mirror of https://github.com/scrapy/scrapy.git
Make redirect dupefilter bypass chain-aware
Extend the self-redirect dupefilter fix to cover the whole redirect chain, not just the immediate predecessor request. A redirect chain that loops back to an earlier request in the same chain (A -> B -> C -> A) previously still got silently dropped, because only the immediate hop's fingerprint was compared. Track the set of fingerprints seen so far in the chain via request.meta and bypass the dupe filter if the new target matches any of them. Refs #1225
This commit is contained in:
parent
4ee3676464
commit
901ec3f08c
|
|
@ -106,6 +106,23 @@ class BaseRedirectMiddleware:
|
|||
reason,
|
||||
]
|
||||
redirected.dont_filter = request.dont_filter
|
||||
if not redirected.dont_filter and self._is_redirect_chain_duplicate(
|
||||
redirected, request
|
||||
):
|
||||
# The dupe filter marks a request's fingerprint as seen as
|
||||
# soon as it is scheduled, before its response (and any
|
||||
# resulting redirect) is known. If this redirect chain loops
|
||||
# back to a URL that fingerprints the same as an earlier
|
||||
# request in this *same* chain -- a literal self-redirect, a
|
||||
# redirect whose query string is just reordered (which the
|
||||
# fingerprinter canonicalizes away), or a longer
|
||||
# A -> B -> C -> A style loop -- the redirected request would
|
||||
# otherwise be treated as an already-seen duplicate and
|
||||
# silently dropped instead of followed. Bypass the filter for
|
||||
# that specific case only, so normal cross-request dedup
|
||||
# (unrelated pages redirecting to the same canonical URL)
|
||||
# still works as before.
|
||||
redirected.dont_filter = True
|
||||
redirected.priority = request.priority + self.priority_adjust
|
||||
logger.debug(
|
||||
"Redirecting (%(reason)s) to %(redirected)s from %(request)s",
|
||||
|
|
@ -120,6 +137,24 @@ class BaseRedirectMiddleware:
|
|||
)
|
||||
raise IgnoreRequest("max redirections reached")
|
||||
|
||||
def _is_redirect_chain_duplicate(
|
||||
self, redirected: Request, request: Request
|
||||
) -> bool:
|
||||
"""Whether *redirected* fingerprints the same as *request* or as any
|
||||
earlier request in the same redirect chain as *request*.
|
||||
|
||||
The set of fingerprints accumulated so far in the chain is carried
|
||||
forward in ``request.meta["redirect_fingerprints"]``, bounded by
|
||||
:setting:`REDIRECT_MAX_TIMES` since it grows by at most one entry per
|
||||
hop.
|
||||
"""
|
||||
assert self.crawler.request_fingerprinter
|
||||
fingerprint = self.crawler.request_fingerprinter.fingerprint
|
||||
seen_fingerprints = set(request.meta.get("redirect_fingerprints", ()))
|
||||
seen_fingerprints.add(fingerprint(request))
|
||||
redirected.meta["redirect_fingerprints"] = seen_fingerprints
|
||||
return fingerprint(redirected) in seen_fingerprints
|
||||
|
||||
def _build_redirect_request(
|
||||
self, source_request: Request, response: Response, *, url: str, **kwargs: Any
|
||||
) -> Request:
|
||||
|
|
|
|||
Loading…
Reference in New Issue