diff --git a/wrolpi/files/test/test_scrape_downloader.py b/wrolpi/files/test/test_scrape_downloader.py
index 33e48151..3c434009 100644
--- a/wrolpi/files/test/test_scrape_downloader.py
+++ b/wrolpi/files/test/test_scrape_downloader.py
@@ -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,6 +277,56 @@ 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 'Zurück' in content
+
+
+@pytest.mark.asyncio
+async def test_execute_collects_multiple_suffixes(test_directory):
+ """The UI submits a comma-separated list of suffixes; a link matching any of them is collected."""
+ prepared = PreparedScrape(
+ url='https://example.com/dir',
+ depth=1,
+ suffix='.pdf,.html',
+ max_pages=100,
+ destination=test_directory,
+ )
+
+ with mock.patch('wrolpi.scrape_downloader.ScrapeHTMLDownloader.fetch_html', fake_fetch_html):
+ executed = await scrape_html_downloader.execute_download(prepared, make_test_ctx())
+
+ assert sorted(executed.download_urls) == sorted([
+ 'https://example.com/one.pdf',
+ 'https://example.com/two.pdf',
+ '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'
diff --git a/wrolpi/scrape_downloader.py b/wrolpi/scrape_downloader.py
index 0c74b8d3..3558401d 100644
--- a/wrolpi/scrape_downloader.py
+++ b/wrolpi/scrape_downloader.py
@@ -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 ''
@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
- charset; aiohttp alone ignores and assumes UTF-8."""
+ charset; aiohttp alone ignores 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.
@@ -107,7 +99,9 @@ class ScrapeHTMLDownloader(Downloader):
async def execute_download(self, prepared: PreparedScrape, ctx: DownloadContext,
download: Download = None) -> ExecutedScrape:
- """Crawl HTML pages up to depth, collecting links that match the configured suffix."""
+ """Crawl HTML pages up to depth, collecting links that match any of the configured suffixes."""
+ # The UI submits a comma-separated list of suffixes.
+ suffixes = tuple(i.strip() for i in prepared.suffix.split(',') if i.strip())
urls = [prepared.url]
download_urls: List[str] = []
page_count = 0
@@ -122,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}')
@@ -143,7 +137,7 @@ class ScrapeHTMLDownloader(Downloader):
logger.debug(f'Not a real anchor: {a}')
continue
child_url = resolve_url(url, href)
- if child_url and child_url.lower().endswith(prepared.suffix):
+ if child_url and child_url.lower().endswith(suffixes):
# Found a file that the User requested.
logger.info(f'ScrapeHTMLDownloader will download {child_url}')
download_urls.append(child_url)