help

[xige]

Re: Is 2.2.x broken?

[Eric Covener]

The fix is the thing I just added to STATUS today

On Tue, Dec 6, 2016 at 1:07 PM, Jacob Champion  wrote:
> I'm trying to test a backport to the 2.2.x tip, but it looks like r1758671
> might have busted our request parsing. Every test we have fails; I think
> ap_rgetline() is no longer dealing with the final CRLF line correctly.
> Reverting that commit fixes things.
>
> Can anyone confirm that it's not just something on my end?
>
> --Jacob



-- 
Eric Covener
cove...@gmail.com

Re: Is 2.2.x broken?

[William A Rowe Jr]

An associated patch was likely overlooked, please go ahead and revert.


On Dec 6, 2016 12:07 PM, "Jacob Champion"  wrote:

> I'm trying to test a backport to the 2.2.x tip, but it looks like r1758671
> might have busted our request parsing. Every test we have fails; I think
> ap_rgetline() is no longer dealing with the final CRLF line correctly.
> Reverting that commit fixes things.
>
> Can anyone confirm that it's not just something on my end?
>
> --Jacob
>

Is 2.2.x broken?

[Jacob Champion]

I'm trying to test a backport to the 2.2.x tip, but it looks like 
r1758671 might have busted our request parsing. Every test we have 
fails; I think ap_rgetline() is no longer dealing with the final CRLF 
line correctly. Reverting that commit fixes things.


Can anyone confirm that it's not just something on my end?

--Jacob

Re: Content-Length header for HTTP 204 and 1xx status codes

[Luca Toscano]

Hi Yann,

2016-12-05 13:54 GMT+01:00 Yann Ylavic :

> Hi Luca,
>
> sorry for the delay (overwhelmed these times)...
>

thanks a lot for the help!


>
> On Mon, Dec 5, 2016 at 1:21 PM, Luca Toscano 
> wrote:
> >
> >
> > 2016-11-30 18:46 GMT+01:00 Luca Toscano :
> >>
> >> Hi everybody,
> >>
> >> while working on https://bz.apache.org/bugzilla/show_bug.cgi?id=51350 a
> >> user asked why httpd send the "Content-Length: 0" header for HTTP 204
> >> responses given the following statement in the RFC:
> >>
> >> https://tools.ietf.org/html/rfc7230#page-30
> >> "A server MUST NOT send a Content-Length header field in any response
> with
> >> a status code of 1xx (Informational) or 204 (No Content)."
> >>
> >> I tried with a simple PHP script returning an HTTP 204 header (via
> >> mod_proxy_fcgi) and indeed I can see the Content-Length: 0. After a bit
> of
> >> digging it seems that ap_content_length_filter in protocol.c adds the
> header
> >> when it evaluates:
> >>
> >> if (!(r->header_only
> >>   && !r->bytes_sent
> >>   && (r->sent_bodyct
> >>   || conf->http_cl_head_zero !=
> >> AP_HTTP_CL_HEAD_ZERO_ENABLE
> >>   || apr_table_get(r->headers_out, "Content-Length"
> {
> >> ap_set_content_length(r, r->bytes_sent);
> >> }
>
> How about adding (yet) another condition to the above:
>
> Index: server/protocol.c
> ===
> --- server/protocol.c(revision 1772657)
> +++ server/protocol.c(working copy)
> @@ -1766,7 +1766,9 @@ AP_CORE_DECLARE_NONSTD(apr_status_t) ap_content_le
>   * such filters update or remove the C-L header, and just use it
>   * if present.
>   */
> -if (!(r->header_only
> +if (!((r->header_only
> +   || r->status == HTTP_NO_CONTENT
> +   || r->status == HTTP_NOT_MODIFIED)
>&& !r->bytes_sent
>&& (r->sent_bodyct
>|| conf->http_cl_head_zero !=
> AP_HTTP_CL_HEAD_ZERO_ENABLE
> ?


This was the other solution that I had in mind (fixing the issue at its
origin rather than patching it afterwards) but I wasn't confident to make
changes to the already crowded if (that seems to be related to a specific
use case).

One caveat that I realized only now: if the backend sets a C-L header (for
a 204 response) it will not be handled by the above if, so if we want to
patch this use case too we'd probably want to add a stricter condition
in ap_http_header_filter.

Follow up: I got tricked by the "r->header_only" condition in the
beginning, I thought that it would have been applied to all the responses
requiring headers and no body, but it applies only to HEAD requests. I
didn't find any trace in the code about how to prevent a HTTP 204 response
body to be sent, except for mod_proxy_http that explicitly handle this
case. I tested the presence of a body in a simple 204 response from a
Perl/PHP cgi/fcgi script with telnet and I confirmed my suspicion.

So this might help:

Index: modules/http/http_filters.c
===
--- modules/http/http_filters.c (revision 1772510)
+++ modules/http/http_filters.c (working copy)
@@ -1297,6 +1297,10 @@
 apr_table_unset(r->headers_out, "Content-Length");
 }

+if (r->status == HTTP_NO_CONTENT || ap_is_HTTP_INFO(r->status)){
+apr_table_unset(r->headers_out, "Content-Length");
+}
+
 ctype = ap_make_content_type(r, r->content_type);
 if (ctype) {
 apr_table_setn(r->headers_out, "Content-Type", ctype);
@@ -1368,7 +1372,7 @@

 ap_pass_brigade(f->next, b2);

-if (r->header_only) {
+if (r->header_only || r->status == HTTP_NO_CONTENT) {
 apr_brigade_cleanup(b);
 ctx->headers_sent = 1;
 return OK;


Does it make any sense?

Thanks,

Luca