Skip to content

Commit f7d77f1

Browse files
committed
fix(scraper): resolve relative links against the post-redirect URL
fetch_links_from_url now returns the effective URL (response.geturl()) and process_url uses it as the base for urljoin. Without this, scraping a URL that redirects - e.g. a GitHub Pages project page where '/repo' 301-redirects to '/repo/' - resolved relative links one directory too high, causing 404s on every file. Add a regression test.
1 parent 3553cc4 commit f7d77f1

2 files changed

Lines changed: 38 additions & 11 deletions

File tree

src/MetaDetective/MetaDetective.py

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -661,7 +661,7 @@ def __init__(self, extensions: List[str]):
661661
self.extensions = {ext.lower() for ext in EXTENSIONS}
662662
self.css_js_pattern = re.compile(r"\.(css|js)($|\?|#)")
663663

664-
def fetch_links_from_url(self, url: str, timeout: int = DEFAULT_HTTP_TIMEOUT) -> List[str]:
664+
def fetch_links_from_url(self, url: str, timeout: int = DEFAULT_HTTP_TIMEOUT) -> Tuple[str, List[str]]:
665665
"""
666666
Fetch all links from a given URL.
667667
@@ -670,14 +670,19 @@ def fetch_links_from_url(self, url: str, timeout: int = DEFAULT_HTTP_TIMEOUT) ->
670670
timeout: Request timeout in seconds
671671
672672
Returns:
673-
List of links found on the page
673+
Tuple of (effective URL after redirects, links found on the page).
674+
The effective URL must be used as the base for resolving relative
675+
links, since e.g. GitHub Pages 301-redirects '/repo' to '/repo/'.
674676
"""
675677
try:
676678
request = urllib.request.Request(url, headers={'User-Agent': USER_AGENT})
677679
with urllib.request.urlopen(request, timeout=timeout) as response:
680+
# Final URL after any redirects; relative links resolve against it.
681+
effective_url = response.geturl() or url
682+
678683
content_type = response.headers.get('Content-Type', '').split(';')[0]
679684
if 'text' not in content_type and 'application' not in content_type:
680-
return []
685+
return effective_url, []
681686

682687
raw_data = response.read()
683688

@@ -688,7 +693,7 @@ def fetch_links_from_url(self, url: str, timeout: int = DEFAULT_HTTP_TIMEOUT) ->
688693
data = raw_data.decode('latin-1')
689694
except UnicodeDecodeError:
690695
Logger.warning(f"Unable to decode content from {url}")
691-
return []
696+
return effective_url, []
692697

693698
parser = LinkParser()
694699
parser.feed(data)
@@ -701,20 +706,20 @@ def fetch_links_from_url(self, url: str, timeout: int = DEFAULT_HTTP_TIMEOUT) ->
701706
if not link or link.strip() == '':
702707
continue
703708
filtered_links.append(link)
704-
return filtered_links
709+
return effective_url, filtered_links
705710

706711
except urllib.error.URLError as e:
707712
if url.startswith("mailto:"):
708713
Logger.info(f"Found mailto link {url}")
709714
else:
710715
Logger.error(f"Unable to open {url} Reason: {e}")
711-
return []
716+
return url, []
712717
except urllib.error.HTTPError as e:
713718
Logger.error(f"HTTP Error for URL {url} Reason: {e.code} - {e.reason}")
714-
return []
719+
return url, []
715720
except Exception as e:
716721
Logger.error(f"Unexpected error fetching {url} Reason: {e}")
717-
return []
722+
return url, []
718723

719724
def is_valid_file_link(self, link: str) -> bool:
720725
"""
@@ -883,12 +888,12 @@ def process_url(self, task: ScrapingTask, task_queue: queue.Queue) -> None:
883888

884889
self.rate_limiter.wait()
885890

886-
links = self.scraper.fetch_links_from_url(task.url)
891+
base_url, links = self.scraper.fetch_links_from_url(task.url)
887892

888893
file_links = []
889894
for link in links:
890895
if self.scraper.is_valid_file_link(link):
891-
absolute_url = urljoin(task.url, link)
896+
absolute_url = urljoin(base_url, link)
892897
file_links.append(absolute_url)
893898

894899
if self.download_dir and not self.scan:
@@ -910,7 +915,7 @@ def process_url(self, task: ScrapingTask, task_queue: queue.Queue) -> None:
910915
if not link.startswith(('http://', 'https://', '/')):
911916
continue
912917

913-
absolute_link = urljoin(task.url, link)
918+
absolute_link = urljoin(base_url, link)
914919
parsed_link = urlparse(absolute_link)
915920

916921
# Validate that it's an HTTP/HTTPS URL

tests/test_MetaDetective.py

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -491,5 +491,27 @@ def test_paths_are_not_urls(self):
491491
self.assertFalse(md.looks_like_url("ftp://example.com/f"))
492492

493493

494+
class TestWebScraperRedirectBase(unittest.TestCase):
495+
"""Relative links must resolve against the URL *after* redirects."""
496+
497+
@patch("src.MetaDetective.MetaDetective.urllib.request.urlopen")
498+
def test_relative_links_resolve_against_effective_url(self, mock_urlopen):
499+
# Simulate GitHub Pages 301-redirecting '/repo' -> '/repo/'
500+
resp = mock_urlopen.return_value.__enter__.return_value
501+
resp.geturl.return_value = "https://host/repo/"
502+
resp.headers.get.return_value = "text/html"
503+
resp.read.return_value = b'<a href="lab/report.pdf">go</a>'
504+
505+
scraper = md.WebScraper(["pdf"])
506+
base, links = scraper.fetch_links_from_url("https://host/repo")
507+
508+
self.assertEqual(base, "https://host/repo/")
509+
self.assertIn("lab/report.pdf", links)
510+
self.assertEqual(
511+
md.urljoin(base, links[0]),
512+
"https://host/repo/lab/report.pdf",
513+
)
514+
515+
494516
if __name__ == "__main__":
495517
unittest.main()

0 commit comments

Comments
 (0)