-
-
Notifications
You must be signed in to change notification settings - Fork 30
Use correct port for wss URLs when ssl_context is None #197
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
Conversation
Signed-off-by: Sam Friedman <sam@golioth.io>
trio-websocket v0.12 introduced a bug that causes the library to use port 80 for wss urls instead of port 443. Details are in the PR: python-trio/trio-websocket#197 Pin the trio-websocket dependency to v0.11.1 until the fix is merged and released. Signed-off-by: Sam Friedman <sam@golioth.io>
trio-websocket v0.12 introduced a bug that causes the library to use port 80 for wss urls instead of port 443. Details are in the PR: python-trio/trio-websocket#197 Pin the trio-websocket dependency to v0.11.1 until the fix is merged and released. Signed-off-by: Sam Friedman <sam@golioth.io>
trio-websocket v0.12 introduced a bug that causes the library to use port 80 for wss urls instead of port 443. Details are in the PR: python-trio/trio-websocket#197 Pin the trio-websocket dependency to v0.11.1 until the fix is merged and released. Signed-off-by: Sam Friedman <sam@golioth.io>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense to me. Not sure why we didn't just enable redefinitions given that would have avoided this (I think).
I'm a bit curious why test cases didn't fail.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🙄
Hey @sam-golioth, it looks like that was the first time we merged one of your PRs! Thanks so much! 🎉 🎂 If you want to keep contributing, we'd love to have you. So, I just sent you an invitation to join the python-trio organization on Github! If you accept, then here's what will happen:
If you want to read more, here's the relevant section in our contributing guide. Alternatively, you're free to decline or ignore the invitation. You'll still be able to contribute as much or as little as you like, and I won't hassle you about joining again. But if you ever change your mind, just let us know and we'll send another invitation. We'd love to have you, but more importantly we want you to do whatever's best for you. If you have any questions, well... I am just a humble Python script, so I probably can't help. But please do post a comment here, or in our chat, or on our forum, whatever's easiest, and someone will help you out! |
Prior to 49b93c1,
_url_to_host()
modifiedssl_context
according to whether the URL containedws
orwss
. That commit introducedreturn_ssl_context
because the return value has a different type set (ssl.SSLContext | bool
) than the passed in argument (ssl.SSLContext | None
). But the determination of which port number to use was still based onssl_context
, which would result in port 80 being used in the case where the user passed in awss://
URL without an SSLContext. Checkingreturn_ssl_context
instead results in the correct behavior.