Scrape: fix multiple suffixes and relative link resolution (#801)

* Scrape: match any of the comma-separated suffixes

The UI's File Suffixes field accepts a comma-separated list, but the
downloader compared each link against the whole string, so a scrape with
more than one suffix never matched anything.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* 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>

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

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 '<a href="zurück.txt">Zurück</a>' 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'

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 '<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.
@@ -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)