Repository: trafficserver Updated Branches: refs/heads/master 148b47455 -> e5b8b1dbd
TS-2614: response to invalid Content-Length for POST should be a 400 error Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo Commit: http://git-wip-us.apache.org/repos/asf/trafficserver/commit/e5b8b1db Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/e5b8b1db Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/e5b8b1db Branch: refs/heads/master Commit: e5b8b1dbd0694060871c7c45e7b80640e5ac766f Parents: 148b474 Author: Ron Barber <[email protected]> Authored: Thu Mar 6 12:16:58 2014 -0600 Committer: James Peach <[email protected]> Committed: Fri Mar 7 09:47:51 2014 -0800 ---------------------------------------------------------------------- CHANGES | 3 + doc/admin/traffic-server-error-messages.en.rst | 6 ++ proxy/config/body_factory/default/Makefile.am | 1 + .../default/request#invalid_content_length | 15 +++ proxy/http/HttpTransact.cc | 54 +++++++--- proxy/http/HttpTransact.h | 1 + proxy/http/Makefile.am | 3 +- proxy/http/RegressionHttpTransact.cc | 103 +++++++++++++++++++ 8 files changed, 169 insertions(+), 17 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/trafficserver/blob/e5b8b1db/CHANGES ---------------------------------------------------------------------- diff --git a/CHANGES b/CHANGES index 2f13215..1f3954d 100644 --- a/CHANGES +++ b/CHANGES @@ -1,6 +1,9 @@ -*- coding: utf-8 -*- Changes with Apache Traffic Server 5.0.0 + *) [TS-2614] Response to invalid Content-Length for POST should be a 400 error + Author: Ron Barber <[email protected]> + *) [TS-2615] Better logging and error handling in SSL client session startup. *) [TS-2613] Can't turn on attach server session to client from records.config http://git-wip-us.apache.org/repos/asf/trafficserver/blob/e5b8b1db/doc/admin/traffic-server-error-messages.en.rst ---------------------------------------------------------------------- diff --git a/doc/admin/traffic-server-error-messages.en.rst b/doc/admin/traffic-server-error-messages.en.rst index eb06180..01586c2 100644 --- a/doc/admin/traffic-server-error-messages.en.rst +++ b/doc/admin/traffic-server-error-messages.en.rst @@ -205,6 +205,12 @@ with corresponding HTTP response codes and customizable files. of the HTTP protocol. ``response#bad_version`` +``Invalid Content Length`` + ``400`` + Could not process this request because the specified ``Content-Length`` + was invalid (less than 0).. + ``request#invalid_content_length`` + ``Invalid HTTP Request`` ``400`` Could not process this ``<client request>`` HTTP method request for ``URL``. http://git-wip-us.apache.org/repos/asf/trafficserver/blob/e5b8b1db/proxy/config/body_factory/default/Makefile.am ---------------------------------------------------------------------- diff --git a/proxy/config/body_factory/default/Makefile.am b/proxy/config/body_factory/default/Makefile.am index 0e9e6f3..2895e56 100644 --- a/proxy/config/body_factory/default/Makefile.am +++ b/proxy/config/body_factory/default/Makefile.am @@ -36,6 +36,7 @@ dist_bodyfactory_DATA = \ redirect\#moved_temporarily \ request\#cycle_detected \ request\#no_content_length \ + request\#invalid_content_length \ request\#no_host \ request\#scheme_unsupported \ request\#syntax_error \ http://git-wip-us.apache.org/repos/asf/trafficserver/blob/e5b8b1db/proxy/config/body_factory/default/request#invalid_content_length ---------------------------------------------------------------------- diff --git a/proxy/config/body_factory/default/request#invalid_content_length b/proxy/config/body_factory/default/request#invalid_content_length new file mode 100644 index 0000000..a54892b --- /dev/null +++ b/proxy/config/body_factory/default/request#invalid_content_length @@ -0,0 +1,15 @@ +<HTML> +<HEAD> +<TITLE>Invalid Content Length</TITLE> +</HEAD> + +<BODY BGCOLOR="white" FGCOLOR="black"> +<H1>Invalid Content Length</H1> +<HR> + +<FONT FACE="Helvetica,Arial"><B> +Description: Could not process this request because +the specified Content-Length was invalid (less than 0). +</B></FONT> +<HR> +</BODY> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/e5b8b1db/proxy/http/HttpTransact.cc ---------------------------------------------------------------------- diff --git a/proxy/http/HttpTransact.cc b/proxy/http/HttpTransact.cc index 8052a38..da1776c 100644 --- a/proxy/http/HttpTransact.cc +++ b/proxy/http/HttpTransact.cc @@ -5291,6 +5291,23 @@ HttpTransact::RequestError_t HttpTransact::check_request_validity(State* s, HTTP } } + ///////////////////////////////////////////////////// + // 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) { + int64_t length = incoming_hdr->get_content_length(); + s->hdr_info.request_content_length = (length >= 0) ? length : HTTP_UNDEFINED_CL; // content length less than zero is invalid + + DebugTxn("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 && @@ -5312,10 +5329,13 @@ HttpTransact::RequestError_t HttpTransact::check_request_validity(State* s, HTTP // Require Content-Length/Transfer-Encoding for POST/PUSH/PUT if ((scheme == URL_WKSIDX_HTTP || scheme == URL_WKSIDX_HTTPS) && (method == HTTP_WKSIDX_POST || method == HTTP_WKSIDX_PUSH || method == HTTP_WKSIDX_PUT) && - ! incoming_hdr->presence(MIME_PRESENCE_CONTENT_LENGTH) && s->client_info.transfer_encoding != CHUNKED_ENCODING) { - - return NO_POST_CONTENT_LENGTH; + if (!incoming_hdr->presence(MIME_PRESENCE_CONTENT_LENGTH)) { + return NO_POST_CONTENT_LENGTH; + } + if (HTTP_UNDEFINED_CL == s->hdr_info.request_content_length) { + return INVALID_POST_CONTENT_LENGTH; + } } } // Check whether a Host header field is missing in the request. @@ -5679,19 +5699,6 @@ HttpTransact::initialize_state_variables_from_request(State* s, HTTPHdr* obsolet s->hdr_info.extension_method = true; } - ////////////////////////////////////////////////// - // get request content length // - ////////////////////////////////////////////////// - if (s->method != HTTP_WKSIDX_TRACE) { - int64_t length = incoming_request->get_content_length(); - s->hdr_info.request_content_length = (length >= 0) ? length : HTTP_UNDEFINED_CL; // content length less than zero is invalid - - DebugTxn("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 transfer encoding is chunked content length is undefined if (s->client_info.transfer_encoding == CHUNKED_ENCODING) { s->hdr_info.request_content_length = HTTP_UNDEFINED_CL; @@ -6433,6 +6440,14 @@ HttpTransact::is_request_valid(State* s, HTTPHdr* incoming_request) const_cast < char *>(URL_MSG)); return false; } + case INVALID_POST_CONTENT_LENGTH : + { + DebugTxn("http_trans", "[is_request_valid] post request with negative content length value"); + SET_VIA_STRING(VIA_DETAIL_TUNNEL, VIA_DETAIL_TUNNEL_NO_FORWARD); + build_error_response(s, HTTP_STATUS_BAD_REQUEST, "Invalid Content Length", "request#invalid_content_length", + const_cast < char *>(URL_MSG)); + return false; + } default: return true; } @@ -8919,3 +8934,10 @@ HttpTransact::change_response_header_because_of_range_request(State *s, HTTPHdr header->set_content_length(s->range_output_cl); } } + +#if TS_HAS_TESTS +void forceLinkRegressionHttpTransact(); +void forceLinkRegressionHttpTransactCaller() { + forceLinkRegressionHttpTransact(); +} +#endif http://git-wip-us.apache.org/repos/asf/trafficserver/blob/e5b8b1db/proxy/http/HttpTransact.h ---------------------------------------------------------------------- diff --git a/proxy/http/HttpTransact.h b/proxy/http/HttpTransact.h index 652df7f..636a81e 100644 --- a/proxy/http/HttpTransact.h +++ b/proxy/http/HttpTransact.h @@ -386,6 +386,7 @@ public: NON_EXISTANT_REQUEST_HEADER, SCHEME_NOT_SUPPORTED, UNACCEPTABLE_TE_REQUIRED, + INVALID_POST_CONTENT_LENGTH, TOTAL_REQUEST_ERROR_TYPES }; http://git-wip-us.apache.org/repos/asf/trafficserver/blob/e5b8b1db/proxy/http/Makefile.am ---------------------------------------------------------------------- diff --git a/proxy/http/Makefile.am b/proxy/http/Makefile.am index 2d559e7..3d48cee 100644 --- a/proxy/http/Makefile.am +++ b/proxy/http/Makefile.am @@ -74,7 +74,8 @@ libhttp_a_SOURCES = \ HttpUpdateSM.h if BUILD_TESTS - libhttp_a_SOURCES += HttpUpdateTester.cc + libhttp_a_SOURCES += HttpUpdateTester.cc \ + RegressionHttpTransact.cc endif #test_UNUSED_SOURCES = \ http://git-wip-us.apache.org/repos/asf/trafficserver/blob/e5b8b1db/proxy/http/RegressionHttpTransact.cc ---------------------------------------------------------------------- diff --git a/proxy/http/RegressionHttpTransact.cc b/proxy/http/RegressionHttpTransact.cc new file mode 100644 index 0000000..2424964 --- /dev/null +++ b/proxy/http/RegressionHttpTransact.cc @@ -0,0 +1,103 @@ +/** @file + + A brief file description + + @section license License + + Licensed to the Apache Software Foundation (ASF) under one + or more contributor license agreements. See the NOTICE file + distributed with this work for additional information + regarding copyright ownership. The ASF licenses this file + to you under the Apache License, Version 2.0 (the + "License"); you may not use this file except in compliance + with the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. + */ + +#include "Regression.h" +#include "HttpTransact.h" +#include "HttpSM.h" + +void forceLinkRegressionHttpTransact() +{ +} + +static void +init_sm(HttpSM *sm) +{ + + sm->init(); + sm->t_state.hdr_info.client_request.create(HTTP_TYPE_REQUEST, NULL); + +} + +static void +setup_client_request(HttpSM * sm, const char * scheme, const char * request) +{ + init_sm(sm); + + MIOBuffer * read_buffer = new_MIOBuffer(HTTP_HEADER_BUFFER_SIZE_INDEX); + IOBufferReader * buffer_reader = read_buffer->alloc_reader(); + read_buffer->write(request, strlen(request)); + + HTTPParser httpParser; + http_parser_init(&httpParser); + int bytes_used = 0; + sm->t_state.hdr_info.client_request.parse_req(&httpParser, buffer_reader, &bytes_used, true /* eos */); + sm->t_state.hdr_info.client_request.url_get()->scheme_set(scheme, strlen(scheme)); + free_MIOBuffer(read_buffer); +} + +/* +pstatus return values +REGRESSION_TEST_PASSED +REGRESSION_TEST_INPROGRESS +REGRESSION_TEST_FAILED +REGRESSION_TEST_NOT_RUN + */ +REGRESSION_TEST(HttpTransact_is_request_valid)(RegressionTest *t, int /* level */, int *pstatus) +{ + HttpTransact transaction; + HttpSM sm; + *pstatus = REGRESSION_TEST_PASSED; + + struct + { + const char *scheme; + const char *req; + bool result; + } requests[] = { + // missing host header + { "http", "GET / HTTP/1.1\r\n\r\n", false}, + // good get request + { "http", "GET / HTTP/1.1\r\nHost: abc.com\r\n\r\n", true}, + // content len < 0 + { "http", "POST / HTTP/1.1\r\nHost: abc.com\r\nContent-Length: -1\r\n\r\n", false}, + { "http", "PUSH / HTTP/1.1\r\nHost: abc.com\r\nContent-Length: -1\r\n\r\n", false}, + { "http", "PUT / HTTP/1.1\r\nHost: abc.com\r\nContent-Length: -1\r\n\r\n", false}, + // valid content len + { "http", "POST / HTTP/1.1\r\nHost: abc.com\r\nContent-Length: 10\r\n\r\n", true}, + { "http", "PUSH / HTTP/1.1\r\nHost: abc.com\r\nContent-Length: 10\r\n\r\n", true}, + { "http", "PUT / HTTP/1.1\r\nHost: abc.com\r\nContent-Length: 10\r\n\r\n", true}, + // Content Length missing + { "http", "POST / HTTP/1.1\r\nHost: abc.com\r\n\r\n", false}, + { "http", "PUSH / HTTP/1.1\r\nHost: abc.com\r\n\r\n", false}, + { "http", "PUT / HTTP/1.1\r\nHost: abc.com\r\n\r\n", false}, + { NULL, NULL, false} + }; + for (int i = 0; requests[i].req; i++) { + setup_client_request(&sm, requests[i].scheme, requests[i].req); + + if (requests[i].result != transaction.is_request_valid(&sm.t_state, &sm.t_state.hdr_info.client_request)) { + rprintf(t, "HttpTransact::is_request_valid - failed for request = '%s'. Expected result was %s request\n", requests[i].req,(requests[i].result ? "valid" :"invalid") ); + *pstatus = REGRESSION_TEST_FAILED; + } + } +}
