You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by yl...@apache.org on 2016/12/12 10:26:16 UTC
svn commit: r1773761 - /httpd/httpd/trunk/modules/http/http_filters.c
Author: ylavic
Date: Mon Dec 12 10:26:16 2016
New Revision: 1773761
URL: http://svn.apache.org/viewvc?rev=1773761&view=rev
Log:
Follow up to r1773293.
When check_headers() fails, clear anything (headers and body) from original/errorneous
response before returning 500.
Modified:
httpd/httpd/trunk/modules/http/http_filters.c
Modified: httpd/httpd/trunk/modules/http/http_filters.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http/http_filters.c?rev=1773761&r1=1773760&r2=1773761&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/http/http_filters.c (original)
+++ httpd/httpd/trunk/modules/http/http_filters.c Mon Dec 12 10:26:16 2016
@@ -632,7 +632,6 @@ apr_status_t ap_http_filter(ap_filter_t
struct check_header_ctx {
request_rec *r;
int strict;
- const char *badheader;
};
/* check a single header, to be used with apr_table_do() */
@@ -644,7 +643,6 @@ static int check_header(void *arg, const
if (name[0] == '\0') {
ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, ctx->r, APLOGNO(02428)
"Empty response header name, aborting request");
- ctx->badheader = name;
return 0;
}
@@ -659,7 +657,6 @@ static int check_header(void *arg, const
"Response header name '%s' contains invalid "
"characters, aborting request",
name);
- ctx->badheader = name;
return 0;
}
@@ -669,7 +666,6 @@ static int check_header(void *arg, const
"Response header '%s' value of '%s' contains invalid "
"characters, aborting request",
name, val);
- ctx->badheader = name;
return 0;
}
return 1;
@@ -684,21 +680,11 @@ static APR_INLINE int check_headers(requ
struct check_header_ctx ctx;
core_server_config *conf =
ap_get_core_module_config(r->server->module_config);
- int rv = 1;
ctx.r = r;
ctx.strict = (conf->http_conformance != AP_HTTP_CONFORMANCE_UNSAFE);
- ctx.badheader = NULL;
-
- while (!apr_table_do(check_header, &ctx, r->headers_out, NULL)){
- if (ctx.badheader) {
- apr_table_unset(r->headers_out, ctx.badheader);
- apr_table_unset(r->err_headers_out, ctx.badheader);
- }
- rv = 0; /* problem has been logged by check_header() */
- }
-
- return rv;
+ return apr_table_do(check_header, &ctx, r->headers_out, NULL) &&
+ apr_table_do(check_header, &ctx, r->err_headers_out, NULL);
}
typedef struct header_struct {
@@ -1191,6 +1177,7 @@ AP_DECLARE_NONSTD(int) ap_send_http_trac
typedef struct header_filter_ctx {
int headers_sent;
+ int headers_error;
} header_filter_ctx;
AP_CORE_DECLARE_NONSTD(apr_status_t) ap_http_header_filter(ap_filter_t *f,
@@ -1205,23 +1192,36 @@ AP_CORE_DECLARE_NONSTD(apr_status_t) ap_
header_filter_ctx *ctx = f->ctx;
const char *ctype;
ap_bucket_error *eb = NULL;
+ int eos = 0;
AP_DEBUG_ASSERT(!r->main);
- if (r->header_only || r->status == HTTP_NO_CONTENT) {
- if (!ctx) {
- ctx = f->ctx = apr_pcalloc(r->pool, sizeof(header_filter_ctx));
- }
- else if (ctx->headers_sent) {
- apr_brigade_cleanup(b);
- return OK;
- }
+ if (!ctx) {
+ ctx = f->ctx = apr_pcalloc(r->pool, sizeof(header_filter_ctx));
+ }
+ else if (ctx->headers_sent) {
+ /* r->header_only or HTTP_NO_CONTENT case below, don't let
+ * the body pass trhough.
+ */
+ apr_brigade_cleanup(b);
+ return APR_SUCCESS;
}
+ if (!ctx->headers_error && !check_headers(r)) {
+ /* Eat body until EOS */
+ ctx->headers_error = 1;
+ }
for (e = APR_BRIGADE_FIRST(b);
e != APR_BRIGADE_SENTINEL(b);
e = APR_BUCKET_NEXT(e))
{
+ if (ctx->headers_error) {
+ if (APR_BUCKET_IS_EOS(e)) {
+ eos = 1;
+ break;
+ }
+ continue;
+ }
if (AP_BUCKET_IS_ERROR(e) && !eb) {
eb = e->data;
continue;
@@ -1235,9 +1235,23 @@ AP_CORE_DECLARE_NONSTD(apr_status_t) ap_
return ap_pass_brigade(f->next, b);
}
}
+ if (ctx->headers_error) {
+ /* We'll come back here from ap_send_error_response(),
+ * so clear anything from this response.
+ */
+ apr_brigade_cleanup(b);
+ if (!eos) {
+ return APR_SUCCESS;
+ }
+ ctx->headers_error = 0;
+ apr_table_clear(r->headers_out);
+ apr_table_clear(r->err_headers_out);
+ r->status = HTTP_INTERNAL_SERVER_ERROR;
+ ap_send_error_response(r, 0);
+ return AP_FILTER_ERROR;
+ }
if (eb) {
int status;
-
status = eb->status;
apr_brigade_cleanup(b);
ap_die(status, r);
@@ -1260,10 +1274,6 @@ AP_CORE_DECLARE_NONSTD(apr_status_t) ap_
r->headers_out);
}
- if (!check_headers(r)) {
- r->status = 500;
- }
-
/*
* Remove the 'Vary' header field if the client can't handle it.
* Since this will have nasty effects on HTTP/1.1 caches, force
@@ -1376,7 +1386,7 @@ AP_CORE_DECLARE_NONSTD(apr_status_t) ap_
if (r->header_only || r->status == HTTP_NO_CONTENT) {
apr_brigade_cleanup(b);
ctx->headers_sent = 1;
- return OK;
+ return APR_SUCCESS;
}
r->sent_bodyct = 1; /* Whatever follows is real body stuff... */
Re: svn commit: r1773761 - /httpd/httpd/trunk/modules/http/http_filters.c
Posted by Eric Covener <co...@gmail.com>.
On Mon, Dec 12, 2016 at 11:18 AM, Yann Ylavic <yl...@gmail.com> wrote:
> On Mon, Dec 12, 2016 at 3:32 PM, Eric Covener <co...@gmail.com> wrote:
>> On Mon, Dec 12, 2016 at 6:25 AM, Yann Ylavic <yl...@gmail.com> wrote:
>>> Tested with "Header set 'X-Bad' '<something with CR>'" with no redirect loop.
>>
>> w/ 'Header always set...' this still repeats
>
> Right, with r1773812 it looks good.
> We'd only recurse if not in internal redirect already, otherwise fall
> through with:
> HTTP/1.1 500 Internal Server Error
> Date: Mon, 12 Dec 2016 16:11:59 GMT
> Server: Apache/2.5.0-dev (Unix) OpenSSL/1.0.2g
> Content-Length: 0
> Connection: close
>
> WDYT?
Still failing my accidental test. Will send you a separate note.
#519 0x000000000048af51 in ap_http_header_filter (f=0x7fffc80061d0,
b=0x7fffc85e9ad0) at http_filters.c:1257
#520 0x00000000004396d0 in ap_pass_brigade (next=0x7fffc80061d0,
bb=0x7fffc85e9ad0) at util_filter.c:610
#521 0x00000000004408a9 in ap_content_length_filter (f=0x7fffc8006198,
b=0x7fffc85e9ad0) at protocol.c:1776
#522 0x00000000004396d0 in ap_pass_brigade (next=0x7fffc8006198,
bb=0x7fffc85e9ad0) at util_filter.c:610
#523 0x00007fffe838daa5 in ap_headers_error_filter (f=0x7fffc85e9850,
in=0x7fffc85e9ad0) at mod_headers.c:917
#524 0x00000000004396d0 in ap_pass_brigade (next=0x7fffc85e9850,
bb=0x7fffc85e9ad0) at util_filter.c:610
#525 0x00007fffe6103dfc in session_output_filter (f=0x7fffc85e9818,
in=0x7fffc85e9ad0) at mod_session.c:489
#526 0x00000000004396d0 in ap_pass_brigade (next=0x7fffc85e9818,
bb=0x7fffc85e9ad0) at util_filter.c:610
#527 0x0000000000440b5d in ap_old_write_filter (f=0x7fffc85e98d8,
bb=0x7fffc85e9ad0) at protocol.c:1845
#528 0x00000000004396d0 in ap_pass_brigade (next=0x7fffc85e98d8,
bb=0x7fffc85e9ad0) at util_filter.c:610
#529 0x00000000004400a6 in end_output_stream (r=0x7fffc8004980) at
protocol.c:1540
#530 0x000000000044011e in ap_finalize_request_protocol
(r=0x7fffc8004980) at protocol.c:1565
#531 0x0000000000486611 in ap_send_error_response (r=0x7fffc8004980,
recursive_error=500) at http_protocol.c:1395
#532 0x0000000000486da7 in ap_die_r (type=500, r=0x7fffc8004980,
recursive_error=500) at http_request.c:224
#533 0x0000000000486dd3 in ap_die (type=500, r=0x7fffc8004980) at
http_request.c:229
#534 0x000000000048af51 in ap_http_header_filter (f=0x7fffc80061d0,
b=0x7fffc85e7790) at http_filters.c:1257
#535 0x00000000004396d0 in ap_pass_brigade (next=0x7fffc80061d0,
bb=0x7fffc85e7790) at util_filter.c:610
#536 0x00000000004408a9 in ap_content_length_filter (f=0x7fffc8006198,
b=0x7fffc85e7790) at protocol.c:1776
#537 0x00000000004396d0 in ap_pass_brigade (next=0x7fffc8006198,
bb=0x7fffc85e7790) at util_filter.c:610
#538 0x00007fffe838daa5 in ap_headers_error_filter (f=0x7fffc85ed568,
in=0x7fffc85e7790) at mod_headers.c:917
#539 0x00000000004396d0 in ap_pass_brigade (next=0x7fffc85ed568,
bb=0x7fffc85e7790) at util_filter.c:610
#540 0x00007fffe6103dfc in session_output_filter (f=0x7fffc85ed530,
in=0x7fffc85e7790) at mod_session.c:489
#541 0x00000000004396d0 in ap_pass_brigade (next=0x7fffc85ed530,
bb=0x7fffc85e7790) at util_filter.c:610
#542 0x0000000000440b5d in ap_old_write_filter (f=0x7fffc85ed5f0,
bb=0x7fffc85e7790) at protocol.c:1845
#543 0x00000000004396d0 in ap_pass_brigade (next=0x7fffc85ed5f0,
bb=0x7fffc85e7790) at util_filter.c:610
#544 0x00000000004400a6 in end_output_stream (r=0x7fffc8004980) at
protocol.c:1540
#545 0x000000000044011e in ap_finalize_request_protocol
(r=0x7fffc8004980) at protocol.c:1565
#546 0x0000000000486611 in ap_send_error_response (r=0x7fffc8004980,
recursive_error=500) at http_protocol.c:1395
#547 0x0000000000486da7 in ap_die_r (type=500, r=0x7fffc8004980,
recursive_error=500) at http_request.c:224
#548 0x0000000000486dd3 in ap_die (type=500, r=0x7fffc8004980) at
http_request.c:229
#549 0x000000000048af51 in ap_http_header_filter (f=0x7fffc80061d0,
b=0x7fffc85ed4e0) at http_filters.c:1257
--
Eric Covener
covener@gmail.com
Re: svn commit: r1773761 -
/httpd/httpd/trunk/modules/http/http_filters.c
Posted by Jacob Champion <ch...@gmail.com>.
On 12/12/2016 10:38 AM, William A Rowe Jr wrote:
> Looks like the right approach, tweaking the test framework to exercise
> this solution. Thx!
I've added a regression test to the suite for this.
--Jacob
Re: svn commit: r1773761 - /httpd/httpd/trunk/modules/http/http_filters.c
Posted by William A Rowe Jr <wr...@rowe-clan.net>.
On Mon, Dec 12, 2016 at 10:18 AM, Yann Ylavic <yl...@gmail.com> wrote:
> On Mon, Dec 12, 2016 at 3:32 PM, Eric Covener <co...@gmail.com> wrote:
> > On Mon, Dec 12, 2016 at 6:25 AM, Yann Ylavic <yl...@gmail.com>
> wrote:
> >> Tested with "Header set 'X-Bad' '<something with CR>'" with no redirect
> loop.
> >
> > w/ 'Header always set...' this still repeats
>
> Right, with r1773812 it looks good.
> We'd only recurse if not in internal redirect already, otherwise fall
> through with:
> HTTP/1.1 500 Internal Server Error
> Date: Mon, 12 Dec 2016 16:11:59 GMT
> Server: Apache/2.5.0-dev (Unix) OpenSSL/1.0.2g
> Content-Length: 0
> Connection: close
>
> WDYT?
>
Looks like the right approach, tweaking the test framework to exercise
this solution. Thx!
Re: svn commit: r1773761 - /httpd/httpd/trunk/modules/http/http_filters.c
Posted by Jim Jagielski <ji...@jaguNET.com>.
Upon inspection it looks good... doing some further testing
but so far, +1
Thx!
> On Dec 12, 2016, at 11:18 AM, Yann Ylavic <yl...@gmail.com> wrote:
>
> On Mon, Dec 12, 2016 at 3:32 PM, Eric Covener <co...@gmail.com> wrote:
>> On Mon, Dec 12, 2016 at 6:25 AM, Yann Ylavic <yl...@gmail.com> wrote:
>>> Tested with "Header set 'X-Bad' '<something with CR>'" with no redirect loop.
>>
>> w/ 'Header always set...' this still repeats
>
> Right, with r1773812 it looks good.
> We'd only recurse if not in internal redirect already, otherwise fall
> through with:
> HTTP/1.1 500 Internal Server Error
> Date: Mon, 12 Dec 2016 16:11:59 GMT
> Server: Apache/2.5.0-dev (Unix) OpenSSL/1.0.2g
> Content-Length: 0
> Connection: close
>
> WDYT?
Re: svn commit: r1773761 - /httpd/httpd/trunk/modules/http/http_filters.c
Posted by Eric Covener <co...@gmail.com>.
On Mon, Dec 12, 2016 at 2:45 PM, Yann Ylavic <yl...@gmail.com> wrote:
> I think it's addressed in r1773861 and r1773862, thanks all for testing.
Looks good, thank you Yann!
--
Eric Covener
covener@gmail.com
Re: svn commit: r1773761 - /httpd/httpd/trunk/modules/http/http_filters.c
Posted by Yann Ylavic <yl...@gmail.com>.
On Mon, Dec 12, 2016 at 7:17 PM, Jacob Champion <ch...@gmail.com> wrote:
> On 12/12/2016 08:18 AM, Yann Ylavic wrote:
>>
>> On Mon, Dec 12, 2016 at 3:32 PM, Eric Covener <co...@gmail.com> wrote:
>>>
>>> On Mon, Dec 12, 2016 at 6:25 AM, Yann Ylavic <yl...@gmail.com>
>>> wrote:
>>>>
>>>> Tested with "Header set 'X-Bad' '<something with CR>'" with no redirect
>>>> loop.
>>>
>>>
>>> w/ 'Header always set...' this still repeats
>>
>>
>> Right, with r1773812 it looks good.
>
>
> Even with r1773812, I'm still getting recursion with the `Header always set`
> case. ap_is_initial_req() is returning true during the ap_die() response.
The issue was that ap_die() calls ap_send_error_response() and not
ap_internal_redirect() when no ErrorDocument is configured (I was
blinded by my ErrorDocument test...).
I think it's addressed in r1773861 and r1773862, thanks all for testing.
Re: svn commit: r1773761 - /httpd/httpd/trunk/modules/http/http_filters.c
Posted by William A Rowe Jr <wr...@rowe-clan.net>.
On Mon, Dec 12, 2016 at 12:59 PM, Eric Covener <co...@gmail.com> wrote:
> On Mon, Dec 12, 2016 at 1:54 PM, William A Rowe Jr <wr...@rowe-clan.net>
> wrote:
> >> The problem seems to be that `Headers always set` negates the header
> >> removal, and the anti-recursion check doesn't seem to be working as
> >> intended.
> >
> >
> > By removal, I'm suggesting this should happen in the http output filter
> > just as we are about to transmit them.
> >
> > So the header will be set, then it would then be un-set, but my issue
> > is that I can't find the programatic pattern for apr_table_do to
> manipulate
> > the elts, and even if it exists, apr_table_do will quit once the first
> bad
> > elt
> > is found and the callback first returns 0, preventing us from reviewing
> the
> > remaining header lines.
>
> We can loop over either apr_table_do or check_headers while they're
> failing, as long as you are removing 1 header each time to make
> progress.
>
Dozens of good headers followed by dozens of bad headers sounds like
a DOS vector. Probably easier if we just iterate the list ourselves and
skip apr_table_do(), although this sounds like a good example for an
APR 1.next enhancement later on.
Re: svn commit: r1773761 - /httpd/httpd/trunk/modules/http/http_filters.c
Posted by Eric Covener <co...@gmail.com>.
On Mon, Dec 12, 2016 at 1:54 PM, William A Rowe Jr <wr...@rowe-clan.net> wrote:
>> The problem seems to be that `Headers always set` negates the header
>> removal, and the anti-recursion check doesn't seem to be working as
>> intended.
>
>
> By removal, I'm suggesting this should happen in the http output filter
> just as we are about to transmit them.
>
> So the header will be set, then it would then be un-set, but my issue
> is that I can't find the programatic pattern for apr_table_do to manipulate
> the elts, and even if it exists, apr_table_do will quit once the first bad
> elt
> is found and the callback first returns 0, preventing us from reviewing the
> remaining header lines.
We can loop over either apr_table_do or check_headers while they're
failing, as long as you are removing 1 header each time to make
progress.
Re: svn commit: r1773761 - /httpd/httpd/trunk/modules/http/http_filters.c
Posted by William A Rowe Jr <wr...@rowe-clan.net>.
On Mon, Dec 12, 2016 at 12:43 PM, Jacob Champion <ch...@gmail.com>
wrote:
> On 12/12/2016 10:40 AM, William A Rowe Jr wrote:
>
>> I expect we still must have a header unset in the 500 case for all invalid
>> header names or values.
>>
>
> The problem seems to be that `Headers always set` negates the header
> removal, and the anti-recursion check doesn't seem to be working as
> intended.
By removal, I'm suggesting this should happen in the http output filter
just as we are about to transmit them.
So the header will be set, then it would then be un-set, but my issue
is that I can't find the programatic pattern for apr_table_do to manipulate
the elts, and even if it exists, apr_table_do will quit once the first bad
elt
is found and the callback first returns 0, preventing us from reviewing the
remaining header lines.
Re: svn commit: r1773761 -
/httpd/httpd/trunk/modules/http/http_filters.c
Posted by Jacob Champion <ch...@gmail.com>.
On 12/12/2016 10:40 AM, William A Rowe Jr wrote:
> I expect we still must have a header unset in the 500 case for all invalid
> header names or values.
The problem seems to be that `Headers always set` negates the header
removal, and the anti-recursion check doesn't seem to be working as
intended.
--Jacob
Re: svn commit: r1773761 - /httpd/httpd/trunk/modules/http/http_filters.c
Posted by William A Rowe Jr <wr...@rowe-clan.net>.
On Mon, Dec 12, 2016 at 12:17 PM, Jacob Champion <ch...@gmail.com>
wrote:
> On 12/12/2016 08:18 AM, Yann Ylavic wrote:
>
>> On Mon, Dec 12, 2016 at 3:32 PM, Eric Covener <co...@gmail.com> wrote:
>>
>>> On Mon, Dec 12, 2016 at 6:25 AM, Yann Ylavic <yl...@gmail.com>
>>> wrote:
>>>
>>>> Tested with "Header set 'X-Bad' '<something with CR>'" with no redirect
>>>> loop.
>>>>
>>>
>>> w/ 'Header always set...' this still repeats
>>>
>>
>> Right, with r1773812 it looks good.
>>
>
> Even with r1773812, I'm still getting recursion with the `Header always
> set` case. ap_is_initial_req() is returning true during the ap_die()
> response.
>
> Can anyone else confirm?
I expect we still must have a header unset in the 500 case for all invalid
header names or values.
A simple test in Strict mode is
Foo?Bar: Bash
A simple test in non-strict mode is
FooBar: Bash\a
Re: svn commit: r1773761 -
/httpd/httpd/trunk/modules/http/http_filters.c
Posted by Jacob Champion <ch...@gmail.com>.
On 12/12/2016 08:18 AM, Yann Ylavic wrote:
> On Mon, Dec 12, 2016 at 3:32 PM, Eric Covener <co...@gmail.com> wrote:
>> On Mon, Dec 12, 2016 at 6:25 AM, Yann Ylavic <yl...@gmail.com> wrote:
>>> Tested with "Header set 'X-Bad' '<something with CR>'" with no redirect loop.
>>
>> w/ 'Header always set...' this still repeats
>
> Right, with r1773812 it looks good.
Even with r1773812, I'm still getting recursion with the `Header always
set` case. ap_is_initial_req() is returning true during the ap_die()
response.
Can anyone else confirm?
--Jacob
Re: svn commit: r1773761 - /httpd/httpd/trunk/modules/http/http_filters.c
Posted by Yann Ylavic <yl...@gmail.com>.
On Mon, Dec 12, 2016 at 3:32 PM, Eric Covener <co...@gmail.com> wrote:
> On Mon, Dec 12, 2016 at 6:25 AM, Yann Ylavic <yl...@gmail.com> wrote:
>> Tested with "Header set 'X-Bad' '<something with CR>'" with no redirect loop.
>
> w/ 'Header always set...' this still repeats
Right, with r1773812 it looks good.
We'd only recurse if not in internal redirect already, otherwise fall
through with:
HTTP/1.1 500 Internal Server Error
Date: Mon, 12 Dec 2016 16:11:59 GMT
Server: Apache/2.5.0-dev (Unix) OpenSSL/1.0.2g
Content-Length: 0
Connection: close
WDYT?
Re: svn commit: r1773761 - /httpd/httpd/trunk/modules/http/http_filters.c
Posted by Eric Covener <co...@gmail.com>.
On Mon, Dec 12, 2016 at 6:25 AM, Yann Ylavic <yl...@gmail.com> wrote:
> Tested with "Header set 'X-Bad' '<something with CR>'" with no redirect loop.
w/ 'Header always set...' this still repeats
#2010 0x0000000000486399 in ap_send_error_response (r=0x7fffc8004980,
recursive_error=0) at http_protocol.c:1336
#2011 0x000000000048af39 in ap_http_header_filter (f=0x7fffc80061d0,
b=0x7fffc8367b58) at http_filters.c:1252
#2012 0x00000000004396d0 in ap_pass_brigade (next=0x7fffc80061d0,
bb=0x7fffc8367b58) at util_filter.c:610
#2013 0x00000000004408a9 in ap_content_length_filter
(f=0x7fffc8006198, b=0x7fffc8367b58) at protocol.c:1776
#2014 0x00000000004396d0 in ap_pass_brigade (next=0x7fffc8006198,
bb=0x7fffc8367b58) at util_filter.c:610
#2015 0x00007fffe838daa5 in ap_headers_error_filter (f=0x7fffc8367a50,
in=0x7fffc8367b58) at mod_headers.c:917
#2016 0x00000000004396d0 in ap_pass_brigade (next=0x7fffc8367a50,
bb=0x7fffc8367b58) at util_filter.c:610
#2017 0x00007fffe6103dfc in session_output_filter (f=0x7fffc8367a18,
in=0x7fffc8367b58) at mod_session.c:489
#2018 0x00000000004396d0 in ap_pass_brigade (next=0x7fffc8367a18,
bb=0x7fffc8367b58) at util_filter.c:610
#2019 0x0000000000440b5d in ap_old_write_filter (f=0x7fffc8367ae0,
bb=0x7fffc8367b58) at protocol.c:1845
#2020 0x00000000004396d0 in ap_pass_brigade (next=0x7fffc8367ae0,
bb=0x7fffc8367b58) at util_filter.c:610
#2021 0x00000000004400a6 in end_output_stream (r=0x7fffc8004980) at
protocol.c:1540
#2022 0x000000000044011e in ap_finalize_request_protocol
(r=0x7fffc8004980) at protocol.c:1565
#2023 0x0000000000486399 in ap_send_error_response (r=0x7fffc8004980,
recursive_error=0) at http_protocol.c:1336
#2024 0x000000000048af39 in ap_http_header_filter (f=0x7fffc80061d0,
b=0x7fffc83679c8) at http_filters.c:1252
--
Eric Covener
covener@gmail.com
Re: svn commit: r1773761 - /httpd/httpd/trunk/modules/http/http_filters.c
Posted by Yann Ylavic <yl...@gmail.com>.
On Mon, Dec 12, 2016 at 11:26 AM, <yl...@apache.org> wrote:
> Author: ylavic
> Date: Mon Dec 12 10:26:16 2016
> New Revision: 1773761
>
> URL: http://svn.apache.org/viewvc?rev=1773761&view=rev
> Log:
> Follow up to r1773293.
> When check_headers() fails, clear anything (headers and body) from original/errorneous
> response before returning 500.
This returns an error 500 with its associated headers and body (not
the ones of the original response).
Tested with "Header set 'X-Bad' '<something with CR>'" with no redirect loop.
> + if (ctx->headers_error) {
> + /* We'll come back here from ap_send_error_response(),
> + * so clear anything from this response.
> + */
> + apr_brigade_cleanup(b);
> + if (!eos) {
> + return APR_SUCCESS;
> + }
> + ctx->headers_error = 0;
> + apr_table_clear(r->headers_out);
> + apr_table_clear(r->err_headers_out);
> + r->status = HTTP_INTERNAL_SERVER_ERROR;
> + ap_send_error_response(r, 0);
We could also use ap_die() here for ErrorDocument to work, in my
"Header set ..." case (with "ErrorDocument /error.html") there is
indeed a second loop because the internal-redirect's request has the
X-Bad header too, but the looping stops here thanks to the
recursive_error caught by ap_die() the second time around (no custom
response in this case).
Maybe Eric's case is another beast so I committed the safer
ap_send_error_response() first, but if it works for everyone I'm happy
to change to ap_die() instead...
Any objection to add this to STATUS (it addresses Bill's nack AFAICT)?
Regards,
Yann.