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

Move ssl-related params from TCPConnector to request args #1128

Closed
asvetlov opened this Issue Aug 26, 2016 · 4 comments

Comments

Projects
None yet
4 participants
@asvetlov
Member

asvetlov commented Aug 26, 2016

Long story short

Now ssl configuration is tightly coupled with TCPConnector.

It is not convenient (user should create a connector and pass it into session) and disallows to use the same ClientSession for working with different ssl contexts etc.

I think we should move verify_ssl, fingerprint and ssl_context into ClientSession.request and ClientSession.ws_connect like we have done for proxy and proxy_auth (see #998).
Current TCPConnector's parameters should be kept as default values which may be overridden by request without any warning/error.

@asvetlov asvetlov added the easy label Aug 26, 2016

@fafhrd91 fafhrd91 added this to the 2.1 milestone Mar 14, 2017

@fafhrd91 fafhrd91 modified the milestones: 2.2, 2.1 Apr 8, 2017

@magicgoose

This comment has been minimized.

magicgoose commented Jun 13, 2017

I think they should not be moved to single requests but to ClientSession instead. Multiple requests to the same host+port can reuse connection (or also not reuse, it's statically unknown in general) —

"""Get from pool or create new connection."""
and TLS params are inherently property of the connection, not a HTTP request, so if the user gives different TLS parameters to these requests, that would be very misleading and even non-stable behavior (because it's not known in general if the connection will be reused for a specific request).

@asvetlov do you agree?

@asvetlov

This comment has been minimized.

Member

asvetlov commented Jun 13, 2017

I think if requests accepts verify and cert for every HTTP request call -- why aiohttp cannot do the same?

@magicgoose

This comment has been minimized.

magicgoose commented Jun 13, 2017

It can, of course, but I'm not sure if there are real advantages of doing this. But on second thought I see that it's probably fine for user, it would be just a little harder to implement. So, okay…

(at first I thought that there would be also things like which versions of TLS and which ciphers to negotiate with the server, and then there's no meaningful way of changing these constraints for next request with the same connection in general way, other than closing the connection and doing the handshake again, but if this level of control is not going to be allowed, then setting just verify and cert per request can be defined easily)

@cecton cecton referenced this issue Aug 9, 2017

Merged

Add SSL related params to ClientSession.request #2184

5 of 5 tasks complete

@cecton cecton self-assigned this Aug 9, 2017

@asvetlov

This comment has been minimized.

Member

asvetlov commented Sep 15, 2017

Fixed by #2184

@asvetlov asvetlov closed this Sep 15, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment