nebius/SWE-rebench-V2-PRs huggingface dataset:
Repository: scrapy/scrapy
Pull request #5225 (2021-08-10 15:10:50)

Allow comma-separated values in the rel attribute
Comma-separated `rel` values are often seen in the wild, because Google allows it (see https://developers.google.com/search/docs/advanced/guidelines/qualify-outbound-links).


Patch:
diff --git a/scrapy/utils/misc.py b/scrapy/utils/misc.py
index 5c986eedc..51cef1e91 100644
--- a/scrapy/utils/misc.py
+++ b/scrapy/utils/misc.py
@@ -138,7 +138,7 @@ def md5sum(file):
 
 def rel_has_nofollow(rel):
     """Return True if link rel attribute has nofollow type"""
-    return rel is not None and 'nofollow' in rel.split()
+    return rel is not None and 'nofollow' in rel.replace(',', ' ').split()
 
 
 def create_instance(objcls, settings, crawler, *args, **kwargs):


Test patch:
diff --git a/tests/test_utils_misc/__init__.py b/tests/test_utils_misc/__init__.py
index e95a3a316..47d73a2dd 100644
--- a/tests/test_utils_misc/__init__.py
+++ b/tests/test_utils_misc/__init__.py
@@ -4,7 +4,7 @@ import unittest
 from unittest import mock
 
 from scrapy.item import Item, Field
-from scrapy.utils.misc import arg_to_iter, create_instance, load_object, set_environ, walk_modules
+from scrapy.utils.misc import arg_to_iter, create_instance, load_object, rel_has_nofollow, set_environ, walk_modules
 
 
 __doctests__ = ['scrapy.utils.misc']
@@ -162,6 +162,15 @@ class UtilsMiscTestCase(unittest.TestCase):
             assert os.environ.get('some_test_environ') == 'test_value'
         assert os.environ.get('some_test_environ') == 'test'
 
+    def test_rel_has_nofollow(self):
+        assert rel_has_nofollow('ugc nofollow') is True
+        assert rel_has_nofollow('ugc,nofollow') is True
+        assert rel_has_nofollow('ugc') is False
+        assert rel_has_nofollow('nofollow') is True
+        assert rel_has_nofollow('nofollowfoo') is False
+        assert rel_has_nofollow('foonofollow') is False
+        assert rel_has_nofollow('ugc,  ,  nofollow') is True
+
 
 if __name__ == "__main__":
     unittest.main()
Repository: scrapy/scrapy
Pull request #5489 (2022-04-30 22:39:26)

Deprecate scrapy.pipelines.images.NoimagesDrop
Deprecate scrapy.pipelines.images.NoimagesDrop

Resolves https://github.com/scrapy/scrapy/issues/5368


Patch:
diff --git a/scrapy/pipelines/images.py b/scrapy/pipelines/images.py
index 67b3224b3..4d59436ab 100644
--- a/scrapy/pipelines/images.py
+++ b/scrapy/pipelines/images.py
@@ -5,12 +5,13 @@ See documentation in topics/media-pipeline.rst
 """
 import functools
 import hashlib
+import warnings
 from contextlib import suppress
 from io import BytesIO
 
 from itemadapter import ItemAdapter
 
-from scrapy.exceptions import DropItem, NotConfigured
+from scrapy.exceptions import DropItem, NotConfigured, ScrapyDeprecationWarning
 from scrapy.http import Request
 from scrapy.pipelines.files import FileException, FilesPipeline
 # TODO: from scrapy.pipelines.media import MediaPipeline
@@ -22,6 +23,10 @@ from scrapy.utils.python import to_bytes
 class NoimagesDrop(DropItem):
     """Product with no images exception"""
 
+    def __init__(self, *args, **kwargs):
+        warnings.warn("The NoimagesDrop class is deprecated", category=ScrapyDeprecationWarning, stacklevel=2)
+        super().__init__(*args, **kwargs)
+
 
 class ImageException(FileException):
     """General image error exception"""


Test patch:
diff --git a/tests/test_pipeline_images.py b/tests/test_pipeline_images.py
index 0082e7a4e..c189d08bf 100644
--- a/tests/test_pipeline_images.py
+++ b/tests/test_pipeline_images.py
@@ -1,17 +1,19 @@
+import dataclasses
 import hashlib
 import io
 import random
 from shutil import rmtree
 from tempfile import mkdtemp
-import dataclasses
+from warnings import catch_warnings
 
 import attr
 from itemadapter import ItemAdapter
 from twisted.trial import unittest
 
+from scrapy.exceptions import ScrapyDeprecationWarning
 from scrapy.http import Request, Response
 from scrapy.item import Field, Item
-from scrapy.pipelines.images import ImagesPipeline
+from scrapy.pipelines.images import ImagesPipeline, NoimagesDrop
 from scrapy.settings import Settings
 from scrapy.utils.python import to_bytes
 
@@ -416,6 +418,22 @@ class ImagesPipelineTestCaseCustomSettings(unittest.TestCase):
                              expected_value)
 
 
+class NoimagesDropTestCase(unittest.TestCase):
+
+    def test_deprecation_warning(self):
+        arg = str()
+        with catch_warnings(record=True) as warnings:
+            NoimagesDrop(arg)
+            self.assertEqual(len(warnings), 1)
+            self.assertEqual(warnings[0].category, ScrapyDeprecationWarning)
+        with catch_warnings(record=True) as warnings:
+            class SubclassedNoimagesDrop(NoimagesDrop):
+                pass
+            SubclassedNoimagesDrop(arg)
+            self.assertEqual(len(warnings), 1)
+            self.assertEqual(warnings[0].category, ScrapyDeprecationWarning)
+
+
 def _create_image(format, *a, **kw):
     buf = io.BytesIO()
     Image.new(*a, **kw).save(buf, format)
Repository: scrapy/scrapy
Pull request #5599 (2022-08-16 04:04:01)

Partial in is_generator_with_return_value
See path in #5592


Patch:
diff --git a/scrapy/utils/misc.py b/scrapy/utils/misc.py
index 4d4fb9600..e0f7ca9e5 100644
--- a/scrapy/utils/misc.py
+++ b/scrapy/utils/misc.py
@@ -9,6 +9,7 @@ from collections import deque
 from contextlib import contextmanager
 from importlib import import_module
 from pkgutil import iter_modules
+from functools import partial
 
 from w3lib.html import replace_entities
 
@@ -226,7 +227,11 @@ def is_generator_with_return_value(callable):
         return value is None or isinstance(value, ast.NameConstant) and value.value is None
 
     if inspect.isgeneratorfunction(callable):
-        src = inspect.getsource(callable)
+        func = callable
+        while isinstance(func, partial):
+            func = func.func
+
+        src = inspect.getsource(func)
         pattern = re.compile(r"(^[\t ]+)")
         code = pattern.sub("", src)
 


Test patch:
diff --git a/tests/test_utils_misc/test_return_with_argument_inside_generator.py b/tests/test_utils_misc/test_return_with_argument_inside_generator.py
index 72277d701..562f72fee 100644
--- a/tests/test_utils_misc/test_return_with_argument_inside_generator.py
+++ b/tests/test_utils_misc/test_return_with_argument_inside_generator.py
@@ -1,5 +1,6 @@
 import unittest
 import warnings
+from functools import partial
 from unittest import mock
 
 from scrapy.utils.misc import is_generator_with_return_value, warn_on_generator_with_return_value
@@ -254,3 +255,10 @@ https://example.org
             warn_on_generator_with_return_value(None, top_level_return_none)
             self.assertEqual(len(w), 1)
             self.assertIn('Unable to determine', str(w[0].message))
+
+    def test_partial(self):
+        def cb(arg1, arg2):
+            yield {}
+
+        partial_cb = partial(cb, arg1=42)
+        assert not is_generator_with_return_value(partial_cb)
Repository: scrapy/scrapy
Pull request #5626 (2022-09-14 12:53:49)

Relax Proxy-Authorization restrictions
This makes it so that the `Proxy-Authentication` header starts working again to set proxy credentials, as long as the Request metadata proxy URL has no other credentials, and for as long as that proxy URL remains the same.

After this change, many of the scenarios that 2.6.2 broke should start working again, but we will still be protecting users against the proxy-rotation scenario that made the original security change necessary.

To do:
- [x] Manually verify that old versions of scrapy-zyte-smartproxy work again.
- [x] Check if docs need changes.


Patch:
diff --git a/scrapy/downloadermiddlewares/httpproxy.py b/scrapy/downloadermiddlewares/httpproxy.py
index 1deda42bd..dd8a7e797 100644
--- a/scrapy/downloadermiddlewares/httpproxy.py
+++ b/scrapy/downloadermiddlewares/httpproxy.py
@@ -78,4 +78,7 @@ class HttpProxyMiddleware:
                     del request.headers[b'Proxy-Authorization']
                 del request.meta['_auth_proxy']
         elif b'Proxy-Authorization' in request.headers:
-            del request.headers[b'Proxy-Authorization']
+            if proxy_url:
+                request.meta['_auth_proxy'] = proxy_url
+            else:
+                del request.headers[b'Proxy-Authorization']


Test patch:
diff --git a/tests/test_downloadermiddleware_httpproxy.py b/tests/test_downloadermiddleware_httpproxy.py
index 70eb94d77..44434f90e 100644
--- a/tests/test_downloadermiddleware_httpproxy.py
+++ b/tests/test_downloadermiddleware_httpproxy.py
@@ -400,6 +400,9 @@ class TestHttpProxyMiddleware(TestCase):
         self.assertNotIn(b'Proxy-Authorization', request.headers)
 
     def test_proxy_authentication_header_proxy_without_credentials(self):
+        """As long as the proxy URL in request metadata remains the same, the
+        Proxy-Authorization header is used and kept, and may even be
+        changed."""
         middleware = HttpProxyMiddleware()
         request = Request(
             'https://example.com',
@@ -408,7 +411,16 @@ class TestHttpProxyMiddleware(TestCase):
         )
         assert middleware.process_request(request, spider) is None
         self.assertEqual(request.meta['proxy'], 'https://example.com')
-        self.assertNotIn(b'Proxy-Authorization', request.headers)
+        self.assertEqual(request.headers['Proxy-Authorization'], b'Basic foo')
+
+        assert middleware.process_request(request, spider) is None
+        self.assertEqual(request.meta['proxy'], 'https://example.com')
+        self.assertEqual(request.headers['Proxy-Authorization'], b'Basic foo')
+
+        request.headers['Proxy-Authorization'] = b'Basic bar'
+        assert middleware.process_request(request, spider) is None
+        self.assertEqual(request.meta['proxy'], 'https://example.com')
+        self.assertEqual(request.headers['Proxy-Authorization'], b'Basic bar')
 
     def test_proxy_authentication_header_proxy_with_same_credentials(self):
         middleware = HttpProxyMiddleware()
Repository: scrapy/scrapy
Pull request #5901 (2023-04-19 03:30:25)

Fix: Request.from_curl() with $-prefixed string literals
Hi @wRAR 

I added an [`action`](https://docs.python.org/3/library/argparse.html#action) for the `--data` argument that removes the _string prefix_ and decodes the string. Although `Request.from_curl()` now works correctly with requests like the one you shared in the issue, I don't know if this is the best way to approach this.


```python
curl_command = "curl 'https://www.example.org/' --data-raw $'{\"$filters\":\"Filter\u0021\"}'"
curl_to_request_kwargs(curl_command)

# output: {'method': 'POST', 'url': 'https://www.example.org/', 'body': '{"$filters":"Filter!"}'}
```


Closes https://github.com/scrapy/scrapy/issues/5899


Patch:
diff --git a/scrapy/utils/curl.py b/scrapy/utils/curl.py
index a2243ae2e..f5dbbd64e 100644
--- a/scrapy/utils/curl.py
+++ b/scrapy/utils/curl.py
@@ -7,6 +7,14 @@ from urllib.parse import urlparse
 from w3lib.http import basic_auth_header
 
 
+class DataAction(argparse.Action):
+    def __call__(self, parser, namespace, values, option_string=None):
+        value = str(values)
+        if value.startswith("$"):
+            value = value[1:]
+        setattr(namespace, self.dest, value)
+
+
 class CurlParser(argparse.ArgumentParser):
     def error(self, message):
         error_msg = f"There was an error parsing the curl command: {message}"
@@ -17,7 +25,7 @@ curl_parser = CurlParser()
 curl_parser.add_argument("url")
 curl_parser.add_argument("-H", "--header", dest="headers", action="append")
 curl_parser.add_argument("-X", "--request", dest="method")
-curl_parser.add_argument("-d", "--data", "--data-raw", dest="data")
+curl_parser.add_argument("-d", "--data", "--data-raw", dest="data", action=DataAction)
 curl_parser.add_argument("-u", "--user", dest="auth")
 
 


Test patch:
diff --git a/tests/test_utils_curl.py b/tests/test_utils_curl.py
index fd4612eba..1816db29b 100644
--- a/tests/test_utils_curl.py
+++ b/tests/test_utils_curl.py
@@ -154,6 +154,15 @@ class CurlToRequestKwargsTest(unittest.TestCase):
         }
         self._test_command(curl_command, expected_result)
 
+    def test_post_data_raw_with_string_prefix(self):
+        curl_command = "curl 'https://www.example.org/' --data-raw $'{\"$filters\":\"Filter\u0021\"}'"
+        expected_result = {
+            "method": "POST",
+            "url": "https://www.example.org/",
+            "body": '{"$filters":"Filter!"}',
+        }
+        self._test_command(curl_command, expected_result)
+
     def test_explicit_get_with_data(self):
         curl_command = "curl httpbin.org/anything -X GET --data asdf"
         expected_result = {
Repository: scrapy/scrapy
Pull request #5960 (2023-06-23 02:59:26)

chore: Implement `pop` method on `BaseSettings` class
fixes: #5959


Patch:
diff --git a/scrapy/settings/__init__.py b/scrapy/settings/__init__.py
index a3b849f7b..8b3bdbabe 100644
--- a/scrapy/settings/__init__.py
+++ b/scrapy/settings/__init__.py
@@ -75,6 +75,8 @@ class BaseSettings(MutableMapping):
     highest priority will be retrieved.
     """
 
+    __default = object()
+
     def __init__(self, values=None, priority="project"):
         self.frozen = False
         self.attributes = {}
@@ -445,6 +447,18 @@ class BaseSettings(MutableMapping):
         else:
             p.text(pformat(self.copy_to_dict()))
 
+    def pop(self, name, default=__default):
+        try:
+            value = self.attributes[name]
+        except KeyError:
+            if default is self.__default:
+                raise
+
+            return SettingsAttribute(default, get_settings_priority("project"))
+        else:
+            self.__delitem__(name)
+            return value
+
 
 class Settings(BaseSettings):
     """


Test patch:
diff --git a/tests/test_settings/__init__.py b/tests/test_settings/__init__.py
index 4a577cd8c..bb6dc67fa 100644
--- a/tests/test_settings/__init__.py
+++ b/tests/test_settings/__init__.py
@@ -451,6 +451,35 @@ class SettingsTest(unittest.TestCase):
         self.assertIsInstance(myhandler_instance, FileDownloadHandler)
         self.assertTrue(hasattr(myhandler_instance, "download_request"))
 
+    def test_pop_item_with_default_value(self):
+        settings = Settings()
+
+        with self.assertRaises(KeyError):
+            settings.pop("DUMMY_CONFIG")
+
+        dummy_config = settings.pop("DUMMY_CONFIG", "dummy_value")
+
+        self.assertEqual(
+            repr(dummy_config), "<SettingsAttribute value='dummy_value' priority=20>"
+        )
+        self.assertEqual(dummy_config.value, "dummy_value")
+
+    def test_pop_item_with_immutable_settings(self):
+        settings = Settings(
+            {"DUMMY_CONFIG": "dummy_value", "OTHER_DUMMY_CONFIG": "other_dummy_value"}
+        )
+
+        self.assertEqual(settings.pop("DUMMY_CONFIG").value, "dummy_value")
+
+        settings.freeze()
+
+        with self.assertRaises(TypeError) as error:
+            settings.pop("OTHER_DUMMY_CONFIG")
+
+        self.assertEqual(
+            str(error.exception), "Trying to modify an immutable Settings object"
+        )
+
 
 if __name__ == "__main__":
     unittest.main()
Repository: scrapy/scrapy
Pull request #6036 (2023-09-01 14:52:43)

Fix enabled addons not being logged


Patch:
diff --git a/scrapy/addons.py b/scrapy/addons.py
index 02dd4fde8..2634bf907 100644
--- a/scrapy/addons.py
+++ b/scrapy/addons.py
@@ -29,7 +29,6 @@ class AddonManager:
             which to read the add-on configuration
         :type settings: :class:`~scrapy.settings.Settings`
         """
-        enabled: List[Any] = []
         for clspath in build_component_list(settings["ADDONS"]):
             try:
                 addoncls = load_object(clspath)
@@ -48,7 +47,7 @@ class AddonManager:
         logger.info(
             "Enabled addons:\n%(addons)s",
             {
-                "addons": enabled,
+                "addons": self.addons,
             },
             extra={"crawler": self.crawler},
         )


Test patch:
diff --git a/tests/test_addons.py b/tests/test_addons.py
index 5d053ed52..b7cac5039 100644
--- a/tests/test_addons.py
+++ b/tests/test_addons.py
@@ -1,6 +1,7 @@
 import itertools
 import unittest
 from typing import Any, Dict
+from unittest.mock import patch
 
 from scrapy import Spider
 from scrapy.crawler import Crawler, CrawlerRunner
@@ -156,3 +157,22 @@ class AddonManagerTest(unittest.TestCase):
             crawler.settings.getwithbase("DOWNLOAD_HANDLERS")["https"], "AddonHandler"
         )
         self.assertEqual(crawler.settings.get(FALLBACK_SETTING), "UserHandler")
+
+    def test_logging_message(self):
+        class LoggedAddon:
+            def update_settings(self, settings):
+                pass
+
+        with patch("scrapy.addons.logger") as logger_mock:
+            with patch("scrapy.addons.create_instance") as create_instance_mock:
+                settings_dict = {
+                    "ADDONS": {LoggedAddon: 1},
+                }
+                addon = LoggedAddon()
+                create_instance_mock.return_value = addon
+                crawler = get_crawler(settings_dict=settings_dict)
+                logger_mock.info.assert_called_once_with(
+                    "Enabled addons:\n%(addons)s",
+                    {"addons": [addon]},
+                    extra={"crawler": crawler},
+                )
Repository: scrapy/scrapy
Pull request #6310 (2024-04-08 07:01:37)

Fix WrappedRequest.get_header raising TypeError if default is None
Fixes https://github.com/scrapy/scrapy/issues/6308 

Alternatives: 

```python
    def get_header(self, name, default=None):
        value = self.request.headers.get(name, default)
        if not value:
            return value
        return to_unicode(value, errors="replace")
 ```
 
 But I don't like repeating value so much 
 
 ```python
    def get_header(self, name, default=None):
        if not (value := self.request.headers.get(name, default)):
            return value
        return to_unicode(value, errors="replace")
 ```
 The parenthesis and walrus operator make the line too convoluted
 
 ```python
     def get_header(self, name, default=None):
         try: 
             to_unicode(self.request.headers.get(name, default), errors="replace")
         except TypeError:
             return default
 ```
Using try/except this way is pythonic, but I'm always afraid I may unexpectedly capture another unrelated exception


Patch:
diff --git a/scrapy/http/cookies.py b/scrapy/http/cookies.py
index 2595f328e..72855bad5 100644
--- a/scrapy/http/cookies.py
+++ b/scrapy/http/cookies.py
@@ -170,7 +170,8 @@ class WrappedRequest:
         return name in self.request.headers
 
     def get_header(self, name, default=None):
-        return to_unicode(self.request.headers.get(name, default), errors="replace")
+        value = self.request.headers.get(name, default)
+        return to_unicode(value, errors="replace") if value is not None else None
 
     def header_items(self):
         return [


Test patch:
diff --git a/tests/test_http_cookies.py b/tests/test_http_cookies.py
index 8b5554914..932644320 100644
--- a/tests/test_http_cookies.py
+++ b/tests/test_http_cookies.py
@@ -43,6 +43,13 @@ class WrappedRequestTest(TestCase):
     def test_get_header(self):
         self.assertEqual(self.wrapped.get_header("content-type"), "text/html")
         self.assertEqual(self.wrapped.get_header("xxxxx", "def"), "def")
+        self.assertEqual(self.wrapped.get_header("xxxxx"), None)
+        wrapped = WrappedRequest(
+            Request(
+                "http://www.example.com/page.html", headers={"empty-binary-header": b""}
+            )
+        )
+        self.assertEqual(wrapped.get_header("empty-binary-header"), "")
 
     def test_header_items(self):
         self.assertEqual(self.wrapped.header_items(), [("Content-Type", ["text/html"])])