Re: svn commit: r1775789 - /httpd/httpd/branches/2.2.x/STATUS

2017-01-03 Thread William A Rowe Jr 
On Tue, Jan 3, 2017 at 11:04 AM, William A Rowe Jr  wrote:
> On Tue, Jan 3, 2017 at 9:55 AM, Eric Covener  wrote:
>> I am not completely following how the branch or patch were assembled,
>> but I am seeing a failure that is missing content from the initial
>> trunk work (1426877)
>> that was also in the initial 2.4.x backport (1772678).
>>
>> It is causing frequent crashes on EOF of a keepalive conn for me
>>
>> The missing bit that sticks out/causes the crash is server/protocol.c
>> ap_read_request():
>>
>> 1276 default:
>> 1277 apr_brigade_destroy(tmp_bb);
>> 1278 r = NULL;
>>  ^ missing
>> 1279 return r;
>> 1280 }
>> 1281 }
>> 1282
>>
>> Bill do you recall if there was perhaps a hand-resolved merge conflict
>> in this area?
>
> That makes perfect sense due to the s/goto traceout/return r/ conflicts,
> none of the patch applied in that section, and I simply missed this line.
> The traceout change wasn't merged since 2.2 has no support for that
> loglevel.
>
> Feel free to patch on the merge branch, or I will fix this myself this
> afternoon.

Looking more closely at this code, the four essential errors all return
NULL, and default: should have returned NULL, which leaves me
wondering why HTTP_REQUEST_TIMEOUT returns the request_rec
when no other error cases do so?

Re: svn commit: r1775789 - /httpd/httpd/branches/2.2.x/STATUS

2017-01-03 Thread William A Rowe Jr 
On Tue, Jan 3, 2017 at 9:55 AM, Eric Covener  wrote:
> I am not completely following how the branch or patch were assembled,
> but I am seeing a failure that is missing content from the initial
> trunk work (1426877)
> that was also in the initial 2.4.x backport (1772678).
>
> It is causing frequent crashes on EOF of a keepalive conn for me
>
> The missing bit that sticks out/causes the crash is server/protocol.c
> ap_read_request():
>
> 1276 default:
> 1277 apr_brigade_destroy(tmp_bb);
> 1278 r = NULL;
>  ^ missing
> 1279 return r;
> 1280 }
> 1281 }
> 1282
>
> Bill do you recall if there was perhaps a hand-resolved merge conflict
> in this area?

That makes perfect sense due to the s/goto traceout/return r/ conflicts,
none of the patch applied in that section, and I simply missed this line.
The traceout change wasn't merged since 2.2 has no support for that
loglevel.

Feel free to patch on the merge branch, or I will fix this myself this
afternoon.

Re: svn commit: r1775789 - /httpd/httpd/branches/2.2.x/STATUS

2017-01-03 Thread Eric Covener 
I am not completely following how the branch or patch were assembled,
but I am seeing a failure that is missing content from the initial
trunk work (1426877)
that was also in the initial 2.4.x backport (1772678).

It is causing frequent crashes on EOF of a keepalive conn for me

The missing bit that sticks out/causes the crash is server/protocol.c
ap_read_request():

1276 default:
1277 apr_brigade_destroy(tmp_bb);
1278 r = NULL;
 ^ missing
1279 return r;
1280 }
1281 }
1282

Bill do you recall if there was perhaps a hand-resolved merge conflict
in this area?

On Fri, Dec 23, 2016 at 12:24 AM,   wrote:
> Author: wrowe
> Date: Fri Dec 23 05:24:54 2016
> New Revision: 1775789
>
> URL: http://svn.apache.org/viewvc?rev=1775789=rev
> Log:
> This was my entire intended commit. But as an alternate strategy, you can
> svn up to r1775787. Not that I intended it, and absolutely not the way we
> should apply it (revert layer by layer 6 commits replicated on the merge
> branch, then apply merge branch in one commit, IMO.)
>
> Sorry folks ;-/
>
> Modified:
> httpd/httpd/branches/2.2.x/STATUS
>
> Modified: httpd/httpd/branches/2.2.x/STATUS
> URL: 
> http://svn.apache.org/viewvc/httpd/httpd/branches/2.2.x/STATUS?rev=1775789=1775788=1775789=diff
> ==
> --- httpd/httpd/branches/2.2.x/STATUS (original)
> +++ httpd/httpd/branches/2.2.x/STATUS Fri Dec 23 05:24:54 2016
> @@ -99,6 +99,32 @@ CURRENT RELEASE NOTES:
>
>  RELEASE SHOWSTOPPERS:
>
> +  *) Rather than odds-and-ends applied out of order, proposing we revert
> + r1757240, r1757256, r1757295, r1758671, r1758672, r1775232, all of
> + which is now recorded in the 2.2.x-merge-http-strict branch, and
> + bring that branch back into 2.2.x for 2.4.32 release.
> + Merges;
> +   -c-1775232 .
> +   -c-1757672 .
> +   -c-1757671 .
> +   -c-1757295 .
> +   -c-1757256 .
> +   -c-1757240 .
> + [here we are back at 2.2.32-dev bump]
> +   -r1775685:1775780 
> https://svn.apache.org/repos/asf/httpd/httpd/branches/2.2.x-merge-http-strict/
> + Roll-up patch of the above (not recommended for casual reading, these
> + would be committed individually as noted above... but for only for 
> sanity
> + testing the end result. Due to intervening CHANGES/ap_mmn changes, there
> + is small delta after reverting the above...)
> +   
> https://raw.githubusercontent.com/wrowe/patches/master/httpd-2.2-HEAD-http-protocol-strict.patch
> +   This patch above does *NOT* apply to the 2.2.31 release, c.f. the 
> delta
> +   of the 2.2.x-merge-http-strict branch for that information. This is 
> for
> +   folks who are testing rollbacks plus 2.4.x activity against 2.2.x 
> HEAD!
> +   Sorry to start from scratch, but yann's correct observation was 
> correct,
> +   that nothing will apply out-of-order, and everything on 2.2 branch had
> +   already become disordered.
> + +1: wrowe
> +
>
>  PATCHES ACCEPTED TO BACKPORT FROM TRUNK:
>[ start all new proposals below, under PATCHES PROPOSED. ]
> @@ -152,44 +178,6 @@ PATCHES PROPOSED TO BACKPORT FROM TRUNK:
>   http://home.apache.org/~ylavic/patches/httpd-2.2.x-r1753592.patch
>+1: ylavic
>
> -  *) Enforce LimitRequestFieldSize after multiple headers with the same
> - name have been merged, Ensure LimitRequestFieldSize is always logged.
> - Downgrade some more log messages indicating client errors from level 
> error
> - to info. Add log messages for various reasons to return 
> HTTP_BAD_REQUEST.
> - Correctly return a 400 (Bad request) in case of a HTTP/0.9 request like
> - "GET @example.org/foo".
> - Add some trace logging to core (using AP_DEBUG_THE_REQUEST macro, 
> because
> - the TRACE5 facilities aren't in 2.2.x branch).
> - Improve error message (PR 54384).
> - Submitted by: sf, rpluem, jailletc36
> - [Note: everything in this patch is modifying logging and brings in the
> - LimitRequestFieldSize logic used for the lifespan of 2.4.x]
> - Trunk version of patch
> - http://svn.apache.org/r951900 (server/protocol.c alone)
> - http://svn.apache.org/r1178566
> - http://svn.apache.org/r1185385
> - http://svn.apache.org/r1188745
> - http://svn.apache.org/r1352911
> - http://svn.apache.org/r1433613
> - Backport: (Adjustments dodging 2.4'isms such as APLOGNO's)
> - 
> https://raw.githubusercontent.com/wrowe/patches/master/backport-2.2.x-r951900-r1178566-r1185385-r1188745-r1352911-r1433613.patch
> - +1: wrowe, covener
> - ylavic: the patch does not apply cleanly? (I tried both w/ and w/o
> - backport-2.2.x-r892678.patch first, conflicts in protocol.c)
> -
> -  *) core: ErrorDocument now works for requests without a Host header.
> - Support custom