httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ruediger Pluem <rpl...@apache.org>
Subject Re: svn commit: r1524161 - in /httpd/httpd/trunk: CHANGES docs/log-message-tags/next-number modules/http/http_filters.c server/protocol.c
Date Wed, 18 Sep 2013 19:11:51 GMT


Yann Ylavic wrote:
> The following patch seems to be "draft-ietf-httpbis-p1-messaging-23,
> section 3.3.3.3" compliant (unlike current code) :

Looks good to me except for one comment.

> 
> </PATCH>
> Index: server/protocol.c
> ===================================================================
> --- server/protocol.c    (revision 1524231)
> +++ server/protocol.c    (working copy)
> @@ -1091,6 +1091,8 @@ request_rec *ap_read_request(conn_rec *conn)
>      }
> 
>      if (!r->assbackwards) {
> +        const char *tenc;
> +
>          ap_get_mime_headers_core(r, tmp_bb);
>          if (r->status != HTTP_OK) {
>              ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r, APLOGNO(00567)
> @@ -1102,14 +1104,30 @@ request_rec *ap_read_request(conn_rec *conn)
>              goto traceout;
>          }
> 
> -        if (apr_table_get(r->headers_in, "Transfer-Encoding")
> -            && apr_table_get(r->headers_in, "Content-Length")) {
> -            /*
> http://tools.ietf.org/html/draft-ietf-httpbis-p1-messaging-23#page-31
> -             * "If a message is received with both a Transfer-Encoding and a
> -             * Content-Length header field, the Transfer-Encoding overrides the
> -             * Content-Length. ... A sender MUST remove the received Content-
> -             * Length field"
> +        tenc = apr_table_get(r->headers_in, "Transfer-Encoding");
> +        if (tenc) {
> +            /* http://tools.ietf.org/html/draft-ietf-httpbis-p1-messaging-23
> +             * Section 3.3.3.3: "If a Transfer-Encoding header field is
> +             * present in a request and the chunked transfer coding is not
> +             * the final encoding ...; the server MUST respond with the 400
> +             * (Bad Request) status code and then close the connection".
>               */
> +            if (strcasecmp(tenc, "chunked") != 0
> +                    && !ap_find_last_token(r->pool, tenc, "chunked")) {
> +                r->status = HTTP_BAD_REQUEST;

I think we miss

 conn->keepalive = AP_CONN_CLOSE;

 here in order to ensure that the connection gets closed after sending the error message.

> +                ap_send_error_response(r, 0);
> +                ap_update_child_status(conn->sbh, SERVER_BUSY_LOG, r);
> +                ap_run_log_transaction(r);
> +                apr_brigade_destroy(tmp_bb);
> +                goto traceout;
> +            }
> +
> +            /* http://tools.ietf.org/html/draft-ietf-httpbis-p1-messaging-23
> +             * Section 3.3.3.3: "If a message is received with both a
> +             * Transfer-Encoding and a Content-Length header field, the
> +             * Transfer-Encoding overrides the Content-Length. ... A sender
> +             * MUST remove the received Content-Length field".
> +             */
>              apr_table_unset(r->headers_in, "Content-Length");
>          }
>      }

Regards

RĂ¼diger

Mime
View raw message