Hi Lukas, On Fri, Nov 24, 2017 at 03:37:02PM +0100, Lukas Tribus wrote: > Great work, 401 is now fine.
Thanks for confirming. > As for closing the connection, this is what I am seeing with the current code: > > - "timeout http-keep-alive" is not used I thought about trying to use it instead of timeout.client but felt uncertain about this. Maybe it would make more sense. What's your opinion ? > - when "timeout client" < "timeout server" and it strikes, we send > GOAWAY with last-stream-id set to the real last used stream id. Chrome > sends FIN right away and the connection is closed (this behavior seems > fine to me) OK. > - when "timeout server" < "timeout client" and it strikes, we send FIN > and the connection closes (no GOAWAY ever crosses the wire) Bad... There's probably something in the error handling part of the stream still being a bit too strong when it comes to closing. > Either way we don't ever try to shutdown TLS cleanly (send a TLS close > notify aka calling ssl_sock_shutw), maybe we should do that instead of > send FIN in the "timeout server" case? In fact we don't make a special case of the timeout server. *normally* the code dealing with the stream (ie between the two sides) doesn't even know it's attached to HTTP/2. And the HTTP/2 gateway doesn't even know what is being done with the bytes it feeds upon request. There's even no way to access the stream from there, let alone know there is a server. That's why I think that what you're observing is more a side effect of the way the "server error" is handled at the stream level. > Which timeout do you have in mind to close an idle HTTP2 connection? Till now it's "client" by default, unless a GOAWAY was already sent, in which case we switch to "client-fin". > Should http-keep-alive be limited to HTTP1? I really don't know and as mentionned above I have been hesitating as well. One point to keep in mind though is that http-keep-alive timeouts sometimes tend to be extremely short on certain systems to limit the number of concurrent idle connections. But with H2 we have a single connection per client, and re-creating it requires some exchanges, so the incentive for keeping it open longer is real. I suspect that some users might prefer to keep a short http-keep-alive for HTTP/1 and a longer one for HTTP/2. But I could be totally wrong. I *think* that's still something reasonable to adjust past the release if it turns out that an initial choice (whatever it is) is wrong. > Current behavior does not > seem to be perfectly consistent yet between client and server timeout. I'll study this again, thanks for this. > Also I'm not sure whether a clean TLS shutdown is required. It should, and I don't know either why it doesn't happen. Thanks! Willy
