Repository: scrapy/scrapy

Issue:
Scrapy warns about itself
I'm using Scrapy 2.6.2. Created a new project, run a spider, and there is 

```
2022-08-02 23:50:26 [py.warnings] WARNING: [redacted]/python3.8/site-packages/scrapy/core/engine.py:279: ScrapyDeprecationWarning: Passing a 'spider' argument to ExecutionEngine.download is deprecated
  return self.download(result, spider) if isinstance(result, Request) else result
```

message in logs. We also have the same warning in our testing suite, but I haven't realized it's not a test-only issue.

Discussion:
codecov[bot]: # [Codecov](https://codecov.io/gh/scrapy/scrapy/pull/5589?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=scrapy) Report
> Merging [#5589](https://codecov.io/gh/scrapy/scrapy/pull/5589?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=scrapy) (aefe6fe) into [master](https://codecov.io/gh/scrapy/scrapy/commit/52d93490f57d4b2284e3d95efa5c622ab8ad16cb?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=scrapy) (52d9349) will **decrease** coverage by `4.02%`.
> The diff coverage is `100.00%`.

```diff
@@            Coverage Diff             @@
##           master    #5589      +/-   ##
==========================================
- Coverage   88.85%   84.82%   -4.03%     
==========================================
  Files         162      162              
  Lines       10962    10962              
  Branches     1894     1894              
==========================================
- Hits         9740     9299     -441     
- Misses        942     1395     +453     
+ Partials      280      268      -12     
```

| [Impacted Files](https://codecov.io/gh/scrapy/scrapy/pull/5589?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=scrapy) | Coverage Δ | |
|---|---|---|
| [scrapy/core/engine.py](https://codecov.io/gh/scrapy/scrapy/pull/5589/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=scrapy#diff-c2NyYXB5L2NvcmUvZW5naW5lLnB5) | `83.95% <100.00%> (ø)` | |
| [scrapy/pipelines/images.py](https://codecov.io/gh/scrapy/scrapy/pull/5589/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=scrapy#diff-c2NyYXB5L3BpcGVsaW5lcy9pbWFnZXMucHk=) | `27.82% <0.00%> (-66.96%)` | :arrow_down: |
| [scrapy/core/http2/stream.py](https://codecov.io/gh/scrapy/scrapy/pull/5589/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=scrapy#diff-c2NyYXB5L2NvcmUvaHR0cDIvc3RyZWFtLnB5) | `27.01% <0.00%> (-64.37%)` | :arrow_down: |
| [scrapy/core/http2/agent.py](https://codecov.io/gh/scrapy/scrapy/pull/5589/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=scrapy#diff-c2NyYXB5L2NvcmUvaHR0cDIvYWdlbnQucHk=) | `36.14% <0.00%> (-60.25%)` | :arrow_down: |
| [scrapy/core/downloader/handlers/http2.py](https://codecov.io/gh/scrapy/scrapy/pull/5589/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=scrapy#diff-c2NyYXB5L2NvcmUvZG93bmxvYWRlci9oYW5kbGVycy9odHRwMi5weQ==) | `43.42% <0.00%> (-56.58%)` | :arrow_down: |
| [scrapy/core/http2/protocol.py](https://codecov.io/gh/scrapy/scrapy/pull/5589/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=scrapy#diff-c2NyYXB5L2NvcmUvaHR0cDIvcHJvdG9jb2wucHk=) | `34.17% <0.00%> (-49.25%)` | :arrow_down: |
| [scrapy/robotstxt.py](https://codecov.io/gh/scrapy/scrapy/pull/5589/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=scrapy#diff-c2NyYXB5L3JvYm90c3R4dC5weQ==) | `75.30% <0.00%> (-22.23%)` | :arrow_down: |
| [scrapy/utils/test.py](https://codecov.io/gh/scrapy/scrapy/pull/5589/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=scrapy#diff-c2NyYXB5L3V0aWxzL3Rlc3QucHk=) | `50.68% <0.00%> (-15.07%)` | :arrow_down: |
| [scrapy/core/downloader/contextfactory.py](https://codecov.io/gh/scrapy/scrapy/pull/5589/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=scrapy#diff-c2NyYXB5L2NvcmUvZG93bmxvYWRlci9jb250ZXh0ZmFjdG9yeS5weQ==) | `75.92% <0.00%> (-11.12%)` | :arrow_down: |
| [scrapy/pipelines/media.py](https://codecov.io/gh/scrapy/scrapy/pull/5589/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=scrapy#diff-c2NyYXB5L3BpcGVsaW5lcy9tZWRpYS5weQ==) | `92.90% <0.00%> (-5.68%)` | :arrow_down: |
| ... and [3 more](https://codecov.io/gh/scrapy/scrapy/pull/5589/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=scrapy) | |

Laerte: The error in CI is not related with the change made.
kmike: If I'm not mistaken, this parameter is deprecated in https://github.com/scrapy/scrapy/pull/5090; it was released with Scrapy 2.6.0 (https://docs.scrapy.org/en/latest/news.html#scrapy-2-6-0-2022-03-01), which happened 2022-03-01. According to https://docs.scrapy.org/en/latest/versioning.html#deprecation-policy, we promise at least one year before the deprecated code is removed, so unfortunately we can't remove it now.
Gallaecio: Instead, we need to capture and silence that warning when triggered by our own code.
Laerte: @Gallaecio I will update the code to this approach.
Gallaecio: I would like to first understand why it is happening in the first place, though. Maybe there is a better way.
Gallaecio: OK, so I think we need to change `download` so that:
* `spider = self.spider` never happens.
* `if spider is None` changes to `if self.spider is None`.

Hopefully that solves the issue.
Laerte: @Gallaecio Hope i getted it the idea.
Gallaecio: Looks good to me, thanks! :rocket:

How do you feel about test coverage? Do you think you can add at least a test to verify that the exception is not logged with default settings? If you do not see it clearly, I can try and have a look later.
Laerte: > Looks good to me, thanks! 🚀
> 
> How do you feel about test coverage? Do you think you can add at least a test to verify that the exception is not logged with default settings? If you do not see it clearly, I can try and have a look later.

I'm vacations, if you can add tests i will be glad, thanks!

Patch:
diff --git a/scrapy/core/engine.py b/scrapy/core/engine.py
index 6602f661d..1228e78da 100644
--- a/scrapy/core/engine.py
+++ b/scrapy/core/engine.py
@@ -257,9 +257,7 @@ class ExecutionEngine:
 
     def download(self, request: Request, spider: Optional[Spider] = None) -> Deferred:
         """Return a Deferred which fires with a Response as result, only downloader middlewares are applied"""
-        if spider is None:
-            spider = self.spider
-        else:
+        if spider is not None:
             warnings.warn(
                 "Passing a 'spider' argument to ExecutionEngine.download is deprecated",
                 category=ScrapyDeprecationWarning,
@@ -267,7 +265,7 @@ class ExecutionEngine:
             )
             if spider is not self.spider:
                 logger.warning("The spider '%s' does not match the open spider", spider.name)
-        if spider is None:
+        if self.spider is None:
             raise RuntimeError(f"No open spider to crawl: {request}")
         return self._download(request, spider).addBoth(self._downloaded, request, spider)
 
@@ -278,11 +276,14 @@ class ExecutionEngine:
         self.slot.remove_request(request)
         return self.download(result, spider) if isinstance(result, Request) else result
 
-    def _download(self, request: Request, spider: Spider) -> Deferred:
+    def _download(self, request: Request, spider: Optional[Spider]) -> Deferred:
         assert self.slot is not None  # typing
 
         self.slot.add_request(request)
 
+        if spider is None:
+            spider = self.spider
+
         def _on_success(result: Union[Response, Request]) -> Union[Response, Request]:
             if not isinstance(result, (Response, Request)):
                 raise TypeError(f"Incorrect type: expected Response or Request, got {type(result)}: {result!r}")
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()