8000 bpo-34975: Add start_tls() method to streams API by icgood · Pull Request #13143 · python/cpython · GitHub
[go: up one dir, main page]
Skip to content

bpo-34975: Add start_tls() method to streams API #13143

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

bpo-34975: Add start_tls() method to streams API #13143

wants to merge 1 commit into from

Conversation

icgood
Copy link
Contributor
@icgood icgood commented May 6, 2019 •
edited
Loading

The existing event loop start_tls() method is not sufficient for
connections using the streams API. The existing StreamReader works
because the new transport passes received data to the original protocol.
The StreamWriter must then write data to the new transport, and the
StreamReaderProtocol must be updated to close the new transport
correctly.

The new StreamWriter start_tls() updates itself and the reader
protocol to the new SSL transport.

https://bugs.python.org/issue34975

@icgood icgood requested review from 1st1 and asvetlov as code owners May 6, 2019 23:08
@icgood
bpo-34975: Add start_tls() method to streams API
7647686 
The existing event loop `start_tls()` method is not sufficient for
connections using the streams API. The existing StreamReader works
because the new transport passes received data to the original protocol.
The StreamWriter must then write data to the new transport, and the
StreamReaderProtocol must be updated to close the new transport
correctly.

The new StreamWriter `start_tls()` updates itself and the reader
protocol to the new SSL transport.
@asvetlov
Copy link
Contributor

Fixed by #13251

@asvetlov asvetlov closed this May 27, 2019
@arhadthedev
Copy link
Member
arhadthedev commented Mar 22, 2022 •
edited by bedevere-bot
Loading

@asvetlov Can this PR be reopened? #13251 got refused and was reverted by #16482 with the following explainer in bpo-38242:

The asyncio.Stream class we have now is overloaded with functionality. It's not composable. It's internal buffer and APIs are designed to parsing text protocols (i.e. parsing a complex binary protocol requires an entirely different buffer implementation).

I'm bringing this PR up because I need to port test.test_poplib and test.test_ftplib from to-be-removed asynchat and they use on-demand switching from plain text to SSL and back.

@icgood Could you also add stop_tls() to match start_tls(), please?

@icgood
Copy link
Contributor Author
icgood commented Mar 22, 2022

@icgood Could you also add stop_tls() to match start_tls(), please?

I'd be happy to, if we get the go-ahead to reopen and proceed. I've been using undocumented API in my library for years now, in lieu of a fix in CPython: https://github.com/icgood/pymap/blob/73c52f805d5fabf91a28f89ff81dd34efcb36486/pymap/imap/__init__.py#L275

@arhadthedev arhadthedev mentioned this pull request Apr 11, 2022
Merged
@icgood icgood mannequin mentioned this pull request May 20, 2022
Closed
@1st1 1st1 mentioned this pull request Dec 28, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@1st1 1st1 Awaiting requested review from 1st1

@asvetlov asvetlov Awaiting requested review from asvetlov

Assignees
No one assigned
Labels
awaiting review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@icgood @asvetlov @arhadthedev @the-knights-who-say-ni @bedevere-bot
0