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

GH-113214: Fix SSLProto exception handling in SSL-over-SSL scenarios #113334

Merged
merged 2 commits into from Dec 20, 2023

Conversation

mjpieters
Copy link
Contributor

@mjpieters mjpieters commented Dec 20, 2023

When wrapped, _SSLProtocolTransport._force_close(exc) is called just like in the unwrapped scenario _SelectorTransport._force_close(exc) or _ProactorBasePipeTransport._force_close(exc) would be called, except here the exception needs to be passed through the SSLProtocol._abort() method, which didn't accept an exception object.

This commit ensures that this path works, in the same way that the uvloop implementation of SSLProto passes on the exception (on which the current implementation of SSLProto is based).

Copy link
Member

@gvanrossum gvanrossum left a comment

Choose a reason for hiding this comment

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

This looks great except a comment nit. Once you fix that I'll merge, and full speed ahead on the 3.12 and 3.11 backports.

Lib/test/test_asyncio/test_sslproto.py Outdated Show resolved Hide resolved
@gvanrossum gvanrossum added needs backport to 3.11 only security fixes needs backport to 3.12 bug and security fixes labels Dec 20, 2023
mjpieters and others added 2 commits December 20, 2023 22:12
When wrapped, `_SSLProtocolTransport._force_close(exc)` is called just like in the unwrapped scenario `_SelectorTransport._force_close(exc)` or `_ProactorBasePipeTransport._force_close(exc)` would be called, except here the exception needs to be passed through the `SSLProtocol._abort()` method, which didn't accept an exception object.

This commit ensures that this path works, in the same way that the uvloop implementation of SSLProto passes on the exception (on which the current implementation of SSLProto is based).
Copy link
Member

@gvanrossum gvanrossum left a comment

Choose a reason for hiding this comment

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

Thanks! Merging now, hopefully the backports will be uneventful (but boy do they generate a lot of notification email :-).

PS. Next time please do not force-push -- it makes the review harder. We squash-merge into main anyway.

@gvanrossum gvanrossum merged commit 1ff0238 into python:main Dec 20, 2023
31 checks passed
@miss-islington-app
Copy link

Thanks @mjpieters for the PR, and @gvanrossum for merging it 🌮🎉.. I'm working now to backport this PR to: 3.11, 3.12.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Dec 20, 2023
…arios (pythonGH-113334)

When wrapped, `_SSLProtocolTransport._force_close(exc)` is called just like in the unwrapped scenario `_SelectorTransport._force_close(exc)` or `_ProactorBasePipeTransport._force_close(exc)` would be called, except here the exception needs to be passed through the `SSLProtocol._abort()` method, which didn't accept an exception object.

This commit ensures that this path works, in the same way that the uvloop implementation of SSLProto passes on the exception (on which the current implementation of SSLProto is based).
(cherry picked from commit 1ff0238)

Co-authored-by: Martijn Pieters <mj@zopatista.com>
@bedevere-app
Copy link

bedevere-app bot commented Dec 20, 2023

GH-113339 is a backport of this pull request to the 3.12 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.12 bug and security fixes label Dec 20, 2023
@bedevere-app
Copy link

bedevere-app bot commented Dec 20, 2023

GH-113340 is a backport of this pull request to the 3.11 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Dec 20, 2023
…arios (pythonGH-113334)

When wrapped, `_SSLProtocolTransport._force_close(exc)` is called just like in the unwrapped scenario `_SelectorTransport._force_close(exc)` or `_ProactorBasePipeTransport._force_close(exc)` would be called, except here the exception needs to be passed through the `SSLProtocol._abort()` method, which didn't accept an exception object.

This commit ensures that this path works, in the same way that the uvloop implementation of SSLProto passes on the exception (on which the current implementation of SSLProto is based).
(cherry picked from commit 1ff0238)

Co-authored-by: Martijn Pieters <mj@zopatista.com>
@bedevere-app bedevere-app bot removed the needs backport to 3.11 only security fixes label Dec 20, 2023
@mjpieters mjpieters deleted the asyncio_fix_ssl_abort branch December 20, 2023 23:11
gvanrossum pushed a commit that referenced this pull request Dec 21, 2023
…narios (GH-113334) (#113339)

When wrapped, `_SSLProtocolTransport._force_close(exc)` is called just like in the unwrapped scenario `_SelectorTransport._force_close(exc)` or `_ProactorBasePipeTransport._force_close(exc)` would be called, except here the exception needs to be passed through the `SSLProtocol._abort()` method, which didn't accept an exception object.

This commit ensures that this path works, in the same way that the uvloop implementation of SSLProto passes on the exception (on which the current implementation of SSLProto is based).

(cherry picked from commit 1ff0238)

Co-authored-by: Martijn Pieters <mj@zopatista.com>
gvanrossum pushed a commit that referenced this pull request Dec 21, 2023
…narios (GH-113334) (#113340)

When wrapped, `_SSLProtocolTransport._force_close(exc)` is called just like in the unwrapped scenario `_SelectorTransport._force_close(exc)` or `_ProactorBasePipeTransport._force_close(exc)` would be called, except here the exception needs to be passed through the `SSLProtocol._abort()` method, which didn't accept an exception object.

This commit ensures that this path works, in the same way that the uvloop implementation of SSLProto passes on the exception (on which the current implementation of SSLProto is based).

(cherry picked from commit 1ff0238)

Co-authored-by: Martijn Pieters <mj@zopatista.com>
ryan-duve pushed a commit to ryan-duve/cpython that referenced this pull request Dec 26, 2023
…arios (python#113334)

When wrapped, `_SSLProtocolTransport._force_close(exc)` is called just like in the unwrapped scenario `_SelectorTransport._force_close(exc)` or `_ProactorBasePipeTransport._force_close(exc)` would be called, except here the exception needs to be passed through the `SSLProtocol._abort()` method, which didn't accept an exception object.

This commit ensures that this path works, in the same way that the uvloop implementation of SSLProto passes on the exception (on which the current implementation of SSLProto is based).
kulikjak pushed a commit to kulikjak/cpython that referenced this pull request Jan 22, 2024
…arios (python#113334)

When wrapped, `_SSLProtocolTransport._force_close(exc)` is called just like in the unwrapped scenario `_SelectorTransport._force_close(exc)` or `_ProactorBasePipeTransport._force_close(exc)` would be called, except here the exception needs to be passed through the `SSLProtocol._abort()` method, which didn't accept an exception object.

This commit ensures that this path works, in the same way that the uvloop implementation of SSLProto passes on the exception (on which the current implementation of SSLProto is based).
aisk pushed a commit to aisk/cpython that referenced this pull request Feb 11, 2024
…arios (python#113334)

When wrapped, `_SSLProtocolTransport._force_close(exc)` is called just like in the unwrapped scenario `_SelectorTransport._force_close(exc)` or `_ProactorBasePipeTransport._force_close(exc)` would be called, except here the exception needs to be passed through the `SSLProtocol._abort()` method, which didn't accept an exception object.

This commit ensures that this path works, in the same way that the uvloop implementation of SSLProto passes on the exception (on which the current implementation of SSLProto is based).
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