On Wed, Dec 7, 2016 at 2:11 PM, Ruediger Pluem <rpl...@apache.org> wrote:
> Sorry for chiming in so late :-(. No worries, good eye... On 12/05/2016 03:34 PM, j...@apache.org wrote: > > Author: jim > > Date: Mon Dec 5 14:34:29 2016 > > New Revision: 1772678 > > > > URL: http://svn.apache.org/viewvc?rev=1772678&view=rev > > Log: > > > Modified: httpd/httpd/branches/2.4.x/modules/http/http_filters.c > > URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/ > modules/http/http_filters.c?rev=1772678&r1=1772677&r2=1772678&view=diff > > ============================================================ > ================== > > --- httpd/httpd/branches/2.4.x/modules/http/http_filters.c (original) > > +++ httpd/httpd/branches/2.4.x/modules/http/http_filters.c Mon Dec 5 > 14:34:29 2016 > > > @@ -668,14 +684,83 @@ apr_status_t ap_http_filter(ap_filter_t > > return APR_SUCCESS; > > } > > > > +struct check_header_ctx { > > + request_rec *r; > > + int strict; > > +}; > > + > > +/* check a single header, to be used with apr_table_do() */ > > +static int check_header(void *arg, const char *name, const char *val) > > +{ > > + struct check_header_ctx *ctx = arg; > > + const char *test; > > + > > + if (name[0] == '\0') { > > + ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, ctx->r, APLOGNO(02428) > > + "Empty response header name, aborting request"); > > + return 0; > > + } > > + > > + if (ctx->strict) { > > + test = ap_scan_http_token(name); > > + } > > + else { > > + test = ap_scan_vchar_obstext(name); > > + } > Here the spec is specific that a field name contains only 'token' characters. A much looser reading for badly written back-ends is that they must not contain CTRL characters, or spaces. > > + if (*test) { > > + ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, ctx->r, APLOGNO(02429) > > + "Response header name '%s' contains invalid " > > + "characters, aborting request", > > + name); > > + return 0; > > + } > > + > > + if (ctx->strict) { > > + test = ap_scan_http_field_content(val); > > What characters are not allowed here that are allowed below? > Here the spec is specific that a field values cannot contain ctrls, excepting the whitespace tab character. This is what that corresponds to; (!c || (apr_iscntrl(c) && c != '\t')) > > + } > > + else { > > + /* Simply terminate scanning on a CTL char, allowing whitespace > */ > > + test = val; > > + do { > > + while (*test == ' ' || *test == '\t') test++; > > + test = ap_scan_vchar_obstext(test); > > + } while (*test == ' ' || *test == '\t'); > The above code else case should simply be thrown away and the ctx->strict test eliminated. This is a legacy of our choices around disallowing the RFC7230 section 3.5 oddball whitespace, including tabs like \f or \v. When we moved to be more literal about avoiding these, this code did become the strict case shown above. vchar_obstext may still be worthwhile to save, it differs from http_field_content only in not accepting HT or SP. > > + } > > + if (*test) { > > + ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, ctx->r, APLOGNO(02430) > > + "Response header '%s' value of '%s' contains > invalid " > > + "characters, aborting request", > > + name, val); > > + return 0; > > + } > > + return 1; > > +} > > + > > Regards > > RĂ¼diger > >