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

handle urllib 2.0 changes #51

Merged
merged 2 commits into from
Jun 2, 2023
Merged

Conversation

greg12000
Copy link
Contributor

HI,

urllib3 >= 2.0 doesn't define urllib3.util.ssl_.DEFAULT_CIPHERS anymore and this creates an import error in the current gpsoauth code.

This change checks the version and removes the import when not needed

The removal is documented here: https://urllib3.readthedocs.io/en/stable/changelog.html#removed
"Removed DEFAULT_CIPHERS, HAS_SNI, USE_DEFAULT_SSLCONTEXT_CIPHERS, from the private module urllib3.util.ssl_ (#2168)."

I ran the pre-commit before submiting. One warning related to the google import was still there but I didn't fix it as it wasn't related to teh same issue.

Best,

@simon-weber
Copy link
Owner

Thanks! Were you able to auth before and after this change? I'd want to check that Google accepts whatever the new ciphers end up being.

@greg12000
Copy link
Contributor Author

greg12000 commented May 28, 2023

I could auth before using urllib < 2 and after with urllib 2.0 + patch.
With urllib3 > 2.0 before the patch I had this error:

Username or password is incorrect (AttributeError("module 'urllib3.util.ssl_' has no attribute 'DEFAULT_CIPHERS'"))

Which is caused by DEFAULT_CIPHERS being removed from urllib3.ssl_ after version 2.0.

On my host (mac OS) the default system ciphers worked.
I haven't checked other systems

@greg12000
Copy link
Contributor Author

another solution is to stick with urllib 1.26.x otherwise in the poetry dependencies.

@simon-weber
Copy link
Owner

There's another issue with urllib3 in a child dependency, so I just constrained it for now: edbba0d

@simon-weber simon-weber closed this Jun 2, 2023
@simon-weber simon-weber reopened this Jun 2, 2023
@simon-weber
Copy link
Owner

I guess we should merge this too for when we remove the constraint. I'll just rerun the formatting on master.

@simon-weber simon-weber merged commit d0e770e into simon-weber:master Jun 2, 2023
0 of 4 checks passed
@simon-weber
Copy link
Owner

Thanks!

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.

None yet

2 participants