GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — GH-113214: Fix SSLProto exception handling in SSL-over-SSL scenarios by mjpieters · Pull Request #113334 · python/cpython

Conversation

@mjpieters

@mjpieters mjpieters commented Dec 20, 2023 •
edited by bedevere-app Bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

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).

  • Issue: asyncio.sslproto.SSLProtocol exception handling triggers TypeError: SSLProtocol._abort() takes 1 positional argument but 2 were given #113214

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@mjpieters
mjpieters requested review from 1st1, asvetlov, gvanrossum, kumaraditya303 and willingc as code owners December 20, 2023 21:16
@bedevere-app bedevere-app Bot added the awaiting review label Dec 20, 2023
@bedevere-app bedevere-app Bot mentioned this pull request Dec 20, 2023
asyncio.sslproto.SSLProtocol exception handling triggers TypeError: SSLProtocol._abort() takes 1 positional argument but 2 were given #113214
Closed
@mjpieters
mjpieters force-pushed the asyncio_fix_ssl_abort branch from c9d76d4 to d4bbdff Compare December 20, 2023 21:20
gvanrossum
gvanrossum reviewed Dec 20, 2023
View reviewed changes

@gvanrossum gvanrossum left a comment

Copy link
Copy Markdown
Member

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.

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

mjpieters reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
Comment thread Lib/test/test_asyncio/test_sslproto.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

@gvanrossum gvanrossum added needs backport to 3.11 only security fixes needs backport to 3.12 only security fixes labels Dec 20, 2023
mjpieters and others added 2 commits December 20, 2023 22:12
@mjpieters
Fix SSLProto exception handling in SSL-over-SSL scenarios
8049c8c
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).
@blurb-it @mjpieters
📜🤖 Added by blurb_it.
998504d
@mjpieters
mjpieters force-pushed the asyncio_fix_ssl_abort branch from d4bbdff to 998504d Compare December 20, 2023 22:12
@mjpieters
mjpieters requested a review from gvanrossum December 20, 2023 22:15
gvanrossum
gvanrossum approved these changes Dec 20, 2023
View reviewed changes

@gvanrossum gvanrossum left a comment

Copy link
Copy Markdown
Member

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.

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@bedevere-app bedevere-app Bot added awaiting merge and removed awaiting review labels Dec 20, 2023
@gvanrossum
gvanrossum merged commit 1ff0238 into python:main Dec 20, 2023
@miss-islington-app

miss-islington-app Bot commented Dec 20, 2023

Copy link
Copy Markdown

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-app bedevere-app Bot removed the awaiting merge label Dec 20, 2023
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Dec 20, 2023
@mjpieters @miss-islington
pythonGH-113214: Fix SSLProto exception handling in SSL-over-SSL scen…
309e699
…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 <[email protected]>
@bedevere-app

bedevere-app Bot commented Dec 20, 2023

Copy link
Copy Markdown

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-app bedevere-app Bot removed the needs backport to 3.12 only security fixes label Dec 20, 2023
@bedevere-app

bedevere-app Bot commented Dec 20, 2023

Copy link
Copy Markdown

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Dec 20, 2023
@mjpieters @miss-islington
pythonGH-113214: Fix SSLProto exception handling in SSL-over-SSL scen…
46cd200
…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 <[email protected]>
@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
@miss-islington @mjpieters
[3.12] GH-113214: Fix SSLProto exception handling in SSL-over-SSL sce…
1927118
…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 <[email protected]>
gvanrossum pushed a commit that referenced this pull request Dec 21, 2023
@miss-islington @mjpieters
[3.11] GH-113214: Fix SSLProto exception handling in SSL-over-SSL sce…
9cbe473
…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 <[email protected]>
ryan-duve pushed a commit to ryan-duve/cpython that referenced this pull request Dec 26, 2023
@mjpieters @ryan-duve
pythonGH-113214: Fix SSLProto exception handling in SSL-over-SSL scen…
87d091a
…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).
@ordinary-jamie ordinary-jamie mentioned this pull request Jan 15, 2024
gh-109534: fix reference leak when SSL handshake fails #114074
Merged
kulikjak pushed a commit to kulikjak/cpython that referenced this pull request Jan 22, 2024
@mjpieters @kulikjak
pythonGH-113214: Fix SSLProto exception handling in SSL-over-SSL scen…
1f55f09
…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
@mjpieters @aisk
pythonGH-113214: Fix SSLProto exception handling in SSL-over-SSL scen…
a807e2a
…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).
Glyphack pushed a commit to Glyphack/cpython that referenced this pull request Sep 2, 2024
@mjpieters @Glyphack
pythonGH-113214: Fix SSLProto exception handling in SSL-over-SSL scen…
f1eeeb6
…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

Reviewers

@gvanrossum gvanrossum gvanrossum approved these changes
@1st1 1st1 Awaiting requested review from 1st1
@asvetlov asvetlov Awaiting requested review from asvetlov
@kumaraditya303 kumaraditya303 Awaiting requested review from kumaraditya303
@willingc willingc Awaiting requested review from willingc

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

2 participants

@mjpieters @gvanrossum
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.