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

AIOHTTPTransport ssl=False by default #529

Open
mariuskava opened this issue Feb 17, 2025 · 5 comments
Open

AIOHTTPTransport ssl=False by default #529

mariuskava opened this issue Feb 17, 2025 · 5 comments
Labels
type: bug An issue or pull request relating to a bug

Comments

@mariuskava
Copy link

Common problems

  • TLS should be enabled by default. AIOHTTPTransport uses ssl=False by default
  • I was expecting library to fail when using self signed certificate on the server.

To Reproduce

  • start a server with a self signed certificate
  • try connect
  • it will succeed instead of failing

Expected behavior
Expect connection to fail when using not trusted certificates

System info (please complete the following information):

  • Any
@leszekhanusz
Copy link
Collaborator

That's pretty bad... Thanks for your report.

The annoying thing is that there are probably some people which are depending on this and changing the default will be a breaking change for them.

A major version bump will be needed to fix this.

@leszekhanusz leszekhanusz added the type: bug An issue or pull request relating to a bug label Feb 17, 2025
@mariuskava
Copy link
Author

yes, there must be a breaking change.. but a patch release could contain a warning:

  • default should be changed to ssl=None
  • if ssl==None: log warning and set ssl=False

@leszekhanusz
Copy link
Collaborator

yes, there must be a breaking change.. but a patch release could contain a warning:

Good idea!

@leszekhanusz
Copy link
Collaborator

leszekhanusz commented Feb 18, 2025

Note that for the warning, the default value cannot be None.

That's because there was a change in aiohttp from version 3.10

Before aiohttp 3.10, aiohttp was using None as the default value, and would verify the certification in that case.
From aiohttp 3.10, it switched to use True as the default value.

So, someone could have used ssl=None previously with AIOHTTPTransport. He would have a typing error with mypy but it would work and gql would verify the certificate. If we set ssl to False if we receive None, then gql will not verify the cert for him in that case.

We could use another default value, like the "warning" string?

@mariuskava
Copy link
Author

good point. Default string value sounds good enough or it could be a predefined sentinel = object()
https://python-patterns.guide/python/sentinel-object/#sentinel-objects

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug An issue or pull request relating to a bug
Projects
None yet
Development

No branches or pull requests

2 participants