Skip to content

#2282 Don't apply SitemapFilter to redirect targets - #2285

Open
rzo1 wants to merge 2 commits into
mainfrom
2282-sitemapfilter-redirects
Open

rzo1 wants to merge 2 commits into
mainfrom
2282-sitemapfilter-redirects

Conversation

@rzo1

@rzo1 rzo1 commented Oct 29, 2026

Copy link
Copy Markdown
Contributor

FetcherBolt.discoverSitemaps() sets foundSitemap=true on the metadata of the URL being fetched, before the fetch. Redirect targets were then filtered with that metadata, and SitemapFilter rejects every URL when foundSitemap=true and isSitemap is not set. As a result, the redirect target of any URL on a site whose robots.txt declares a sitemap never reached the status store.

Changes:

  • New StatusEmitterBolt.redirectMetadata(Metadata), which returns a copy of the metadata without foundSitemap if it is present.
  • It is used for the metadata passed to protocol.getProtocolOutput in FetcherBolt and SimpleFetcherBolt (okhttp filters each redirect hop with it since Run redirect hops through the URL filters #2128), for the REDIRECTION outlinks emitted by both fetcher bolts, and for meta refresh redirects in JSoupParserBolt.
  • The parse and status tuples still carry foundSitemap, so SitemapFilter still applies to the page's outlinks.

Tests:

  • AbstractFetcherBoltTest (runs for FetcherBolt and SimpleFetcherBolt): a 301 with only SitemapFilter in the chain, with and without a Sitemap: line in robots.txt, for http.allow.redirects false and true. The "with sitemap" cases fail on main.
  • JSoupParserBoltTest.testHTMLRedirWithSitemapFilter: meta refresh with foundSitemap=true. Fails on main.
  • New SitemapFilterTest for the outlink behaviour that has to stay.

Fixes #2282

FetcherBolt.discoverSitemaps() sets foundSitemap=true on the metadata of
the URL being fetched, so SitemapFilter rejected its redirect target
whenever robots.txt declared a sitemap. Filter redirect targets with a
copy of the metadata without foundSitemap: in the protocol (redirects
followed by okhttp), for REDIRECTION outlinks in FetcherBolt and
SimpleFetcherBolt, and for meta refresh redirects in JSoupParserBolt.
Outlinks of the page are still filtered as before.
@rzo1
rzo1 requested a review from jnioche October 29, 2026 09:00
@rzo1 rzo1 added this to the 4.0.0 milestone Oct 29, 2026
@rzo1
rzo1 requested a review from dpol1 October 29, 2026 09:00
@rzo1 rzo1 self-assigned this Oct 29, 2026

@dpol1 dpol1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we also assert foundSitemap on the parse tuple in redirectIsFollowedByProtocolWithSitemapFilter? That would cover SimpleFetcherBolt too.

@rzo1

rzo1 commented Oct 30, 2026

Copy link
Copy Markdown
Contributor Author

Good point, added. Thanks!

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SitemapFilter drops redirect targets on sites that declare a sitemap in robots.txt

2 participants