Re: HEAD response's Content-Length stripped when zero
On Wed, Apr 29, 2015 at 2:46 PM, Eric Covener cove...@gmail.com wrote: Maybe we could remember here if the CL was set by ap_content_length_filter or someone else? I finally committed (in r1678215) a fix that moves all the logic in ap_content_length_filter(). Thanks, Yann.
Re: HEAD response's Content-Length stripped when zero
On Wed, Apr 29, 2015 at 8:19 AM, Yann Ylavic ylavic@gmail.com wrote:
Hence how about removing this whole block (is there any module today
outsmarting httpd that cannot be considered as buggy?) or least
disable it for forwarded responses, eg:
Index: modules/http/http_filters.c
===
--- modules/http/http_filters.c(revision 1676716)
+++ modules/http/http_filters.c(working copy)
@@ -1292,6 +1292,7 @@ AP_CORE_DECLARE_NONSTD(apr_status_t) ap_http_heade
* The default (unset) behavior is to squelch the C-L in this case.
*/
if (r-header_only
+ !r-proxyreq
(clheader = apr_table_get(r-headers_out, Content-Length))
!strcmp(clheader, 0)
conf-http_cl_head_zero != AP_HTTP_CL_HEAD_ZERO_ENABLE) {
Maybe we could remember here if the CL was set by
ap_content_length_filter or someone else?
--
Eric Covener
cove...@gmail.com
HEAD response's Content-Length stripped when zero
Due to ap_http_header_filter():
/* This is a hack, but I can't find anyway around it. The idea is that
* we don't want to send out 0 Content-Lengths if it is a head request.
* This happens when modules try to outsmart the server, and return
* if they see a HEAD request. Apache 1.3 handlers were supposed to
* just return in that situation, and the core handled the HEAD. In
* 2.0, if a handler returns, then the core sends an EOS bucket down
* the filter stack, and the content-length filter computes a C-L of
* zero and that gets put in the headers, and we end up sending a
* zero C-L to the client. We can't just remove the C-L filter,
* because well behaved 2.0 handlers will send their data down the stack,
* and we will compute a real C-L for the head request. RBB
*
* Allow modification of this behavior through the
* HttpContentLengthHeadZero directive.
*
* The default (unset) behavior is to squelch the C-L in this case.
*/
if (r-header_only
(clheader = apr_table_get(r-headers_out, Content-Length))
!strcmp(clheader, 0)
conf-http_cl_head_zero != AP_HTTP_CL_HEAD_ZERO_ENABLE) {
apr_table_unset(r-headers_out, Content-Length);
}
I find this workaround a bit too much hacky because Content-Length:
0 could be the real one for the corresponding GET request, either
given by a module or forwarded from a backend.
Hence how about removing this whole block (is there any module today
outsmarting httpd that cannot be considered as buggy?) or least
disable it for forwarded responses, eg:
Index: modules/http/http_filters.c
===
--- modules/http/http_filters.c(revision 1676716)
+++ modules/http/http_filters.c(working copy)
@@ -1292,6 +1292,7 @@ AP_CORE_DECLARE_NONSTD(apr_status_t) ap_http_heade
* The default (unset) behavior is to squelch the C-L in this case.
*/
if (r-header_only
+ !r-proxyreq
(clheader = apr_table_get(r-headers_out, Content-Length))
!strcmp(clheader, 0)
conf-http_cl_head_zero != AP_HTTP_CL_HEAD_ZERO_ENABLE) {
--
?
Re: HEAD response's Content-Length stripped when zero
On Wed, Apr 29, 2015 at 2:46 PM, Eric Covener cove...@gmail.com wrote:
On Wed, Apr 29, 2015 at 8:19 AM, Yann Ylavic ylavic@gmail.com wrote:
Hence how about removing this whole block (is there any module today
outsmarting httpd that cannot be considered as buggy?) or least
disable it for forwarded responses, eg:
Index: modules/http/http_filters.c
===
--- modules/http/http_filters.c(revision 1676716)
+++ modules/http/http_filters.c(working copy)
@@ -1292,6 +1292,7 @@ AP_CORE_DECLARE_NONSTD(apr_status_t) ap_http_heade
* The default (unset) behavior is to squelch the C-L in this case.
*/
if (r-header_only
+ !r-proxyreq
(clheader = apr_table_get(r-headers_out, Content-Length))
!strcmp(clheader, 0)
conf-http_cl_head_zero != AP_HTTP_CL_HEAD_ZERO_ENABLE) {
Maybe we could remember here if the CL was set by
ap_content_length_filter or someone else?
Yes, I first thought about this, hence checking r-clength instead,
but that would probably require to initialize it to something like
UNSET (-1), or we still could not distinguish it from a zero length
set...
That would be:
Index: modules/http/http_core.c
===
--- modules/http/http_core.c(revision 1676716)
+++ modules/http/http_core.c(working copy)
@@ -254,6 +254,9 @@ static int ap_process_http_connection(conn_rec *c)
static int http_create_request(request_rec *r)
{
+/* Content-Length unset by default. */
+r-clength = -1;
+
if (!r-main !r-prev) {
ap_add_output_filter_handle(ap_byterange_filter_handle,
NULL, r, r-connection);
Index: modules/http/http_filters.c
===
--- modules/http/http_filters.c(revision 1676716)
+++ modules/http/http_filters.c(working copy)
@@ -1292,6 +1292,7 @@ AP_CORE_DECLARE_NONSTD(apr_status_t) ap_http_heade
* The default (unset) behavior is to squelch the C-L in this case.
*/
if (r-header_only
+ (r-clength 0)
(clheader = apr_table_get(r-headers_out, Content-Length))
!strcmp(clheader, 0)
conf-http_cl_head_zero != AP_HTTP_CL_HEAD_ZERO_ENABLE) {
--
Re: HEAD response's Content-Length stripped when zero
On Wed, Apr 29, 2015 at 2:46 PM, Eric Covener cove...@gmail.com wrote:
On Wed, Apr 29, 2015 at 8:19 AM, Yann Ylavic ylavic@gmail.com wrote:
Hence how about removing this whole block (is there any module today
outsmarting httpd that cannot be considered as buggy?) or least
disable it for forwarded responses, eg:
Index: modules/http/http_filters.c
===
--- modules/http/http_filters.c(revision 1676716)
+++ modules/http/http_filters.c(working copy)
@@ -1292,6 +1292,7 @@ AP_CORE_DECLARE_NONSTD(apr_status_t) ap_http_heade
* The default (unset) behavior is to squelch the C-L in this case.
*/
if (r-header_only
+ !r-proxyreq
(clheader = apr_table_get(r-headers_out, Content-Length))
!strcmp(clheader, 0)
conf-http_cl_head_zero != AP_HTTP_CL_HEAD_ZERO_ENABLE) {
Maybe we could remember here if the CL was set by
ap_content_length_filter or someone else?
The patch below tries your proposal...
Set a note If we detect that the header_only CL is from the origin in
ap_content_length_filter(), and don't strip the Content-Length if that
note is set in ap_http_header_filter().
We could also avoid the note by setting r-clength = -1 instead in
ap_content_length_filter().
This should work both with buggy modules and the normal cases (including proxy).
I guess we wouldn't need HttpContentLengthHeadZero from r1554303
anymore (and several third-party patches that simply disable the whole
unconditionally).
Do it sound better?
Index: server/protocol.c
===
--- server/protocol.c(revision 1676716)
+++ server/protocol.c(working copy)
@@ -1533,7 +1533,7 @@ AP_CORE_DECLARE_NONSTD(apr_status_t) ap_content_le
* We can only set a C-L in the response header if we haven't already
* sent any buckets on to the next output filter for this request.
*/
-if (ctx-data_sent == 0 eos
+if (ctx-data_sent == 0 eos) {
/* don't whack the C-L if it has already been set for a HEAD
* by something like proxy. the brigade only has an EOS bucket
* in this case, making r-bytes_sent zero.
@@ -1544,9 +1544,13 @@ 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.
*/
-!(r-header_only r-bytes_sent == 0
-apr_table_get(r-headers_out, Content-Length))) {
-ap_set_content_length(r, r-bytes_sent);
+if (r-header_only r-bytes_sent == 0
+apr_table_get(r-headers_out, Content-Length)) {
+apr_table_setn(r-notes, origin-ho, 1);
+}
+else {
+ap_set_content_length(r, r-bytes_sent);
+}
}
ctx-data_sent = 1;
Index: modules/http/http_filters.c
===
--- modules/http/http_filters.c(revision 1676716)
+++ modules/http/http_filters.c(working copy)
@@ -1292,9 +1292,11 @@ AP_CORE_DECLARE_NONSTD(apr_status_t) ap_http_heade
* The default (unset) behavior is to squelch the C-L in this case.
*/
if (r-header_only
- (clheader = apr_table_get(r-headers_out, Content-Length))
- !strcmp(clheader, 0)
- conf-http_cl_head_zero != AP_HTTP_CL_HEAD_ZERO_ENABLE) {
+ !r-clength
+ !apr_table_get(r-notes, origin-ho)
+ (clheader = apr_table_get(r-headers_out, Content-Length))
+ !strcmp(clheader, 0)
+ conf-http_cl_head_zero != AP_HTTP_CL_HEAD_ZERO_ENABLE) {
apr_table_unset(r-headers_out, Content-Length);
}
--
Re: HEAD response's Content-Length stripped when zero
(sorry for the patches spam / confused proposal)
On Wed, Apr 29, 2015 at 6:07 PM, Yann Ylavic ylavic@gmail.com wrote:
We could also avoid the note by setting r-clength = -1 instead in
ap_content_length_filter().
Another candidate could be r-sent_bodyct = 1, eg:
Index: server/protocol.c
===
--- server/protocol.c(revision 1676716)
+++ server/protocol.c(working copy)
@@ -1533,7 +1533,7 @@ AP_CORE_DECLARE_NONSTD(apr_status_t) ap_content_le
* We can only set a C-L in the response header if we haven't already
* sent any buckets on to the next output filter for this request.
*/
-if (ctx-data_sent == 0 eos
+if (ctx-data_sent == 0 eos) {
/* don't whack the C-L if it has already been set for a HEAD
* by something like proxy. the brigade only has an EOS bucket
* in this case, making r-bytes_sent zero.
@@ -1544,9 +1544,16 @@ 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.
*/
-!(r-header_only r-bytes_sent == 0
-apr_table_get(r-headers_out, Content-Length))) {
-ap_set_content_length(r, r-bytes_sent);
+if (r-header_only r-bytes_sent == 0
+apr_table_get(r-headers_out, Content-Length)) {
+/* Tell ap_http_header_filter() we don't want the C-L to
+ * be stripped (ie. the original no-body is sent now).
+ */
+r-sent_bodyct = 1;
+}
+else {
+ap_set_content_length(r, r-bytes_sent);
+}
}
ctx-data_sent = 1;
Index: modules/http/http_filters.c
===
--- modules/http/http_filters.c(revision 1676716)
+++ modules/http/http_filters.c(working copy)
@@ -1292,9 +1292,9 @@ AP_CORE_DECLARE_NONSTD(apr_status_t) ap_http_heade
* The default (unset) behavior is to squelch the C-L in this case.
*/
if (r-header_only
- (clheader = apr_table_get(r-headers_out, Content-Length))
- !strcmp(clheader, 0)
- conf-http_cl_head_zero != AP_HTTP_CL_HEAD_ZERO_ENABLE) {
+ !r-sent_bodyct
+ (clheader = apr_table_get(r-headers_out, Content-Length))
+ !strcmp(clheader, 0)) {
apr_table_unset(r-headers_out, Content-Length);
}
--
Re: HEAD response's Content-Length stripped when zero
On Wed, Apr 29, 2015 at 3:17 PM, Yann Ylavic ylavic@gmail.com wrote:
On Wed, Apr 29, 2015 at 2:46 PM, Eric Covener cove...@gmail.com wrote:
On Wed, Apr 29, 2015 at 8:19 AM, Yann Ylavic ylavic@gmail.com wrote:
Hence how about removing this whole block (is there any module today
outsmarting httpd that cannot be considered as buggy?) or least
disable it for forwarded responses, eg:
Index: modules/http/http_filters.c
===
--- modules/http/http_filters.c(revision 1676716)
+++ modules/http/http_filters.c(working copy)
@@ -1292,6 +1292,7 @@ AP_CORE_DECLARE_NONSTD(apr_status_t) ap_http_heade
* The default (unset) behavior is to squelch the C-L in this case.
*/
if (r-header_only
+ !r-proxyreq
(clheader = apr_table_get(r-headers_out, Content-Length))
!strcmp(clheader, 0)
conf-http_cl_head_zero != AP_HTTP_CL_HEAD_ZERO_ENABLE) {
Maybe we could remember here if the CL was set by
ap_content_length_filter or someone else?
Yes, I first thought about this, hence checking r-clength instead,
Hmm, I have read ap_content_length_filter() instead of
ap_set_content_length(), please forget about my previous answer(s)...
I need to think more about this.
Re: HEAD response's Content-Length stripped when zero
On Wed, Apr 29, 2015 at 3:39 PM, Yann Ylavic ylavic@gmail.com wrote:
On Wed, Apr 29, 2015 at 3:17 PM, Yann Ylavic ylavic@gmail.com wrote:
On Wed, Apr 29, 2015 at 2:46 PM, Eric Covener cove...@gmail.com wrote:
On Wed, Apr 29, 2015 at 8:19 AM, Yann Ylavic ylavic@gmail.com wrote:
Hence how about removing this whole block (is there any module today
outsmarting httpd that cannot be considered as buggy?) or least
disable it for forwarded responses, eg:
Index: modules/http/http_filters.c
===
--- modules/http/http_filters.c(revision 1676716)
+++ modules/http/http_filters.c(working copy)
@@ -1292,6 +1292,7 @@ AP_CORE_DECLARE_NONSTD(apr_status_t) ap_http_heade
* The default (unset) behavior is to squelch the C-L in this case.
*/
if (r-header_only
+ !r-proxyreq
(clheader = apr_table_get(r-headers_out, Content-Length))
!strcmp(clheader, 0)
conf-http_cl_head_zero != AP_HTTP_CL_HEAD_ZERO_ENABLE) {
Maybe we could remember here if the CL was set by
ap_content_length_filter or someone else?
Yes, I first thought about this, hence checking r-clength instead,
Hmm, I have read ap_content_length_filter() instead of
ap_set_content_length(), please forget about my previous answer(s)...
I need to think more about this.
Actually I found http://svn.apache.org/r1554303 which make it configurable.
Maybe we can backport it instead (why a MAJOR bump?? I'd like to see
it in 2.2.x too...).
I'd argue that the default should be HttpContentLengthHeadZero on
though (at least in trunk), still users of buggy/legacy modules could
turn it off.
Re: HEAD response's Content-Length stripped when zero
On Wed, Apr 29, 2015 at 3:17 PM, Yann Ylavic ylavic@gmail.com wrote: Index: modules/http/http_filters.c === --- modules/http/http_filters.c(revision 1676716) +++ modules/http/http_filters.c(working copy) @@ -1292,6 +1292,7 @@ AP_CORE_DECLARE_NONSTD(apr_status_t) ap_http_heade * The default (unset) behavior is to squelch the C-L in this case. */ if (r-header_only + (r-clength 0) We probably still require: + !r-proxyreq since mod_proxy does not call ap_set_content_length().
