Merge pull request #6213 from Laerte/master

Cleanup deprecated fingerprint code in scrapy.utils.request
This commit is contained in:
Andrey Rakhmatullin 2024-01-31 21:00:48 +04:00 committed by GitHub
commit 6f73dc0e67
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 11 additions and 457 deletions

View File

@ -469,60 +469,6 @@ import path.
.. autoclass:: scrapy.utils.request.RequestFingerprinter
.. setting:: REQUEST_FINGERPRINTER_IMPLEMENTATION
REQUEST_FINGERPRINTER_IMPLEMENTATION
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
.. versionadded:: 2.7
Default: ``'2.6'``
Determines which request fingerprinting algorithm is used by the default
request fingerprinter class (see :setting:`REQUEST_FINGERPRINTER_CLASS`).
Possible values are:
- ``'2.6'`` (default)
This implementation uses the same request fingerprinting algorithm as
Scrapy 2.6 and earlier versions.
Even though this is the default value for backward compatibility reasons,
it is a deprecated value.
- ``'2.7'``
This implementation was introduced in Scrapy 2.7 to fix an issue of the
previous implementation.
New projects should use this value. The :command:`startproject` command
sets this value in the generated ``settings.py`` file.
If you are using the default value (``'2.6'``) for this setting, and you are
using Scrapy components where changing the request fingerprinting algorithm
would cause undesired results, you need to carefully decide when to change the
value of this setting, or switch the :setting:`REQUEST_FINGERPRINTER_CLASS`
setting to a custom request fingerprinter class that implements the 2.6 request
fingerprinting algorithm and does not log this warning (
:ref:`2.6-request-fingerprinter` includes an example implementation of such a
class).
Scenarios where changing the request fingerprinting algorithm may cause
undesired results include, for example, using the HTTP cache middleware (see
:class:`~scrapy.downloadermiddlewares.httpcache.HttpCacheMiddleware`).
Changing the request fingerprinting algorithm would invalidate the current
cache, requiring you to redownload all requests again.
Otherwise, set :setting:`REQUEST_FINGERPRINTER_IMPLEMENTATION` to ``'2.7'`` in
your settings to switch already to the request fingerprinting implementation
that will be the only request fingerprinting implementation available in a
future version of Scrapy, and remove the deprecation warning triggered by using
the default value (``'2.6'``).
.. _2.6-request-fingerprinter:
.. _custom-request-fingerprinter:
Writing your own request fingerprinter

View File

@ -260,7 +260,7 @@ REFERER_ENABLED = True
REFERRER_POLICY = "scrapy.spidermiddlewares.referer.DefaultReferrerPolicy"
REQUEST_FINGERPRINTER_CLASS = "scrapy.utils.request.RequestFingerprinter"
REQUEST_FINGERPRINTER_IMPLEMENTATION = "2.6"
REQUEST_FINGERPRINTER_IMPLEMENTATION = "SENTINEL"
RETRY_ENABLED = True
RETRY_TIMES = 2 # initial response + 2 retries = 3 requests

View File

@ -88,6 +88,5 @@ ROBOTSTXT_OBEY = True
#HTTPCACHE_STORAGE = "scrapy.extensions.httpcache.FilesystemCacheStorage"
# Set settings whose default value is deprecated to a future-proof value
REQUEST_FINGERPRINTER_IMPLEMENTATION = "2.7"
TWISTED_REACTOR = "twisted.internet.asyncioreactor.AsyncioSelectorReactor"
FEED_EXPORT_ENCODING = "utf-8"

View File

@ -34,9 +34,6 @@ from scrapy.utils.python import to_bytes, to_unicode
if TYPE_CHECKING:
from scrapy.crawler import Crawler
_deprecated_fingerprint_cache: "WeakKeyDictionary[Request, Dict[Tuple[Optional[Tuple[bytes, ...]], bool], str]]"
_deprecated_fingerprint_cache = WeakKeyDictionary()
def _serialize_headers(
headers: Iterable[bytes], request: Request
@ -47,120 +44,6 @@ def _serialize_headers(
yield from request.headers.getlist(header)
def request_fingerprint(
request: Request,
include_headers: Optional[Iterable[Union[bytes, str]]] = None,
keep_fragments: bool = False,
) -> str:
"""
Return the request fingerprint as an hexadecimal string.
The request fingerprint is a hash that uniquely identifies the resource the
request points to. For example, take the following two urls:
http://www.example.com/query?id=111&cat=222
http://www.example.com/query?cat=222&id=111
Even though those are two different URLs both point to the same resource
and are equivalent (i.e. they should return the same response).
Another example are cookies used to store session ids. Suppose the
following page is only accessible to authenticated users:
http://www.example.com/members/offers.html
Lots of sites use a cookie to store the session id, which adds a random
component to the HTTP Request and thus should be ignored when calculating
the fingerprint.
For this reason, request headers are ignored by default when calculating
the fingerprint. If you want to include specific headers use the
include_headers argument, which is a list of Request headers to include.
Also, servers usually ignore fragments in urls when handling requests,
so they are also ignored by default when calculating the fingerprint.
If you want to include them, set the keep_fragments argument to True
(for instance when handling requests with a headless browser).
"""
if include_headers or keep_fragments:
message = (
"Call to deprecated function "
"scrapy.utils.request.request_fingerprint().\n"
"\n"
"If you are using this function in a Scrapy component because you "
"need a non-default fingerprinting algorithm, and you are OK "
"with that non-default fingerprinting algorithm being used by "
"all Scrapy components and not just the one calling this "
"function, use crawler.request_fingerprinter.fingerprint() "
"instead in your Scrapy component (you can get the crawler "
"object from the 'from_crawler' class method), and use the "
"'REQUEST_FINGERPRINTER_CLASS' setting to configure your "
"non-default fingerprinting algorithm.\n"
"\n"
"Otherwise, consider using the "
"scrapy.utils.request.fingerprint() function instead.\n"
"\n"
"If you switch to 'fingerprint()', or assign the "
"'REQUEST_FINGERPRINTER_CLASS' setting a class that uses "
"'fingerprint()', the generated fingerprints will not only be "
"bytes instead of a string, but they will also be different from "
"those generated by 'request_fingerprint()'. Before you switch, "
"make sure that you understand the consequences of this (e.g. "
"cache invalidation) and are OK with them; otherwise, consider "
"implementing your own function which returns the same "
"fingerprints as the deprecated 'request_fingerprint()' function."
)
else:
message = (
"Call to deprecated function "
"scrapy.utils.request.request_fingerprint().\n"
"\n"
"If you are using this function in a Scrapy component, and you "
"are OK with users of your component changing the fingerprinting "
"algorithm through settings, use "
"crawler.request_fingerprinter.fingerprint() instead in your "
"Scrapy component (you can get the crawler object from the "
"'from_crawler' class method).\n"
"\n"
"Otherwise, consider using the "
"scrapy.utils.request.fingerprint() function instead.\n"
"\n"
"Either way, the resulting fingerprints will be returned as "
"bytes, not as a string, and they will also be different from "
"those generated by 'request_fingerprint()'. Before you switch, "
"make sure that you understand the consequences of this (e.g. "
"cache invalidation) and are OK with them; otherwise, consider "
"implementing your own function which returns the same "
"fingerprints as the deprecated 'request_fingerprint()' function."
)
warnings.warn(message, category=ScrapyDeprecationWarning, stacklevel=2)
processed_include_headers: Optional[Tuple[bytes, ...]] = None
if include_headers:
processed_include_headers = tuple(
to_bytes(h.lower()) for h in sorted(include_headers)
)
cache = _deprecated_fingerprint_cache.setdefault(request, {})
cache_key = (processed_include_headers, keep_fragments)
if cache_key not in cache:
fp = hashlib.sha1()
fp.update(to_bytes(request.method))
fp.update(
to_bytes(canonicalize_url(request.url, keep_fragments=keep_fragments))
)
fp.update(request.body or b"")
if processed_include_headers:
for part in _serialize_headers(processed_include_headers, request):
fp.update(part)
cache[cache_key] = fp.hexdigest()
return cache[cache_key]
def _request_fingerprint_as_bytes(*args: Any, **kwargs: Any) -> bytes:
with warnings.catch_warnings():
warnings.simplefilter("ignore")
return bytes.fromhex(request_fingerprint(*args, **kwargs))
_fingerprint_cache: "WeakKeyDictionary[Request, Dict[Tuple[Optional[Tuple[bytes, ...]], bool], bytes]]"
_fingerprint_cache = WeakKeyDictionary()
@ -258,33 +141,15 @@ class RequestFingerprinter:
"REQUEST_FINGERPRINTER_IMPLEMENTATION"
)
else:
implementation = "2.6"
if implementation == "2.6":
implementation = "SENTINEL"
if implementation != "SENTINEL":
message = (
"'2.6' is a deprecated value for the "
"'REQUEST_FINGERPRINTER_IMPLEMENTATION' setting.\n"
"\n"
"It is also the default value. In other words, it is normal "
"to get this warning if you have not defined a value for the "
"'REQUEST_FINGERPRINTER_IMPLEMENTATION' setting. This is so "
"for backward compatibility reasons, but it will change in a "
"future version of Scrapy.\n"
"\n"
"See the documentation of the "
"'REQUEST_FINGERPRINTER_IMPLEMENTATION' setting for "
"information on how to handle this deprecation."
"'REQUEST_FINGERPRINTER_IMPLEMENTATION' is a deprecated setting.\n"
"And it will be removed in future version of Scrapy."
)
warnings.warn(message, category=ScrapyDeprecationWarning, stacklevel=2)
self._fingerprint = _request_fingerprint_as_bytes
elif implementation == "2.7":
self._fingerprint = fingerprint
else:
raise ValueError(
f"Got an invalid value on setting "
f"'REQUEST_FINGERPRINTER_IMPLEMENTATION': "
f"{implementation!r}. Valid values are '2.6' (deprecated) "
f"and '2.7'."
)
self._fingerprint = fingerprint
def fingerprint(self, request: Request) -> bytes:
return self._fingerprint(request)

View File

@ -86,8 +86,6 @@ def get_crawler(
# Set by default settings that prevent deprecation warnings.
settings: Dict[str, Any] = {}
if prevent_warnings:
settings["REQUEST_FINGERPRINTER_IMPLEMENTATION"] = "2.7"
settings.update(settings_dict or {})
runner = CrawlerRunner(settings)
crawler = runner.create_crawler(spidercls or TestSpider)

View File

@ -2,22 +2,15 @@ import json
import unittest
import warnings
from hashlib import sha1
from typing import Dict, Mapping, Optional, Tuple, Union
from typing import Dict, Optional, Tuple, Union
from weakref import WeakKeyDictionary
import pytest
from w3lib.url import canonicalize_url
from scrapy.http import Request
from scrapy.utils.deprecate import ScrapyDeprecationWarning
from scrapy.utils.python import to_bytes
from scrapy.utils.request import (
_deprecated_fingerprint_cache,
_fingerprint_cache,
_request_fingerprint_as_bytes,
fingerprint,
request_authenticate,
request_fingerprint,
request_httprepr,
request_to_curl,
)
@ -233,168 +226,6 @@ class FingerprintTest(unittest.TestCase):
self.assertEqual(actual, expected)
class RequestFingerprintTest(FingerprintTest):
function = staticmethod(request_fingerprint)
cache = _deprecated_fingerprint_cache
known_hashes: Tuple[Tuple[Request, Union[bytes, str], Dict], ...] = (
(
Request("http://example.org"),
"b2e5245ef826fd9576c93bd6e392fce3133fab62",
{},
),
(
Request("https://example.org"),
"bd10a0a89ea32cdee77917320f1309b0da87e892",
{},
),
(
Request("https://example.org?a"),
"2fb7d48ae02f04b749f40caa969c0bc3c43204ce",
{},
),
(
Request("https://example.org?a=b"),
"42e5fe149b147476e3f67ad0670c57b4cc57856a",
{},
),
(
Request("https://example.org?a=b&a"),
"d23a9787cb56c6375c2cae4453c5a8c634526942",
{},
),
(
Request("https://example.org?a=b&a=c"),
"9a18a7a8552a9182b7f1e05d33876409e421e5c5",
{},
),
(
Request("https://example.org", method="POST"),
"ba20a80cb5c5ca460021ceefb3c2467b2bfd1bc6",
{},
),
(
Request("https://example.org", body=b"a"),
"4bb136e54e715a4ea7a9dd1101831765d33f2d60",
{},
),
(
Request("https://example.org", method="POST", body=b"a"),
"6c6595374a304b293be762f7b7be3f54e9947c65",
{},
),
(
Request("https://example.org#a", headers={"A": b"B"}),
"bd10a0a89ea32cdee77917320f1309b0da87e892",
{},
),
(
Request("https://example.org#a", headers={"A": b"B"}),
"515b633cb3ca502a33a9d8c890e889ec1e425e65",
{"include_headers": ["A"]},
),
(
Request("https://example.org#a", headers={"A": b"B"}),
"505c96e7da675920dfef58725e8c957dfdb38f47",
{"keep_fragments": True},
),
(
Request("https://example.org#a", headers={"A": b"B"}),
"d6f673cdcb661b7970c2b9a00ee63e87d1e2e5da",
{"include_headers": ["A"], "keep_fragments": True},
),
(
Request("https://example.org/ab"),
"4e2870fee58582d6f81755e9b8fdefe3cba0c951",
{},
),
(
Request("https://example.org/a", body=b"b"),
"4e2870fee58582d6f81755e9b8fdefe3cba0c951",
{},
),
)
def setUp(self) -> None:
warnings.simplefilter("ignore", ScrapyDeprecationWarning)
def tearDown(self) -> None:
warnings.simplefilter("default", ScrapyDeprecationWarning)
@pytest.mark.xfail(reason="known bug kept for backward compatibility", strict=True)
def test_part_separation(self):
super().test_part_separation()
class RequestFingerprintDeprecationTest(unittest.TestCase):
def test_deprecation_default_parameters(self):
with pytest.warns(ScrapyDeprecationWarning) as warnings:
request_fingerprint(Request("http://www.example.com"))
messages = [str(warning.message) for warning in warnings]
self.assertTrue(
any("Call to deprecated function" in message for message in messages)
)
self.assertFalse(any("non-default" in message for message in messages))
def test_deprecation_non_default_parameters(self):
with pytest.warns(ScrapyDeprecationWarning) as warnings:
request_fingerprint(Request("http://www.example.com"), keep_fragments=True)
messages = [str(warning.message) for warning in warnings]
self.assertTrue(
any("Call to deprecated function" in message for message in messages)
)
self.assertTrue(any("non-default" in message for message in messages))
class RequestFingerprintAsBytesTest(FingerprintTest):
function = staticmethod(_request_fingerprint_as_bytes)
cache = _deprecated_fingerprint_cache
known_hashes = RequestFingerprintTest.known_hashes
def test_caching(self):
r1 = Request("http://www.example.com/hnnoticiaj1.aspx?78160,199")
self.assertEqual(
self.function(r1), bytes.fromhex(self.cache[r1][self.default_cache_key])
)
@pytest.mark.xfail(reason="known bug kept for backward compatibility", strict=True)
def test_part_separation(self):
super().test_part_separation()
def test_hashes(self):
actual = [
self.function(request, **kwargs) for request, _, kwargs in self.known_hashes
]
expected = [
bytes.fromhex(_fingerprint) for _, _fingerprint, _ in self.known_hashes
]
self.assertEqual(actual, expected)
_fingerprint_cache_2_6: Mapping[Request, Tuple[None, bool]] = WeakKeyDictionary()
def request_fingerprint_2_6(request, include_headers=None, keep_fragments=False):
if include_headers:
include_headers = tuple(to_bytes(h.lower()) for h in sorted(include_headers))
cache = _fingerprint_cache_2_6.setdefault(request, {})
cache_key = (include_headers, keep_fragments)
if cache_key not in cache:
fp = sha1()
fp.update(to_bytes(request.method))
fp.update(
to_bytes(canonicalize_url(request.url, keep_fragments=keep_fragments))
)
fp.update(request.body or b"")
if include_headers:
for hdr in include_headers:
if hdr in request.headers:
fp.update(hdr)
for v in request.headers.getlist(hdr):
fp.update(v)
cache[cache_key] = fp.hexdigest()
return cache[cache_key]
REQUEST_OBJECTS_TO_TEST = (
Request("http://www.example.com/"),
Request("http://www.example.com/query?id=111&cat=222"),
@ -424,94 +255,16 @@ REQUEST_OBJECTS_TO_TEST = (
)
class BackwardCompatibilityTestCase(unittest.TestCase):
def test_function_backward_compatibility(self):
include_headers_to_test = (
None,
["Accept-Language"],
["accept-language", "sessionid"],
["SESSIONID", "Accept-Language"],
)
for request_object in REQUEST_OBJECTS_TO_TEST:
for include_headers in include_headers_to_test:
for keep_fragments in (False, True):
with warnings.catch_warnings():
warnings.simplefilter("ignore")
fp = request_fingerprint(
request_object,
include_headers=include_headers,
keep_fragments=keep_fragments,
)
old_fp = request_fingerprint_2_6(
request_object,
include_headers=include_headers,
keep_fragments=keep_fragments,
)
self.assertEqual(fp, old_fp)
def test_component_backward_compatibility(self):
for request_object in REQUEST_OBJECTS_TO_TEST:
with warnings.catch_warnings():
warnings.simplefilter("ignore")
crawler = get_crawler(prevent_warnings=False)
fp = crawler.request_fingerprinter.fingerprint(request_object)
old_fp = request_fingerprint_2_6(request_object)
self.assertEqual(fp.hex(), old_fp)
def test_custom_component_backward_compatibility(self):
"""Tests that the backward-compatible request fingerprinting class featured
in the documentation is indeed backward compatible and does not cause a
warning to be logged."""
class RequestFingerprinter:
cache = WeakKeyDictionary()
def fingerprint(self, request):
if request not in self.cache:
fp = sha1()
fp.update(to_bytes(request.method))
fp.update(to_bytes(canonicalize_url(request.url)))
fp.update(request.body or b"")
self.cache[request] = fp.digest()
return self.cache[request]
for request_object in REQUEST_OBJECTS_TO_TEST:
with warnings.catch_warnings() as logged_warnings:
settings = {
"REQUEST_FINGERPRINTER_CLASS": RequestFingerprinter,
}
crawler = get_crawler(settings_dict=settings)
fp = crawler.request_fingerprinter.fingerprint(request_object)
old_fp = request_fingerprint_2_6(request_object)
self.assertEqual(fp.hex(), old_fp)
self.assertFalse(logged_warnings)
class RequestFingerprinterTestCase(unittest.TestCase):
def test_default_implementation(self):
with warnings.catch_warnings(record=True) as logged_warnings:
crawler = get_crawler(prevent_warnings=False)
crawler = get_crawler()
request = Request("https://example.com")
self.assertEqual(
crawler.request_fingerprinter.fingerprint(request),
_request_fingerprint_as_bytes(request),
fingerprint(request),
)
self.assertTrue(logged_warnings)
def test_deprecated_implementation(self):
settings = {
"REQUEST_FINGERPRINTER_IMPLEMENTATION": "2.6",
}
with warnings.catch_warnings(record=True) as logged_warnings:
crawler = get_crawler(settings_dict=settings)
request = Request("https://example.com")
self.assertEqual(
crawler.request_fingerprinter.fingerprint(request),
_request_fingerprint_as_bytes(request),
)
self.assertTrue(logged_warnings)
def test_recommended_implementation(self):
settings = {
"REQUEST_FINGERPRINTER_IMPLEMENTATION": "2.7",
}
@ -522,14 +275,7 @@ class RequestFingerprinterTestCase(unittest.TestCase):
crawler.request_fingerprinter.fingerprint(request),
fingerprint(request),
)
self.assertFalse(logged_warnings)
def test_unknown_implementation(self):
settings = {
"REQUEST_FINGERPRINTER_IMPLEMENTATION": "2.5",
}
with self.assertRaises(ValueError):
get_crawler(settings_dict=settings)
self.assertTrue(logged_warnings)
class CustomRequestFingerprinterTestCase(unittest.TestCase):