http: malformed request handling improvement

handle case when sum of the DATA frame payload lengths do not equal
value of content-length header

Type: improvement

Change-Id: I71c4114fb52d7ae1dba9645a3dcf41ffe3516e1f
Signed-off-by: Matus Fabian <[email protected]>

diff --git a/extras/hs-test/infra/suite_h2.go b/extras/hs-test/infra/suite_h2.go
index f05d451..6e910b9 100644 (file)
--- a/extras/hs-test/infra/suite_h2.go
+++ b/extras/hs-test/infra/suite_h2.go
@@ -208,8 +208,7 @@ var http2Tests = []h2specTest{
        // {desc: "http2/5.5/2"},
        {desc: "http2/6.1/1"},
        {desc: "http2/6.1/2"},
-       // TODO: http_static url handler POST expect data immediately
-       // {desc: "http2/6.1/3"},
+       {desc: "http2/6.1/3"},
        {desc: "http2/6.2/1"},
        {desc: "http2/6.2/2"},
        {desc: "http2/6.2/3"},
@@ -267,21 +266,26 @@ var http2Tests = []h2specTest{
        {desc: "http2/8.1.2.3/5"},
        {desc: "http2/8.1.2.3/6"},
        {desc: "http2/8.1.2.3/7"},
-       // TODO: http_static url handler POST expect data immediately
-       // {desc: "http2/8.1.2.6/1"},
+       {desc: "http2/8.1.2.6/1"},
        {desc: "http2/8.1.2.6/2"},
        {desc: "http2/8.1.2/1"},
        {desc: "http2/8.1/1"},
        {desc: "http2/8.2/1"},
 }
 
+const (
+       GenericTestGroup int = 1
+       HpackTestGroup   int = 2
+       Http2TestGroup   int = 3
+)
+
 var specs = []struct {
-       tg    *spec.TestGroup
+       tg    int
        tests []h2specTest
 }{
-       {generic.Spec(), genericTests},
-       {hpack.Spec(), hpackTests},
-       {http2.Spec(), http2Tests},
+       {GenericTestGroup, genericTests},
+       {HpackTestGroup, hpackTests},
+       {Http2TestGroup, http2Tests},
 }
 
 // Marked as pending since http plugin is not build with http/2 enabled by default
@@ -300,8 +304,8 @@ var _ = Describe("H2SpecSuite", Pending, Ordered, ContinueOnFailure, func() {
                s.TearDownTest()
        })
 
-       for _, spec := range specs {
-               for _, test := range spec.tests {
+       for _, sp := range specs {
+               for _, test := range sp.tests {
                        test := test
                        testName := "http2_test.go/h2spec_" + strings.ReplaceAll(test.desc, "/", "_")
                        It(testName, func(ctx SpecContext) {
@@ -321,13 +325,24 @@ var _ = Describe("H2SpecSuite", Pending, Ordered, ContinueOnFailure, func() {
                                        Sections:     []string{test.desc},
                                        Verbose:      true,
                                }
-
                                // capture h2spec output so it will be in log
                                oldStdout := os.Stdout
                                r, w, _ := os.Pipe()
                                os.Stdout = w
 
-                               spec.tg.Test(conf)
+                               var tg *spec.TestGroup
+                               switch sp.tg {
+                               case GenericTestGroup:
+                                       tg = generic.Spec()
+                                       break
+                               case HpackTestGroup:
+                                       tg = hpack.Spec()
+                                       break
+                               case Http2TestGroup:
+                                       tg = http2.Spec()
+                                       break
+                               }
+                               tg.Test(conf)
 
                                oChan := make(chan string)
                                go func() {
@@ -341,7 +356,7 @@ var _ = Describe("H2SpecSuite", Pending, Ordered, ContinueOnFailure, func() {
                                os.Stdout = oldStdout
                                o := <-oChan
                                s.Log(o)
-                               s.AssertEqual(0, spec.tg.FailedCount)
+                               s.AssertEqual(0, tg.FailedCount)
                        })
                }
        }

diff --git a/src/plugins/http/http2/http2.c b/src/plugins/http/http2/http2.c
index 9dbca64..bbfa25a 100644 (file)
--- a/src/plugins/http/http2/http2.c
+++ b/src/plugins/http/http2/http2.c
@@ -422,6 +422,7 @@ http2_req_state_wait_transport_method (http_conn_t *hc, http2_req_t *req,
   msg.data.headers_len = req->base.headers_len;
   msg.data.headers_ctx = pointer_to_uword (req->base.headers);
   msg.data.upgrade_proto = HTTP_UPGRADE_PROTO_NA;
+  msg.data.body_offset = req->base.control_data_len;
   msg.data.body_len = req->base.body_len;
 
   svm_fifo_seg_t segs[2] = { { (u8 *) &msg, sizeof (msg) },
@@ -442,16 +443,22 @@ http2_req_state_transport_io_more_data (http_conn_t *hc, http2_req_t *req,
                                        transport_send_params_t *sp,
                                        http2_error_t *error)
 {
+  if (req->payload_len > req->base.to_recv)
+    {
+      HTTP_DBG (1, "received more data than expected");
+      http2_stream_error (hc, req, HTTP2_ERROR_PROTOCOL_ERROR, sp);
+      return HTTP_SM_STOP;
+    }
   req->base.to_recv -= req->payload_len;
-  if (req->base.to_recv == 0)
+  if (req->stream_state == HTTP2_STREAM_STATE_HALF_CLOSED &&
+      req->base.to_recv != 0)
     {
-      if (req->stream_state != HTTP2_STREAM_STATE_HALF_CLOSED)
-       {
-         http2_stream_error (hc, req, HTTP2_ERROR_PROTOCOL_ERROR, sp);
-         return HTTP_SM_STOP;
-       }
-      http_req_state_change (&req->base, HTTP_REQ_STATE_WAIT_APP_REPLY);
+      HTTP_DBG (1, "peer closed stream but don't send all data");
+      http2_stream_error (hc, req, HTTP2_ERROR_PROTOCOL_ERROR, sp);
+      return HTTP_SM_STOP;
     }
+  if (req->base.to_recv == 0)
+    http_req_state_change (&req->base, HTTP_REQ_STATE_WAIT_APP_REPLY);
   http_io_as_write (&req->base, req->payload, req->payload_len);
   http_app_worker_rx_notify (&req->base);