You are viewing a plain text version of this content. The canonical link for it is here.
Posted to bugs@httpd.apache.org by bu...@apache.org on 2018/05/16 18:07:30 UTC

[Bug 62380] New: Existing header not overwritten when using the 'always' condition with Header set

https://bz.apache.org/bugzilla/show_bug.cgi?id=62380

            Bug ID: 62380
           Summary: Existing header not overwritten when using the
                    'always' condition with Header set
           Product: Apache httpd-2
           Version: 2.4.29
          Hardware: All
                OS: Linux
            Status: NEW
          Severity: normal
          Priority: P2
         Component: mod_headers
          Assignee: bugs@httpd.apache.org
          Reporter: angeloxx+apache@angeloxx.ch
  Target Milestone: ---

Hi, I'm reporting a bug that is already well described here
(https://bugs.launchpad.net/ubuntu/+source/apache2/+bug/1178090):

I have an application that sets some headers, but I also have Apache setting
them as well to handle some special cases. I'm using the mod_headers syntax of
'Header always set X-Foo "bar"'. I specifically use the 'always' condition
table, as I want to include these headers on non-2xx responses (such as 301,
302). However, if I use 'always' (instead of the default 'onsuccess' condition
table), the headers are duplicated, which goes against what I have an
application that sets some headers, but I also have Apache setting them as well
to handle some special cases. I'm using the mod_headers syntax of 'Header
always set X-Foo "bar"'. I specifically use the 'always' condition table, as I
want to include these headers on non-2xx responses (such as 301, 302). However,
if I use 'always' (instead of the default 'onsuccess' condition table), the
headers are duplicated, which goes against what the 'set' action is supposed to
do (overwrite any existing header).

I'm confirm that this bug affect the more recent Apache 2.4.29, compiled from
the official source distribution on Linux RHEL 7.4

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 62380] Existing header not overwritten when using the 'always' condition with Header set

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=62380

Luca Toscano <to...@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |WONTFIX

--- Comment #11 from Luca Toscano <to...@gmail.com> ---
Forgot to update this thread. I added the missing 2.4 configuration bits in
r1844475 and also added the wishlist for a normalized header array in r1844403
(for probably httpd 2.6+).

Closing since I don't see more things to do, but please re-open if necessary.

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 62380] Existing header not overwritten when using the 'always' condition with Header set

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=62380

--- Comment #2 from angeloxx <an...@angeloxx.ch> ---
Hi, I've tested the patch and the problem persists.

If the proxied application send the involved header, the:
- Header always set -> duplicate the header when the backend application
returns an error page (e.g. 404)
- Header set -> works correctly

Thank you for your interest.

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 62380] Existing header not overwritten when using the 'always' condition with Header set

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=62380

--- Comment #9 from Luca Toscano <to...@gmail.com> ---
After a chat on the dev@ mailing list we agreed to updated the documentation
for the moment, so I started it with trunk:

https://httpd.apache.org/docs/trunk/mod/mod_headers.html#header

Will update 2.4 if nobody will come up with further suggestions about how to
improve the above section.

The absence of a "normalized" headers list for HTTP responses is not a good
feature in my opinion, but trying to introduce it to 2.4.x would probably cause
more damages than positive effects on existing configurations/setups. I believe
that this could be a good fix for the next 2.X httpd.

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 62380] Existing header not overwritten when using the 'always' condition with Header set

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=62380

--- Comment #7 from Luca Toscano <to...@gmail.com> ---
While reading mod_headers' Header set documentation though I'd say that the
best course of action would be that rather than blindly add a header to
err_headers_out, it should also "blank" any header with the same name present
in headers_out.

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 62380] Existing header not overwritten when using the 'always' condition with Header set

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=62380

--- Comment #6 from Luca Toscano <to...@gmail.com> ---
Simple patch to prove the above point (branch 2.4.x):

Index: modules/proxy/mod_proxy_http.c
===================================================================
--- modules/proxy/mod_proxy_http.c      (revision 1841528)
+++ modules/proxy/mod_proxy_http.c      (working copy)
@@ -1014,7 +1014,7 @@
             return;
        }
     }
-    apr_table_add(r->headers_out, key, value);
+    apr_table_add(r->err_headers_out, key, value);
 }

 /*

But I have no idea if it makes sense or not (probably the latter), so I'll wait
for a more expert opinion :)

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 62380] Existing header not overwritten when using the 'always' condition with Header set

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=62380

--- Comment #1 from Luca Toscano <to...@gmail.com> ---
Hi,

this bug sounds similar to
https://bz.apache.org/bugzilla/show_bug.cgi?id=61860, can you double check?
There is a patch available for trunk that should apply to 2.4.x too, if you
have time/patience to test it would be really helpful :)

Thanks!

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 62380] Existing header not overwritten when using the 'always' condition with Header set

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=62380

--- Comment #8 from Luca Toscano <to...@gmail.com> ---
Changing mod_headers might not be as easy as I thought. The following simple
patch solves this specific use case:

Index: modules/metadata/mod_headers.c
===================================================================
--- modules/metadata/mod_headers.c      (revision 1843742)
+++ modules/metadata/mod_headers.c      (working copy)
@@ -791,9 +791,12 @@
             }
             break;
         case hdr_set:
-            if (r->headers_in != headers &&
-                !ap_cstr_casecmp(hdr->header, "Content-Type")) {
-                 ap_set_content_type(r, process_tags(hdr, r));
+            if (r->headers_in != headers) {
+                    if (!ap_cstr_casecmp(hdr->header, "Content-Type")) {
+                        ap_set_content_type(r, process_tags(hdr, r));
+                    } else if (r->err_headers_out == headers) {
+                        apr_table_unset(r->headers_out, hdr->header);
+                    }
             }
             apr_table_setn(headers, hdr->header, process_tags(hdr, r));
             break;

But then how about 'merge', 'setifempty', etc..? The fact that there are two
separate list of headers ('onsuccess' vs 'always') makes it really tricky to
fix all the use cases without introducing regressions.

For example, how is it the right way to handle the following example?

'onsuccess' header list -> X-Foo: bla
'always' header list -> []

Header always merge X-Foo anotherone

In this case the current behavior, i.e. duplication, seems the most "logic" one
given what written above, other solutions would be inconsistent (like moving
X-Foo: bla to the always header list before applying the merge). 

The same example with 'setifempty' is also strange, since not setting X-Foo in
the 'always' list would be incorrect, so duplication seems legitimate.

As far as I can see what it is missing from the documentation is that, to be
completely sure about avoiding header duplication, one should unset an header
in the 'onsuccess' list before doing anything with the 'always' one, like:

Header (onsuccess) unset X-Foo
Header always set X-Foo somevalue

Using the above the mod_proxy_http example works as expected, not duplicating
any header.

Any opinion from other people reading? :)

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 62380] Existing header not overwritten when using the 'always' condition with Header set

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=62380

--- Comment #3 from Luca Toscano <to...@gmail.com> ---
Really strange, I am not able to repro with this simple proxy_fcgi/php test
case:

<?php
header('X-Foo: bar');
http_response_code(404)
?>

Header always set X-Foo "bar" (global httpd.conf)

If you have time/patience can you please add a bare minimum testing/repro-case
that I can use?

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 62380] Existing header not overwritten when using the 'always' condition with Header set

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=62380

--- Comment #4 from Luca Toscano <to...@gmail.com> ---
Tested with mod_proxy_http and actually I can repro, the header is duplicated..

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 62380] Existing header not overwritten when using the 'always' condition with Header set

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=62380

Luca Toscano <to...@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |lhu5@cdc.gov

--- Comment #10 from Luca Toscano <to...@gmail.com> ---
*** Bug 62233 has been marked as a duplicate of this bug. ***

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 62380] Existing header not overwritten when using the 'always' condition with Header set

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=62380

--- Comment #5 from Luca Toscano <to...@gmail.com> ---
So as far as I can see, when mod_headers executes ap_headers_output_filter and
gets to:

    /* do the fixup */
    do_headers_fixup(f->r, f->r->err_headers_out, dirconf->fixup_err, 0);
    do_headers_fixup(f->r, f->r->headers_out, dirconf->fixup_out, 0);

it duplicates the headers (only in the mod_proxy_http use case). This seems to
be related to the fact that the X-Foo: bar header ends up in err_headers_out
for mod_proxy_fcgi, meanwhile it ends up in headers_out in mod_proxy_http
(before reaching ap_headers_output_filter). So in the first case, no
duplication occurs since X-Foo: bar is set two times in the same place
(err_headers_out), meanwhile in the latter it gets set in err_headers_out and
headers_out, ending up "rendered" two times.

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org