Scrape: resolve relative links against the page's directory

resolve_url appended a relative href to the page URL, so a link to
"a.txt" on ".../dir/page.html" became ".../dir/page.html/a.txt" and the
file download 404'd. Use urljoin, and resolve against the final URL after
redirects so a directory URL without a trailing slash still works.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Roland 2026-10-08 19:59:23 -06:00
parent 1670b655b2
commit 53b6dcc1c4
No known key found for this signature in database
GPG key ID: 1BA620CE6C9A4680
2 changed files with 44 additions and 22 deletions

View file

@ -11,6 +11,7 @@ from wrolpi.scrape_downloader import (
ExecutedScrape,
PreparedScrape,
ScrapeHTMLDownloader,
resolve_url,
scrape_html_downloader,
)
@ -29,7 +30,8 @@ EXAMPLE_HTTP = '''
async def fake_fetch_html(self, url: str):
return EXAMPLE_HTTP
# Servers redirect a directory URL to its trailing-slash form.
return f'{url.rstrip("/")}/', EXAMPLE_HTTP
async def fake_file_do_download(*a, **kwargs):
@ -275,7 +277,7 @@ async def test_fetch_html_decodes_declared_charset(test_directory, simple_file_s
(test_directory / 'page.html').write_bytes(html.encode(encoding))
host, port = simple_file_server.server_address
content = await ScrapeHTMLDownloader.fetch_html(f'http://127.0.0.1:{port}/page.html')
_, content = await ScrapeHTMLDownloader.fetch_html(f'http://127.0.0.1:{port}/page.html')
assert '<a href="zurück.txt">Zurück</a>' in content
@ -300,3 +302,31 @@ async def test_execute_collects_multiple_suffixes(test_directory):
'https://example.com/dir/three.pdf',
'https://example.com/dir/other.html',
])
@pytest.mark.parametrize('page_url,href,expected', [
# A link on a page resolves against the page's directory, not the page itself.
('http://example.com/archive/archive.html', 'bc-a02.txt', 'http://example.com/archive/bc-a02.txt'),
('http://example.com/archive/', 'bc-a02.txt', 'http://example.com/archive/bc-a02.txt'),
('http://example.com/archive/archive.html', '../index.html', 'http://example.com/index.html'),
('http://example.com/archive/archive.html', './a.txt', 'http://example.com/archive/a.txt'),
('http://example.com/archive/archive.html', '/root.txt', 'http://example.com/root.txt'),
('http://example.com/archive/archive.html', 'https://other.com/x.txt', 'https://other.com/x.txt'),
('https://example.com/archive/archive.html', '//cdn.example.com/x.txt', 'https://cdn.example.com/x.txt'),
('http://example.com/archive/archive.html', 'a.txt?v=1', 'http://example.com/archive/a.txt?v=1'),
])
def test_resolve_url(page_url, href, expected):
assert resolve_url(page_url, href) == expected
@pytest.mark.asyncio
async def test_fetch_html_returns_redirected_url(test_directory, simple_file_server):
"""Links in a directory listing resolve against the redirected URL (with trailing slash)."""
(test_directory / 'archive').mkdir()
(test_directory / 'archive' / 'a.txt').write_text('a')
host, port = simple_file_server.server_address
final_url, content = await ScrapeHTMLDownloader.fetch_html(f'http://127.0.0.1:{port}/archive')
assert final_url == f'http://127.0.0.1:{port}/archive/'
assert resolve_url(final_url, 'a.txt') == f'http://127.0.0.1:{port}/archive/a.txt'

View file

@ -1,8 +1,8 @@
import pathlib
from copy import copy
from dataclasses import dataclass
from typing import List
from urllib.parse import urlparse
from typing import List, Tuple
from urllib.parse import urljoin
from bs4.dammit import UnicodeDammit
from sqlalchemy.orm import Session
@ -14,20 +14,9 @@ from wrolpi.errors import UnrecoverableDownloadError
logger = logger.getChild(__name__)
def resolve_url(parent_url: str, url: str):
parsed_domain = urlparse(parent_url)
parsed = urlparse(url)
if parsed.scheme:
# URL is complete or external.
return url
elif parsed.path.startswith('/'):
# URL is absolute from domain.
return f'{parsed_domain.scheme}://{parsed_domain.netloc}{url}'
# Assume URL is relative to the parent URL page.
parent_url = parent_url.rstrip('/')
return f'{parent_url}/{url}'
def resolve_url(page_url: str, url: str) -> str:
"""Resolve a link the way a browser does: relative to the page's directory (or absolute/external)."""
return urljoin(page_url, url)
@dataclass
@ -64,13 +53,16 @@ class ScrapeHTMLDownloader(Downloader):
return '<ScrapeHTMLDownloader>'
@staticmethod
async def fetch_html(url: str) -> str:
async def fetch_html(url: str) -> Tuple[str, str]:
"""Fetch and decode a page. A charset in the Content-Type header wins, then the page's
<meta> charset; aiohttp alone ignores <meta> and assumes UTF-8."""
<meta> charset; aiohttp alone ignores <meta> and assumes UTF-8.
Returns the final URL after redirects (links resolve against it) and the page content."""
async with aiohttp_get(url, timeout=60 * 5) as response:
body = await response.read()
known_encodings = [response.charset] if response.charset else []
return UnicodeDammit(body, known_definite_encodings=known_encodings, is_html=True).unicode_markup
final_url = str(response.url)
return final_url, UnicodeDammit(body, known_definite_encodings=known_encodings, is_html=True).unicode_markup
def prepare_download(self, session: Session, download: Download) -> PreparedScrape:
"""Validate settings and ensure the destination directory exists.
@ -124,7 +116,7 @@ class ScrapeHTMLDownloader(Downloader):
break
# Get the HTML of the URL.
content = await self.fetch_html(url)
url, content = await self.fetch_html(url)
page_count += 1
if not content:
logger.error(f'Failed to download url: {url}')