From d74126f13f98d2ab6f2c68551ba375abc74e509b Mon Sep 17 00:00:00 2001 From: Sigmund Xia Date: Sun, 8 Sep 2024 17:06:11 +0800 Subject: [PATCH 1/5] add test which will cause error --- fasthttpadaptor/adaptor_test.go | 47 +++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/fasthttpadaptor/adaptor_test.go b/fasthttpadaptor/adaptor_test.go index 229e368550..4fb7ff98d8 100644 --- a/fasthttpadaptor/adaptor_test.go +++ b/fasthttpadaptor/adaptor_test.go @@ -139,6 +139,53 @@ func TestNewFastHTTPHandler(t *testing.T) { } } +func TestMiddleware(t *testing.T) { + expectedMethod := fasthttp.MethodPost + expectedRequestURI := "/foo/bar?baz=123" + expectedHost := "foobar.com" + expectedRemoteAddr := "1.2.3.4:6789" + + var ctx fasthttp.RequestCtx + var req fasthttp.Request + + req.Header.SetMethod(expectedMethod) + req.SetRequestURI(expectedRequestURI) + req.Header.SetHost(expectedHost) + req.Header.SetCookie("cookieOne", "valueCookieOne") + req.Header.SetCookie("cookieTwo", "valueCookieTwo") + + remoteAddr, err := net.ResolveTCPAddr("tcp", expectedRemoteAddr) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + ctx.Init(&req, remoteAddr, nil) + + nethttpH := func(w http.ResponseWriter, r *http.Request) { + // real handler warped by middleware, in this example do nothing + } + fasthttpH := NewFastHTTPHandler(http.HandlerFunc(nethttpH)) + + netMiddleware := func(_ http.ResponseWriter, r *http.Request) { + // assume middleware do some change on r, such as reset header's host + r.Header.Set("Host", "example.com") + // convert ctx again in case request may modify by middleware + ctx.Request.Header.Set("Host", r.Header.Get("Host")) + // since cookies of r are not changed, expect "cookieOne=valueCookieOne" + cookie, _ := r.Cookie("cookieOne") + if err != nil { + // will error, but if line 172 is commented, then no error will happen + t.Errorf("should not error") + } + if cookie.Value != "valueCookieOne" { + t.Errorf("cookie error, expect %s, find %s", "valueCookieOne", cookie.Value) + } + // instead of using responseWriter and r, use ctx again, like what have done in fiber + fasthttpH(&ctx) + } + fastMiddleware := NewFastHTTPHandler(http.HandlerFunc(netMiddleware)) + fastMiddleware(&ctx) +} + func setContextValueMiddleware(next fasthttp.RequestHandler, key string, value any) fasthttp.RequestHandler { return func(ctx *fasthttp.RequestCtx) { ctx.SetUserValue(key, value) From a165213f6370f59e5d6414f4f573eb6519c472ca Mon Sep 17 00:00:00 2001 From: Sigmund Xia Date: Sun, 8 Sep 2024 17:08:32 +0800 Subject: [PATCH 2/5] rename test --- fasthttpadaptor/adaptor_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fasthttpadaptor/adaptor_test.go b/fasthttpadaptor/adaptor_test.go index 4fb7ff98d8..83c2dd5525 100644 --- a/fasthttpadaptor/adaptor_test.go +++ b/fasthttpadaptor/adaptor_test.go @@ -139,7 +139,7 @@ func TestNewFastHTTPHandler(t *testing.T) { } } -func TestMiddleware(t *testing.T) { +func TestNewFastHTTPHandlerWithMiddleware(t *testing.T) { expectedMethod := fasthttp.MethodPost expectedRequestURI := "/foo/bar?baz=123" expectedHost := "foobar.com" From 8f2d75e8ddc4d5ae091adb6e57f1e398a6a8acf2 Mon Sep 17 00:00:00 2001 From: Sigmund Xia Date: Sun, 8 Sep 2024 20:28:09 +0800 Subject: [PATCH 3/5] fix improper memory reuse in NewFastHTTPHandler --- fasthttpadaptor/adaptor_test.go | 2 +- header.go | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/fasthttpadaptor/adaptor_test.go b/fasthttpadaptor/adaptor_test.go index 83c2dd5525..94ecf9f8d3 100644 --- a/fasthttpadaptor/adaptor_test.go +++ b/fasthttpadaptor/adaptor_test.go @@ -139,7 +139,7 @@ func TestNewFastHTTPHandler(t *testing.T) { } } -func TestNewFastHTTPHandlerWithMiddleware(t *testing.T) { +func TestNewFastHTTPHandlerWithCookies(t *testing.T) { expectedMethod := fasthttp.MethodPost expectedRequestURI := "/foo/bar?baz=123" expectedHost := "foobar.com" diff --git a/header.go b/header.go index b6d8ffec92..28ff307c21 100644 --- a/header.go +++ b/header.go @@ -1286,8 +1286,9 @@ func (h *RequestHeader) VisitAll(f func(key, value []byte)) { h.collectCookies() if len(h.cookies) > 0 { - h.bufV = appendRequestCookieBytes(h.bufV[:0], h.cookies) - f(strCookie, h.bufV) + buf := make([]byte, 0) + buf = appendRequestCookieBytes(buf[:0], h.cookies) + f(strCookie, buf) } visitArgs(h.h, f) if h.ConnectionClose() { From 6a1233aee2480d8820262e8df880439d33cba4fe Mon Sep 17 00:00:00 2001 From: Sigmund Xia Date: Sun, 8 Sep 2024 21:32:10 +0800 Subject: [PATCH 4/5] move deep copy from VisitAll to f --- fasthttpadaptor/request.go | 4 +++- header.go | 5 ++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/fasthttpadaptor/request.go b/fasthttpadaptor/request.go index 62a85234ae..b720eb9c6d 100644 --- a/fasthttpadaptor/request.go +++ b/fasthttpadaptor/request.go @@ -5,6 +5,7 @@ import ( "io" "net/http" "net/url" + "strings" "github.com/valyala/fasthttp" ) @@ -58,7 +59,8 @@ func ConvertRequest(ctx *fasthttp.RequestCtx, r *http.Request, forServer bool) e case "Transfer-Encoding": r.TransferEncoding = append(r.TransferEncoding, sv) default: - r.Header.Set(sk, sv) + svCopy := strings.Clone(sv) + r.Header.Set(sk, svCopy) } }) diff --git a/header.go b/header.go index 28ff307c21..b6d8ffec92 100644 --- a/header.go +++ b/header.go @@ -1286,9 +1286,8 @@ func (h *RequestHeader) VisitAll(f func(key, value []byte)) { h.collectCookies() if len(h.cookies) > 0 { - buf := make([]byte, 0) - buf = appendRequestCookieBytes(buf[:0], h.cookies) - f(strCookie, buf) + h.bufV = appendRequestCookieBytes(h.bufV[:0], h.cookies) + f(strCookie, h.bufV) } visitArgs(h.h, f) if h.ConnectionClose() { From 181eddbd3aebfa5ce2b78719355dd0dfaaf16fa9 Mon Sep 17 00:00:00 2001 From: Sigmund Xia Date: Sun, 8 Sep 2024 22:40:11 +0800 Subject: [PATCH 5/5] copy value string only for cookie --- fasthttpadaptor/request.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/fasthttpadaptor/request.go b/fasthttpadaptor/request.go index b720eb9c6d..35266664db 100644 --- a/fasthttpadaptor/request.go +++ b/fasthttpadaptor/request.go @@ -59,8 +59,10 @@ func ConvertRequest(ctx *fasthttp.RequestCtx, r *http.Request, forServer bool) e case "Transfer-Encoding": r.TransferEncoding = append(r.TransferEncoding, sv) default: - svCopy := strings.Clone(sv) - r.Header.Set(sk, svCopy) + if sk == fasthttp.HeaderCookie { + sv = strings.Clone(sv) + } + r.Header.Set(sk, sv) } })