trafficserver-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From jpe...@apache.org
Subject git commit: TS-2614: response to invalid Content-Length for POST should be a 400 error
Date Fri, 07 Mar 2014 17:52:26 GMT
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 <rbarber@yahoo-inc.com>
Authored: Thu Mar 6 12:16:58 2014 -0600
Committer: James Peach <jpeach@apache.org>
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 <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

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;
+    }
+  }
+}


Mime
View raw message