httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From William A Rowe Jr <wr...@rowe-clan.net>
Subject Re: svn commit: r1772678 [1/2] - in /httpd/httpd/branches/2.4.x: ./ docs/manual/mod/ include/ modules/http/ server/
Date Wed, 07 Dec 2016 21:43:51 GMT
On Wed, Dec 7, 2016 at 2:11 PM, Ruediger Pluem <rpluem@apache.org> wrote:

> Sorry for chiming in so late :-(.


No worries, good eye...

On 12/05/2016 03:34 PM, jim@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
>
>

Mime
View raw message