Re: svn commit: r1754548 - /httpd/httpd/trunk/server/protocol.c

[William A Rowe Jr]

On Thu, Aug 18, 2016 at 5:00 AM, William A Rowe Jr 
wrote:

> Just FWIW, this still is not fixed for the legacy header parser.
>
> It *is* now fixed for the HTTP Request Line parser. Relaxing the
> whitespace rule (as we still do by default) only lets 1+ SP/HTAB
> slip through, and then recomposes with single SP delimiters.
>
> Of the subset \f \r \v \n I can't think of any possible application.
> Whitespace of ' ' and \t makes (some) sense in the real world.
> If anyone has a real-world example of a user-agent which used
> these legitimately, I'd love a pointer.
>

Committed in 1756847, either Strict or StrictWhitespace will reject
these quirks, Unsafe and LenientWhitespace together are required
to continue to handle such headers, and never permitted for the
request line itself.

Re: svn commit: r1754548 - /httpd/httpd/trunk/server/protocol.c

[William A Rowe Jr]

Just FWIW, this still is not fixed for the legacy header parser.

It *is* now fixed for the HTTP Request Line parser. Relaxing the
whitespace rule (as we still do by default) only lets 1+ SP/HTAB
slip through, and then recomposes with single SP delimiters.

Of the subset \f \r \v \n I can't think of any possible application.
Whitespace of ' ' and \t makes (some) sense in the real world.
If anyone has a real-world example of a user-agent which used
these legitimately, I'd love a pointer.


On Thu, Aug 18, 2016 at 4:34 AM, Plüm, Rüdiger, Vodafone Group <
ruediger.pl...@vodafone.com> wrote:

> +1
>
> Regards
>
> Rüdiger
>
> > -Original Message-
> > From: Jacob Champion [mailto:champio...@gmail.com]
> > Sent: Donnerstag, 4. August 2016 22:35
> > To: dev@httpd.apache.org
> > Subject: Re: svn commit: r1754548 - /httpd/httpd/trunk/server/protocol.c
> >
> > On 08/04/2016 01:11 PM, William A Rowe Jr wrote:
> > > At our kindest, we would like to let people keep upgrading on the 2.2
> > > or 2.4 branches of httpd for other fixes, without breaking their
> > > deployments.
> > >
> > > I'm 100% in favor of recognizing-and-rejecting (and terminating the
> > > connection) for any obs-fold occurrences on 2.6 / 3.0, if that's the
> > > group consensus.
> >
> > +1 to both.
> >
> > --Jacob
>
>

RE: svn commit: r1754548 - /httpd/httpd/trunk/server/protocol.c

[Plüm , Rüdiger , Vodafone Group]

+1

Regards

Rüdiger

> -Original Message-
> From: Jacob Champion [mailto:champio...@gmail.com]
> Sent: Donnerstag, 4. August 2016 22:35
> To: dev@httpd.apache.org
> Subject: Re: svn commit: r1754548 - /httpd/httpd/trunk/server/protocol.c
> 
> On 08/04/2016 01:11 PM, William A Rowe Jr wrote:
> > At our kindest, we would like to let people keep upgrading on the 2.2
> > or 2.4 branches of httpd for other fixes, without breaking their
> > deployments.
> >
> > I'm 100% in favor of recognizing-and-rejecting (and terminating the
> > connection) for any obs-fold occurrences on 2.6 / 3.0, if that's the
> > group consensus.
> 
> +1 to both.
> 
> --Jacob

Re: svn commit: r1754548 - /httpd/httpd/trunk/server/protocol.c

[William A Rowe Jr]

On Aug 5, 2016 3:43 PM, "William A Rowe Jr"  wrote:
>
> Our strict mode parsing still permits simple "\n" line termination rather
than the CRLF as defined by spec. Here again, I can't find a security or
integrity issue.
>
> In neither case do we send bad data as request headers to a backend or
bad data in a response.

RFC7230 3.5 provided the answer, no issue here since we don't propagate the
legacy behavior in our own requests or responses.

Re: svn commit: r1754548 - /httpd/httpd/trunk/server/protocol.c

[William A Rowe Jr]

On Aug 4, 2016 3:02 PM, "Roy T. Fielding"  wrote:
>>
>> On Aug 3, 2016, at 2:28 PM, William A Rowe Jr 
wrote:
>
>> If not, then stripping trailing whitespace from the line prior to
obs-fold and
>> eating all leading whitespace on the obs-fold line will result in a
single SP
>> character, which should be just fine unless spaces were significant
within
>> a quoted value. The only way for the client to preserve such significant
>> spaces would be to place them after the opening quote before the
obs-fold.
>
> obs-fold is not allowed inside quoted text, so we need not worry about
> messing with such a construct.

Roy especially, and others familiar with the explicit purpose and meaning
of the spec...

We do not guard against an obs-fold occurring within a field-vchar segment,
our unfolding occurs beforehand...

field-content = field-vchar [ 1*( SP / HTAB ) field-vchar ]

>From a security and integrity perspective I can find no reason to guard
against an obs-fold within a quoted string, for example. Whitespace
compression to a single SP occurs to that token sent by an inept client
still using obs-fold, but this appears to have no negative side-effects.

Our strict mode parsing still permits simple "\n" line termination rather
than the CRLF as defined by spec. Here again, I can't find a security or
integrity issue.

In neither case do we send bad data as request headers to a backend or bad
data in a response.

Is there any justification for changing either of these permissive
behaviors?

Re: svn commit: r1754548 - /httpd/httpd/trunk/server/protocol.c

[Jacob Champion]

On 08/04/2016 01:11 PM, William A Rowe Jr wrote:

At our kindest, we would like to let people keep upgrading on the 2.2
or 2.4 branches of httpd for other fixes, without breaking their
deployments.

I'm 100% in favor of recognizing-and-rejecting (and terminating the
connection) for any obs-fold occurrences on 2.6 / 3.0, if that's the
group consensus.


+1 to both.

--Jacob

Re: svn commit: r1754548 - /httpd/httpd/trunk/server/protocol.c

[William A Rowe Jr]

Thanks for the feedback...

On Thu, Aug 4, 2016 at 3:02 PM, Roy T. Fielding  wrote:

> On Aug 3, 2016, at 2:28 PM, William A Rowe Jr  wrote:
>
> So AIUI, the leading SP / TAB whitespace in a field is a no-op (usually
> represented by a single space by convention), and trailing whitespace
> in the field value is a no-op, all leading tabs/spaces (beyond one SP)
> in the obs-fold line is a no-op. Is there any reason to preserve trailing
> spaces before the obs-fold?
>
>
> Not given our implementation.  The buffer efficiency argument is for other
> kinds of parsers that are not reading just one line at a time.
>

And because we are merging line-at-a-time, we have both factors, the cost
of stripping back trailing whitespace before an obs-fold (we should be doing
this at the end of the header field value in any case), v.s. buffer
allocation
for no-op whitespace.

If not, then stripping trailing whitespace from the line prior to obs-fold
> and
> eating all leading whitespace on the obs-fold line will result in a single
> SP
> character, which should be just fine unless spaces were significant within
> a quoted value. The only way for the client to preserve such significant
> spaces would be to place them after the opening quote before the obs-fold.
>
>
> obs-fold is not allowed inside quoted text, so we need not worry about
> messing with such a construct.
>

That solves that, thanks for the clarity.


> Note that obs-fold has been formally deprecated outside of message/http.
> We can remove its handling at any time we are willing to accept the risk
> of strange error reports.  I do not believe it is part of our versioning
> contract.
>

At our kindest, we would like to let people keep upgrading on the 2.2 or
2.4
branches of httpd for other fixes, without breaking their deployments.

I'm 100% in favor of recognizing-and-rejecting (and terminating the
connection)
for any obs-fold occurrences on 2.6 / 3.0, if that's the group consensus.

It still must be supported in media type message/http, but that shouldn't
apply
to anything in the core of httpd. Third party module authors would have to
make
appropriate accommodations if they directly handle this content.

Re: svn commit: r1754548 - /httpd/httpd/trunk/server/protocol.c

[Roy T. Fielding]

> On Aug 3, 2016, at 2:28 PM, William A Rowe Jr  wrote:
> 
> So AIUI, the leading SP / TAB whitespace in a field is a no-op (usually
> represented by a single space by convention), and trailing whitespace 
> in the field value is a no-op, all leading tabs/spaces (beyond one SP) 
> in the obs-fold line is a no-op. Is there any reason to preserve trailing 
> spaces before the obs-fold?

Not given our implementation.  The buffer efficiency argument is for other
kinds of parsers that are not reading just one line at a time.

> If not, then stripping trailing whitespace from the line prior to obs-fold and
> eating all leading whitespace on the obs-fold line will result in a single SP
> character, which should be just fine unless spaces were significant within
> a quoted value. The only way for the client to preserve such significant 
> spaces would be to place them after the opening quote before the obs-fold.

obs-fold is not allowed inside quoted text, so we need not worry about
messing with such a construct.

Note that obs-fold has been formally deprecated outside of message/http.
We can remove its handling at any time we are willing to accept the risk
of strange error reports.  I do not believe it is part of our versioning 
contract.

Roy

Re: svn commit: r1754548 - /httpd/httpd/trunk/server/protocol.c

[William A Rowe Jr]

On Wed, Aug 3, 2016 at 4:28 PM, William A Rowe Jr 
wrote:

> On Wed, Aug 3, 2016 at 1:53 PM, Roy T. Fielding  wrote:
>
>> > On Aug 3, 2016, at 11:44 AM, Jacob Champion 
>> wrote:
>> >
>> > On 07/31/2016 09:18 AM, William A Rowe Jr wrote:
>> >>> So all the trailing SP/HTAB are part of obs-fold IMHO.
>> >>> Should we replace all of them (plus the CRLF) with a single SP or with
>> >>> as many SP?
>> >>
>> >> Hmmm... Good point. Advancing over them in our HTTP_STRICT mode seems
>> >> best, if we have a consensus on this.
>> >
>> > Agreed that we should process all the obs-fold whitespace, and not just
>> one byte.
>> >
>> > Replacing each byte with a separate space (as opposed to condensing
>> into a single space) *might* help prevent adversaries from playing games
>> with header length checks in more complicated/layered systems. That's
>> probably a stretch though. And if we consume the CRLF in a different layer
>> of logic, adding on two spaces just to keep everything "consistent" may
>> also be a stretch. I'm not feeling strongly either way.
>>
>> What the spec is trying to say is that we can either replace all those
>> bytes
>> with a single SP (semantically speaking they are the same) or we we can
>> replace
>> them all with a sequence of SP (still the same, but doesn't require
>> splitting
>> or recomposing the buffer).
>>
>> > >> > So the obs-fold itself consists of CR LF [ SP | TAB ]
>> > >>
>> > >>obs-fold = CRLF 1*( SP / HTAB )
>> > >>
>> >
>> > Note that this section of the spec has Errata associated with it; I'm
>> reading through the conversation [1] and it's seeming like they *may* want
>> to treat OWS preceding the CRLF as part of the obs-fold as well. I don't
>> know what our position is on adopting pieces of Errata that have been Held
>> for Document Update.
>>
>> No, that is just an ABNF issue for matching purposes.  We don't use it.
>>
>
> So AIUI, the leading SP / TAB whitespace in a field is a no-op (usually
> represented by a single space by convention), and trailing whitespace
> in the field value is a no-op, all leading tabs/spaces (beyond one SP)
> in the obs-fold line is a no-op. Is there any reason to preserve trailing
> spaces before the obs-fold?
>
> If not, then stripping trailing whitespace from the line prior to obs-fold
> and
> eating all leading whitespace on the obs-fold line will result in a single
> SP
> character, which should be just fine unless spaces were significant within
> a quoted value. The only way for the client to preserve such significant
> spaces would be to place them after the opening quote before the obs-fold.
>

To ensure the patch is entirely correct on my 'next try', I'd like
clarification
here before resuming this code review and correction. If we know the spaces
and tabs around an obs-fold, both leading - and trailing, are all
collapsible
to a single SP, and the leading and trailing spaces around the field value
are all discarded, then the code and resulting patch gets simpler, not more
confusing.

Re: svn commit: r1754548 - /httpd/httpd/trunk/server/protocol.c

[William A Rowe Jr]

On Wed, Aug 3, 2016 at 1:53 PM, Roy T. Fielding  wrote:

> > On Aug 3, 2016, at 11:44 AM, Jacob Champion 
> wrote:
> >
> > On 07/31/2016 09:18 AM, William A Rowe Jr wrote:
> >>> So all the trailing SP/HTAB are part of obs-fold IMHO.
> >>> Should we replace all of them (plus the CRLF) with a single SP or with
> >>> as many SP?
> >>
> >> Hmmm... Good point. Advancing over them in our HTTP_STRICT mode seems
> >> best, if we have a consensus on this.
> >
> > Agreed that we should process all the obs-fold whitespace, and not just
> one byte.
> >
> > Replacing each byte with a separate space (as opposed to condensing into
> a single space) *might* help prevent adversaries from playing games with
> header length checks in more complicated/layered systems. That's probably a
> stretch though. And if we consume the CRLF in a different layer of logic,
> adding on two spaces just to keep everything "consistent" may also be a
> stretch. I'm not feeling strongly either way.
>
> What the spec is trying to say is that we can either replace all those
> bytes
> with a single SP (semantically speaking they are the same) or we we can
> replace
> them all with a sequence of SP (still the same, but doesn't require
> splitting
> or recomposing the buffer).
>
> > >> > So the obs-fold itself consists of CR LF [ SP | TAB ]
> > >>
> > >>obs-fold = CRLF 1*( SP / HTAB )
> > >>
> >
> > Note that this section of the spec has Errata associated with it; I'm
> reading through the conversation [1] and it's seeming like they *may* want
> to treat OWS preceding the CRLF as part of the obs-fold as well. I don't
> know what our position is on adopting pieces of Errata that have been Held
> for Document Update.
>
> No, that is just an ABNF issue for matching purposes.  We don't use it.
>

So AIUI, the leading SP / TAB whitespace in a field is a no-op (usually
represented by a single space by convention), and trailing whitespace
in the field value is a no-op, all leading tabs/spaces (beyond one SP)
in the obs-fold line is a no-op. Is there any reason to preserve trailing
spaces before the obs-fold?

If not, then stripping trailing whitespace from the line prior to obs-fold
and
eating all leading whitespace on the obs-fold line will result in a single
SP
character, which should be just fine unless spaces were significant within
a quoted value. The only way for the client to preserve such significant
spaces would be to place them after the opening quote before the obs-fold.

Re: svn commit: r1754548 - /httpd/httpd/trunk/server/protocol.c

[Jacob Champion]

On 08/03/2016 12:05 PM, Jacob Champion wrote:

So if there is an HTAB directly *before* the obs-fold CRLF, we should
not try to replace with a SP?


Ad never mind here; looks like we already strip trailing OWS from 
the end of the line before the fold. That'll teach me to theorycraft 
before reading the code.


Thanks Roy for the clarifications. :)

--Jacob

Re: svn commit: r1754548 - /httpd/httpd/trunk/server/protocol.c

[Jacob Champion]

On 08/03/2016 12:05 PM, Jacob Champion wrote:

Right, I was just wondering out loud if condensing into a single space
could give anyone the chance to defeat a header length check in a
multi-layered system. It's admittedly a pretty "tinfoil hat" concern.


Never mind. It would need to be the other way around (expanding a header 
value rather than contracting) to cause a problem for a backend.


--Jacob

Re: svn commit: r1754548 - /httpd/httpd/trunk/server/protocol.c

[Jacob Champion]

On 08/03/2016 11:53 AM, Roy T. Fielding wrote:

Replacing each byte with a separate space (as opposed to condensing into a single space) 
*might* help prevent adversaries from playing games with header length checks in more 
complicated/layered systems. That's probably a stretch though. And if we consume the CRLF 
in a different layer of logic, adding on two spaces just to keep everything 
"consistent" may also be a stretch. I'm not feeling strongly either way.


What the spec is trying to say is that we can either replace all those bytes
with a single SP (semantically speaking they are the same) or we we can replace
them all with a sequence of SP (still the same, but doesn't require splitting
or recomposing the buffer).


Right, I was just wondering out loud if condensing into a single space 
could give anyone the chance to defeat a header length check in a 
multi-layered system. It's admittedly a pretty "tinfoil hat" concern.



So the obs-fold itself consists of CR LF [ SP | TAB ]


   obs-fold = CRLF 1*( SP / HTAB )



Note that this section of the spec has Errata associated with it; I'm reading 
through the conversation [1] and it's seeming like they *may* want to treat OWS 
preceding the CRLF as part of the obs-fold as well. I don't know what our 
position is on adopting pieces of Errata that have been Held for Document 
Update.


No, that is just an ABNF issue for matching purposes.  We don't use it.


So if there is an HTAB directly *before* the obs-fold CRLF, we should 
not try to replace with a SP?


--Jacob

Re: svn commit: r1754548 - /httpd/httpd/trunk/server/protocol.c

[Roy T. Fielding]

> On Aug 3, 2016, at 11:44 AM, Jacob Champion  wrote:
> 
> On 07/31/2016 09:18 AM, William A Rowe Jr wrote:
>>> So all the trailing SP/HTAB are part of obs-fold IMHO.
>>> Should we replace all of them (plus the CRLF) with a single SP or with
>>> as many SP?
>> 
>> Hmmm... Good point. Advancing over them in our HTTP_STRICT mode seems
>> best, if we have a consensus on this.
> 
> Agreed that we should process all the obs-fold whitespace, and not just one 
> byte.
> 
> Replacing each byte with a separate space (as opposed to condensing into a 
> single space) *might* help prevent adversaries from playing games with header 
> length checks in more complicated/layered systems. That's probably a stretch 
> though. And if we consume the CRLF in a different layer of logic, adding on 
> two spaces just to keep everything "consistent" may also be a stretch. I'm 
> not feeling strongly either way.

What the spec is trying to say is that we can either replace all those bytes
with a single SP (semantically speaking they are the same) or we we can replace
them all with a sequence of SP (still the same, but doesn't require splitting
or recomposing the buffer).

> >> > So the obs-fold itself consists of CR LF [ SP | TAB ]
> >>
> >>obs-fold = CRLF 1*( SP / HTAB )
> >>
> 
> Note that this section of the spec has Errata associated with it; I'm reading 
> through the conversation [1] and it's seeming like they *may* want to treat 
> OWS preceding the CRLF as part of the obs-fold as well. I don't know what our 
> position is on adopting pieces of Errata that have been Held for Document 
> Update.

No, that is just an ABNF issue for matching purposes.  We don't use it.

Roy

Re: svn commit: r1754548 - /httpd/httpd/trunk/server/protocol.c

[Jacob Champion]

On 07/31/2016 09:18 AM, William A Rowe Jr wrote:

So all the trailing SP/HTAB are part of obs-fold IMHO.
Should we replace all of them (plus the CRLF) with a single SP or with
as many SP?


Hmmm... Good point. Advancing over them in our HTTP_STRICT mode seems
best, if we have a consensus on this.


Agreed that we should process all the obs-fold whitespace, and not just 
one byte.


Replacing each byte with a separate space (as opposed to condensing into 
a single space) *might* help prevent adversaries from playing games with 
header length checks in more complicated/layered systems. That's 
probably a stretch though. And if we consume the CRLF in a different 
layer of logic, adding on two spaces just to keep everything 
"consistent" may also be a stretch. I'm not feeling strongly either way.


>> > So the obs-fold itself consists of CR LF [ SP | TAB ]
>>
>>obs-fold = CRLF 1*( SP / HTAB )
>>

Note that this section of the spec has Errata associated with it; I'm 
reading through the conversation [1] and it's seeming like they *may* 
want to treat OWS preceding the CRLF as part of the obs-fold as well. I 
don't know what our position is on adopting pieces of Errata that have 
been Held for Document Update.


--Jacob

[1] https://www.ietf.org/mail-archive/web/httpbisa/current/msg23721.html

Re: svn commit: r1754548 - /httpd/httpd/trunk/server/protocol.c

[William A Rowe Jr]

On Jul 31, 2016 3:17 AM, "Yann Ylavic"  wrote:
>
> On Sun, Jul 31, 2016 at 12:56 AM, William A Rowe Jr 
wrote:
> > On Jul 30, 2016 4:36 PM, "Yann Ylavic"  wrote:
> >>
> >> On Sat, Jul 30, 2016 at 11:22 PM, Yann Ylavic 
> >> wrote:
> >> > On Fri, Jul 29, 2016 at 6:24 PM,   wrote:
> >> >> Author: wrowe
> >> >> Date: Fri Jul 29 16:24:14 2016
> >> >> New Revision: 1754548
> >> >>
> >> >> URL: http://svn.apache.org/viewvc?rev=1754548=rev
> >> >> Log:
> >> >> Strictly observe spec on obs-fold
> >> >>
> >> >> Modified:
> >> >> httpd/httpd/trunk/server/protocol.c
> >> > []
> >> >>
> >> >>  memcpy(last_field + last_len, field, len +1); /* +1
> >> >> for nul */
> >> >> +/* Replace obs-fold w/ SP per RFC 7230 3.2.4 */
> >> >> +if (conf->http_conformance &
> >> >> AP_HTTP_CONFORMANCE_STRICT) {
> >> >> +last_field[last_len] = ' ';
> >> >> +}
> >> >
> >> > The wording is:
> >> >A user agent that receives an obs-fold in a response message that
is
> >> >not within a message/http container MUST replace each received
> >> >obs-fold with one or more SP octets prior to interpreting the
field
> >> >value.
> >>
> >> Please forget the "user agent" part, the wording for
> >> server/proxy/gateway is the same though, from "MUST replace each
> >> received..."
> >>
> >> >
> >> > Not sure if it means that one HTAB or more than one SP/HTAB of each
> >> > obs-fold should be replaced by a single SP (that's what I think), or
> >> > if it's that all HTAB should be replaced by a SP (keeping as many
> >> > "spaces").
> >> >
> >> > In any case the above code will replace one HTAB only, we probably
> >> > need a loop here.
> >> >
> >> >>  last_len += len;
> >> >>  folded = 1;
> >> >>  }
> >> >
> >> > Regards,
> >> > Yann.
> >
> > So the obs-fold itself consists of CR LF [ SP | TAB ]
>
>obs-fold = CRLF 1*( SP / HTAB )
>
> So all the trailing SP/HTAB are part of obs-fold IMHO.
> Should we replace all of them (plus the CRLF) with a single SP or with
> as many SP?

Hmmm... Good point. Advancing over them in our HTTP_STRICT mode seems best,
if we have a consensus on this.

Re: svn commit: r1754548 - /httpd/httpd/trunk/server/protocol.c

[Yann Ylavic]

On Sun, Jul 31, 2016 at 12:56 AM, William A Rowe Jr  wrote:
> On Jul 30, 2016 4:36 PM, "Yann Ylavic"  wrote:
>>
>> On Sat, Jul 30, 2016 at 11:22 PM, Yann Ylavic 
>> wrote:
>> > On Fri, Jul 29, 2016 at 6:24 PM,   wrote:
>> >> Author: wrowe
>> >> Date: Fri Jul 29 16:24:14 2016
>> >> New Revision: 1754548
>> >>
>> >> URL: http://svn.apache.org/viewvc?rev=1754548=rev
>> >> Log:
>> >> Strictly observe spec on obs-fold
>> >>
>> >> Modified:
>> >> httpd/httpd/trunk/server/protocol.c
>> > []
>> >>
>> >>  memcpy(last_field + last_len, field, len +1); /* +1
>> >> for nul */
>> >> +/* Replace obs-fold w/ SP per RFC 7230 3.2.4 */
>> >> +if (conf->http_conformance &
>> >> AP_HTTP_CONFORMANCE_STRICT) {
>> >> +last_field[last_len] = ' ';
>> >> +}
>> >
>> > The wording is:
>> >A user agent that receives an obs-fold in a response message that is
>> >not within a message/http container MUST replace each received
>> >obs-fold with one or more SP octets prior to interpreting the field
>> >value.
>>
>> Please forget the "user agent" part, the wording for
>> server/proxy/gateway is the same though, from "MUST replace each
>> received..."
>>
>> >
>> > Not sure if it means that one HTAB or more than one SP/HTAB of each
>> > obs-fold should be replaced by a single SP (that's what I think), or
>> > if it's that all HTAB should be replaced by a SP (keeping as many
>> > "spaces").
>> >
>> > In any case the above code will replace one HTAB only, we probably
>> > need a loop here.
>> >
>> >>  last_len += len;
>> >>  folded = 1;
>> >>  }
>> >
>> > Regards,
>> > Yann.
>
> So the obs-fold itself consists of CR LF [ SP | TAB ]

   obs-fold = CRLF 1*( SP / HTAB )

So all the trailing SP/HTAB are part of obs-fold IMHO.
Should we replace all of them (plus the CRLF) with a single SP or with
as many SP?

Re: svn commit: r1754548 - /httpd/httpd/trunk/server/protocol.c

[William A Rowe Jr]

On Jul 30, 2016 4:36 PM, "Yann Ylavic"  wrote:
>
> On Sat, Jul 30, 2016 at 11:22 PM, Yann Ylavic 
wrote:
> > On Fri, Jul 29, 2016 at 6:24 PM,   wrote:
> >> Author: wrowe
> >> Date: Fri Jul 29 16:24:14 2016
> >> New Revision: 1754548
> >>
> >> URL: http://svn.apache.org/viewvc?rev=1754548=rev
> >> Log:
> >> Strictly observe spec on obs-fold
> >>
> >> Modified:
> >> httpd/httpd/trunk/server/protocol.c
> > []
> >>
> >>  memcpy(last_field + last_len, field, len +1); /* +1
for nul */
> >> +/* Replace obs-fold w/ SP per RFC 7230 3.2.4 */
> >> +if (conf->http_conformance &
AP_HTTP_CONFORMANCE_STRICT) {
> >> +last_field[last_len] = ' ';
> >> +}
> >
> > The wording is:
> >A user agent that receives an obs-fold in a response message that is
> >not within a message/http container MUST replace each received
> >obs-fold with one or more SP octets prior to interpreting the field
> >value.
>
> Please forget the "user agent" part, the wording for
> server/proxy/gateway is the same though, from "MUST replace each
> received..."
>
> >
> > Not sure if it means that one HTAB or more than one SP/HTAB of each
> > obs-fold should be replaced by a single SP (that's what I think), or
> > if it's that all HTAB should be replaced by a SP (keeping as many
> > "spaces").
> >
> > In any case the above code will replace one HTAB only, we probably
> > need a loop here.
> >
> >>  last_len += len;
> >>  folded = 1;
> >>  }
> >
> > Regards,
> > Yann.

So the obs-fold itself consists of CR LF [ SP | TAB ]

Any whitespace following the single sp/tab is field whitespace. I
considered and determined we do not further collapse whitespace, as these
may be in quoted field contents.

But the CR LF TAB becomes SP per spec.

Re: svn commit: r1754548 - /httpd/httpd/trunk/server/protocol.c

[Yann Ylavic]

On Sat, Jul 30, 2016 at 11:22 PM, Yann Ylavic  wrote:
> On Fri, Jul 29, 2016 at 6:24 PM,   wrote:
>> Author: wrowe
>> Date: Fri Jul 29 16:24:14 2016
>> New Revision: 1754548
>>
>> URL: http://svn.apache.org/viewvc?rev=1754548=rev
>> Log:
>> Strictly observe spec on obs-fold
>>
>> Modified:
>> httpd/httpd/trunk/server/protocol.c
> []
>>
>>  memcpy(last_field + last_len, field, len +1); /* +1 for nul 
>> */
>> +/* Replace obs-fold w/ SP per RFC 7230 3.2.4 */
>> +if (conf->http_conformance & AP_HTTP_CONFORMANCE_STRICT) {
>> +last_field[last_len] = ' ';
>> +}
>
> The wording is:
>A user agent that receives an obs-fold in a response message that is
>not within a message/http container MUST replace each received
>obs-fold with one or more SP octets prior to interpreting the field
>value.

Please forget the "user agent" part, the wording for
server/proxy/gateway is the same though, from "MUST replace each
received..."

>
> Not sure if it means that one HTAB or more than one SP/HTAB of each
> obs-fold should be replaced by a single SP (that's what I think), or
> if it's that all HTAB should be replaced by a SP (keeping as many
> "spaces").
>
> In any case the above code will replace one HTAB only, we probably
> need a loop here.
>
>>  last_len += len;
>>  folded = 1;
>>  }
>
> Regards,
> Yann.

Re: svn commit: r1754548 - /httpd/httpd/trunk/server/protocol.c

[Yann Ylavic]

On Fri, Jul 29, 2016 at 6:24 PM,   wrote:
> Author: wrowe
> Date: Fri Jul 29 16:24:14 2016
> New Revision: 1754548
>
> URL: http://svn.apache.org/viewvc?rev=1754548=rev
> Log:
> Strictly observe spec on obs-fold
>
> Modified:
> httpd/httpd/trunk/server/protocol.c
[]
>
>  memcpy(last_field + last_len, field, len +1); /* +1 for nul 
> */
> +/* Replace obs-fold w/ SP per RFC 7230 3.2.4 */
> +if (conf->http_conformance & AP_HTTP_CONFORMANCE_STRICT) {
> +last_field[last_len] = ' ';
> +}

The wording is:
   A user agent that receives an obs-fold in a response message that is
   not within a message/http container MUST replace each received
   obs-fold with one or more SP octets prior to interpreting the field
   value.

Not sure if it means that one HTAB or more than one SP/HTAB of each
obs-fold should be replaced by a single SP (that's what I think), or
if it's that all HTAB should be replaced by a SP (keeping as many
"spaces").

In any case the above code will replace one HTAB only, we probably
need a loop here.

>  last_len += len;
>  folded = 1;
>  }

Regards,
Yann.