Skip to content

Commit

Permalink
change error handling for bad resp headers
Browse files Browse the repository at this point in the history
 - avoid looping between ap_die and the http filter
 - remove the header that failed the check
 - keep calling apr_table_do until our fn stops matching


This is still not great. We get the original body, a 500 status
code and status line.

(r1773285 + fix for first return from check_headers)




git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1773293 13f79535-47bb-0310-9956-ffa450edef68
  • Loading branch information
covener committed Dec 8, 2016
1 parent 6f7d9b1 commit e663a7b
Showing 1 changed file with 16 additions and 5 deletions.
21 changes: 16 additions & 5 deletions modules/http/http_filters.c
Expand Up @@ -632,6 +632,7 @@ apr_status_t ap_http_filter(ap_filter_t *f, apr_bucket_brigade *b,
struct check_header_ctx {
request_rec *r;
int strict;
const char *badheader;
};

/* check a single header, to be used with apr_table_do() */
Expand All @@ -643,6 +644,7 @@ static int check_header(void *arg, const char *name, const char *val)
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;
}

Expand All @@ -657,6 +659,7 @@ static int check_header(void *arg, const char *name, const char *val)
"Response header name '%s' contains invalid "
"characters, aborting request",
name);
ctx->badheader = name;
return 0;
}

Expand All @@ -666,6 +669,7 @@ static int check_header(void *arg, const char *name, const char *val)
"Response header '%s' value of '%s' contains invalid "
"characters, aborting request",
name, val);
ctx->badheader = name;
return 0;
}
return 1;
Expand All @@ -680,13 +684,21 @@ static APR_INLINE int check_headers(request_rec *r)
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);
if (!apr_table_do(check_header, &ctx, r->headers_out, NULL))
return 0; /* problem has been logged by check_header() */
ctx.badheader = NULL;

return 1;
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;
}

typedef struct header_struct {
Expand Down Expand Up @@ -1249,8 +1261,7 @@ AP_CORE_DECLARE_NONSTD(apr_status_t) ap_http_header_filter(ap_filter_t *f,
}

if (!check_headers(r)) {
ap_die(HTTP_INTERNAL_SERVER_ERROR, r);
return AP_FILTER_ERROR;
r->status = 500;
}

/*
Expand Down

0 comments on commit e663a7b

Please sign in to comment.