Skip to content
Browse files

TS-2614: response to invalid Content-Length for POST should be a 400 …

…error
  • Loading branch information...
1 parent 148b474 commit e5b8b1dbd0694060871c7c45e7b80640e5ac766f Ron Barber committed with jpeach
View
3 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 <rbarber@yahoo-inc.com>
+
*) [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
View
6 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``.
View
1 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 \
View
15 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>
View
54 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
View
1 proxy/http/HttpTransact.h
@@ -386,6 +386,7 @@ class HttpTransact
NON_EXISTANT_REQUEST_HEADER,
SCHEME_NOT_SUPPORTED,
UNACCEPTABLE_TE_REQUIRED,
+ INVALID_POST_CONTENT_LENGTH,
TOTAL_REQUEST_ERROR_TYPES
};
View
3 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 = \
View
103 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;
+ }
+ }
+}

0 comments on commit e5b8b1d

Please sign in to comment.
Something went wrong with that request. Please try again.