From: Matus Fabian <matfabia@cisco.com>
Date: Fri, 11 Apr 2025 13:38:07 +0000 (-0400)
Subject: http: malformed request handling improvement
X-Git-Tag: v25.10-rc0~93
X-Git-Url: https://gerrit.fd.io/r/gitweb?a=commitdiff_plain;h=refs%2Fchanges%2F89%2F42789%2F4;p=vpp.git

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 <matfabia@cisco.com>
---

diff --git a/extras/hs-test/infra/suite_h2.go b/extras/hs-test/infra/suite_h2.go
index f05d4510e77..6e910b9faf4 100644
--- 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 9dbca646955..bbfa25a8a1f 100644
--- 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);