Agreed.

On Sep 18, 2013, at 3:11 PM, Ruediger Pluem <[email protected]> wrote:

> 
> 
> 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
> 

Reply via email to