Code Review / vpp.git / commitdiff
? search:
summary | shortlog | log | commit | commitdiff | review | tree
raw | patch | inline | side by side (parent: 182d8b2)
http: fix server sending all status codes 81/40881/3
authorMatus Fabian <matfabia@cisco.com>
Fri, 10 May 2024 14:20:40 +0000 (16:20 +0200)
committerFlorin Coras <florin.coras@gmail.com>
Tue, 14 May 2024 22:36:31 +0000 (22:36 +0000)
Type: fix

Change-Id: I4bc748e3091c2fbe0142d1b74d21a543a62c4ce0
Signed-off-by: Matus Fabian <matfabia@cisco.com>
extras/hs-test/http_test.go patch | blob | history
extras/hs-test/script/build_hst.sh patch | blob | history
src/plugins/hs_apps/http_cli.c patch | blob | history
src/plugins/http/http.c patch | blob | history

diff --git a/extras/hs-test/http_test.go b/extras/hs-test/http_test.go
index fe12f5a..63a5b8e 100644 (file)
--- a/extras/hs-test/http_test.go
+++ b/extras/hs-test/http_test.go
@@ -2,6 +2,7 @@ package main
 
 import (
        "fmt"
+       "net/http"
        "os"
        "strings"
        "time"
@@ -13,7 +14,9 @@ func init() {
        registerNsTests(HttpTpsTest)
        registerVethTests(HttpCliTest)
        registerNoTopoTests(NginxHttp3Test, NginxAsServerTest,
-               NginxPerfCpsTest, NginxPerfRpsTest, NginxPerfWrkTest, HeaderServerTest)
+               NginxPerfCpsTest, NginxPerfRpsTest, NginxPerfWrkTest, HeaderServerTest,
+               HttpStaticMovedTest, HttpStaticNotFoundTest, HttpCliMethodNotAllowedTest,
+               HttpCliBadRequestTest)
        registerNoTopoSoloTests(HttpStaticPromTest)
 }
 
@@ -91,19 +94,79 @@ func HttpStaticPromTest(s *NoTopoSuite) {
        s.assertNil(err)
 }
 
+func HttpStaticMovedTest(s *NoTopoSuite) {
+       vpp := s.getContainerByName("vpp").vppInstance
+       vpp.container.exec("mkdir -p /tmp/tmp.aaa")
+       vpp.container.createFile("/tmp/tmp.aaa/index.html", "<http><body><p>Hello</p></body></http>")
+       serverAddress := s.getInterfaceByName(tapInterfaceName).peer.ip4AddressString()
+       s.log(vpp.vppctl("http static server www-root /tmp uri tcp://" + serverAddress + "/80 debug"))
+
+       client := &http.Client{
+               CheckRedirect: func(req *http.Request, via []*http.Request) error {
+                       return http.ErrUseLastResponse
+               },
+       }
+       req, err := http.NewRequest("GET", "http://"+serverAddress+":80/tmp.aaa", nil)
+       s.assertNil(err, fmt.Sprint(err))
+       resp, err := client.Do(req)
+       s.assertNil(err, fmt.Sprint(err))
+       defer resp.Body.Close()
+       s.assertEqual(301, resp.StatusCode)
+       s.assertNotEqual("", resp.Header.Get("Location"))
+}
+
+func HttpStaticNotFoundTest(s *NoTopoSuite) {
+       vpp := s.getContainerByName("vpp").vppInstance
+       serverAddress := s.getInterfaceByName(tapInterfaceName).peer.ip4AddressString()
+       s.log(vpp.vppctl("http static server www-root /tmp uri tcp://" + serverAddress + "/80 debug"))
+
+       req, err := http.NewRequest("GET", "http://"+serverAddress+":80/notfound.html", nil)
+       s.assertNil(err, fmt.Sprint(err))
+       resp, err := http.DefaultClient.Do(req)
+       s.assertNil(err, fmt.Sprint(err))
+       defer resp.Body.Close()
+       s.assertEqual(404, resp.StatusCode)
+}
+
+func HttpCliMethodNotAllowedTest(s *NoTopoSuite) {
+       vpp := s.getContainerByName("vpp").vppInstance
+       serverAddress := s.getInterfaceByName(tapInterfaceName).peer.ip4AddressString()
+       vpp.vppctl("http cli server")
+
+       req, err := http.NewRequest("POST", "http://"+serverAddress+":80/test", nil)
+       s.assertNil(err, fmt.Sprint(err))
+       resp, err := http.DefaultClient.Do(req)
+       s.assertNil(err, fmt.Sprint(err))
+       defer resp.Body.Close()
+       s.assertEqual(405, resp.StatusCode)
+       // TODO: need to be fixed in http code
+       //s.assertNotEqual("", resp.Header.Get("Allow"))
+}
+
+func HttpCliBadRequestTest(s *NoTopoSuite) {
+       vpp := s.getContainerByName("vpp").vppInstance
+       serverAddress := s.getInterfaceByName(tapInterfaceName).peer.ip4AddressString()
+       vpp.vppctl("http cli server")
+
+       req, err := http.NewRequest("GET", "http://"+serverAddress+":80", nil)
+       s.assertNil(err, fmt.Sprint(err))
+       resp, err := http.DefaultClient.Do(req)
+       s.assertNil(err, fmt.Sprint(err))
+       defer resp.Body.Close()
+       s.assertEqual(400, resp.StatusCode)
+}
+
 func HeaderServerTest(s *NoTopoSuite) {
-       query := "show/version"
        vpp := s.getContainerByName("vpp").vppInstance
        serverAddress := s.getInterfaceByName(tapInterfaceName).peer.ip4AddressString()
        vpp.vppctl("http cli server")
 
-       curlCont := s.getContainerByName("curl")
-       args := fmt.Sprintf("curl -i -s http://%s:80/%s", serverAddress, query)
-       curlCont.extraRunningArgs = args
-       o, err := curlCont.combinedOutput()
+       req, err := http.NewRequest("GET", "http://"+serverAddress+":80/show/version", nil)
        s.assertNil(err, fmt.Sprint(err))
-       s.log(o)
-       s.assertContains(o, "Server: http_cli_server")
+       resp, err := http.DefaultClient.Do(req)
+       s.assertNil(err, fmt.Sprint(err))
+       defer resp.Body.Close()
+       s.assertEqual("http_cli_server", resp.Header.Get("Server"))
 }
 
 func NginxAsServerTest(s *NoTopoSuite) {
diff --git a/extras/hs-test/script/build_hst.sh b/extras/hs-test/script/build_hst.sh
index 33a8393..cc2d00b 100755 (executable)
--- a/extras/hs-test/script/build_hst.sh
+++ b/extras/hs-test/script/build_hst.sh
@@ -67,9 +67,9 @@ docker_build hs-test/vpp vpp
 docker_build hs-test/nginx-ldp nginx
 docker_build hs-test/nginx-server nginx-server
 docker_build hs-test/build build
-docker_build hs-test/curl curl
 if [ "$HST_EXTENDED_TESTS" = true ] ; then
     docker_build hs-test/nginx-http3 nginx-http3
+    docker_build hs-test/curl curl
 fi
 
 # cleanup detached images
diff --git a/src/plugins/hs_apps/http_cli.c b/src/plugins/hs_apps/http_cli.c
index 5d4d49c..f42f653 100644 (file)
--- a/src/plugins/hs_apps/http_cli.c
+++ b/src/plugins/hs_apps/http_cli.c
@@ -323,6 +323,13 @@ hcs_ts_rx_callback (session_t *ts)
       return 0;
     }
 
+  if (msg.data.len == 0)
+    {
+      hs->tx_buf = 0;
+      start_send_data (hs, HTTP_STATUS_BAD_REQUEST);
+      return 0;
+    }
+
   /* send the command to a new/recycled vlib process */
   vec_validate (args.buf, msg.data.len - 1);
   rv = svm_fifo_dequeue (ts->rx_fifo, msg.data.len, args.buf);
diff --git a/src/plugins/http/http.c b/src/plugins/http/http.c
index 526e665..59cb094 100644 (file)
--- a/src/plugins/http/http.c
+++ b/src/plugins/http/http.c
@@ -740,6 +740,10 @@ http_state_wait_app_reply (http_conn_t *hc, transport_send_params_t *sp)
 
   switch (msg.code)
     {
+    case HTTP_STATUS_NOT_FOUND:
+    case HTTP_STATUS_METHOD_NOT_ALLOWED:
+    case HTTP_STATUS_BAD_REQUEST:
+    case HTTP_STATUS_INTERNAL_ERROR:
     case HTTP_STATUS_OK:
       header =
        format (0, http_response_template, http_status_code_str[msg.code],
@@ -760,6 +764,7 @@ http_state_wait_app_reply (http_conn_t *hc, transport_send_params_t *sp)
       /* Location: http(s)://new-place already queued up as data */
       break;
     default:
+      clib_warning ("unsupported status code: %d", msg.code);
       return HTTP_SM_ERROR;
     }
 
Vector Packet Processing