classification
Title: start_tls() difficult when using asyncio.start_server()
Type: enhancement Stage: resolved
Components: asyncio Versions: Python 3.8
process
Status: closed Resolution: wont fix
Dependencies: Superseder: Merge StreamWriter and StreamReader into just asyncio.Stream
View: 36889
Assigned To: asvetlov Nosy List: asvetlov, icgood, yselivanov
Priority: normal Keywords: patch

Created on 2018-10-13 20:03 by icgood, last changed 2019-10-28 18:49 by asvetlov. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 13143 closed icgood, 2019-05-06 23:09
Messages (8)
msg327665 - (view) Author: Ian Good (icgood) * Date: 2018-10-13 20:03
There does not seem to be a public API for replacing the transport of the StreamReader / StreamWriter provided to the callback of a call to asyncio.start_server().

The only way I have found to use the new SSL transport is to update protected members of the StreamReaderProtocol object, e.g.:

    async def callback(reader, writer):
        loop = asyncio.get_event_loop()
        transport = writer.transport
        protocol = transport.get_protocol()
        new_transport = await loop.start_tls(
            transport, protocol, ssl_context, server_side=True)
        protocol._stream_reader = StreamReader(loop=loop)
        protocol._client_connected_cb = do_stuff_after_start_tls
        protocol.connection_made(new_transport)

    async def do_stuff_after_start_tls(ssl_reader, ssl_writer): ...
msg327674 - (view) Author: Andrew Svetlov (asvetlov) * (Python committer) Date: 2018-10-13 21:35
Thanks for raising the problem.
I'm in the middle of streams API refactoring.

https://github.com/asvetlov/cpython/blob/async-streams/Lib/asyncio/streams.py#L801-L812 is a draft.

A help is very appreciated.
Would you pick up this snippet and make a pull request with tests and documentation update?
msg327678 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2018-10-13 22:38
One thing: I'm -1 on adding starttls to current stream api; let's add it only to the new one (same for sendfile)
msg342012 - (view) Author: Ian Good (icgood) * Date: 2019-05-09 23:42
I added start_tls() to StreamWriter. My implementation returns a new StreamWriter that should be used from then on, but it could be adapted to modify the current writer in-place (let me know).

I've added docs, an integration test, and done some additional "real-world" testing with an IMAP server I work on. Specifically, "openssl s_client -connect xxx -starttls imap" works like a charm, and no errors/warnings are logged on disconnect.
msg342215 - (view) Author: Andrew Svetlov (asvetlov) * (Python committer) Date: 2019-05-11 18:56
I believe stream transport should be replaced.
If you have time please do this change.

Also please take a look at #36889 for merging StreamWriter and StreamReader.
It can simplify the replacement strategy.

Anyway, please keep your PR open at least.
The plan is: either merge your PR before writer/reader merging or land unified streams first and adopt the PR to new API.

Another thing that should be done before 3.8 feature freeze is support for sendfile in streams API. Hopefully, it is much simpler: no need for transport changing etc.
msg343679 - (view) Author: Andrew Svetlov (asvetlov) * (Python committer) Date: 2019-05-27 19:59
Fixed by #36889
msg355586 - (view) Author: Ian Good (icgood) * Date: 2019-10-28 18:35
#36889 was reverted, so this is not resolved.

I'm guessing this needs to be moved to 3.9 now too. Is my original PR worth revisiting? https://github.com/python/cpython/pull/13143/files
msg355587 - (view) Author: Andrew Svetlov (asvetlov) * (Python committer) Date: 2019-10-28 18:49
I think it should be closed; Yuri thinks that current streams API is frozen for the sake of shiny brand new streams (don't exist yet).
History
Date User Action Args
2019-10-28 18:49:42asvetlovsetstatus: open -> closed
resolution: wont fix
messages: + msg355587
2019-10-28 18:35:53icgoodsetstatus: closed -> open
resolution: fixed -> (no value)
messages: + msg355586
2019-05-27 19:59:57asvetlovsetstatus: open -> closed
superseder: Merge StreamWriter and StreamReader into just asyncio.Stream
messages: + msg343679

resolution: fixed
stage: patch review -> resolved
2019-05-11 18:56:22asvetlovsetmessages: + msg342215
2019-05-09 23:42:39icgoodsettype: enhancement
messages: + msg342012
2019-05-06 23:09:01icgoodsetkeywords: + patch
stage: patch review
pull_requests: + pull_request13056
2018-10-13 22:38:52yselivanovsetmessages: + msg327678
2018-10-13 21:35:08asvetlovsetassignee: asvetlov
messages: + msg327674
versions: + Python 3.8, - Python 3.7
2018-10-13 20:03:13icgoodcreate