Skip to content
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

Cleanup METHOD_SSLv3 #5634

Merged
merged 1 commit into from
Sep 26, 2022
Merged

Cleanup METHOD_SSLv3 #5634

merged 1 commit into from
Sep 26, 2022

Conversation

gliptak
Copy link
Contributor

@gliptak gliptak commented Sep 25, 2022

Signed-off-by: Gábor Lipták gliptak@gmail.com

docs generation is not running for PRs?

Fixes #5632, fixes #5635.

@codecov
Copy link

codecov bot commented Sep 25, 2022

Codecov Report

Merging #5634 (79a4bc3) into master (14b1565) will increase coverage by 0.44%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #5634      +/-   ##
==========================================
+ Coverage   88.40%   88.85%   +0.44%     
==========================================
  Files         162      162              
  Lines       10850    10964     +114     
  Branches     1861     1894      +33     
==========================================
+ Hits         9592     9742     +150     
+ Misses        975      942      -33     
+ Partials      283      280       -3     
Impacted Files Coverage Δ
scrapy/core/downloader/contextfactory.py 87.03% <ø> (+0.24%) ⬆️
scrapy/core/downloader/tls.py 97.05% <ø> (-0.09%) ⬇️
scrapy/utils/decorators.py 85.71% <0.00%> (-2.29%) ⬇️
scrapy/contracts/__init__.py 83.59% <0.00%> (-0.41%) ⬇️
scrapy/extension.py 100.00% <0.00%> (ø)
scrapy/spiderloader.py 100.00% <0.00%> (ø)
scrapy/commands/check.py 71.01% <0.00%> (ø)
scrapy/spiders/__init__.py 100.00% <0.00%> (ø)
scrapy/http/response/text.py 100.00% <0.00%> (ø)
scrapy/extensions/logstats.py 100.00% <0.00%> (ø)
... and 62 more

@Gallaecio
Copy link
Member

We need to update https://docs.scrapy.org/en/latest/topics/settings.html#downloader-client-tls-method as well.

Signed-off-by: Gábor Lipták <gliptak@gmail.com>
@gliptak
Copy link
Contributor Author

gliptak commented Sep 25, 2022

@Gallaecio updated

@kmike
Copy link
Member

kmike commented Sep 26, 2022

Hey! In install_requires we list pyOpenSSL 21.0 as a minimum requirement; should we support SSLv3 if older pyOpenSSL is installed? I.e. do getattr instead of removing the support completely?

@Gallaecio @wRAR what do you think about backporting the fix (e.g. for 2.6.3)?

@Gallaecio
Copy link
Member

Gallaecio commented Sep 26, 2022

Since web browsers only support TLS 1.2+, and SSLv3 is worse than TLS 1.0, I don’t think it is a big deal. In fact, I would be OK with dropping TLS 1.0 and 1.1 as well from there, just as the latest Twisted does, based on the argument that we aim to replicate browser behavior.

Not a strong opinion, though. @wRAR?

About backporting the fix, +1, since 2.7 may not be released for a while, and pip install Scrapy right now gives you a broken Scrapy, requiring a workaround.

@kmike
Copy link
Member

kmike commented Sep 26, 2022

Ok, let's kill SSLv3 :) Thanks for the help @gliptak!

@kmike kmike merged commit 07b079d into scrapy:master Sep 26, 2022
@gliptak gliptak deleted the tls1 branch September 26, 2022 16:54
@xhMiao
Copy link

xhMiao commented Sep 27, 2022

how to kill SSLv3?

@kmike
Copy link
Member

kmike commented Sep 27, 2022

It's killed in master @xhMiao; we're also going to do 2.6.3 release (#5637), but it's not released yet. For now you can pin pyOpenSSL version, as described in #5635 (comment).

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.

SSLv3_METHOD where did you come from???????????????
4 participants