You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by William A Rowe Jr <wr...@rowe-clan.net> on 2016/10/14 21:16:36 UTC

Re: svn commit: r1764961 - in /httpd/httpd/trunk: docs/manual/mod/core.xml modules/http/http_filters.c server/core.c server/gen_test_char.c server/protocol.c server/util.c

On Fri, Oct 14, 2016 at 3:48 PM, <wr...@apache.org> wrote:

> Author: wrowe
> Date: Fri Oct 14 20:48:43 2016
> New Revision: 1764961
>
> URL: http://svn.apache.org/viewvc?rev=1764961&view=rev
> Log:
> [...]
> Apply HttpProtocolOptions Strict to chunk header parsing, invalid
> whitespace is invalid, line termination must follow CRLF convention.
>
> [...]



> static apr_status_t parse_chunk_size(http_ctx_t *ctx, const char *buffer,
> [...]



> -        else if (c == ' ' || c == '\t') {
> +        else if (!strict && (c == ' ' || c == '\t')) {
>              /* Be lenient up to 10 BWS (term from rfc7230 - 3.2.3).
>               */
>              ctx->state = BODY_CHUNK_CR;
>

I'm not sure where this myth came from...

https://tools.ietf.org/html/rfc7230#section-4.1

has *NO* provision for BWS in the chunk size.

Re: svn commit: r1764961 - in /httpd/httpd/trunk: docs/manual/mod/core.xml modules/http/http_filters.c server/core.c server/gen_test_char.c server/protocol.c server/util.c

Posted by William A Rowe Jr <wr...@rowe-clan.net>.
On Mon, Oct 17, 2016 at 1:48 PM, Roy T. Fielding <fi...@gbiv.com> wrote:

> On Oct 15, 2016, at 2:10 AM, William A Rowe Jr <wr...@rowe-clan.net>
> wrote:
>
> On Sat, Oct 15, 2016 at 3:54 AM, William A Rowe Jr <wr...@rowe-clan.net>
> wrote:
>
>> On Fri, Oct 14, 2016 at 4:44 PM, Roy T. Fielding <fi...@gbiv.com>
>> wrote:
>>
>>> Right, though several people have requested it now as errata. Seems
>>> likely to be in the final update for STD.
>>>
>>
>> In the HttpProtocolOptions Unsafe mode, it is tolerated.
>>
>> Should it be the proper 'Strict' behavior to parse (never generate) such
>> noise?
>>
>
> FWIW, I see very little harm in potentially unsafe chunk headers because
> it becomes a serious chore to inject between alternating \r-only vs
> \n-only
> vs space trailing chunk headers. I'm not suggesting it can't be done, but
> most requests-with-body are intrinsically not idempotent, so one must be
> extremely clever to affect cache history.
>
> But it isn't impossible, so if the editors follow the way of BWS vs.
> follow
> the absolute explicit statements about HTTP request field names and
> the trailing ':', I'd be somewhat disappointed. Tighten ambiguity where
> there was little ambiguity before. Make explicit the real ambiguity for
> all user-agents and servers to implement. /shrug.
>
>
> We tried.  People complained.
>
> In any case, BWS only includes *( SP / HTAB ).  Not much ambiguity there.
>

Fair enough. There is no BWS allowed at present, nor a bare CR or LF, at
this
point. httpd is free to respond with any action it likes.

The original and distributed behaviors allow CRLF or LF, CR followed by
other
than LF was disallowed. The new trunk behavior disallows a bare LF also.

The original action was *(SP / HTAB), the distributed behavior restricts
this
to 10 SP/HTAB characters, the new trunk behavior disallows SP / HTAB
between the final hex digit and ';' delimiter. Note that we don't support
the
true *(SP / HTAB) rule by limiting it very severely.

I favor leaving the new no-space-tolerance rule but will accept the group's
choices, Roy appears to concede to accepting some BWS. I guess a quick
poll is in order... opinions?

Re: svn commit: r1764961 - in /httpd/httpd/trunk: docs/manual/mod/core.xml modules/http/http_filters.c server/core.c server/gen_test_char.c server/protocol.c server/util.c

Posted by "Roy T. Fielding" <fi...@gbiv.com>.
> On Oct 15, 2016, at 2:10 AM, William A Rowe Jr <wr...@rowe-clan.net> wrote:
> 
> On Sat, Oct 15, 2016 at 3:54 AM, William A Rowe Jr <wrowe@rowe-clan.net <ma...@rowe-clan.net>> wrote:
> On Fri, Oct 14, 2016 at 4:44 PM, Roy T. Fielding <fielding@gbiv.com <ma...@gbiv.com>> wrote:
> Right, though several people have requested it now as errata. Seems likely to be in the final update for STD.
> 
> In the HttpProtocolOptions Unsafe mode, it is tolerated.
> 
> Should it be the proper 'Strict' behavior to parse (never generate) such noise? 
> 
> FWIW, I see very little harm in potentially unsafe chunk headers because
> it becomes a serious chore to inject between alternating \r-only vs \n-only 
> vs space trailing chunk headers. I'm not suggesting it can't be done, but
> most requests-with-body are intrinsically not idempotent, so one must be
> extremely clever to affect cache history. 
> 
> But it isn't impossible, so if the editors follow the way of BWS vs. follow 
> the absolute explicit statements about HTTP request field names and
> the trailing ':', I'd be somewhat disappointed. Tighten ambiguity where
> there was little ambiguity before. Make explicit the real ambiguity for
> all user-agents and servers to implement. /shrug.

We tried.  People complained.

In any case, BWS only includes *( SP / HTAB ).  Not much ambiguity there.

....Roy


Re: svn commit: r1764961 - in /httpd/httpd/trunk: docs/manual/mod/core.xml modules/http/http_filters.c server/core.c server/gen_test_char.c server/protocol.c server/util.c

Posted by William A Rowe Jr <wr...@rowe-clan.net>.
On Sat, Oct 15, 2016 at 3:54 AM, William A Rowe Jr <wr...@rowe-clan.net>
wrote:

> On Fri, Oct 14, 2016 at 4:44 PM, Roy T. Fielding <fi...@gbiv.com>
> wrote:
>
>> Right, though several people have requested it now as errata. Seems
>> likely to be in the final update for STD.
>>
>
> In the HttpProtocolOptions Unsafe mode, it is tolerated.
>
> Should it be the proper 'Strict' behavior to parse (never generate) such
> noise?
>

FWIW, I see very little harm in potentially unsafe chunk headers because
it becomes a serious chore to inject between alternating \r-only vs \n-only
vs space trailing chunk headers. I'm not suggesting it can't be done, but
most requests-with-body are intrinsically not idempotent, so one must be
extremely clever to affect cache history.

But it isn't impossible, so if the editors follow the way of BWS vs. follow
the absolute explicit statements about HTTP request field names and
the trailing ':', I'd be somewhat disappointed. Tighten ambiguity where
there was little ambiguity before. Make explicit the real ambiguity for
all user-agents and servers to implement. /shrug.

Cheers,

Bill

Re: svn commit: r1764961 - in /httpd/httpd/trunk: docs/manual/mod/core.xml modules/http/http_filters.c server/core.c server/gen_test_char.c server/protocol.c server/util.c

Posted by William A Rowe Jr <wr...@rowe-clan.net>.
On Fri, Oct 14, 2016 at 4:44 PM, Roy T. Fielding <fi...@gbiv.com> wrote:

> Right, though several people have requested it now as errata. Seems likely
> to be in the final update for STD.
>

In the HttpProtocolOptions Unsafe mode, it is tolerated.

Should it be the proper 'Strict' behavior to parse (never generate) such
noise?

Re: svn commit: r1764961 - in /httpd/httpd/trunk: docs/manual/mod/core.xml modules/http/http_filters.c server/core.c server/gen_test_char.c server/protocol.c server/util.c

Posted by "Roy T. Fielding" <fi...@gbiv.com>.
Right, though several people have requested it now as errata. Seems likely to be in the final update for STD.

....Roy


> On Oct 14, 2016, at 2:16 PM, William A Rowe Jr <wr...@rowe-clan.net> wrote:
> 
>> On Fri, Oct 14, 2016 at 3:48 PM, <wr...@apache.org> wrote:
>> Author: wrowe
>> Date: Fri Oct 14 20:48:43 2016
>> New Revision: 1764961
>> 
>> URL: http://svn.apache.org/viewvc?rev=1764961&view=rev
>> Log:
>> [...]
>> Apply HttpProtocolOptions Strict to chunk header parsing, invalid
>> whitespace is invalid, line termination must follow CRLF convention.
>> 
>> [...]
>  
>> static apr_status_t parse_chunk_size(http_ctx_t *ctx, const char *buffer,
>> [...]
>  
>> -        else if (c == ' ' || c == '\t') {
>> +        else if (!strict && (c == ' ' || c == '\t')) {
>>              /* Be lenient up to 10 BWS (term from rfc7230 - 3.2.3).
>>               */
>>              ctx->state = BODY_CHUNK_CR;
> 
> I'm not sure where this myth came from... 
> 
> https://tools.ietf.org/html/rfc7230#section-4.1
> 
> has *NO* provision for BWS in the chunk size.