You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Ruediger Pluem <rp...@apache.org> on 2016/12/07 20:11:46 UTC
Re: svn commit: r1772678 [1/2] - in /httpd/httpd/branches/2.4.x: ./
docs/manual/mod/ include/ modules/http/ server/
Sorry for chiming in so late :-(.
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);
> + }
> + 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?
> + }
> + 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');
> + }
> + 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
Re: svn commit: r1772678 [1/2] - in /httpd/httpd/branches/2.4.x: ./
docs/manual/mod/ include/ modules/http/ server/
Posted by William A Rowe Jr <wr...@rowe-clan.net>.
On Wed, Dec 7, 2016 at 2:11 PM, Ruediger Pluem <rp...@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
>
>