]> granicus.if.org Git - apache/commitdiff
Introduce ap_scan_http_token / ap_scan_http_field_content for a much
authorWilliam A. Rowe Jr <wrowe@apache.org>
Fri, 29 Jul 2016 17:37:41 +0000 (17:37 +0000)
committerWilliam A. Rowe Jr <wrowe@apache.org>
Fri, 29 Jul 2016 17:37:41 +0000 (17:37 +0000)
more efficient pass through the header text; rather than reparsing
the strings over and over under the HTTP_CONFORMANCE_STRICT fules.

Improve logic and legibility by eliminating multiple repetitive tests
of the STRICT flag, and simply reorder 'classic' behavior first and
this new parser second to simplify the diff. Because of the whitespace
change (which I had wished to dodge), reading this --ignore-all-space
is a whole lot easier. Particularly against 2.4.x branch, which is now
identical in the 'classic' logic flow. Both of which I'll share with dev@

git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1754556 13f79535-47bb-0310-9956-ffa450edef68

server/protocol.c

index e5cc5cc74a19b7b0d5a97683bd26a54446714464..8ecfe8bb063b9a92a2dc8e96886130922e9c54ce 100644 (file)
@@ -900,71 +900,106 @@ AP_DECLARE(void) ap_get_mime_headers_core(request_rec *r, apr_bucket_brigade *bb
                     return;
                 }
 
-                if (!(value = strchr(last_field, ':'))) { /* Find ':' or    */
-                    r->status = HTTP_BAD_REQUEST;      /* abort bad request */
-                    apr_table_setn(r->notes, "error-notes",
-                                   apr_psprintf(r->pool,
-                                               "Request header field is "
-                                               "missing ':' separator.<br />\n"
-                                               "<pre>\n%.*s</pre>\n", 
-                                               (int)LOG_NAME_MAX_LEN,
-                                               ap_escape_html(r->pool,
-                                                              last_field)));
-                    ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r, APLOGNO(00564)
-                                  "Request header field is missing ':' "
-                                  "separator: %.*s", (int)LOG_NAME_MAX_LEN,
-                                  last_field);
-                    return;
-                }
+                if (!(conf->http_conformance & AP_HTTP_CONFORMANCE_STRICT)) {
+                {
+                    /* Not Strict, using the legacy parser */
+
+                    if (!(value = strchr(last_field, ':'))) { /* Find ':' or */
+                        r->status = HTTP_BAD_REQUEST;   /* abort bad request */
+                        apr_table_setn(r->notes, "error-notes",
+                            apr_psprintf(r->pool,
+                                         "Request header field is "
+                                         "missing ':' separator.<br />\n"
+                                         "<pre>\n%.*s</pre>\n", 
+                                         (int)LOG_NAME_MAX_LEN,
+                                         ap_escape_html(r->pool,
+                                                        last_field)));
+                        ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r, APLOGNO(00564)
+                                      "Request header field is missing ':' "
+                                      "separator: %.*s", (int)LOG_NAME_MAX_LEN,
+                                      last_field);
+                        return;
+                    }
 
-                tmp_field = value - 1; /* last character of field-name */
+                    tmp_field = value - 1; /* last character of field-name */
 
-                *value++ = '\0'; /* NUL-terminate at colon */
+                    *value++ = '\0'; /* NUL-terminate at colon */
 
-                while (*value == ' ' || *value == '\t') {
-                    ++value;            /* Skip to start of value   */
-                }
+                    while (*value == ' ' || *value == '\t') {
+                         ++value;            /* Skip to start of value   */
+                    }
 
-                /* Strip LWS after field-name: */
-                while (tmp_field > last_field
-                       && (*tmp_field == ' ' || *tmp_field == '\t')) {
-                    *tmp_field-- = '\0';
-                }
+                    /* Strip LWS after field-name: */
+                    while (tmp_field > last_field
+                           && (*tmp_field == ' ' || *tmp_field == '\t')) {
+                        *tmp_field-- = '\0';
+                    }
 
-                /* Strip LWS after field-value: */
-                tmp_field = last_field + last_len - 1;
-                while (tmp_field > value
-                       && (*tmp_field == ' ' || *tmp_field == '\t')) {
-                    *tmp_field-- = '\0';
+                    /* Strip LWS after field-value: */
+                    tmp_field = last_field + last_len - 1;
+                    while (tmp_field > value
+                           && (*tmp_field == ' ' || *tmp_field == '\t')) {
+                        *tmp_field-- = '\0';
+                    }
                 }
+                else /* Using strict RFC7230 parsing */
+                {
+                    /* Ensure valid token chars before ':' per RFC 7230 3.2.4 */
+                    if (!(value = (char *)ap_scan_http_token(last_field))
+                            || *value != ':') {
+                        r->status = HTTP_BAD_REQUEST;
+                        apr_table_setn(r->notes, "error-notes",
+                            apr_psprintf(r->pool,
+                                         "Request header field name "
+                                         "is malformed.<br />\n"
+                                         "<pre>\n%.*s</pre>\n", 
+                                         (int)LOG_NAME_MAX_LEN,
+                                         ap_escape_html(r->pool, last_field)));
+                        ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r, APLOGNO(02426)
+                                      "Request header field name is malformed: "
+                                      "%.*s", (int)LOG_NAME_MAX_LEN, last_field);
+                        return;
+                    }
 
-                if (conf->http_conformance & AP_HTTP_CONFORMANCE_STRICT) {
-                    int err = 0;
+                    *value++ = '\0'; /* NUL-terminate last_field name at ':' */
 
-                    if (*last_field == '\0') {
-                        err = HTTP_BAD_REQUEST;
-                        ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r, APLOGNO(02425)
-                                      "Empty request header field name not allowed");
+                    while (*value == ' ' || *value == '\t') {
+                        ++value;     /* Skip LWS of value */
                     }
-                    else if (ap_has_cntrl(last_field)) {
-                        err = HTTP_BAD_REQUEST;
-                        ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r, APLOGNO(02426)
-                                      "[HTTP strict] Request header field name contains "
-                                      "control character: %.*s",
-                                      (int)LOG_NAME_MAX_LEN, last_field);
+
+                    /* Find invalid, non-HT ctrl char, or the trailing NULL */
+                    tmp_field = (char *)ap_scan_http_field_content(value);
+                
+                    /* Strip LWS after field-value, if string not empty */
+                    if (*value && (*tmp_field == '\0')) {
+                        tmp_field--;
+                        while (*tmp_field == ' ' || *tmp_field == '\t') {
+                            *tmp_field-- = '\0';
+                        }
+                        ++tmp_field;
                     }
-                    else if (ap_has_cntrl(value)) {
-                        err = HTTP_BAD_REQUEST;
+
+                    /* Reject value for all garbage input (CTRLs excluding HT)
+                     * e.g. only VCHAR / SP / HT / obs-text are allowed per
+                     * RFC7230 3.2.6 - leave all more explicit rule enforcement
+                     * for specific header handler logic later in the cycle
+                     */
+                    if (*tmp_field != '\0') {
+                        r->status = HTTP_BAD_REQUEST;
+                        apr_table_setn(r->notes, "error-notes",
+                            apr_psprintf(r->pool,
+                                         "Request header value "
+                                         "is malformed.<br />\n"
+                                         "<pre>\n%.*s</pre>\n", 
+                                         (int)LOG_NAME_MAX_LEN,
+                                         ap_escape_html(r->pool, value)));
                         ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r, APLOGNO(02427)
-                                      "Request header field '%.*s' contains "
-                                      "control character", (int)LOG_NAME_MAX_LEN,
-                                      last_field);
-                    }
-                    if (err && !(conf->http_conformance & AP_HTTP_CONFORMANCE_LOGONLY)) {
-                        r->status = err;
+                                      "Request header value is malformed: "
+                                      "%.*s", (int)LOG_NAME_MAX_LEN, value);
                         return;
                     }
                 }
+
                 apr_table_addn(r->headers_in, last_field, value);
 
                 /* reset the alloc_len so that we'll allocate a new