svn commit: r1832092 - /httpd/httpd/trunk/modules/http/http_protocol.c

[el...@apache.org]

Author: elukey
Date: Wed May 23 11:22:34 2018
New Revision: 1832092

URL: http://svn.apache.org/viewvc?rev=1832092&view=rev
Log:
http_protocol.c: avoid duplicate headers when using
                 ap_send_error_response

While debugging PR 61860 I found a chain of events
that leads to duplicate headers in the HTTP response
if Header always set is used in the httpd's config.

As far as can understand, mod_headers uses err_headers_out
when dealing with the 'always' setting, since it is expected
to be preserved even on error. The current code seems
to correctly preserve the important headers like Location
(happening before the bit of code that changed), but then
it swaps headers_out with err_headers_out and clears
err_headers_out. My understanding is that this will cause
mod_headers, if called again, to re-insert the headers
set via 'always' again in err_headers_out, leading to a
duplication in the response. So far I managed to reproduce this
only with the specific use case outlined by the PR, but
there might be more.

r1831585 was added to the test suite for PR 61860,
(marked as TODO), and now it seems to pass as expected.

Since this part of the codebase has been working fine
for years I was reluctant to change it, but it seems
the right change to me. I didn't run into any regression
while testing this change (including running the test suite),
but as always I'd be glad to get feedback from a more expert
eye. If I missed a clear regression I'll make sure to
add it to the test suite as part of the follow up.


Modified:
    httpd/httpd/trunk/modules/http/http_protocol.c

Modified: httpd/httpd/trunk/modules/http/http_protocol.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http/http_protocol.c?rev=1832092&r1=1832091&r2=1832092&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/http/http_protocol.c (original)
+++ httpd/httpd/trunk/modules/http/http_protocol.c Wed May 23 11:22:34 2018
@@ -1262,16 +1262,13 @@ AP_DECLARE(void) ap_send_error_response(
     }
 
     if (!r->assbackwards) {
-        apr_table_t *tmp = r->headers_out;
 
         /* For all HTTP/1.x responses for which we generate the message,
          * we need to avoid inheriting the "normal status" header fields
          * that may have been set by the request handler before the
          * error or redirect, except for Location on external redirects.
          */
-        r->headers_out = r->err_headers_out;
-        r->err_headers_out = tmp;
-        apr_table_clear(r->err_headers_out);
+        apr_table_clear(r->headers_out);
 
         if (ap_is_HTTP_REDIRECT(status) || (status == HTTP_CREATED)) {
             if ((location != NULL) && *location) {