http: state machine fix

When client sends second request without waiting for response of the
first request http_ts_rx_callback should drop request (pipelining is
not supported) instead of invoking return to state machine which can
lead to erroneous state, e.g. reading random data from server app
fifo.

Added simple http static server url handler for testing to simulate
long running request processing, for now hardcoded delay 5 seconds.

Type: fix

Change-Id: Ied9f7e2e4ee64c982f045c0f7f99a2dc5d7a2108
Signed-off-by: Matus Fabian <matfabia@cisco.com>

2 files changed, 100 insertions, 1 deletions

diff --git a/extras/hs-test/http_test.go b/extras/hs-test/http_test.go
index e20efd6f35c..29bd272fe99 100644
--- a/extras/hs-test/http_test.go
+++ b/extras/hs-test/http_test.go
@@ -5,7 +5,10 @@ import (
 	"fmt"
 	"github.com/onsi/gomega/gmeasure"
 	"io"
+	"net"
 	"net/http"
+	"net/http/httptrace"
+	"os"
 	"time"
 
 	. "fd.io/hs-test/infra"
@@ -13,7 +16,7 @@ import (
 
 func init() {
 	RegisterVethTests(HttpCliTest, HttpCliConnectErrorTest)
-	RegisterNoTopoTests(HeaderServerTest,
+	RegisterNoTopoTests(HeaderServerTest, HttpPersistentConnectionTest, HttpPipeliningTest,
 		HttpStaticMovedTest, HttpStaticNotFoundTest, HttpCliMethodNotAllowedTest,
 		HttpCliBadRequestTest, HttpStaticBuildInUrlGetIfStatsTest, HttpStaticBuildInUrlPostIfStatsTest,
 		HttpInvalidRequestLineTest, HttpMethodNotImplementedTest, HttpInvalidHeadersTest,
@@ -56,6 +59,98 @@ func HttpTpsTest(s *NoTopoSuite) {
 	s.RunBenchmark("HTTP tps 10M", 10, 0, httpDownloadBenchmark, url)
 }
 
+func HttpPersistentConnectionTest(s *NoTopoSuite) {
+	// testing url handler app do not support multi-thread
+	s.SkipIfMultiWorker()
+	vpp := s.GetContainerByName("vpp").VppInstance
+	serverAddress := s.GetInterfaceByName(TapInterfaceName).Peer.Ip4AddressString()
+	s.Log(vpp.Vppctl("http static server uri tcp://" + serverAddress + "/80 url-handlers"))
+	s.Log(vpp.Vppctl("test-url-handler enable"))
+
+	transport := http.DefaultTransport
+	transport.(*http.Transport).Proxy = nil
+	transport.(*http.Transport).DisableKeepAlives = false
+	client := &http.Client{
+		Transport: transport,
+		Timeout:   time.Second * 30,
+		CheckRedirect: func(req *http.Request, via []*http.Request) error {
+			return http.ErrUseLastResponse
+		}}
+
+	req, err := http.NewRequest("GET", "http://"+serverAddress+":80/test1", nil)
+	s.AssertNil(err, fmt.Sprint(err))
+	resp, err := client.Do(req)
+	s.AssertNil(err, fmt.Sprint(err))
+	defer resp.Body.Close()
+	s.Log(DumpHttpResp(resp, true))
+	s.AssertEqual(200, resp.StatusCode)
+	s.AssertEqual(false, resp.Close)
+	body, err := io.ReadAll(resp.Body)
+	s.AssertNil(err, fmt.Sprint(err))
+	s.AssertEqual(string(body), "hello")
+	o1 := vpp.Vppctl("show session verbose proto http state ready")
+	s.Log(o1)
+	s.AssertContains(o1, "ESTABLISHED")
+
+	req, err = http.NewRequest("GET", "http://"+serverAddress+":80/test2", nil)
+	s.AssertNil(err, fmt.Sprint(err))
+	clientTrace := &httptrace.ClientTrace{
+		GotConn: func(info httptrace.GotConnInfo) {
+			s.AssertEqual(true, info.Reused, "connection not reused")
+		},
+	}
+	req = req.WithContext(httptrace.WithClientTrace(req.Context(), clientTrace))
+	resp, err = client.Do(req)
+	s.AssertNil(err, fmt.Sprint(err))
+	defer resp.Body.Close()
+	s.Log(DumpHttpResp(resp, true))
+	s.AssertEqual(200, resp.StatusCode)
+	s.AssertEqual(false, resp.Close)
+	body, err = io.ReadAll(resp.Body)
+	s.AssertNil(err, fmt.Sprint(err))
+	s.AssertEqual(string(body), "some data")
+	s.AssertNil(err, fmt.Sprint(err))
+	o2 := vpp.Vppctl("show session verbose proto http state ready")
+	s.Log(o2)
+	s.AssertContains(o2, "ESTABLISHED")
+	s.AssertEqual(o1, o2)
+}
+
+func HttpPipeliningTest(s *NoTopoSuite) {
+	// testing url handler app do not support multi-thread
+	s.SkipIfMultiWorker()
+	vpp := s.GetContainerByName("vpp").VppInstance
+	serverAddress := s.GetInterfaceByName(TapInterfaceName).Peer.Ip4AddressString()
+	s.Log(vpp.Vppctl("http static server uri tcp://" + serverAddress + "/80 url-handlers debug"))
+	s.Log(vpp.Vppctl("test-url-handler enable"))
+
+	req1 := "GET /test_delayed HTTP/1.1\r\nHost:" + serverAddress + ":80\r\nUser-Agent:test\r\n\r\n"
+	req2 := "GET /test1 HTTP/1.1\r\nHost:" + serverAddress + ":80\r\nUser-Agent:test\r\n\r\n"
+
+	conn, err := net.DialTimeout("tcp", serverAddress+":80", time.Second*30)
+	s.AssertNil(err, fmt.Sprint(err))
+	defer conn.Close()
+	err = conn.SetDeadline(time.Now().Add(time.Second * 15))
+	s.AssertNil(err, fmt.Sprint(err))
+	n, err := conn.Write([]byte(req1))
+	s.AssertNil(err, fmt.Sprint(err))
+	s.AssertEqual(n, len([]rune(req1)))
+	// send second request a bit later so first is already in progress
+	time.Sleep(500 * time.Millisecond)
+	n, err = conn.Write([]byte(req2))
+	s.AssertNil(err, fmt.Sprint(err))
+	s.AssertEqual(n, len([]rune(req2)))
+	reply := make([]byte, 1024)
+	n, err = conn.Read(reply)
+	s.AssertNil(err, fmt.Sprint(err))
+	s.Log(string(reply))
+	s.AssertContains(string(reply), "delayed data", "first request response not received")
+	s.AssertNotContains(string(reply), "hello", "second request response received")
+	// make sure response for second request is not received later
+	_, err = conn.Read(reply)
+	s.AssertMatchError(err, os.ErrDeadlineExceeded, "second request response received")
+}
+
 func HttpCliTest(s *VethsSuite) {
 	serverContainer := s.GetContainerByName("server-vpp")
 	clientContainer := s.GetContainerByName("client-vpp")
diff --git a/extras/hs-test/infra/hst_suite.go b/extras/hs-test/infra/hst_suite.go
index 2cf241afa64..1f1d54b1b94 100644
--- a/extras/hs-test/infra/hst_suite.go
+++ b/extras/hs-test/infra/hst_suite.go
@@ -224,6 +224,10 @@ func (s *HstSuite) AssertNotEmpty(object interface{}, msgAndArgs ...interface{})
 	Expect(object).ToNot(BeEmpty(), msgAndArgs...)
 }
 
+func (s *HstSuite) AssertMatchError(actual, expected error, msgAndArgs ...interface{}) {
+	Expect(actual).To(MatchError(expected))
+}
+
 func (s *HstSuite) CreateLogger() {
 	suiteName := s.GetCurrentSuiteName()
 	var err error