Skip to content

Relax Proxy-Authorization restrictions #5626

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Oct 26, 2022

Conversation

Gallaecio
Copy link
Member

@Gallaecio Gallaecio commented Sep 14, 2022

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:

  • Manually verify that old versions of scrapy-zyte-smartproxy work again.
  • Check if docs need changes.

@Gallaecio Gallaecio requested review from kmike and wRAR September 14, 2022 12:53
@codecov
Copy link

codecov bot commented Sep 14, 2022

Codecov Report

Merging #5626 (46dd152) into master (92be5ba) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #5626   +/-   ##
=======================================
  Coverage   88.66%   88.66%           
=======================================
  Files         162      162           
  Lines       10995    10997    +2     
  Branches     1799     1800    +1     
=======================================
+ Hits         9749     9751    +2     
  Misses        966      966           
  Partials      280      280           
Impacted Files Coverage Δ
scrapy/downloadermiddlewares/httpproxy.py 96.61% <100.00%> (+0.11%) ⬆️

@wRAR
Copy link
Member

wRAR commented Oct 13, 2022

Should this go into 2.7?

@Gallaecio
Copy link
Member Author

Verified that a simple spider hangs with pip install scrapy 'scrapy-zyte-smartproxy<2.2' but works after doing pip install git+https://github.com/Gallaecio/scrapy.git@more-lenient-proxying.

Since the only mention of Proxy-Authentication is in the changelog, I have decided against making any documentation changes. It is better to discourage Proxy-Authentication usage, even if it will now work for additional scenarios. A mention of this in the changelog should be enough.

@Gallaecio Gallaecio added this to the Scrapy 2.8 milestone Oct 26, 2022
@kmike
Copy link
Member

kmike commented Oct 26, 2022

Thanks @Gallaecio!

What do you think about making a 2.7.1 release, and not waiting for 2.8 to release this fix? It seems there are some other small fixes in master. The current set of merged PRs, + maybe #5689?

@kmike kmike merged commit a1075b8 into scrapy:master Oct 26, 2022
@Gallaecio
Copy link
Member Author

Sounds good to me!

@Gallaecio Gallaecio modified the milestones: Scrapy 2.8, Scrapy 2.7.1 Oct 26, 2022
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.

3 participants