Re: 2.4 pause - mod_http2 patchset Upgrade h2c vs mod_ssl Upgrade tls

[Yann Ylavic]

On Tue, Dec 8, 2015 at 3:07 AM, William A Rowe Jr  wrote:
>>
>> The handler handling the first request has to read the body before the
>> switch, eg. mod_proxy could forward the request without the client
>> having switched to any other protocol.
>
> Whoa... First the handler can't process the request until the switch, and
> more importantly the protocol upgrade is hop by hop, review s6.7 again
> please.

Hop by hop does not mean that Upgrade has to be done as soon as
possible, but that it must not be forwarded (this is client <=> httpd
stuff only, any the backend/origin wouldn't notice).

The handler has to process an HTTP/1 body (that is, plain text when
http: is used), because this is what the client will send (s6.7 states
that the first request is HTTP/1, including the body, IOW the client
can't change that between the header and the body).
So if we were to set aside the first request body on Upgrade, do the
switch, and then run the handler, the latter should not expect
anything else than HTTP/1 for the body (this is how the body is, be it
aside or not).

Moreover, if we choose to honor the Upgrade after the body, we must
produce a response which is Upgraded, that means for the TLS/1.x case
that we must first enable mod_ssl, read/write/do the TLS handshake so
that the response is really TLS/1.x upgraded.
We can indeed do that by reading/setaside the body, send the 101
switch to the client, do the handshake, play the setaside HTTP/1 body
through the input filter, and finally be HTTP/1+TLS on the (first)
response and further requests.
But I find this a bit complicated, not to talk about resources usage /
possible DOS...

>
> The mechanism to address your use case is HTTP/1.1 CONNECT.

My use case is a normal HTTP/1 request with a body, w/ or w/o
100-continue, through mod_proxy_http, not CONNECT.
And I don't want Upgrade to break or make this overly complicated.

>
>> That does not mean it has to (or prereqs a) setaside body, this could
>> be streamed (and is even desirable in the mod_proxy case).
>
> Not possible to read plaintext and write TLS, for example.  The body must be
> consumed first.

The HTTP/1 Upgrade described in the RFC is actually "read plaintext
and write TLS", AIUI.

>
>> So couldn't we handle the switch (101) in an output filter instead
>> (before the first bytes are sent)?
>
> No, for TLS, absolutely not.
>
> You presume the handler consumed the body, and you have no assurance of
> that.  Which means the output filter now needs to attend to the input filter
> state, and we are back where we started with the no 101 before the upgrade
> can be satisfied.

That's why I'd rather go with *not* honoring the Upgrade until a
request comes with no body.

Re: 2.4 pause - mod_http2 patchset Upgrade h2c vs mod_ssl Upgrade tls

[Stefan Eissing]

> Am 08.12.2015 um 01:58 schrieb William A Rowe Jr :
> 
> On Mon, Dec 7, 2015 at 6:35 PM, Yann Ylavic  wrote:
> On Tue, Dec 8, 2015 at 1:27 AM, William A Rowe Jr  wrote:
> > On Mon, Dec 7, 2015 at 6:15 PM, Yann Ylavic  wrote:
> >>
> >> On Tue, Dec 8, 2015 at 1:07 AM, Yann Ylavic  wrote:
> >> >
> >> > the body ought to be
> >> > set aside for any (relevant) TLS response (which needs the
> >> > handshake...).
> >>
> >> Hmm, no need to set aside, *unless* with must produce a response
> >> before the entire body (and the handshake) is read.
> >> But we'd better not Upgrade in this case...
> >
> >
> > Yes, there is a set aside, because the handler will read from the filter
> > stack... the handler phase has not yet occurred, and other content
> > input filters may be inserted to transform the request body.
> >
> > The upgrade switch would occur before the request content handler
> > is invoked, in all cases (post_read_request, or later during fixups
> > or the very beginning of invoke_handler).
> 
> But this isn't what the RFC says, right?
> The body of the first request is never Upgraded, so why would we read
> it using the Upgraded protocol?
> 
> How do you mean?  The RFC states we must read the request body
> (following a 100-continue) prior to a 101-switching protocols.  AFTER
> the protocol is switched, we are ready to invoke the handler, which
> will read the request body (which I suggest we have set aside within 
> the http input filter) and it will write to the output filter stack, but to 
> a different stack of protocol and network filters.

No, that is not what the RFC says. HTTP/1 switching protocols does not need
to happen simultaneously on upstream and downstream. Instead, the server
switches directly after the 101 response, the client switches after the last
byte of the request.

This is problematic for protocols like HTTP/2 who may require to receive
data in the new protocol format *before* they are able to read an arbitrarily
long request body. So, every HTTP/2 implementation will have a request
body maximum length that it can upgrade with. For interop reasons, me and
other implementors choose to have 0 as max length, since every clients
will run into that limit immediately and learn to cope with that.

Please remember that such a request will be answered correctly and that 
the client can upgrade to h2c on the next one, or anytime it likes. The
upgrade possibilities are announced by Apache on the first response, so
it should be easy to make a decision.

//Stefan

Re: 2.4 pause - mod_http2 patchset Upgrade h2c vs mod_ssl Upgrade tls

[Stefan Eissing]

> Am 08.12.2015 um 10:48 schrieb Yann Ylavic :
> 
> On Tue, Dec 8, 2015 at 3:07 AM, William A Rowe Jr  wrote:
> [...]
> That's why I'd rather go with *not* honoring the Upgrade until a
> request comes with no body.

++1

Re: 2.4 pause - mod_http2 patchset Upgrade h2c vs mod_ssl Upgradetls

[Stefan Eissing]

> Am 07.12.2015 um 23:42 schrieb Jacob Champion :
> 
> On 12/07/2015 02:30 PM, Bert Huijben wrote:
>> Is this a h2 limitation or a mod_h2 limitation?
>> 
>> If I would like h2c upgrade from Subversion without an additional
>> request I would have to send the upgrade request with a very short
>> OPTIONS request that has a body.
> 
> The HTTP/2 spec (RFC 7540, sec 3.2) appears to allow the sending of a request 
> body in an h2c upgrade (h2 upgrades are prohibited). So it would appear to be 
> a mod_http2 limitation for now.
> 
> Hopefully Stefan can confirm tomorrow (it's pretty late in Germany).

It is possible, but difficult to do unless you can read the body and set it 
aside somewhere. Which again imposes an arbitrary (for the client) limit. 
Experience shows that this leads to design of clients that work will in 
developments, but then encounter requests usage/sites/servers that have other 
limits and break.

If Apache accepts body lengths of up to 64KB (or something configurable) and 
some other server implements 8K and some third 1MB, how will that help design 
of a client that, for some reason, *wants* an upgrade to succeed? 

//Stefan

Re: Upgrade Summary

[Stefan Eissing]

Bert,

why can't you use h2c in direct mode? I assume you could store server support 
for this in the checkout meta data somewhere.

//Stefan

> Am 08.12.2015 um 11:30 schrieb Bert Huijben :
> 
>> -Original Message-
>> From: Stefan Eissing [mailto:stefan.eiss...@greenbytes.de]
>> Sent: dinsdag 8 december 2015 11:07
>> To: dev@httpd.apache.org
>> Subject: Upgrade Summary
>> 
>> Trying to summarize the status of the discussion and where the issues are
>> with the current Upgrade implementation.
>> 
>> Clarified:
>> A. any 100 must be sent out *before* a 101 response
>> B. request bodies are to be read in the original protocol, input filters
> like
>> chunk can be used, indeed are necessary, as if the request is being
>> processed normally
>> C. if a protocol supports upgrade on request bodies is up to the protocol
>> implementation and needs to be checked in the "propose" phase
>> 
>> Open:
>> 1. Protocols like Websocket need to take over the 101 sending themselves
> in
>> the "switch protocol" phase. (correct, Jacob?). Should we delegate the
>> sending of the 101 to the protocol switch handler?
> 
> If possible I would recommend avoiding this. I think the original protocol
> should setup the response and then the final protocol should somehow be able
> to annotate this.
> 
> The problems we try to solve now originate from the problem of doing things
> different in different protocol handlers, while in theory many upgrades are
> very similar.
> 
> The TLS and H2C upgrades both begin in one form and end in a different form.
> Websockets are kind of different in that they require a bad request response
> in a specific case. I'm not sure in which protocol this error needs to be
> send though.
> 
> In TLS and H2C further errors can always be produced in the new protocol, so
> when the handshake succeeds things can just go on.
> 
>> 2. General handling of request bodies. Options:
>>  a setaside in core of up to nnn bytes before switch invocation
>>  b do nothing, let protocol switch handler care about it
> 
> For Subversion to be able to use upgrade we would need to support a small
> body on a request (few hundred bytes. Content-Length header provided).
> 
> During our current (already optimized) handshake all requests have bodies,
> and introducing an additional request just to upgrade will slow things down
> quite measurable on operation like 'svn log', that are mostly bound by the
> handshake time.
> 
> We can't just switch the handshake to be something else... our handshake was
> built upon WEBDAV and DELTA/V. We added several headers to avoid many
> requests we previously performed, but we can't move away from that initial
> OPTIONS request without slowing down against all older servers.
> 
>   Bert

Re: No H2 Window updates!

[Jan Ehrhardt]

Eric Covener in gmane.comp.apache.devel (Mon, 7 Dec 2015 11:03:15 -0500):
>On Mon, Dec 7, 2015 at 10:56 AM, Jan Ehrhardt  wrote:
>> Stefan Eissing in gmane.comp.apache.devel (Mon, 7 Dec 2015 13:58:07
>> +0100):
>>>if you could find the time to verify that the duplicate headers are gone
>>>in the current 2.4.x version, that'd be nice. Thanks!
>>
>> They are gone! Time to T 2.4.18.
>
>Thanks for driving this one Jan.

My pleasure. To confirm: I have been working on our devserver with
mod_http2 1.0.11 a couple of hours now. No problems.
-- 
Jan

RE: Upgrade Summary

[Bert Huijben]

> -Original Message-
> From: Stefan Eissing [mailto:stefan.eiss...@greenbytes.de]
> Sent: dinsdag 8 december 2015 11:55
> To: dev@httpd.apache.org
> Subject: Re: Upgrade Summary
> 
> 
> > Am 08.12.2015 um 11:44 schrieb Yann Ylavic :
> >
> > On Tue, Dec 8, 2015 at 11:07 AM, Stefan Eissing
> >  wrote:
> >> [...]
> >> Open:
> >> 1. Protocols like Websocket need to take over the 101 sending
> themselves in the "switch protocol" phase. (correct, Jacob?). Should we
> delegate the sending of the 101 to the protocol switch handler?
> >> 2. General handling of request bodies. Options:
> >>  a setaside in core of up to nnn bytes before switch invocation
> >>  b do nothing, let protocol switch handler care about it
> >
> > a. is tempting, where nnn would be the limit above which we don't honor
> Upgrade.
> > But that could be a default behaviour, i.e. when "Protocols ... http1"
> > is finally elected.
> > Possibly specific modules would bypass that?
> >
> >> 3. When to do the upgrade dance:
> >>  a post_read_request: upgrade precedes authentication
> >
> > Looks like it can't be RFC compliant, is it?
> 
> I think it will not be, right.

I read the spec as H2c and HTTP/1.1 are equivalent protocols. Handling
authentication *after* switching should work just like when not switching.

As client I would like to switch as soon as possible, as at that point I can
start multiple requests.. potentially with their own auth, using the same or
different realms; or even different auth schemes.


I don't think we should require all auth to happen on HTTP/1.1.

With equivalent protocols we should switch as soon as possible... I don't
think servers should be configured as H2c only for authenticated users.
Especially if they also allow H2direct.

Bert

Upgrade Summary

[Stefan Eissing]

Trying to summarize the status of the discussion and where the issues are with 
the current Upgrade implementation.

Clarified:
A. any 100 must be sent out *before* a 101 response
B. request bodies are to be read in the original protocol, input filters like 
chunk can be used, indeed are necessary, as if the request is being processed 
normally
C. if a protocol supports upgrade on request bodies is up to the protocol 
implementation and needs to be checked in the "propose" phase

Open:
1. Protocols like Websocket need to take over the 101 sending themselves in the 
"switch protocol" phase. (correct, Jacob?). Should we delegate the sending of 
the 101 to the protocol switch handler?
2. General handling of request bodies. Options:
  a setaside in core of up to nnn bytes before switch invocation
  b do nothing, let protocol switch handler care about it
3. When to do the upgrade dance:
  a post_read_request: upgrade precedes authentication
  b handler: upgrade only honored on authenticated and otherwise ok requests
  c both: introduce separate hooks? have an additional parameter? more 
complexity
4. status code of protocol switch handler: if we move the task of 101 sending, 
the switch handler might not do it and keep the connection on the "old" 
protocol. Then a connection close is not necessary. So, we would do the close 
only when the switch handler returns APR_EOF.
5. Will it be possible to migrate the current TLS upgrade handling to this 
revised scheme?

Anything I missed?

PS. Re 5: with change 1+4, a TLS upgrade switcher could install an output 
filter, return and have it being processed normally. The output filter would 
then send the 101 and do the TLS handshake. Would that work?

> Am 08.12.2015 um 10:42 schrieb Stefan Eissing :
> 
> 
>> Am 07.12.2015 um 23:42 schrieb Jacob Champion :
>> 
>> On 12/07/2015 02:30 PM, Bert Huijben wrote:
>>> Is this a h2 limitation or a mod_h2 limitation?
>>> 
>>> If I would like h2c upgrade from Subversion without an additional
>>> request I would have to send the upgrade request with a very short
>>> OPTIONS request that has a body.
>> 
>> The HTTP/2 spec (RFC 7540, sec 3.2) appears to allow the sending of a 
>> request body in an h2c upgrade (h2 upgrades are prohibited). So it would 
>> appear to be a mod_http2 limitation for now.
>> 
>> Hopefully Stefan can confirm tomorrow (it's pretty late in Germany).
> 
> It is possible, but difficult to do unless you can read the body and set it 
> aside somewhere. Which again imposes an arbitrary (for the client) limit. 
> Experience shows that this leads to design of clients that work will in 
> developments, but then encounter requests usage/sites/servers that have other 
> limits and break.
> 
> If Apache accepts body lengths of up to 64KB (or something configurable) and 
> some other server implements 8K and some third 1MB, how will that help design 
> of a client that, for some reason, *wants* an upgrade to succeed? 
> 
> //Stefan
> 

RE: 2.4 pause - mod_http2 patchset Upgrade h2c vs mod_ssl Upgrade tls

[Bert Huijben]

> -Original Message-
> From: Stefan Eissing [mailto:stefan.eiss...@greenbytes.de]
> Sent: dinsdag 8 december 2015 10:25
> To: dev@httpd.apache.org
> Subject: Re: 2.4 pause - mod_http2 patchset Upgrade h2c vs mod_ssl
> Upgrade tls
> 
> 
> > Am 08.12.2015 um 01:58 schrieb William A Rowe Jr  clan.net>:
> >
> > On Mon, Dec 7, 2015 at 6:35 PM, Yann Ylavic 
> wrote:
> > On Tue, Dec 8, 2015 at 1:27 AM, William A Rowe Jr  clan.net> wrote:
> > > On Mon, Dec 7, 2015 at 6:15 PM, Yann Ylavic 
> wrote:
> > >>
> > >> On Tue, Dec 8, 2015 at 1:07 AM, Yann Ylavic 
> wrote:
> > >> >
> > >> > the body ought to be
> > >> > set aside for any (relevant) TLS response (which needs the
> > >> > handshake...).
> > >>
> > >> Hmm, no need to set aside, *unless* with must produce a response
> > >> before the entire body (and the handshake) is read.
> > >> But we'd better not Upgrade in this case...
> > >
> > >
> > > Yes, there is a set aside, because the handler will read from the
filter
> > > stack... the handler phase has not yet occurred, and other content
> > > input filters may be inserted to transform the request body.
> > >
> > > The upgrade switch would occur before the request content handler
> > > is invoked, in all cases (post_read_request, or later during fixups
> > > or the very beginning of invoke_handler).
> >
> > But this isn't what the RFC says, right?
> > The body of the first request is never Upgraded, so why would we read
> > it using the Upgraded protocol?
> >
> > How do you mean?  The RFC states we must read the request body
> > (following a 100-continue) prior to a 101-switching protocols.  AFTER
> > the protocol is switched, we are ready to invoke the handler, which
> > will read the request body (which I suggest we have set aside within
> > the http input filter) and it will write to the output filter stack, but
to
> > a different stack of protocol and network filters.
> 
> No, that is not what the RFC says. HTTP/1 switching protocols does not
need
> to happen simultaneously on upstream and downstream. Instead, the server
> switches directly after the 101 response, the client switches after the
last
> byte of the request.

The client can only switch once it has confirmation from the server, that it
is really going to perform the upgrade. If the server doesn't accept the
upgrade it must continue using 1.1.

So it has to stop producing data after that first request and first check if
there is a 101 response before it can pipeline the next request.
 
In pipelining clients such as serf this makes a huge difference.

At some points during update/checkout we may have tens of requests pipelined
on a single connection... but that process starts a bit after the initial
handshake. I'm trying to be in H2(c) at that point, to avoid opening the
additional connections at that point, which we currently have.


Once the client has the 101 it can switch protocol for both input and output
and start sending the next request. Waiting for the 101 is a full stall.

Bert

Re: 2.4 pause - mod_http2 patchset Upgrade h2c vs mod_ssl Upgradetls

[Stefan Eissing]

Bert,

I do not understand. How do you want to make pipelining requests and protocol 
upgrades work together? I assume you talk about http pipelining where you send 
a second request before you receive the response for the first one...

//Stefan

> Am 08.12.2015 um 11:27 schrieb Bert Huijben :
> 
> 
> 
>> -Original Message-
>> From: Stefan Eissing [mailto:stefan.eiss...@greenbytes.de]
>> Sent: dinsdag 8 december 2015 10:43
>> To: dev@httpd.apache.org
>> Subject: Re: 2.4 pause - mod_http2 patchset Upgrade h2c vs mod_ssl
>> Upgradetls
> 
> 
>> If Apache accepts body lengths of up to 64KB (or something configurable)
>> and some other server implements 8K and some third 1MB, how will that
>> help design of a client that, for some reason, *wants* an upgrade to
>> succeed?
> 
> For Subversion this would make a 100% difference.
> 
> Currently all true Subversion servers are implemented via httpd.
> 
> 
> Upgrade to h2c is currently not interesting for webbrowsers as all of them
> have decided not to implement it... But it makes a huge difference for web
> applications that talk to specific servers.
> 
> Which is very similar to that TLS upgrade that is interested for
> webprinting... These usecases don't use combinations of random clients, with
> random servers... They use specific combinations.
> 
>   Bert
> 

Re: Upgrade Summary

[Stefan Eissing]

> Am 08.12.2015 um 12:18 schrieb Yann Ylavic :
> 
> On Tue, Dec 8, 2015 at 11:54 AM, Stefan Eissing
>  wrote:
>> 
>> Am 08.12.2015 um 11:44 schrieb Yann Ylavic :
>>> 
>>> On Tue, Dec 8, 2015 at 11:07 AM, Stefan Eissing
 
 PS. Re 5: with change 1+4, a TLS upgrade switcher could install an output 
 filter, return and have it being processed normally. The output filter 
 would then send the 101 and do the TLS handshake. Would that work?
>>> 
>>> The issue is that this output filter would have to both read and write
>>> during the TLS handshake, not sure this is suitable.
>> 
>> Ok, maybe from pre_read_request hook then?
> 
> Hmm, pre_read of the second request, interesting.
> But maybe output filter could work too after all, we'd need to set the
> correct CONN_SENSE for event to do right thing?
> pre_read may be simpler though, based on a flag in the conn_rec?

mod_ssl could have that flag in its SSLConnRec, i would assume...

RE: 2.4 pause - mod_http2 patchset Upgrade h2c vs mod_ssl Upgradetls

[Bert Huijben]

> -Original Message-
> From: Stefan Eissing [mailto:stefan.eiss...@greenbytes.de]
> Sent: dinsdag 8 december 2015 10:43
> To: dev@httpd.apache.org
> Subject: Re: 2.4 pause - mod_http2 patchset Upgrade h2c vs mod_ssl
> Upgradetls


> If Apache accepts body lengths of up to 64KB (or something configurable)
> and some other server implements 8K and some third 1MB, how will that
> help design of a client that, for some reason, *wants* an upgrade to
> succeed?

For Subversion this would make a 100% difference.

Currently all true Subversion servers are implemented via httpd.


Upgrade to h2c is currently not interesting for webbrowsers as all of them
have decided not to implement it... But it makes a huge difference for web
applications that talk to specific servers.

Which is very similar to that TLS upgrade that is interested for
webprinting... These usecases don't use combinations of random clients, with
random servers... They use specific combinations.

Bert

Re: Upgrade Summary

[Yann Ylavic]

On Tue, Dec 8, 2015 at 11:54 AM, Stefan Eissing
 wrote:
>
> Am 08.12.2015 um 11:44 schrieb Yann Ylavic :
>>
>> On Tue, Dec 8, 2015 at 11:07 AM, Stefan Eissing
>>>
>>> PS. Re 5: with change 1+4, a TLS upgrade switcher could install an output 
>>> filter, return and have it being processed normally. The output filter 
>>> would then send the 101 and do the TLS handshake. Would that work?
>>
>> The issue is that this output filter would have to both read and write
>> during the TLS handshake, not sure this is suitable.
>
> Ok, maybe from pre_read_request hook then?

Hmm, pre_read of the second request, interesting.
But maybe output filter could work too after all, we'd need to set the
correct CONN_SENSE for event to do right thing?
pre_read may be simpler though, based on a flag in the conn_rec?

RE: Upgrade Summary

[Bert Huijben]

> -Original Message-
> From: Bert Huijben [mailto:b...@qqmail.nl]
> Sent: dinsdag 8 december 2015 12:41
> To: dev@httpd.apache.org
> Subject: RE: Upgrade Summary
> 


> I don't think we should require all auth to happen on HTTP/1.1.

If you want to go for equivalent implementations for things like the TLS
upgrade... then you really don't want to have AUTH completed before upgrade.

My preferred implementation would issue the 101 on HTTP/1.1 and then produce
the 401 on H2.
(I would be happy with an EOS on the response headers... I'm not interested
in the body :-))

I can handle '100 continue' before the 101 on HTTP/1.1, but also after
switching to H2 and before the actual result on H2... But I'm happy that we
don't have to support all possible scenarios there.

Bert

Re: Upgrade Summary

[Yann Ylavic]

On Tue, Dec 8, 2015 at 12:18 PM, Yann Ylavic  wrote:
> On Tue, Dec 8, 2015 at 11:54 AM, Stefan Eissing
>  wrote:
>>
>> Am 08.12.2015 um 11:44 schrieb Yann Ylavic :
>>>
>>> On Tue, Dec 8, 2015 at 11:07 AM, Stefan Eissing

 PS. Re 5: with change 1+4, a TLS upgrade switcher could install an output 
 filter, return and have it being processed normally. The output filter 
 would then send the 101 and do the TLS handshake. Would that work?
>>>
>>> The issue is that this output filter would have to both read and write
>>> during the TLS handshake, not sure this is suitable.
>>
>> Ok, maybe from pre_read_request hook then?
>
> Hmm, pre_read of the second request, interesting.

Actually no, the previous response has to be Upgraded, so for the TLS
case we'd have needed the handshake already.
Also, pre_read has no server_rec selected, so on which configuration
would we base the upgrade on...

> But maybe output filter could work too after all, we'd need to set the
> correct CONN_SENSE for event to do right thing?

Looks like the most suitable option if that can work.

RE: Upgrade Summary

[Bert Huijben]

> -Original Message-
> From: Stefan Eissing [mailto:stefan.eiss...@greenbytes.de]
> Sent: dinsdag 8 december 2015 13:22
> To: dev@httpd.apache.org
> Subject: Re: Upgrade Summary
> 
> 
> > Am 08.12.2015 um 12:40 schrieb Bert Huijben :
> >> -Original Message-
> >> From: Stefan Eissing [mailto:stefan.eiss...@greenbytes.de]
> >> Sent: dinsdag 8 december 2015 11:55
> >> To: dev@httpd.apache.org
> >> Subject: Re: Upgrade Summary
>  [...]3. When to do the upgrade dance:
>  a post_read_request: upgrade precedes authentication
> >>>
> >>> Looks like it can't be RFC compliant, is it?
> >>
> >> I think it will not be, right.
> >
> > I read the spec as H2c and HTTP/1.1 are equivalent protocols. Handling
> > authentication *after* switching should work just like when not
switching.
> 
> It does. We are only talking about upgrade.
> 
> > As client I would like to switch as soon as possible, as at that point I
can
> > start multiple requests.. potentially with their own auth, using the
same or
> > different realms; or even different auth schemes.
> 
> Again, your easiest choice is a direct h2c connection. The second easiest
is
> an initial OPTIONS * request without *request* body. The response may
> have
> any body length it wants.
> 
> The options * will, as I understand it, not trigger any authentication. In
> another mail, you describe that you already send such a request as 1st
thing.
> But you said it has a body. What does that contain, I wonder?
> 
> If you can live without that request body, we are all fine and have a
simple
> implementation. If we need to implement upgrades to h2c on some length
> request bodies, I personally do not have the time to do that right away
> among
> all other changes that are being discussed. At least not this week.
> 
> So, what is so relevant about the OPTIONS request body, may I ask?

I can't live without that request body. That would just add another request
to my handshake... the thing I'm trying to avoid.

I'm guessing the body of that initial body is something like
[[

   

]]
(content-type text/xml of course)

And this request is mostly handled by mod_dav, and further annotated with
additional headers by mod_dav_svn.

I can't just redesign DAV to optimize for http/2. If we had a timemachine,
we would have made other decisions.

Would have been nice if we knew DELTA/V was never really implemented outside
Subversion. But now we have compatibility guarantees with older Subversion
clients/servers and other implementations that may use some of the features.

Bert

Re: Upgrade Summary

[Stefan Eissing]

> Am 08.12.2015 um 14:08 schrieb Bert Huijben :
> [...]
> (I'm pretty sure we find new interesting proxy server scenarios :-( )

Not only that. Someone tested h2c against his server from various Tor exit 
nodes and got a failure rate > 10% because of network middle boxes...

Fun times.

Re: Upgrade Summary

[Yann Ylavic]

On Tue, Dec 8, 2015 at 11:07 AM, Stefan Eissing
 wrote:
> Trying to summarize the status of the discussion and where the issues are 
> with the current Upgrade implementation.
>
> Clarified:
> A. any 100 must be sent out *before* a 101 response
> B. request bodies are to be read in the original protocol, input filters like 
> chunk can be used, indeed are necessary, as if the request is being processed 
> normally
> C. if a protocol supports upgrade on request bodies is up to the protocol 
> implementation and needs to be checked in the "propose" phase

Agreed.

>
> Open:
> 1. Protocols like Websocket need to take over the 101 sending themselves in 
> the "switch protocol" phase. (correct, Jacob?). Should we delegate the 
> sending of the 101 to the protocol switch handler?
> 2. General handling of request bodies. Options:
>   a setaside in core of up to nnn bytes before switch invocation
>   b do nothing, let protocol switch handler care about it

a. is tempting, where nnn would be the limit above which we don't honor Upgrade.
But that could be a default behaviour, i.e. when "Protocols ... http1"
is finally elected.
Possibly specific modules would bypass that?

> 3. When to do the upgrade dance:
>   a post_read_request: upgrade precedes authentication

Looks like it can't be RFC compliant, is it?

>   b handler: upgrade only honored on authenticated and otherwise ok requests

This is probably the right thing to do, would OPTIONS be special is this case?

>   c both: introduce separate hooks? have an additional parameter? more 
> complexity

Depends on whether OPTIONS (others?) need special treatment.

> 4. status code of protocol switch handler: if we move the task of 101 
> sending, the switch handler might not do it and keep the connection on the 
> "old" protocol. Then a connection close is not necessary. So, we would do the 
> close only when the switch handler returns APR_EOF.
> 5. Will it be possible to migrate the current TLS upgrade handling to this 
> revised scheme?

We must do that IMHO, the scheme should be suite for all the cases.

>
> PS. Re 5: with change 1+4, a TLS upgrade switcher could install an output 
> filter, return and have it being processed normally. The output filter would 
> then send the 101 and do the TLS handshake. Would that work?

The issue is that this output filter would have to both read and write
during the TLS handshake, not sure this is suitable.

Re: Upgrade Summary

[Stefan Eissing]

> Am 08.12.2015 um 11:44 schrieb Yann Ylavic :
> 
> On Tue, Dec 8, 2015 at 11:07 AM, Stefan Eissing
>  wrote:
>> [...]
>> Open:
>> 1. Protocols like Websocket need to take over the 101 sending themselves in 
>> the "switch protocol" phase. (correct, Jacob?). Should we delegate the 
>> sending of the 101 to the protocol switch handler?
>> 2. General handling of request bodies. Options:
>>  a setaside in core of up to nnn bytes before switch invocation
>>  b do nothing, let protocol switch handler care about it
> 
> a. is tempting, where nnn would be the limit above which we don't honor 
> Upgrade.
> But that could be a default behaviour, i.e. when "Protocols ... http1"
> is finally elected.
> Possibly specific modules would bypass that?
> 
>> 3. When to do the upgrade dance:
>>  a post_read_request: upgrade precedes authentication
> 
> Looks like it can't be RFC compliant, is it?

I think it will not be, right.

>>  b handler: upgrade only honored on authenticated and otherwise ok requests
> 
> This is probably the right thing to do, would OPTIONS be special is this case?

It currently is insofar as core installs a storage handler to invoke the, 
otherwise
same, upgrade handling code. It's a special case, but nothing the protocol 
handlers
need to care about...

>>  c both: introduce separate hooks? have an additional parameter? more 
>> complexity
> 
> Depends on whether OPTIONS (others?) need special treatment.

The current design works with it while exposing in the API only one set of 
hooks. If
those get called from one or the other does not matter. For each request, the 
hooks
are invoked at most once.

>> 4. status code of protocol switch handler: if we move the task of 101 
>> sending, the switch handler might not do it and keep the connection on the 
>> "old" protocol. Then a connection close is not necessary. So, we would do 
>> the close only when the switch handler returns APR_EOF.
>> 5. Will it be possible to migrate the current TLS upgrade handling to this 
>> revised scheme?
> 
> We must do that IMHO, the scheme should be suite for all the cases.

Agreed.

>> PS. Re 5: with change 1+4, a TLS upgrade switcher could install an output 
>> filter, return and have it being processed normally. The output filter would 
>> then send the 101 and do the TLS handshake. Would that work?
> 
> The issue is that this output filter would have to both read and write
> during the TLS handshake, not sure this is suitable.

Ok, maybe from pre_read_request hook then?

Re: Upgrade Summary

[Stefan Eissing]

> Am 08.12.2015 um 12:40 schrieb Bert Huijben :
>> -Original Message-
>> From: Stefan Eissing [mailto:stefan.eiss...@greenbytes.de]
>> Sent: dinsdag 8 december 2015 11:55
>> To: dev@httpd.apache.org
>> Subject: Re: Upgrade Summary
 [...]3. When to do the upgrade dance:
 a post_read_request: upgrade precedes authentication
>>> 
>>> Looks like it can't be RFC compliant, is it?
>> 
>> I think it will not be, right.
> 
> I read the spec as H2c and HTTP/1.1 are equivalent protocols. Handling
> authentication *after* switching should work just like when not switching.

It does. We are only talking about upgrade.

> As client I would like to switch as soon as possible, as at that point I can
> start multiple requests.. potentially with their own auth, using the same or
> different realms; or even different auth schemes.

Again, your easiest choice is a direct h2c connection. The second easiest is 
an initial OPTIONS * request without *request* body. The response may have 
any body length it wants. 

The options * will, as I understand it, not trigger any authentication. In
another mail, you describe that you already send such a request as 1st thing.
But you said it has a body. What does that contain, I wonder?

If you can live without that request body, we are all fine and have a simple
implementation. If we need to implement upgrades to h2c on some length
request bodies, I personally do not have the time to do that right away among
all other changes that are being discussed. At least not this week.

So, what is so relevant about the OPTIONS request body, may I ask?

//Stefan

Re: 2.4.18 T

[Jim Jagielski]

This will be happening today, afternoon, eastern time.

Re: Upgrade Summary

[Yann Ylavic]

If so, for 2.4.18 we should probably backport Bill's proposal in
STATUS (r1717816), so that OPTIONS works as in 2.4.16...

On Tue, Dec 8, 2015 at 1:54 PM, Stefan Eissing
 wrote:
> I see. Delta-V goodies.
>
> My proposal therefore is:
> - keep the upgrade/protocol switch mechanism as is for the 2.4.18 release
> - make the agreed upon changes, including TLS upgrade and small content h2c 
> upgrades for the next release
>
> Since the mechanism is already out with 2.4.17, I see no sense in delaying 
> 2.4.18 at this point and rather give us time to change it later, so it works 
> for everyone.
>
> //Stefan
>
>> Am 08.12.2015 um 13:46 schrieb Bert Huijben :
>>
>>
>>
>>> -Original Message-
>>> From: Stefan Eissing [mailto:stefan.eiss...@greenbytes.de]
>>> Sent: dinsdag 8 december 2015 13:22
>>> To: dev@httpd.apache.org
>>> Subject: Re: Upgrade Summary
>>>
>>>
 Am 08.12.2015 um 12:40 schrieb Bert Huijben :
> -Original Message-
> From: Stefan Eissing [mailto:stefan.eiss...@greenbytes.de]
> Sent: dinsdag 8 december 2015 11:55
> To: dev@httpd.apache.org
> Subject: Re: Upgrade Summary
>>> [...]3. When to do the upgrade dance:
>>> a post_read_request: upgrade precedes authentication
>>
>> Looks like it can't be RFC compliant, is it?
>
> I think it will not be, right.

 I read the spec as H2c and HTTP/1.1 are equivalent protocols. Handling
 authentication *after* switching should work just like when not
>> switching.
>>>
>>> It does. We are only talking about upgrade.
>>>
 As client I would like to switch as soon as possible, as at that point I
>> can
 start multiple requests.. potentially with their own auth, using the
>> same or
 different realms; or even different auth schemes.
>>>
>>> Again, your easiest choice is a direct h2c connection. The second easiest
>> is
>>> an initial OPTIONS * request without *request* body. The response may
>>> have
>>> any body length it wants.
>>>
>>> The options * will, as I understand it, not trigger any authentication. In
>>> another mail, you describe that you already send such a request as 1st
>> thing.
>>> But you said it has a body. What does that contain, I wonder?
>>>
>>> If you can live without that request body, we are all fine and have a
>> simple
>>> implementation. If we need to implement upgrades to h2c on some length
>>> request bodies, I personally do not have the time to do that right away
>>> among
>>> all other changes that are being discussed. At least not this week.
>>>
>>> So, what is so relevant about the OPTIONS request body, may I ask?
>>
>> I can't live without that request body. That would just add another request
>> to my handshake... the thing I'm trying to avoid.
>>
>> I'm guessing the body of that initial body is something like
>> [[
>> 
>>   
>> 
>> ]]
>> (content-type text/xml of course)
>>
>> And this request is mostly handled by mod_dav, and further annotated with
>> additional headers by mod_dav_svn.
>>
>> I can't just redesign DAV to optimize for http/2. If we had a timemachine,
>> we would have made other decisions.
>>
>> Would have been nice if we knew DELTA/V was never really implemented outside
>> Subversion. But now we have compatibility guarantees with older Subversion
>> clients/servers and other implementations that may use some of the features.
>>
>>   Bert
>

Re: Upgrade Summary

[Stefan Eissing]

I see. Delta-V goodies.

My proposal therefore is:
- keep the upgrade/protocol switch mechanism as is for the 2.4.18 release
- make the agreed upon changes, including TLS upgrade and small content h2c 
upgrades for the next release

Since the mechanism is already out with 2.4.17, I see no sense in delaying 
2.4.18 at this point and rather give us time to change it later, so it works 
for everyone.

//Stefan

> Am 08.12.2015 um 13:46 schrieb Bert Huijben :
> 
> 
> 
>> -Original Message-
>> From: Stefan Eissing [mailto:stefan.eiss...@greenbytes.de]
>> Sent: dinsdag 8 december 2015 13:22
>> To: dev@httpd.apache.org
>> Subject: Re: Upgrade Summary
>> 
>> 
>>> Am 08.12.2015 um 12:40 schrieb Bert Huijben :
 -Original Message-
 From: Stefan Eissing [mailto:stefan.eiss...@greenbytes.de]
 Sent: dinsdag 8 december 2015 11:55
 To: dev@httpd.apache.org
 Subject: Re: Upgrade Summary
>> [...]3. When to do the upgrade dance:
>> a post_read_request: upgrade precedes authentication
> 
> Looks like it can't be RFC compliant, is it?
 
 I think it will not be, right.
>>> 
>>> I read the spec as H2c and HTTP/1.1 are equivalent protocols. Handling
>>> authentication *after* switching should work just like when not
> switching.
>> 
>> It does. We are only talking about upgrade.
>> 
>>> As client I would like to switch as soon as possible, as at that point I
> can
>>> start multiple requests.. potentially with their own auth, using the
> same or
>>> different realms; or even different auth schemes.
>> 
>> Again, your easiest choice is a direct h2c connection. The second easiest
> is
>> an initial OPTIONS * request without *request* body. The response may
>> have
>> any body length it wants.
>> 
>> The options * will, as I understand it, not trigger any authentication. In
>> another mail, you describe that you already send such a request as 1st
> thing.
>> But you said it has a body. What does that contain, I wonder?
>> 
>> If you can live without that request body, we are all fine and have a
> simple
>> implementation. If we need to implement upgrades to h2c on some length
>> request bodies, I personally do not have the time to do that right away
>> among
>> all other changes that are being discussed. At least not this week.
>> 
>> So, what is so relevant about the OPTIONS request body, may I ask?
> 
> I can't live without that request body. That would just add another request
> to my handshake... the thing I'm trying to avoid.
> 
> I'm guessing the body of that initial body is something like
> [[
> 
>   
> 
> ]]
> (content-type text/xml of course)
> 
> And this request is mostly handled by mod_dav, and further annotated with
> additional headers by mod_dav_svn.
> 
> I can't just redesign DAV to optimize for http/2. If we had a timemachine,
> we would have made other decisions.
> 
> Would have been nice if we knew DELTA/V was never really implemented outside
> Subversion. But now we have compatibility guarantees with older Subversion
> clients/servers and other implementations that may use some of the features.
> 
>   Bert

RE: Upgrade Summary

[Bert Huijben]

> -Original Message-
> From: Stefan Eissing [mailto:stefan.eiss...@greenbytes.de]
> Sent: dinsdag 8 december 2015 11:07
> To: dev@httpd.apache.org
> Subject: Upgrade Summary
> 
> Trying to summarize the status of the discussion and where the issues are
> with the current Upgrade implementation.
> 
> Clarified:
> A. any 100 must be sent out *before* a 101 response
> B. request bodies are to be read in the original protocol, input filters
like
> chunk can be used, indeed are necessary, as if the request is being
> processed normally
> C. if a protocol supports upgrade on request bodies is up to the protocol
> implementation and needs to be checked in the "propose" phase
> 
> Open:
> 1. Protocols like Websocket need to take over the 101 sending themselves
in
> the "switch protocol" phase. (correct, Jacob?). Should we delegate the
> sending of the 101 to the protocol switch handler?

If possible I would recommend avoiding this. I think the original protocol
should setup the response and then the final protocol should somehow be able
to annotate this.

The problems we try to solve now originate from the problem of doing things
different in different protocol handlers, while in theory many upgrades are
very similar.

The TLS and H2C upgrades both begin in one form and end in a different form.
Websockets are kind of different in that they require a bad request response
in a specific case. I'm not sure in which protocol this error needs to be
send though.

In TLS and H2C further errors can always be produced in the new protocol, so
when the handshake succeeds things can just go on.

> 2. General handling of request bodies. Options:
>   a setaside in core of up to nnn bytes before switch invocation
>   b do nothing, let protocol switch handler care about it

For Subversion to be able to use upgrade we would need to support a small
body on a request (few hundred bytes. Content-Length header provided).

During our current (already optimized) handshake all requests have bodies,
and introducing an additional request just to upgrade will slow things down
quite measurable on operation like 'svn log', that are mostly bound by the
handshake time.

We can't just switch the handshake to be something else... our handshake was
built upon WEBDAV and DELTA/V. We added several headers to avoid many
requests we previously performed, but we can't move away from that initial
OPTIONS request without slowing down against all older servers.

Bert

Re: 2.4 pause - mod_http2 patchset Upgrade h2c vs mod_ssl Upgradetls

[Yann Ylavic]

On Tue, Dec 8, 2015 at 11:36 AM, Stefan Eissing
 wrote:
>
> I do not understand. How do you want to make pipelining requests and protocol 
> upgrades work together? I assume you talk about http pipelining where you 
> send a second request before you receive the response for the first one...

I suppose pipelining would start from the second request...

RE: 2.4 pause - mod_http2 patchset Upgrade h2c vs mod_ssl Upgradetls

[Bert Huijben]

> -Original Message-
> From: Stefan Eissing [mailto:stefan.eiss...@greenbytes.de]
> Sent: dinsdag 8 december 2015 11:36
> To: dev@httpd.apache.org
> Subject: Re: 2.4 pause - mod_http2 patchset Upgrade h2c vs mod_ssl
> Upgradetls
> 
> Bert,
> 
> I do not understand. How do you want to make pipelining requests and
> protocol upgrades work together? I assume you talk about http pipelining
> where you send a second request before you receive the response for the
> first one...

I can't... but you were explaining that I could just switch to the new
protocol after my request... which I can't.

I have to perform a full stop... which is a performance killer in a client
that is optimized for pipelining. (Serf and libsvn_ra_serf are 100%
optimized for pipelining)

That is why I want to upgrade on the first request.


At the start of any session we currently perform 2 requests with full stop
behavior, to support user scnearios:
* one to detect if we have HTTP/1.1 or HTTP/1.0.

This is an OPTIONS request with a small Content-Length body. This will
already hand over the capabilities of the Subversion server and some
interesting URLS. If we find an old server here, the full handshake will
have more requests.

* A second request with a small chunked body to see if the server supports
chunked encoding (assuming we found 1.1).
If users use an nginx frontend (which is not uncommon) this fails and we
explicitly cache all requests to have explicit Content-Length headers in
this case.

After this we have many different scenarios, of which many want to pipeline,
or open multiple connections as soon as possible.

For Subversion I would like to switch to H2c in one of these two requests.
The first one looks like the simplest one. (Combining upgrading with chunked
makes things even harder).



I can't store whether a server supports H2 and just use it. Subversion is
used on laptops that are used on multiple networks.


Receiving Alt-Svc in the first response and just pipelining the upgrade
between further requests if there is a match, is the only other way I could
upgrade without a performance penalty on all servers that don't support H2c.

Bert

RE: Upgrade Summary

[Bert Huijben]

> -Original Message-
> From: Stefan Eissing [mailto:stefan.eiss...@greenbytes.de]
> Sent: dinsdag 8 december 2015 13:54
> To: dev@httpd.apache.org
> Subject: Re: Upgrade Summary
> 
> I see. Delta-V goodies.
> 
> My proposal therefore is:
> - keep the upgrade/protocol switch mechanism as is for the 2.4.18 release
> - make the agreed upon changes, including TLS upgrade and small content
> h2c upgrades for the next release
> 
> Since the mechanism is already out with 2.4.17, I see no sense in delaying
> 2.4.18 at this point and rather give us time to change it later, so it
works for
> everyone.

Sure... my upgrade requests for Subversion are for 2.4.x future... Certainly
no rush here.

I don't expect this upgrade support to reach Subversion far before 1.10. 

Perhaps we will backport some of this to 1.9, but I don't think we can just
enable H2 support in a minor release without testing on a bigger group
first.
(I'm pretty sure we find new interesting proxy server scenarios :-( )

Bert

Upgrade when !ap_request_has_body(r) only for 2.4.18? (was: svn commit: r1718595 - /httpd/httpd/branches/2.4.x/STATUS)

[Yann Ylavic]

On Tue, Dec 8, 2015 at 2:30 PM,   wrote:
> Author: ylavic
> Date: Tue Dec  8 13:30:30 2015
> New Revision: 1718595
>
> URL: http://svn.apache.org/viewvc?rev=1718595=rev
> Log:
> Comment about ap_request_has_body() check for Upgrade.
>
> Modified:
> httpd/httpd/branches/2.4.x/STATUS
>
[]
>   trunk patch: http://svn.apache.org/r1717816
>   +1: wrowe, icing
> + ylavic: how about adding !ap_request_has_body(r) to the test then?

E.g. (on top of r1717816):

Index: modules/ssl/ssl_engine_kernel.c
===
--- modules/ssl/ssl_engine_kernel.c(revision 1718341)
+++ modules/ssl/ssl_engine_kernel.c(working copy)
@@ -252,7 +252,7 @@ int ssl_hook_ReadReq(request_rec *r)
  * and OPTIONS * request processing is completed.
  */
 if (sc->enabled == SSL_ENABLED_OPTIONAL && !(sslconn && sslconn->ssl)
-&& !r->main) {
+&& ap_is_initial_req(r) && !ap_has_request_body(r)) {
 apr_table_setn(r->headers_out, "Upgrade", "TLS/1.0, HTTP/1.1");
 apr_table_mergen(r->headers_out, "Connection", "upgrade");
 }
--

Re: Upgrade when !ap_request_has_body(r) only for 2.4.18? (was: svn commit: r1718595 - /httpd/httpd/branches/2.4.x/STATUS)

[Yann Ylavic]

On Tue, Dec 8, 2015 at 2:37 PM, Yann Ylavic  wrote:
> On Tue, Dec 8, 2015 at 2:30 PM,   wrote:
>> Author: ylavic
>> Date: Tue Dec  8 13:30:30 2015
>> New Revision: 1718595
>>
>> URL: http://svn.apache.org/viewvc?rev=1718595=rev
>> Log:
>> Comment about ap_request_has_body() check for Upgrade.
>>
>> Modified:
>> httpd/httpd/branches/2.4.x/STATUS
>>
> []
>>   trunk patch: http://svn.apache.org/r1717816
>>   +1: wrowe, icing
>> + ylavic: how about adding !ap_request_has_body(r) to the test then?
>
> E.g. (on top of r1717816):

Actually, since there is already an Upgrade handling above, wouldn't a more
correct patch be (trunk):

Index: modules/ssl/ssl_engine_kernel.c
===
--- modules/ssl/ssl_engine_kernel.c(revision 1718341)
+++ modules/ssl/ssl_engine_kernel.c(working copy)
@@ -230,10 +230,13 @@ int ssl_hook_ReadReq(request_rec *r)

 /* Perform TLS upgrade here if "SSLEngine optional" is configured,
  * SSL is not already set up for this connection, and the client
- * has sent a suitable Upgrade header. */
+ * has sent a suitable Upgrade header. Note this must happen before
+ * map_to_storage and OPTIONS * request processing is completed.
+ */
 if (sc->enabled == SSL_ENABLED_OPTIONAL && !myConnConfig(r->connection)
 && (upgrade = apr_table_get(r->headers_in, "Upgrade")) != NULL
-&& ap_find_token(r->pool, upgrade, "TLS/1.0")) {
+&& ap_find_token(r->pool, upgrade, "TLS/1.0")
+&& !r->main && !ap_has_request_body(r)) {
 if (upgrade_connection(r)) {
 return AP_FILTER_ERROR;
 }
@@ -246,17 +249,6 @@ int ssl_hook_ReadReq(request_rec *r)
 sslconn = myConnConfig(r->connection->master);
 }

-/* If "SSLEngine optional" is configured, this is not an SSL
- * connection, and this isn't a subrequest, send an Upgrade
- * response header.  Note this must happen before map_to_storage
- * and OPTIONS * request processing is completed.
- */
-if (sc->enabled == SSL_ENABLED_OPTIONAL && !(sslconn && sslconn->ssl)
-&& !r->main) {
-apr_table_setn(r->headers_out, "Upgrade", "TLS/1.0, HTTP/1.1");
-apr_table_mergen(r->headers_out, "Connection", "upgrade");
-}
-
 if (!sslconn) {
 return DECLINED;
 }
?

Re: Upgrade when !ap_request_has_body(r) only for 2.4.18? (was: svn commit: r1718595 - /httpd/httpd/branches/2.4.x/STATUS)

[Jim Jagielski]

My only suggestion is that instead of willy-nilly suggesting
patches that will be included in a release, that we actually take
time to think of the correct patch, to implement it and TEST against
it and only THEN have it backported.

Please.

We don't want to put out a semi-immediate 2.4.19 because one
of these last-minute patches broke other stuff. It looks, and
is, really, really bad and amateur.

Re: Upgrade when !ap_request_has_body(r) only for 2.4.18? (was: svn commit: r1718595 - /httpd/httpd/branches/2.4.x/STATUS)

[William A Rowe Jr]

On Tue, Dec 8, 2015 at 7:37 AM, Yann Ylavic  wrote:

> On Tue, Dec 8, 2015 at 2:30 PM,   wrote:
> > Author: ylavic
> > Date: Tue Dec  8 13:30:30 2015
> > New Revision: 1718595
> >
> > URL: http://svn.apache.org/viewvc?rev=1718595=rev
> > Log:
> > Comment about ap_request_has_body() check for Upgrade.
> >
> > Modified:
> > httpd/httpd/branches/2.4.x/STATUS
> >
> []
> >   trunk patch: http://svn.apache.org/r1717816
> >   +1: wrowe, icing
> > + ylavic: how about adding !ap_request_has_body(r) to the test then?
>
> E.g. (on top of r1717816):
>
> Index: modules/ssl/ssl_engine_kernel.c
> ===
> --- modules/ssl/ssl_engine_kernel.c(revision 1718341)
> +++ modules/ssl/ssl_engine_kernel.c(working copy)
> @@ -252,7 +252,7 @@ int ssl_hook_ReadReq(request_rec *r)
>   * and OPTIONS * request processing is completed.
>   */
>  if (sc->enabled == SSL_ENABLED_OPTIONAL && !(sslconn && sslconn->ssl)
> -&& !r->main) {
> +&& ap_is_initial_req(r) && !ap_has_request_body(r)) {
>  apr_table_setn(r->headers_out, "Upgrade", "TLS/1.0, HTTP/1.1");
>  apr_table_mergen(r->headers_out, "Connection", "upgrade");
>  }
>
>
The problem is that you are disabling *advertising* of the protocol based
on the content of a request body.  *This* request isn't going to be the
target
of the user's advertisement, that applies to their *next* request.  It
doesn't
change what I think you meant to change.

On the next request, your patch disables the advertising in an OPTIONS *
request because you didn't send the first advertisement; that won't work
out.
You are also disabling per-context advertisement which is allowed by
RFC7230.
E.g. content under /cups/devices/ may actually have an SSLRequireSSL.  Right
now the 426 code path relies on advertising headers already being present.

We save lots of response bytes on kept-alive SSLEngine optional resources
by advertising less frequently as you suggest above, but we can't have an
_initial_req toggle without later re-injecting these into all 426 exception
paths,
per the upgrade required exception's definition.

The core Protocols API already suffers this problem, e.g. WebSockets.  They
may apply to all resources within /webapp/ but not to other resources on the
server, and the current API won't present the upgrade header in response to
the specific resources desired.

Since we have the read-ahead buffering already, I don't see a reason to
simply
disable it at this time, the patch to switch the sequence of 101 and 100
should
be straightforward.  Since we aren't converging quite yet on an API fix to
solve
this multifaceted case, I'll look at the simple fix within mod_ssl alone.
Since
the logic was valid under RFC2718/2616 I don't think we have to do a hard
stop
to fix such a bug for RFC7230 here today, but can incorporate such fixes in
2.4.19 soon.

Cheers,

Bill

Re: Upgrade when !ap_request_has_body(r) only for 2.4.18? (was: svn commit: r1718595 - /httpd/httpd/branches/2.4.x/STATUS)

[William A Rowe Jr]

On Tue, Dec 8, 2015 at 8:34 AM, Stefan Eissing  wrote:

> +1 for deferring any upgrade changes that do not fix real issues - like
> the one proposed for backport by Bill - to 2.4.19
>

Agreed, as spelled out in my top-post, simplest path to 2.4.18, and these
interesting discussions over the past day point to no single simple path.

Note that the pre-patch behavior caused tls to overwrite h2c, now the
protocol
API will overwrite tls upgrade advertisements on the first trip through.

Does it make sense to @bug the new Protocol API's stating that these
remain experimental and still subject to change, and refer prospective
developer/consumers to dev@httpd?  It seems something will change
in a later 2.4 release, and its simply a matter of what is the appropriate
straight path that can satisfy all of the prospective upgrade consumers.

Re: Upgrade when !ap_request_has_body(r) only for 2.4.18? (was: svn commit: r1718595 - /httpd/httpd/branches/2.4.x/STATUS)

[Stefan Eissing]

With the current implementation, that seems wise. This restriction can be 
removed once the change we discussed with output filter/hook is working.

> Am 08.12.2015 um 14:37 schrieb Yann Ylavic :
> 
> On Tue, Dec 8, 2015 at 2:30 PM,   wrote:
>> Author: ylavic
>> Date: Tue Dec  8 13:30:30 2015
>> New Revision: 1718595
>> 
>> URL: http://svn.apache.org/viewvc?rev=1718595=rev
>> Log:
>> Comment about ap_request_has_body() check for Upgrade.
>> 
>> Modified:
>>httpd/httpd/branches/2.4.x/STATUS
>> 
> []
>>  trunk patch: http://svn.apache.org/r1717816
>>  +1: wrowe, icing
>> + ylavic: how about adding !ap_request_has_body(r) to the test then?
> 
> E.g. (on top of r1717816):
> 
> Index: modules/ssl/ssl_engine_kernel.c
> ===
> --- modules/ssl/ssl_engine_kernel.c(revision 1718341)
> +++ modules/ssl/ssl_engine_kernel.c(working copy)
> @@ -252,7 +252,7 @@ int ssl_hook_ReadReq(request_rec *r)
>  * and OPTIONS * request processing is completed.
>  */
> if (sc->enabled == SSL_ENABLED_OPTIONAL && !(sslconn && sslconn->ssl)
> -&& !r->main) {
> +&& ap_is_initial_req(r) && !ap_has_request_body(r)) {
> apr_table_setn(r->headers_out, "Upgrade", "TLS/1.0, HTTP/1.1");
> apr_table_mergen(r->headers_out, "Connection", "upgrade");
> }
> --

Re: Upgrade when !ap_request_has_body(r) only for 2.4.18? (was: svn commit: r1718595 - /httpd/httpd/branches/2.4.x/STATUS)

[Stefan Eissing]

> Am 08.12.2015 um 16:18 schrieb William A Rowe Jr :
> 
> On Tue, Dec 8, 2015 at 8:34 AM, Stefan Eissing  
> wrote:
> +1 for deferring any upgrade changes that do not fix real issues - like the 
> one proposed for backport by Bill - to 2.4.19
> 
> Agreed, as spelled out in my top-post, simplest path to 2.4.18, and these
> interesting discussions over the past day point to no single simple path.
> 
> Note that the pre-patch behavior caused tls to overwrite h2c, now the protocol
> API will overwrite tls upgrade advertisements on the first trip through.
> 
> Does it make sense to @bug the new Protocol API's stating that these
> remain experimental and still subject to change, and refer prospective 
> developer/consumers to dev@httpd?  It seems something will change
> in a later 2.4 release, and its simply a matter of what is the appropriate
> straight path that can satisfy all of the prospective upgrade consumers.

+1

Re: Upgrade when !ap_request_has_body(r) only for 2.4.18? (was: svn commit: r1718595 - /httpd/httpd/branches/2.4.x/STATUS)

[Stefan Eissing]

+1 for deferring any upgrade changes that do not fix real issues - like the one 
proposed for backport by Bill - to 2.4.19

> Am 08.12.2015 um 15:29 schrieb William A Rowe Jr :
> 
> On Tue, Dec 8, 2015 at 7:37 AM, Yann Ylavic  wrote:
> On Tue, Dec 8, 2015 at 2:30 PM,   wrote:
> > Author: ylavic
> > Date: Tue Dec  8 13:30:30 2015
> > New Revision: 1718595
> >
> > URL: http://svn.apache.org/viewvc?rev=1718595=rev
> > Log:
> > Comment about ap_request_has_body() check for Upgrade.
> >
> > Modified:
> > httpd/httpd/branches/2.4.x/STATUS
> >
> []
> >   trunk patch: http://svn.apache.org/r1717816
> >   +1: wrowe, icing
> > + ylavic: how about adding !ap_request_has_body(r) to the test then?
> 
> E.g. (on top of r1717816):
> 
> Index: modules/ssl/ssl_engine_kernel.c
> ===
> --- modules/ssl/ssl_engine_kernel.c(revision 1718341)
> +++ modules/ssl/ssl_engine_kernel.c(working copy)
> @@ -252,7 +252,7 @@ int ssl_hook_ReadReq(request_rec *r)
>   * and OPTIONS * request processing is completed.
>   */
>  if (sc->enabled == SSL_ENABLED_OPTIONAL && !(sslconn && sslconn->ssl)
> -&& !r->main) {
> +&& ap_is_initial_req(r) && !ap_has_request_body(r)) {
>  apr_table_setn(r->headers_out, "Upgrade", "TLS/1.0, HTTP/1.1");
>  apr_table_mergen(r->headers_out, "Connection", "upgrade");
>  }
> 
> 
> The problem is that you are disabling *advertising* of the protocol based
> on the content of a request body.  *This* request isn't going to be the target
> of the user's advertisement, that applies to their *next* request.  It doesn't
> change what I think you meant to change.
> 
> On the next request, your patch disables the advertising in an OPTIONS *
> request because you didn't send the first advertisement; that won't work out.
> You are also disabling per-context advertisement which is allowed by RFC7230.
> E.g. content under /cups/devices/ may actually have an SSLRequireSSL.  Right
> now the 426 code path relies on advertising headers already being present.
> 
> We save lots of response bytes on kept-alive SSLEngine optional resources 
> by advertising less frequently as you suggest above, but we can't have an 
> _initial_req toggle without later re-injecting these into all 426 exception 
> paths,
> per the upgrade required exception's definition.
> 
> The core Protocols API already suffers this problem, e.g. WebSockets.  They
> may apply to all resources within /webapp/ but not to other resources on the
> server, and the current API won't present the upgrade header in response to
> the specific resources desired.
> 
> Since we have the read-ahead buffering already, I don't see a reason to simply
> disable it at this time, the patch to switch the sequence of 101 and 100 
> should
> be straightforward.  Since we aren't converging quite yet on an API fix to 
> solve
> this multifaceted case, I'll look at the simple fix within mod_ssl alone.  
> Since
> the logic was valid under RFC2718/2616 I don't think we have to do a hard stop
> to fix such a bug for RFC7230 here today, but can incorporate such fixes in
> 2.4.19 soon.
> 
> Cheers,
> 
> Bill

Re: Upgrade when !ap_request_has_body(r) only? (was: for 2.4.18)

[Yann Ylavic]

On Tue, Dec 8, 2015 at 3:29 PM, William A Rowe Jr  wrote:
> On Tue, Dec 8, 2015 at 7:37 AM, Yann Ylavic  wrote:
>>
>> E.g. (on top of r1717816):
>
> The problem is that you are disabling *advertising* of the protocol based
> on the content of a request body.  *This* request isn't going to be the
> target
> of the user's advertisement, that applies to their *next* request.  It
> doesn't
> change what I think you meant to change.

Yes, you are right, the real Upgrade is done above, so when we are
here we just prepare the response headers.
Let's think/agree more about how to handle Upgrades in the next release(s).

Regards,
Yann.

Re: Upgrade when !ap_request_has_body(r) only for 2.4.18? (was: svn commit: r1718595 - /httpd/httpd/branches/2.4.x/STATUS)

[Yann Ylavic]

On Tue, Dec 8, 2015 at 3:17 PM, Jim Jagielski  wrote:
> My only suggestion is that instead of willy-nilly suggesting
> patches that will be included in a release, that we actually take
> time to think of the correct patch, to implement it and TEST against
> it and only THEN have it backported.
>
> Please.

Suggestions have to start somewhere, I did not mean to rush on this,
just expecting feedbacks (including ones like yours, which is indeed
very sensible :)

My point was that if we were backport r1717816 in 2.4.18 (for OPTIONS
to work back), we needed more changes for RFC-compliance wrt TLS/1.x
Upgrades, the one w/o the other is not suitable.

So I think we all agree on the need to think/test more about this ;)

Re: Protocol API @bug warnings for 2.4.18?

[Stefan Eissing]

+1 Fine by me.

> Am 08.12.2015 um 16:31 schrieb William A Rowe Jr :
> 
> On Tue, Dec 8, 2015 at 9:21 AM, Stefan Eissing  
> wrote:
> 
> > On Tue, Dec 8, 2015 at 8:34 AM, Stefan Eissing 
> >  wrote:
> > +1 for deferring any upgrade changes
> >
> > Agreed, as spelled out in my top-post, simplest path to 2.4.18, and these
> > interesting discussions over the past day point to no single simple path.
> >
> > Does it make sense to @bug the new Protocol API's stating that these
> > remain experimental and still subject to change, and refer prospective
> > developer/consumers to dev@httpd?  It seems something will change
> > in a later 2.4 release, and its simply a matter of what is the appropriate
> > straight path that can satisfy all of the prospective upgrade consumers.
> 
> +1
> 
> Something like the attached?  Edits/suggestions welcome.
> 
> 
> 
>  
> 
> 

Re: Upgrade when !ap_request_has_body(r) only for 2.4.18? (was: svn commit: r1718595 - /httpd/httpd/branches/2.4.x/STATUS)

[Jim Jagielski]

> On Dec 8, 2015, at 11:11 AM, Yann Ylavic  wrote:
> 
> On Tue, Dec 8, 2015 at 3:17 PM, Jim Jagielski  wrote:
>> My only suggestion is that instead of willy-nilly suggesting
>> patches that will be included in a release, that we actually take
>> time to think of the correct patch, to implement it and TEST against
>> it and only THEN have it backported.
>> 
>> Please.
> 
> Suggestions have to start somewhere, I did not mean to rush on this,
> just expecting feedbacks (including ones like yours, which is indeed
> very sensible :)
> 
> My point was that if we were backport r1717816 in 2.4.18 (for OPTIONS
> to work back), we needed more changes for RFC-compliance wrt TLS/1.x
> Upgrades, the one w/o the other is not suitable.
> 
> So I think we all agree on the need to think/test more about this ;)

Oh, agreed 100%.

Protocol API @bug warnings for 2.4.18?

[William A Rowe Jr]

On Tue, Dec 8, 2015 at 9:21 AM, Stefan Eissing  wrote:

>
> > On Tue, Dec 8, 2015 at 8:34 AM, Stefan Eissing <
> stefan.eiss...@greenbytes.de> wrote:
> > +1 for deferring any upgrade changes
> >
> > Agreed, as spelled out in my top-post, simplest path to 2.4.18, and these
> > interesting discussions over the past day point to no single simple path.
> >
> > Does it make sense to @bug the new Protocol API's stating that these
> > remain experimental and still subject to change, and refer prospective
> > developer/consumers to dev@httpd?  It seems something will change
> > in a later 2.4 release, and its simply a matter of what is the
> appropriate
> > straight path that can satisfy all of the prospective upgrade consumers.
>
> +1
>

Something like the attached?  Edits/suggestions welcome.
Index: include/http_protocol.h
===
--- include/http_protocol.h	(revision 1718631)
+++ include/http_protocol.h	(working copy)
@@ -736,6 +736,10 @@
  *   NULL to indicated that the hooks are free to propose 
  * @param proposals The list of protocol identifiers proposed by the hooks
  * @return OK or DECLINED
+ * @bug This API or implementation and order of operations should be considered
+ * experimental and will continue to evolve in future 2.4 releases, with
+ * a corresponding minor module magic number (MMN) bump to indicate the
+ * API revision level.
  */
 AP_DECLARE_HOOK(int,protocol_propose,(conn_rec *c, request_rec *r,
   server_rec *s,
@@ -765,6 +769,10 @@
  * @param choices A list of protocol identifiers, normally the clients whishes
  * @param proposals the list of protocol identifiers proposed by the hooks
  * @return OK or DECLINED
+ * @bug This API or implementation and order of operations should be considered
+ * experimental and will continue to evolve in future 2.4 releases, with
+ * a corresponding minor module magic number (MMN) bump to indicate the
+ * API revision level.
  */
 AP_DECLARE_HOOK(int,protocol_switch,(conn_rec *c, request_rec *r,
  server_rec *s,
@@ -780,6 +788,10 @@
  *
  * @param c The current connection
  * @return The identifier of the protocol in place or NULL
+ * @bug This API or implementation and order of operations should be considered
+ * experimental and will continue to evolve in future 2.4 releases, with
+ * a corresponding minor module magic number (MMN) bump to indicate the
+ * API revision level.
  */
 AP_DECLARE_HOOK(const char *,protocol_get,(const conn_rec *c))
 
@@ -798,6 +810,10 @@
  * @param report_all include also protocols less preferred than the current one
  * @param pupgrades on return, possible protocols to upgrade to in descending order 
  * of preference. Maybe NULL if none are available.
+ * @bug This API or implementation and order of operations should be considered
+ * experimental and will continue to evolve in future 2.4 releases, with
+ * a corresponding minor module magic number (MMN) bump to indicate the
+ * API revision level.
  */
 AP_DECLARE(apr_status_t) ap_get_protocol_upgrades(conn_rec *c, request_rec *r, 
   server_rec *s, int report_all, 
@@ -817,6 +833,10 @@
  * @param s The server/virtual host selected
  * @param choices A list of protocol identifiers, normally the clients whishes
  * @return The selected protocol or NULL if no protocol could be agreed upon
+ * @bug This API or implementation and order of operations should be considered
+ * experimental and will continue to evolve in future 2.4 releases, with
+ * a corresponding minor module magic number (MMN) bump to indicate the
+ * API revision level.
  */
 AP_DECLARE(const char *) ap_select_protocol(conn_rec *c, request_rec *r, 
 server_rec *s,
@@ -835,6 +855,10 @@
  * APR_EINVAL,  if the protocol is already in place
  * APR_NOTIMPL, if no module performed the switch
  * Other errors where appropriate
+ * @bug This API or implementation and order of operations should be considered
+ * experimental and will continue to evolve in future 2.4 releases, with
+ * a corresponding minor module magic number (MMN) bump to indicate the
+ * API revision level.
  */
 AP_DECLARE(apr_status_t) ap_switch_protocol(conn_rec *c, request_rec *r, 
 server_rec *s,
@@ -850,6 +874,10 @@
  *
  * @param c The connection to determine the protocol for
  * @return the protocol in use, never NULL
+ * @bug This API or implementation and order of operations should be considered
+ * experimental and will continue to evolve in future 2.4 releases, with
+ * a corresponding minor module magic number (MMN) bump to indicate the
+ * API revision level.
  */
 AP_DECLARE(const char *) ap_get_protocol(conn_rec *c);
 
@@ -866,6 +894,10 @@
  * @param s The server/virtual host 

Re: Upgrade when !ap_request_has_body(r) only for 2.4.18? (was: svn commit: r1718595 - /httpd/httpd/branches/2.4.x/STATUS)

[Jim Jagielski]

So what is current (r1718631) is likely what will be tagged and rolled
and, assuming enuff +1s, released.

Re: Protocol API @bug warnings for 2.4.18?

[Yann Ylavic]

On Tue, Dec 8, 2015 at 4:31 PM, William A Rowe Jr  wrote:
> On Tue, Dec 8, 2015 at 9:21 AM, Stefan Eissing
>  wrote:
>>
>>
>> > On Tue, Dec 8, 2015 at 8:34 AM, Stefan Eissing
>> >  wrote:
>> > +1 for deferring any upgrade changes
>> >
>> > Agreed, as spelled out in my top-post, simplest path to 2.4.18, and
>> > these
>> > interesting discussions over the past day point to no single simple
>> > path.
>> >
>> > Does it make sense to @bug the new Protocol API's stating that these
>> > remain experimental and still subject to change, and refer prospective
>> > developer/consumers to dev@httpd?  It seems something will change
>> > in a later 2.4 release, and its simply a matter of what is the
>> > appropriate
>> > straight path that can satisfy all of the prospective upgrade consumers.
>>
>> +1
>
>
> Something like the attached?  Edits/suggestions welcome.

+1, we discussed this already for 2.4.17 but it did not make its way.
Thanks for doing it.

Re: 2.4 pause - mod_http2 patchset Upgrade h2c vs mod_ssl Upgradetls

[Jacob Champion]

On 12/08/2015 01:42 AM, Stefan Eissing wrote:

If Apache accepts body lengths of up to 64KB (or something
configurable) and some other server implements 8K and some third 1MB,
how will that help design of a client that, for some reason, *wants*
an upgrade to succeed?


I don't disagree with your premise, and I think that the zero-length 
body requirement is the correct thing to do for now. I will theorycraft 
in another email to the list.


--Jacob

[VOTE] Release Apache httpd 2.4.18 as GA

[Jim Jagielski]

The pre-release test tarballs for Apache httpd 2.4.18 can be found
at the usual place:

http://httpd.apache.org/dev/dist/

I'm calling a VOTE on releasing these as Apache httpd 2.4.18 GA.

[ ] +1: Good to go
[ ] +0: meh
[ ] -1: Danger Will Robinson. And why.

Vote will last the normal 72 hrs.

NOTE: The *-deps are only there for convenience.

Thx!

Re: Upgrade Summary

[Jacob Champion]

On 12/08/2015 02:07 AM, Stefan Eissing wrote:

C. if a protocol supports upgrade on request bodies is up to the protocol implementation 
and needs to be checked in the "propose" phase


Only if the module's intent is to simply ignore upgrade requests if they 
have bodies. Other modules might choose to propose the protocol anyway, 
and explicitly fail the request with a 4xx if the upgrade comes to them.



1. Protocols like Websocket need to take over the 101 sending themselves in the 
"switch protocol" phase. (correct, Jacob?). Should we delegate the sending of 
the 101 to the protocol switch handler?


It's more that I need to be able to interrupt the upgrade if necessary 
-- whether that's handled by a separate hook, or by delegating the 
upgrade entirely to me, is up for debate. I worry about the duplication 
of the Upgrade code if you delegate that to the modules though... seems 
like core should understand how to upgrade, and handle it for the modules.


In my head the ideal handshake API goes like this:

- Request comes in with a proposed set of protocols to Upgrade to.
- Core uses the propose hook to get a list of protocols that are valid 
upgrades for the current request target. A protocol is chosen.
- The module that won the proposal is given one last chance to check the 
incoming request and fail the upgrade with an immediate HTTP response.

- Otherwise, core sends the 101.
- The switch handler takes over the connection.
- The connection is closed after the switch handler returns.

I am still not sure where the authnz calls fit in, though. h2c and 
tls+http/1.1 can send a 401 after the protocol switch if they want 
(right?), but WebSocket needs to send it before the protocol switch, and 
having to replicate the authnz checks in my switch handler would be a 
pain...



4. status code of protocol switch handler: if we move the task of 101 sending, the switch 
handler might not do it and keep the connection on the "old" protocol. Then a 
connection close is not necessary. So, we would do the close only when the switch handler 
returns APR_EOF.


That's an interesting idea. Seems like it would work.

--Jacob

Re: On the Upgrade request body limit

[Jacob Champion]

On 12/08/2015 12:45 PM, Jim Jagielski wrote:

Well

We are not NGINX. We are also not Microsoft. We don't create HTTP
response codes willy-nilly. We actually try to *honor* the actual
specs.


Yes, I agree 100%. :) Trust me, I've dealt with enough non-standard HTTP 
"extensions"; I'm not suggesting we just start doing stuff willy-nilly.


There are people who have worked on those specs on this mailing list, 
though, who might be interested in making official improvements and 
additions -- or have opinions on why those additions would not actually 
be improvements. This seemed like a nice place to talk about it, since 
we have implementation experts who can weigh in. If that's not on topic, 
I can try to take it somewhere else.


--Jacob

On the Upgrade request body limit

[Jacob Champion]

I wrote this in response to Stefan's note on the zero-length request 
body limit for h2c Upgrades, then realized it would further fragment the 
(already massive) conversation, so here it is.


Stefan wrote:

It is possible, but difficult to do unless you can read the body and

> set it aside somewhere. Which again imposes an arbitrary (for the
> client) limit. Experience shows that this leads to design of clients
> that work will in developments, but then encounter requests
> usage/sites/servers that have other limits and break.

This is true of other things about HTTP/1.1 too -- how does a client 
figure out what the maximum request size for a server is? -- but it's 
made harder in this case by the fact that


1) 100 Continue is sent whether the request is upgraded or not, which 
makes it impossible to use a continue handshake to quickly determine 
upgrade limits, and


2) returning 413 during an upgrade would be ambiguous anyway. Is the 413 
really related to the request target, or is it just because I tried to 
upgrade and my request was temporarily limited?


It seems to me that we're missing something in HTTP/1.1 that would make 
these complex Upgrade scenarios robust. IIUC, the point of allowing 
request bodies during upgrades was to decrease the number of round 
trips, but if implementations can't actually make use of it in practice 
then that's not very useful...


If it were enough of a problem to fix (and I'm not claiming it is, 
necessarily), how would we fix it? Would we need a new 1xx response 
(e.g. 103 Upgrade Declined, with headers indicating the related protocol 
and the reason for the failure)? A new header in OPTIONS? Something else 
entirely?


Or is the correct "fix" to rearchitect so the limit during upgrades can 
be exactly the same as during a normal request?


--Jacob

Re: On the Upgrade request body limit

[Jim Jagielski]

Well

We are not NGINX. We are also not Microsoft. We don't create HTTP
response codes willy-nilly. We actually try to *honor* the actual
specs.

> On Dec 8, 2015, at 3:22 PM, Jacob Champion  wrote:
> 
> I wrote this in response to Stefan's note on the zero-length request body 
> limit for h2c Upgrades, then realized it would further fragment the (already 
> massive) conversation, so here it is.
> 
> Stefan wrote:
>> It is possible, but difficult to do unless you can read the body and
> > set it aside somewhere. Which again imposes an arbitrary (for the
> > client) limit. Experience shows that this leads to design of clients
> > that work will in developments, but then encounter requests
> > usage/sites/servers that have other limits and break.
> 
> This is true of other things about HTTP/1.1 too -- how does a client figure 
> out what the maximum request size for a server is? -- but it's made harder in 
> this case by the fact that
> 
> 1) 100 Continue is sent whether the request is upgraded or not, which makes 
> it impossible to use a continue handshake to quickly determine upgrade 
> limits, and
> 
> 2) returning 413 during an upgrade would be ambiguous anyway. Is the 413 
> really related to the request target, or is it just because I tried to 
> upgrade and my request was temporarily limited?
> 
> It seems to me that we're missing something in HTTP/1.1 that would make these 
> complex Upgrade scenarios robust. IIUC, the point of allowing request bodies 
> during upgrades was to decrease the number of round trips, but if 
> implementations can't actually make use of it in practice then that's not 
> very useful...
> 
> If it were enough of a problem to fix (and I'm not claiming it is, 
> necessarily), how would we fix it? Would we need a new 1xx response (e.g. 103 
> Upgrade Declined, with headers indicating the related protocol and the reason 
> for the failure)? A new header in OPTIONS? Something else entirely?
> 
> Or is the correct "fix" to rearchitect so the limit during upgrades can be 
> exactly the same as during a normal request?
> 
> --Jacob

Re: On the Upgrade request body limit

[Yann Ylavic]

On Tue, Dec 8, 2015 at 11:43 PM, William A Rowe Jr  wrote:
> On Tue, Dec 8, 2015 at 4:17 PM, Yann Ylavic  wrote:
>>
>> Why any HTTP/1 response? If the Upgrade is accepted, ISTM that the
>> response must be Upgraded, no?
>
> The server is always free to accept or ignore the Upgrade: request... and
> once the 101-switching protocols has been sent, the connection must be
> in the offered protocol.
>
> 6.7.  Upgrade

Oh, right, but in this case the Upgrade is aborted, so there should be
no 101 switch from httpd, the whole transaction is HTTP/1 (until
possibly further request).

Re: On the Upgrade request body limit

[Yann Ylavic]

On Tue, Dec 8, 2015 at 11:00 PM, William A Rowe Jr  wrote:
>
> Apache httpd server offers limits on the number
> of incoming headers, the number of bytes in those headers, the number of
> bytes in the request line, the length of the URI.  The maximum request body
> length.  This is just one more limit, but not an actual *limitation*,
> because the request will be processed, simply not by changing protocols
> first, per RFC7230.

Agreed.

> On Tue, Dec 8, 2015 at 2:22 PM, Jacob Champion  wrote:
>>
>> 1) 100 Continue is sent whether the request is upgraded or not, which
>> makes it impossible to use a continue handshake to quickly determine upgrade
>> limits, and

100-continue is sent only if the client Expect's it, that's not mandatory.

>
>> 2) returning 413 during an upgrade would be ambiguous anyway. Is the 413
>> really related to the request target, or is it just because I tried to
>> upgrade and my request was temporarily limited?
>
> It never would.  The possible results are the normal response in http/1.1
> protocol, or 426 to tell the client it still must change protocols first.
[]
>
> Our httpd http/1.1 server
> doesn't and cannot care, it can read-ahead the same network bucket it was
> going to read anyways, and let the new protocol handler decide, very early
> or very late in the cycle, whether the upgrade will happen.  If it won't,
> httpd must continue to serve the response sticking to http/1.1.

Agreed, the module/handler can read the (HTTP/1) body if it needs it,
or ignore it, in any case the core must do the right thing for this
body, and if some Protocols was elected, do the 101 switch before any
response bytes.
For empty response (i.e. simple/single 101 response), couldn't the
module/handler simply flush?
If so, an output filter could do the job...

>
> RFC7230 is baked, expect no changes.  The request (including 100-continue +
> body) is read, the protocol is switched if possible, otherwise the response
> is sent in http/1.1.  Those are our constraints.

Agreed (again), and that constraints are not that bad ("normal"
Upgrade handshakes should not have a huge payload anyway), so we
should be able to do an Upgrade in a single round trip.

Re: On the Upgrade request body limit

[William A Rowe Jr]

On Tue, Dec 8, 2015 at 4:17 PM, Yann Ylavic  wrote:

> On Tue, Dec 8, 2015 at 11:00 PM, William A Rowe Jr 
> wrote:
> >
> > Define complex, robust.  Request (upgrade: somespec) -> 100 continue ->
> > request body <- [ http/1.1 response | 101 - switching protocols <- new
> > protocol response ].
>
> As I read the RFC, the simple(st) case is:
> Request (upgrade: somespec) -> request body <- 101 (upgrade: somespec)
> <- new protocol response or read
>

Whereas the 100-continue case is:
> Request (upgrade: somespec, expect: 100-continue) [ <- 100-continue ->
> request body ] <- 101 (upgrade: somespec) <- new protocol response or
> read
>

Right, and I didn't color the complexity of using C-L vs. T-E encoding for
the request body, either.  Both may reply in http/1.1 or with a
101-switching
followed by something recognizable (e.g. tls following handshake and filter
insertion) or inscrutable (e.g. websocket, h2c) to the http/1.1 protocol
stack.


> Why any HTTP/1 response? If the Upgrade is accepted, ISTM that the
> response must be Upgraded, no?
>

The server is always free to accept or ignore the Upgrade: request... and
once the 101-switching protocols has been sent, the connection must be
in the offered protocol.

6.7 .  Upgrade

   The "Upgrade" header field is intended to provide a simple mechanism
   for transitioning from HTTP/1.1 to some other protocol on the same
   connection.  A client MAY send a list of protocols in the Upgrade
   header field of a request to invite the server to switch to one or
   more of those protocols, in order of descending preference, before
   sending the final response.  A server MAY ignore a received Upgrade
   header field if it wishes to continue using the current protocol on
   that connection.  Upgrade cannot be used to insist on a protocol
   change.

Re: On the Upgrade request body limit

[Jacob Champion]

On 12/08/2015 03:01 PM, Yann Ylavic wrote:

Jacob,

On Tue, Dec 8, 2015 at 11:17 PM, Yann Ylavic  wrote:


As I read the RFC, the simple(st) case is:
Request (upgrade: somespec) -> request body <- 101 (upgrade: somespec)
<- new protocol response or read


Wouldn't that work for the WebSocket case?
If the new protocol wants to read after the Switch I don't think
anything prevents it...
The headers included in the 101 response could be set by a hook.


Sorry I'm responding so slowly; I'm trying to grok all the responses 
before I reply, and they're coming in fast. ;)


Yes, this works fine for the WebSocket case. WebSocket is easier because 
WebSocket upgrades are only allowed for the GET method and there is no 
request body (that I have to worry about, anyway).


The motivation for my original post wasn't WebSocket. I made it with the 
assumption (provided by Stefan) that the h2c upgrade is more limited 
than other protocol upgrades in how big a request body it can handle; 
i.e. that the specs themselves painted mod_http2 into a corner. If that 
assumption is incorrect, and there's a way for it to use the same limit 
as every other request, then I think my discussion point is moot.


--Jacob

Re: Upgrade Summary

[William A Rowe Jr]

On Tue, Dec 8, 2015 at 3:43 PM, Jacob Champion  wrote:

> On 12/08/2015 01:03 PM, Jacob Champion wrote:
>
>> - The module that won the proposal is given one last chance to check the
>> incoming request and fail the upgrade with an immediate HTTP response.
>>
>
> And to add to this, for completeness: mod_websocket also needs to add any
> required headers (e.g. Sec-WebSocket-Accept/Protocol/Extensions) to the 101
> response during this same conceptual "check" phase.


It seems prudent in the API design to allow the target of the switch to also
specify any exceptional 100-continue interim response headers as well as
these 101-switching protocols interim response headers.

Re: On the Upgrade request body limit

[Jacob Champion]

On 12/08/2015 02:00 PM, William A Rowe Jr wrote:

Apache httpd server offers limits on the
number of incoming headers, the number of bytes in those headers, the
number of bytes in the request line, the length of the URI.  The maximum
request body length.  This is just one more limit, but not an actual
*limitation*, because the request will be processed, simply not by
changing protocols first, per RFC7230.


I agree with your point -- that's what I was trying to point out with my 
OP -- but I think general-purpose clients that are trying to ensure 
their upgrades succeed as quickly as possible, as often as possible, and 
are thwarted in that by an invisible request body limit, might consider 
it a limitation. That the request succeeds is not necessarily their only 
design goal.



As I'll point out again in closing, h2c isn't defined by RFC7230, and if
it decides to ignore requests with bodies, that's up to mod_http2 and
other implementations.  But RFC7230 has nothing to say to h2c on this,
and rather encourages protocol implementors to accept already-read C-L
or T-E: chunked request bodies in a hand-over into their upgraded protocol.


Agreed. And like I said in my response to Yann, if it turns out that the 
arbitrary upgrade-body-limit for h2c can be done away with entirely, an 
official addition to HTTP would be completely unnecessary.



There is no need for the client to determine the upgrade limit, because
the body of the request is sent.  If the server -arbitrarily- chooses to
satisfy it, there will be a 101 following the body transmission.  But
the spec makes absolutely clear that it is the server's prerogative to
upgrade or not, irrespective of the particular upgrade requested.

If the server demands that the upgrade happen, before a request can be
honored, it must reply with a 426 until the client offers to upgrade.
If the client demands that the upgrade happen, it must perform the
simplest applicable request (e.g. OPTIONS *) with the desired upgrade
semantics and verify that the upgrade occurred.


Stefan is, I think, adding more shades of grey here. OPTIONS * is the 
most likely upgrade to succeed, but also guarantees the addition of an 
"unnecessary" (depending on your point of view) round-trip. Clients may 
be willing to make a trade-off in initial success rate for an average 
increase in performance, but not if the server implementations are so 
different that their success rate plummets.


Whether this is enough of a concern to "fix" is a matter of opinion. But 
it appears that there has already been an agreement of sorts to reject 
h2c response bodies from the major implementors, if I understood Stefan 
correctly, so they at least are concerned. It would be nice to read that 
conversation.



2) returning 413 during an upgrade would be ambiguous anyway. Is the
413 really related to the request target, or is it just because I
tried to upgrade and my request was temporarily limited?


It never would.  The possible results are the normal response in
http/1.1 protocol, or 426 to tell the client it still must change
protocols first.


I'm not sure I understand what you mean. My point was simply that while 
a pure HTTP/1.1 client implementation can try to optimize its "limits 
discovery" through the use of a 100-continue handshake, and immediately 
making adjustments if a 4xx comes back, clients have no way to apply the 
same discovery to an artificial limit on upgrade-requests.


And again, whether such a limit is justified to begin with still remains 
to be seen, I think.



Define complex, robust.  Request (upgrade: somespec) -> 100 continue ->
request body <- [ http/1.1 response | 101 - switching protocols <- new
protocol response ].


Hopefully I've clarified what I mean by this -- not "robust" in terms of 
whether the request breaks or not, but "robust" in terms of a high 
success rate for optimally-performant upgrades to h2c.


--Jacob

Re: On the Upgrade request body limit

[Yann Ylavic]

Jacob,

On Tue, Dec 8, 2015 at 11:17 PM, Yann Ylavic  wrote:
>
> As I read the RFC, the simple(st) case is:
> Request (upgrade: somespec) -> request body <- 101 (upgrade: somespec)
> <- new protocol response or read

Wouldn't that work for the WebSocket case?
If the new protocol wants to read after the Switch I don't think
anything prevents it...
The headers included in the 101 response could be set by a hook.

Re: Upgrade Summary

[Roy T. Fielding]

> On Dec 8, 2015, at 2:07 AM, Stefan Eissing  
> wrote:
> 
> Trying to summarize the status of the discussion and where the issues are 
> with the current Upgrade implementation.
> 
> Clarified:
> A. any 100 must be sent out *before* a 101 response
> B. request bodies are to be read in the original protocol, input filters like 
> chunk can be used, indeed are necessary, as if the request is being processed 
> normally

Yes.

> C. if a protocol supports upgrade on request bodies is up to the protocol 
> implementation and needs to be checked in the "propose" phase

In some respects, yes, but Upgrade is defined by HTTP/1 and no other
protocol applies until after it is done.  That means ignoring any
idiotic handshake requirements of the "new" protocol that aren't
consistent with HTTP/1.  It also means h2's requirements on Upgrade
are irrelevant except for when it requires not upgrading to h2.

The client's assumption must be that the Upgrade will fail and any
attempt to use Expect will timeout, so the entire message will be
sent eventually regardless of anyone's stupid hacks to try and game
the protocol.  Hence, the server has no choice but to receive the
entire message as HTTP/1.1 even if it thinks it could have responded
fast enough to interrupt the client in mid-send.

> 
> Open:
> 1. Protocols like Websocket need to take over the 101 sending themselves in 
> the "switch protocol" phase. (correct, Jacob?). Should we delegate the 
> sending of the 101 to the protocol switch handler?

That seems unlikely.

> 2. General handling of request bodies. Options:
>  a setaside in core of up to nnn bytes before switch invocation
>  b do nothing, let protocol switch handler care about it

I think folks are confusing implementation with protocol.  There is no need
for the protocol being read on a connection to be the same protocol that is
being written in response.  In other words, the incoming connection remains
reading HTTP/1 bits until the message is finished, regardless of the decision
to upgrade the response stream -- the other protocol engine doesn't care.

This should be easily handled by adding a filter that translates the HTTP/1
incoming body (if any) to a single channel of the new protocol.  Just fake it.
There is no need to wait or set aside the bytes, unless that is desired for
other reasons (e.g., denial of denial of service attacks).

> 3. When to do the upgrade dance:
>  a post_read_request: upgrade precedes authentication
>  b handler: upgrade only honored on authenticated and otherwise ok requests
>  c both: introduce separate hooks? have an additional parameter? more 
> complexity

(a).  We do want to upgrade non-ok responses.  If the "new" protocol wants to
send a canned HTTP/1.1 error, it can do so without our help.

> 4. status code of protocol switch handler: if we move the task of 101 
> sending, the switch handler might not do it and keep the connection on the 
> "old" protocol. Then a connection close is not necessary. So, we would do the 
> close only when the switch handler returns APR_EOF.

Eh?

> 5. Will it be possible to migrate the current TLS upgrade handling to this 
> revised scheme?

TLS upgrade is never done with a body (and is broken code anyway).  Just fix it.
Note that the upgrade token is "TLS" -- it does not have to be "TLS/1.0".

Roy

Re: reverse proxy wishlist

[jean-frederic clere]

On 12/03/2015 03:59 PM, Jim Jagielski wrote:
> I put out a call on Twitter regarding this, but wanted to
> close the loop here as well.
> 
> What would *you* like to see as new features or enhancements
> w/ mod_proxy, esp reverse proxy. I was thinking about some
> sort of active backend monitoring, utilizing watchdog, which
> could also maybe, eventually, pull in performance and load
> data for the backend for a more accurate LB provider. But
> what about new LB methods? Any ideas there?

I need a more dynamic list of workers and balancers basically in
mod_cluster we are able to add and remove those. Our health check is
done by the nodes which also requires some changes in the proxy logic too.

Cheers

Jean-Frederic

Re: reverse proxy wishlist

[jean-frederic clere]

On 12/03/2015 07:39 PM, Houser, Rick wrote:
> I would definitely expect to see a substantial improvement if the thread 
> reservation could be delayed until the response headers are fully received.

That is something tricky you need to mix blocking and non-blocking of
have a bunch (configurable) threads that receive the data and are able
to know they have a complete message they can forward to the worker
thread that does the send (which also needs a bunch of threads dedicated
to send in a non-blocking way to the client).

Cheers

Jean-Frederic

Re: On the Upgrade request body limit

[Yann Ylavic]

On Tue, Dec 8, 2015 at 11:00 PM, William A Rowe Jr  wrote:
>
> Define complex, robust.  Request (upgrade: somespec) -> 100 continue ->
> request body <- [ http/1.1 response | 101 - switching protocols <- new
> protocol response ].

As I read the RFC, the simple(st) case is:
Request (upgrade: somespec) -> request body <- 101 (upgrade: somespec)
<- new protocol response or read

Whereas the 100-continue case is:
Request (upgrade: somespec, expect: 100-continue) [ <- 100-continue ->
request body ] <- 101 (upgrade: somespec) <- new protocol response or
read

Why any HTTP/1 response? If the Upgrade is accepted, ISTM that the
response must be Upgraded, no?

Re: Upgrade Summary

[Jacob Champion]

On 12/08/2015 01:03 PM, Jacob Champion wrote:

- The module that won the proposal is given one last chance to check the
incoming request and fail the upgrade with an immediate HTTP response.


And to add to this, for completeness: mod_websocket also needs to add 
any required headers (e.g. Sec-WebSocket-Accept/Protocol/Extensions) to 
the 101 response during this same conceptual "check" phase.


--Jacob

Re: Upgrade Summary

[Jacob Champion]

On 12/08/2015 02:30 AM, Bert Huijben wrote:

The TLS and H2C upgrades both begin in one form and end in a different form.
Websockets are kind of different in that they require a bad request response
in a specific case. I'm not sure in which protocol this error needs to be
send though.


For WebSocket, any upgrade failures have to be sent in the original 
HTTP/1.1 protocol using HTTP error codes.


WebSocket is different from TLS/h2c because, conceptually, you're 
upgrading the connection to a specific request target (a ws:// URL). The 
"response" sent back to the client after the upgrade (if you want to 
call it a response) is simply the establishment of a WebSocket 
connection for that URL.


--Jacob

Re: On the Upgrade request body limit

[Jim Jagielski]

> On Dec 8, 2015, at 4:00 PM, Jacob Champion  wrote:
> 
> On 12/08/2015 12:45 PM, Jim Jagielski wrote:
>> Well
>> 
>> We are not NGINX. We are also not Microsoft. We don't create HTTP
>> response codes willy-nilly. We actually try to *honor* the actual
>> specs.
> 
> Yes, I agree 100%. :) Trust me, I've dealt with enough non-standard HTTP 
> "extensions"; I'm not suggesting we just start doing stuff willy-nilly.
> 
> There are people who have worked on those specs on this mailing list, though, 
> who might be interested in making official improvements and additions -- or 
> have opinions on why those additions would not actually be improvements. This 
> seemed like a nice place to talk about it, since we have implementation 
> experts who can weigh in. If that's not on topic, I can try to take it 
> somewhere else.
> 

I agree that adding in some of the http/2 lists on this thread
would make sense.

Re: On the Upgrade request body limit

[William A Rowe Jr]

On Tue, Dec 8, 2015 at 2:22 PM, Jacob Champion  wrote:

> I wrote this in response to Stefan's note on the zero-length request body
> limit for h2c Upgrades, then realized it would further fragment the
> (already massive) conversation, so here it is.
>

And I agree to keep the other thread as clean as possible, so to your
thoughts...


> Stefan wrote:
>
>> It is possible, but difficult to do unless you can read the body and
>>
> > set it aside somewhere. Which again imposes an arbitrary (for the
> > client) limit. Experience shows that this leads to design of clients
> > that work will in developments, but then encounter requests
> > usage/sites/servers that have other limits and break.
>

That's nonsense, of course.  Apache httpd server offers limits on the
number of incoming headers, the number of bytes in those headers, the
number of bytes in the request line, the length of the URI.  The maximum
request body length.  This is just one more limit, but not an actual
*limitation*, because the request will be processed, simply not by changing
protocols first, per RFC7230.

As I'll point out again in closing, h2c isn't defined by RFC7230, and if it
decides to ignore requests with bodies, that's up to mod_http2 and other
implementations.  But RFC7230 has nothing to say to h2c on this, and rather
encourages protocol implementors to accept already-read C-L or T-E: chunked
request bodies in a hand-over into their upgraded protocol.

This is true of other things about HTTP/1.1 too -- how does a client figure
> out what the maximum request size for a server is? -- but it's made harder
> in this case by the fact that
>
> 1) 100 Continue is sent whether the request is upgraded or not, which
> makes it impossible to use a continue handshake to quickly determine
> upgrade limits, and
>

There is no need for the client to determine the upgrade limit, because the
body of the request is sent.  If the server -arbitrarily- chooses to
satisfy it, there will be a 101 following the body transmission.  But the
spec makes absolutely clear that it is the server's prerogative to upgrade
or not, irrespective of the particular upgrade requested.

If the server demands that the upgrade happen, before a request can be
honored, it must reply with a 426 until the client offers to upgrade.  If
the client demands that the upgrade happen, it must perform the simplest
applicable request (e.g. OPTIONS *) with the desired upgrade semantics and
verify that the upgrade occurred.

2) returning 413 during an upgrade would be ambiguous anyway. Is the 413
> really related to the request target, or is it just because I tried to
> upgrade and my request was temporarily limited?
>

It never would.  The possible results are the normal response in http/1.1
protocol, or 426 to tell the client it still must change protocols first.

That new protocol can make whatever demands on the protocol it wants to.
That the only acceptable upgrade to h2c will be an OPTIONS * with no
request body.  Or that POST upgrades will be refused.  Our httpd http/1.1
server doesn't and cannot care, it can read-ahead the same network bucket
it was going to read anyways, and let the new protocol handler decide, very
early or very late in the cycle, whether the upgrade will happen.  If it
won't, httpd must continue to serve the response sticking to http/1.1.

It seems to me that we're missing something in HTTP/1.1 that would make
> these complex Upgrade scenarios robust. IIUC, the point of allowing request
> bodies during upgrades was to decrease the number of round trips, but if
> implementations can't actually make use of it in practice then that's not
> very useful...
>

Define complex, robust.  Request (upgrade: somespec) -> 100 continue ->
request body <- [ http/1.1 response | 101 - switching protocols <- new
protocol response ].

The spec spells it out.  That is as clear as it gets.  One will happen, or
the other.

If it were enough of a problem to fix (and I'm not claiming it is,
> necessarily), how would we fix it? Would we need a new 1xx response (e.g.
> 103 Upgrade Declined, with headers indicating the related protocol and the
> reason for the failure)? A new header in OPTIONS? Something else entirely?
>

I was thinking about this... even within the 100-continue it might be
possible to add some commentary / hint of what might happen next.  Request
and response headers may contain (comments).

Or is the correct "fix" to rearchitect so the limit during upgrades can be
> exactly the same as during a normal request?


RFC7230 is baked, expect no changes.  The request (including 100-continue +
body) is read, the protocol is switched if possible, otherwise the response
is sent in http/1.1.  Those are our constraints.

Is there an advantage to stating no body accepted when we are free to
either upgrade or stay with http/1.1, at our whim? I'd suggest not and will
strongly object to the TLS filter behaving this way. I'll let http/2
protocol gurus argue what