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

migrate trio.ssl from star import to explicit imports, to aid static analysis issue #542 #752

Merged
merged 18 commits into from
Dec 13, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 0 additions & 47 deletions trio/_ssl.py
Original file line number Diff line number Diff line change
Expand Up @@ -159,53 +159,6 @@
from . import _sync
from ._util import ConflictDetector

__all__ = ["SSLStream", "SSLListener"]

################################################################
# Faking the stdlib ssl API
################################################################


def _reexport(name):
try:
value = getattr(_stdlib_ssl, name)
except AttributeError:
pass
else:
globals()[name] = value
__all__.append(name)


# Intentionally not re-exported:
# SSLContext
for _name in [
"SSLError",
"SSLZeroReturnError",
"SSLSyscallError",
"SSLEOFError",
"CertificateError",
"create_default_context",
"match_hostname",
"cert_time_to_seconds",
"DER_cert_to_PEM_cert",
"PEM_cert_to_DER_cert",
"get_default_verify_paths",
"Purpose",
"enum_certificates",
"enum_crls",
"SSLSession",
"VerifyMode",
"VerifyFlags",
"Options",
"AlertDescription",
"SSLErrorNumber",
]:
_reexport(_name)

for _name in _stdlib_ssl.__dict__.keys():
if _name == _name.upper():
_reexport(_name)
Copy link
Member

Choose a reason for hiding this comment

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

This code is like trio.socket: it re-exports all the uppercased symbols. So we need some similar handling: a fake try/import, plus moving this loop into trio/ssl.py. With this PR right now it looks like we aren't re-exporting the uppercased symbols at all?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ups, I overlooked this totally, yes of course, this has to be addressed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

They are now imported, but see my comment below about external dependencies like ssl library and version.


################################################################
# SSLStream
################################################################
Expand Down
95 changes: 93 additions & 2 deletions trio/ssl.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,96 @@
# temporaries, imports, etc. when implementing the module. So we put the
# implementation in an underscored module, and then re-export the public parts
# here.
from ._ssl import *
from ._ssl import __all__

# Trio-specific symbols:
from ._ssl import (SSLStream, SSLListener)

# Symbols re-exported from the stdlib ssl module:

# Always available
from ssl import (
cert_time_to_seconds, CertificateError, create_default_context,
DER_cert_to_PEM_cert, get_default_verify_paths, match_hostname,
PEM_cert_to_DER_cert, Purpose, SSLEOFError, SSLError, SSLSyscallError,
SSLZeroReturnError
)

# Added in python 3.6
try:
from ssl import AlertDescription, SSLErrorNumber, SSLSession, VerifyFlags, VerifyMode, Options
except ImportError:
pass

# Added in python 3.7
try:
from ssl import SSLCertVerificationError, TLSVersion
except ImportError:
pass

# Windows-only
try:
from ssl import enum_certificates, enum_crls
except ImportError:
pass

# Fake import to enable static analysis tools to catch the names
# (Real import is below)
try:
from ssl import (
AF_INET, ALERT_DESCRIPTION_ACCESS_DENIED,
ALERT_DESCRIPTION_BAD_CERTIFICATE_HASH_VALUE,
ALERT_DESCRIPTION_BAD_CERTIFICATE_STATUS_RESPONSE,
ALERT_DESCRIPTION_BAD_CERTIFICATE, ALERT_DESCRIPTION_BAD_RECORD_MAC,
ALERT_DESCRIPTION_CERTIFICATE_EXPIRED,
ALERT_DESCRIPTION_CERTIFICATE_REVOKED,
ALERT_DESCRIPTION_CERTIFICATE_UNKNOWN,
ALERT_DESCRIPTION_CERTIFICATE_UNOBTAINABLE,
ALERT_DESCRIPTION_CLOSE_NOTIFY, ALERT_DESCRIPTION_DECODE_ERROR,
ALERT_DESCRIPTION_DECOMPRESSION_FAILURE,
ALERT_DESCRIPTION_DECRYPT_ERROR, ALERT_DESCRIPTION_HANDSHAKE_FAILURE,
ALERT_DESCRIPTION_ILLEGAL_PARAMETER,
ALERT_DESCRIPTION_INSUFFICIENT_SECURITY,
ALERT_DESCRIPTION_INTERNAL_ERROR, ALERT_DESCRIPTION_NO_RENEGOTIATION,
ALERT_DESCRIPTION_PROTOCOL_VERSION, ALERT_DESCRIPTION_RECORD_OVERFLOW,
ALERT_DESCRIPTION_UNEXPECTED_MESSAGE, ALERT_DESCRIPTION_UNKNOWN_CA,
ALERT_DESCRIPTION_UNKNOWN_PSK_IDENTITY,
ALERT_DESCRIPTION_UNRECOGNIZED_NAME,
ALERT_DESCRIPTION_UNSUPPORTED_CERTIFICATE,
ALERT_DESCRIPTION_UNSUPPORTED_EXTENSION,
ALERT_DESCRIPTION_USER_CANCELLED, CERT_NONE, CERT_OPTIONAL,
CERT_REQUIRED, CHANNEL_BINDING_TYPES, HAS_ALPN, HAS_ECDH,
HAS_NEVER_CHECK_COMMON_NAME, HAS_NPN, HAS_SNI, OP_ALL,
OP_ALLOW_UNSAFE_LEGACY_RENEGOTIATION, OP_COOKIE_EXCHANGE,
OP_DONT_INSERT_EMPTY_FRAGMENTS, OP_EPHEMERAL_RSA,
OP_LEGACY_SERVER_CONNECT, OP_MICROSOFT_BIG_SSLV3_BUFFER,
OP_MICROSOFT_SESS_ID_BUG, OP_MSIE_SSLV2_RSA_PADDING,
OP_NETSCAPE_CA_DN_BUG, OP_NETSCAPE_CHALLENGE_BUG,
OP_NETSCAPE_DEMO_CIPHER_CHANGE_BUG,
OP_NETSCAPE_REUSE_CIPHER_CHANGE_BUG, OP_NO_QUERY_MTU, OP_PKCS1_CHECK_1,
OP_PKCS1_CHECK_2, OP_SSLEAY_080_CLIENT_DH_BUG,
OP_SSLREF2_REUSE_CERT_TYPE_BUG, OP_TLS_BLOCK_PADDING_BUG,
OP_TLS_D5_BUG, OP_TLS_ROLLBACK_BUG, SSL_ERROR_NONE,
SSL_ERROR_NO_SOCKET, OP_CIPHER_SERVER_PREFERENCE, OP_NO_COMPRESSION,
OP_NO_RENEGOTIATION, OP_NO_TICKET, OP_SINGLE_DH_USE,
OP_SINGLE_ECDH_USE, OPENSSL_VERSION_INFO, OPENSSL_VERSION_NUMBER,
OPENSSL_VERSION, PEM_FOOTER, PEM_HEADER, PROTOCOL_TLS_CLIENT,
PROTOCOL_TLS_SERVER, PROTOCOL_TLS, SO_TYPE, SOCK_STREAM, SOL_SOCKET,
SSL_ERROR_EOF, SSL_ERROR_INVALID_ERROR_CODE, SSL_ERROR_SSL,
SSL_ERROR_SYSCALL, SSL_ERROR_WANT_CONNECT, SSL_ERROR_WANT_READ,
SSL_ERROR_WANT_WRITE, SSL_ERROR_WANT_X509_LOOKUP,
SSL_ERROR_ZERO_RETURN, VERIFY_CRL_CHECK_CHAIN, VERIFY_CRL_CHECK_LEAF,
VERIFY_DEFAULT, VERIFY_X509_STRICT, VERIFY_X509_TRUSTED_FIRST
)
Copy link
Member

Choose a reason for hiding this comment

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

These don't need to be fake! It's a well-defined set of things that should always be available. So we can just do a regular old import without any try/except.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Some actually don't but others do. I tried to import them directly but the test failed on some versions of python and with windows. I decided to revert back to being them all fake since doing a subset one way and the rest the other is more confusing and IMHO not worth the extra lines of code. Is this fine with you?

except ImportError:
pass

# Dynamically re-export whatever constants this particular Python happens to
# have:
import ssl as _stdlib_ssl
globals().update(
{
_name: getattr(_stdlib_ssl, _name)
for _name in _stdlib_ssl.__dict__.keys()
if _name.isupper() and not _name.startswith('_')
}
)
Copy link
Member

Choose a reason for hiding this comment

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

Once we switch to a regular import above, we can delete this.

1 change: 0 additions & 1 deletion trio/tests/test_exports.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ def public_namespaces(module):

# Not yet set up for static analysis:
NAMESPACES.remove("trio.hazmat")
NAMESPACES.remove("trio.ssl")


# pylint/jedi often have trouble with alpha releases, where Python's internals
Expand Down
4 changes: 2 additions & 2 deletions trio/tests/test_ssl.py
Original file line number Diff line number Diff line change
Expand Up @@ -336,10 +336,10 @@ def ssl_lockstep_stream_pair(**kwargs):
def test_exports():
# Just a quick check to make sure _reexport isn't totally broken
assert hasattr(tssl, "SSLError")
assert "SSLError" in tssl.__all__
assert "SSLError" in tssl.__dict__.keys()

assert hasattr(tssl, "Purpose")
assert "Purpose" in tssl.__all__
assert "Purpose" in tssl.__dict__.keys()

# Intentionally omitted
assert not hasattr(tssl, "SSLContext")
Expand Down