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

2016-10-18 Thread William A Rowe Jr 
On Sat, Oct 15, 2016 at 6:23 PM, Rainer Jung 
wrote:

> Am 14.10.2016 um 22:48 schrieb wr...@apache.org:
>
>> Author: wrowe
>> Date: Fri Oct 14 20:48:43 2016
>> New Revision: 1764961
>>
>> URL: http://svn.apache.org/viewvc?rev=1764961=rev
>> Log:
>>
>> Dropped the never-released ap_has_cntrls() as it had very limited
>> and inefficient application at that, added ap_scan_vchar_obstext()
>> to accomplish a similar purpose.
>>
>
> Should we note the API changes (2 removals, one addition) in the mmn file?
> At least the original addition of ap_has_cntrl() is still noted there.
>

Worse, it was an mmn major. If we remove a struct member, a minor
bump doesn't suffice.

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

2016-10-17 Thread William A Rowe Jr 
On Mon, Oct 17, 2016 at 1:48 PM, Roy T. Fielding  wrote:

> On Oct 15, 2016, at 2:10 AM, William A Rowe Jr 
> wrote:
>
> On Sat, Oct 15, 2016 at 3:54 AM, William A Rowe Jr 
> wrote:
>
>> On Fri, Oct 14, 2016 at 4:44 PM, Roy T. Fielding 
>> 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

2016-10-17 Thread Roy T. Fielding 
> On Oct 15, 2016, at 2:10 AM, William A Rowe Jr  wrote:
> 
> On Sat, Oct 15, 2016 at 3:54 AM, William A Rowe Jr  > wrote:
> On Fri, Oct 14, 2016 at 4:44 PM, Roy T. Fielding  > 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

2016-10-16 Thread Rainer Jung 

NP, thanks!

Am 16.10.2016 um 04:35 schrieb William A Rowe Jr:

I see you made nearly all the adjustments I missed, apologies that I
neglected the includes/ update.


On Oct 15, 2016 9:01 PM, "William A Rowe Jr" > wrote:

Yes, sorry... I meant to commit these all at once.  Patch incoming.


On Oct 15, 2016 6:23 PM, "Rainer Jung" > wrote:

Am 14.10.2016 um 22:48 schrieb wr...@apache.org
:

Author: wrowe
Date: Fri Oct 14 20:48:43 2016
New Revision: 1764961

URL: http://svn.apache.org/viewvc?rev=1764961=rev

Log:

Dropped the never-released ap_has_cntrls() as it had very
limited
and inefficient application at that, added
ap_scan_vchar_obstext()
to accomplish a similar purpose.


Should we note the API changes (2 removals, one addition) in the
mmn file? At least the original addition of ap_has_cntrl() is
still noted there.

Regards,

Rainer

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

2016-10-15 Thread William A Rowe Jr 
I see you made nearly all the adjustments I missed, apologies that I
neglected the includes/ update.

On Oct 15, 2016 9:01 PM, "William A Rowe Jr"  wrote:

> Yes, sorry... I meant to commit these all at once.  Patch incoming.
>
> On Oct 15, 2016 6:23 PM, "Rainer Jung"  wrote:
>
>> Am 14.10.2016 um 22:48 schrieb wr...@apache.org:
>>
>>> Author: wrowe
>>> Date: Fri Oct 14 20:48:43 2016
>>> New Revision: 1764961
>>>
>>> URL: http://svn.apache.org/viewvc?rev=1764961=rev
>>> Log:
>>>
>>> Dropped the never-released ap_has_cntrls() as it had very limited
>>> and inefficient application at that, added ap_scan_vchar_obstext()
>>> to accomplish a similar purpose.
>>>
>>
>> Should we note the API changes (2 removals, one addition) in the mmn
>> file? At least the original addition of ap_has_cntrl() is still noted there.
>>
>> Regards,
>>
>> Rainer
>>
>>

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

2016-10-15 Thread William A Rowe Jr 
Yes, sorry... I meant to commit these all at once.  Patch incoming.

On Oct 15, 2016 6:23 PM, "Rainer Jung"  wrote:

> Am 14.10.2016 um 22:48 schrieb wr...@apache.org:
>
>> Author: wrowe
>> Date: Fri Oct 14 20:48:43 2016
>> New Revision: 1764961
>>
>> URL: http://svn.apache.org/viewvc?rev=1764961=rev
>> Log:
>>
>> Dropped the never-released ap_has_cntrls() as it had very limited
>> and inefficient application at that, added ap_scan_vchar_obstext()
>> to accomplish a similar purpose.
>>
>
> Should we note the API changes (2 removals, one addition) in the mmn file?
> At least the original addition of ap_has_cntrl() is still noted there.
>
> Regards,
>
> Rainer
>
>

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

2016-10-15 Thread Rainer Jung 

Am 14.10.2016 um 22:48 schrieb wr...@apache.org:

Author: wrowe
Date: Fri Oct 14 20:48:43 2016
New Revision: 1764961

URL: http://svn.apache.org/viewvc?rev=1764961=rev
Log:

Dropped the never-released ap_has_cntrls() as it had very limited
and inefficient application at that, added ap_scan_vchar_obstext()
to accomplish a similar purpose.


Should we note the API changes (2 removals, one addition) in the mmn 
file? At least the original addition of ap_has_cntrl() is still noted there.


Regards,

Rainer

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

2016-10-15 Thread William A Rowe Jr 
On Sat, Oct 15, 2016 at 3:54 AM, William A Rowe Jr 
wrote:

> On Fri, Oct 14, 2016 at 4:44 PM, Roy T. Fielding 
> 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

2016-10-15 Thread William A Rowe Jr 
On Fri, Oct 14, 2016 at 4:44 PM, Roy T. Fielding  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

2016-10-14 Thread Roy T. Fielding 
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  wrote:
> 
>> On Fri, Oct 14, 2016 at 3:48 PM,  wrote:
>> Author: wrowe
>> Date: Fri Oct 14 20:48:43 2016
>> New Revision: 1764961
>> 
>> URL: http://svn.apache.org/viewvc?rev=1764961=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

2016-10-14 Thread William A Rowe Jr 
On Fri, Oct 14, 2016 at 3:48 PM,  wrote:

> Author: wrowe
> Date: Fri Oct 14 20:48:43 2016
> New Revision: 1764961
>
> URL: http://svn.apache.org/viewvc?rev=1764961=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.