diff --git a/proxy/http/HttpSM.cc b/proxy/http/HttpSM.cc index 36b29e002b7..2747dfa568c 100644 --- a/proxy/http/HttpSM.cc +++ b/proxy/http/HttpSM.cc @@ -899,11 +899,17 @@ HttpSM::state_read_client_request_header(int event, void *data) } } - if (t_state.hdr_info.client_request.method_get_wksidx() == HTTP_WKSIDX_TRACE || - (t_state.hdr_info.client_request.get_content_length() == 0 && - t_state.client_info.transfer_encoding != HttpTransact::CHUNKED_ENCODING)) { + // Call to ensure the content-length and transfer_encoding elements in client_request are filled in + HttpTransact::set_client_request_state(&t_state, &t_state.hdr_info.client_request); + + if (t_state.hdr_info.client_request.get_content_length() == 0 && + t_state.client_info.transfer_encoding != HttpTransact::CHUNKED_ENCODING) { // Enable further IO to watch for client aborts ua_entry->read_vio->reenable(); + } else if (t_state.hdr_info.client_request.method_get_wksidx() == HTTP_WKSIDX_TRACE) { + // Trace with request body is not allowed + call_transact_and_set_next_state(HttpTransact::BadRequest); + return 0; } else { // Disable further I/O on the client since there could // be body that we are tunneling POST/PUT/CONNECT or diff --git a/proxy/http/HttpTransact.cc b/proxy/http/HttpTransact.cc index b995d1535a5..6713f44c975 100644 --- a/proxy/http/HttpTransact.cc +++ b/proxy/http/HttpTransact.cc @@ -5346,6 +5346,9 @@ HttpTransact::add_client_ip_to_outgoing_request(State *s, HTTPHdr *request) HttpTransact::RequestError_t HttpTransact::check_request_validity(State *s, HTTPHdr *incoming_hdr) { + // Called also on receiving request. Not sure if we need to call this again in case + // the transfer-encoding and content-length headers changed + set_client_request_state(s, incoming_hdr); if (incoming_hdr == nullptr) { return NON_EXISTANT_REQUEST_HEADER; } @@ -5369,44 +5372,6 @@ HttpTransact::check_request_validity(State *s, HTTPHdr *incoming_hdr) int scheme = incoming_url->scheme_get_wksidx(); int method = incoming_hdr->method_get_wksidx(); - // Check for chunked encoding - if (incoming_hdr->presence(MIME_PRESENCE_TRANSFER_ENCODING)) { - MIMEField *field = incoming_hdr->field_find(MIME_FIELD_TRANSFER_ENCODING, MIME_LEN_TRANSFER_ENCODING); - if (field) { - HdrCsvIter enc_val_iter; - int enc_val_len; - const char *enc_value = enc_val_iter.get_first(field, &enc_val_len); - - while (enc_value) { - const char *wks_value = hdrtoken_string_to_wks(enc_value, enc_val_len); - if (wks_value == HTTP_VALUE_CHUNKED) { - s->client_info.transfer_encoding = CHUNKED_ENCODING; - break; - } - enc_value = enc_val_iter.get_next(&enc_val_len); - } - } - } - - ///////////////////////////////////////////////////// - // get request content length // - // To avoid parsing content-length twice, we set // - // s->hdr_info.request_content_length here rather // - // than in initialize_state_variables_from_request // - ///////////////////////////////////////////////////// - if (method != HTTP_WKSIDX_TRACE) { - if (incoming_hdr->presence(MIME_PRESENCE_CONTENT_LENGTH)) { - s->hdr_info.request_content_length = incoming_hdr->get_content_length(); - } else { - s->hdr_info.request_content_length = HTTP_UNDEFINED_CL; // content length less than zero is invalid - } - - TxnDebug("http_trans", "[init_stat_vars_from_req] set req cont length to %" PRId64, s->hdr_info.request_content_length); - - } else { - s->hdr_info.request_content_length = 0; - } - if (!((scheme == URL_WKSIDX_HTTP) && (method == HTTP_WKSIDX_GET))) { if (scheme != URL_WKSIDX_HTTP && scheme != URL_WKSIDX_HTTPS && method != HTTP_WKSIDX_CONNECT && !((scheme == URL_WKSIDX_WS || scheme == URL_WKSIDX_WSS) && s->is_websocket)) { @@ -5485,6 +5450,47 @@ HttpTransact::check_request_validity(State *s, HTTPHdr *incoming_hdr) return NO_REQUEST_HEADER_ERROR; } +void +HttpTransact::set_client_request_state(State *s, HTTPHdr *incoming_hdr) +{ + if (incoming_hdr == nullptr) { + return; + } + + // Set transfer_encoding value + if (incoming_hdr->presence(MIME_PRESENCE_TRANSFER_ENCODING)) { + MIMEField *field = incoming_hdr->field_find(MIME_FIELD_TRANSFER_ENCODING, MIME_LEN_TRANSFER_ENCODING); + if (field) { + HdrCsvIter enc_val_iter; + int enc_val_len; + const char *enc_value = enc_val_iter.get_first(field, &enc_val_len); + + while (enc_value) { + const char *wks_value = hdrtoken_string_to_wks(enc_value, enc_val_len); + if (wks_value == HTTP_VALUE_CHUNKED) { + s->client_info.transfer_encoding = CHUNKED_ENCODING; + break; + } + enc_value = enc_val_iter.get_next(&enc_val_len); + } + } + } + + ///////////////////////////////////////////////////// + // get request content length // + // To avoid parsing content-length twice, we set // + // s->hdr_info.request_content_length here rather // + // than in initialize_state_variables_from_request // + ///////////////////////////////////////////////////// + if (incoming_hdr->presence(MIME_PRESENCE_CONTENT_LENGTH)) { + s->hdr_info.request_content_length = incoming_hdr->get_content_length(); + } else { + s->hdr_info.request_content_length = HTTP_UNDEFINED_CL; // content length less than zero is invalid + } + + TxnDebug("http_trans", "[set_client_request_state] set req cont length to %" PRId64, s->hdr_info.request_content_length); +} + HttpTransact::ResponseError_t HttpTransact::check_response_validity(State *s, HTTPHdr *incoming_hdr) { diff --git a/proxy/http/HttpTransact.h b/proxy/http/HttpTransact.h index bb432c6d520..9273b2c5ce7 100644 --- a/proxy/http/HttpTransact.h +++ b/proxy/http/HttpTransact.h @@ -1008,6 +1008,7 @@ class HttpTransact static void add_client_ip_to_outgoing_request(State *s, HTTPHdr *request); static RequestError_t check_request_validity(State *s, HTTPHdr *incoming_hdr); static ResponseError_t check_response_validity(State *s, HTTPHdr *incoming_hdr); + static void set_client_request_state(State *s, HTTPHdr *incoming_hdr); static bool delete_all_document_alternates_and_return(State *s, bool cache_hit); static bool does_client_request_permit_cached_response(const OverridableHttpConfigParams *p, CacheControlResult *c, HTTPHdr *h, char *via_string); diff --git a/tests/gold_tests/headers/gold/bad_good_request.gold b/tests/gold_tests/headers/gold/bad_good_request.gold index 217afe03aea..ceef1e5e159 100644 --- a/tests/gold_tests/headers/gold/bad_good_request.gold +++ b/tests/gold_tests/headers/gold/bad_good_request.gold @@ -1,23 +1,9 @@ -HTTP/1.1 400 Invalid HTTP Request -Date: `` -Connection: close -Server: ATS/`` -Cache-Control: no-store -Content-Type: text/html -Content-Language: en -Content-Length: 219 - - - +``HTTP/1.1 400 Invalid HTTP Request +``Connection: close +``Server: ATS/`` +``Content-Length: 219 +`` Bad Request - - - -

Bad Request

-
- - -Description: Could not process this request. - -
- +``

Bad Request

+``Description: Could not process this request. +`` diff --git a/tests/gold_tests/headers/good_request_after_bad.test.py b/tests/gold_tests/headers/good_request_after_bad.test.py index c5cbf10c37d..9a76975dbd7 100644 --- a/tests/gold_tests/headers/good_request_after_bad.test.py +++ b/tests/gold_tests/headers/good_request_after_bad.test.py @@ -22,6 +22,7 @@ Test.Summary = ''' Verify that request following a ill-formed request is not processed ''' +Test.ContinueOnFail = True ts = Test.MakeATSProcess("ts", enable_cache=True) ts.Disk.records_config.update({'proxy.config.diags.debug.tags': 'http', @@ -64,3 +65,29 @@ ts.Variables.port) tr.Processes.Default.ReturnCode = 0 tr.Processes.Default.Streams.stdout = 'gold/bad_te_value.gold' + +# TRACE request with a body +tr = Test.AddTestRun("Trace request with a body") +tr.Processes.Default.Command = 'printf "TRACE /foo HTTP/1.1\r\nHost: bob\r\nContent-length:2\r\n\r\nokGET / HTTP/1.1\r\nHost: boa\r\n\r\n" | nc 127.0.0.1 {}'.format( + ts.Variables.port) +tr.Processes.Default.ReturnCode = 0 +tr.Processes.Default.Streams.stdout = 'gold/bad_good_request.gold' + +tr = Test.AddTestRun("Trace request with a chunked body") +tr.Processes.Default.Command = 'printf "TRACE /foo HTTP/1.1\r\nHost: bob\r\ntransfer-encoding: chunked\r\n\r\n2\r\nokGGET / HTTP/1.1\r\nHost: boa\r\n\r\n" | nc 127.0.0.1 {}'.format( + ts.Variables.port) +tr.Processes.Default.ReturnCode = 0 +tr.Processes.Default.Streams.stdout = 'gold/bad_good_request.gold' + +tr = Test.AddTestRun("Trace request with a chunked body via curl") +tr.Processes.Default.Command = 'curl -v --http1.1 --header "Transfer-Encoding: chunked" -d aaa -X TRACE -k http://127.0.0.1:{}/foo'.format( + ts.Variables.port) +tr.Processes.Default.ReturnCode = 0 +tr.Processes.Default.Streams.All = 'gold/bad_good_request.gold' + +tr = Test.AddTestRun("Trace request via curl") +tr.Processes.Default.Command = 'curl -v --http1.1 -X TRACE -k http://127.0.0.1:{}/bar'.format(ts.Variables.port) +tr.Processes.Default.ReturnCode = 0 +tr.Processes.Default.Streams.All = Testers.ContainsExpression( + r"HTTP/1.1 501 Unsupported method \('TRACE'\)", + "microserver does not support TRACE")