Skip to content

Match pyOpenSSL and service_identity to Twisted #5632

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 1 commit into from
Sep 25, 2022
Merged

Match pyOpenSSL and service_identity to Twisted #5632

merged 1 commit into from
Sep 25, 2022

Conversation

gliptak
Copy link
Contributor

@gliptak gliptak commented Sep 25, 2022

Resolves #5621

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

Correcting https://github.com/scrapy/scrapy/actions/runs/3052958225/jobs/4922962575

    from twisted.internet import error, tcp, udp
.tox/py/lib/python3.8/site-packages/twisted/internet/tcp.py:38: in <module>
    from twisted.internet._newtls import (
.tox/py/lib/python3.8/site-packages/twisted/internet/_newtls.py:18: in <module>
    from twisted.protocols.tls import TLSMemoryBIOFactory, TLSMemoryBIOProtocol
.tox/py/lib/python3.8/site-packages/twisted/protocols/tls.py:45: in <module>
    from twisted.internet._sslverify import _setAcceptableProtocols
.tox/py/lib/python3.8/site-packages/twisted/internet/_sslverify.py:1828: in <module>
    defaultCiphers = OpenSSLAcceptableCiphers.fromOpenSSLCipherString(
.tox/py/lib/python3.8/site-packages/twisted/internet/_sslverify.py:1807: in fromOpenSSLCipherString
    SSL.TLS_METHOD,
E   AttributeError: module 'OpenSSL.SSL' has no attribute 'TLS_METHOD'

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

codecov bot commented Sep 25, 2022

Codecov Report

Merging #5632 (385acd5) into master (5f19420) will increase coverage by 0.19%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #5632      +/-   ##
==========================================
+ Coverage   88.66%   88.85%   +0.19%     
==========================================
  Files         162      162              
  Lines       10965    10965              
  Branches     1894     1894              
==========================================
+ Hits         9722     9743      +21     
+ Misses        963      942      -21     
  Partials      280      280              
Impacted Files Coverage Δ
scrapy/core/downloader/handlers/http11.py 94.11% <0.00%> (+0.98%) ⬆️
scrapy/robotstxt.py 97.53% <0.00%> (+22.22%) ⬆️

Copy link
Member

@Gallaecio Gallaecio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work!

@wRAR I would like to hear from you if you think there is some other approach we should explore here. But I did not realize upgrading some dependencies would do the job, and I am more than fine with this approach.

@@ -327,7 +327,7 @@ def test_reactor_default(self):

def test_reactor_default_twisted_reactor_select(self):
log = self.run_script('reactor_default_twisted_reactor_select.py')
if platform.system() == 'Windows':
if platform.system() in ['Windows', 'Darwin']:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hang on in there, Linux!

@Gallaecio
Copy link
Member

Gallaecio commented Sep 25, 2022

@wRAR I would like to hear from you if you think there is some other approach we should explore here.

Nevermind, I believe you were already aware of this, we were just deciding whether the side effect (not supporting TLS ≤1.1) was OK.

@kmike
Copy link
Member

kmike commented Sep 25, 2022

Thanks @gliptak!

@kmike kmike merged commit 14b1565 into scrapy:master Sep 25, 2022
@kmike
Copy link
Member

kmike commented Sep 25, 2022

It seems tests are failing now with a different error, weird! See e.g. https://github.com/scrapy/scrapy/actions/runs/3123148006/jobs/5065580786 - there's a lot of errors like this:

___________________ ERROR collecting tests/test_webclient.py ___________________
tests/test_webclient.py:23: in
from scrapy.core.downloader.contextfactory import ScrapyClientContextFactory
scrapy/core/downloader/contextfactory.py:11: in
from scrapy.core.downloader.tls import DEFAULT_CIPHERS, openssl_methods, ScrapyClientTLSOptions
scrapy/core/downloader/tls.py:23: in
METHOD_SSLv3: SSL.SSLv3_METHOD, # SSL 3 (NOT recommended)
E AttributeError: module 'OpenSSL.SSL' has no attribute 'SSLv3_METHOD'

@gliptak gliptak mentioned this pull request Sep 25, 2022
@Gallaecio
Copy link
Member

weird!

It is your fault, you should have merged a few minutes earlier! https://pypi.org/project/pyOpenSSL/#history 🤦

@gliptak gliptak deleted the ssl1 branch September 25, 2022 21:10
@kmike kmike mentioned this pull request Sep 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.

Support for Twisted 22.8.0
3 participants