From 65e989e8b8bcf2541d840fcf27dcf8a11931b9db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sigmund=20Xia=20=E5=A4=8F=E5=A4=A9=E7=9D=BF?= Date: Wed, 11 Sep 2024 02:05:52 +0800 Subject: [PATCH] Fix improper memory reuse in NewFastHTTPHandler (#1860) * add test which will cause error * rename test * fix improper memory reuse in NewFastHTTPHandler * move deep copy from VisitAll to f * copy value string only for cookie --- fasthttpadaptor/adaptor_test.go | 47 +++++++++++++++++++++++++++++++++ fasthttpadaptor/request.go | 4 +++ 2 files changed, 51 insertions(+) diff --git a/fasthttpadaptor/adaptor_test.go b/fasthttpadaptor/adaptor_test.go index 229e368550..94ecf9f8d3 100644 --- a/fasthttpadaptor/adaptor_test.go +++ b/fasthttpadaptor/adaptor_test.go @@ -139,6 +139,53 @@ func TestNewFastHTTPHandler(t *testing.T) { } } +func TestNewFastHTTPHandlerWithCookies(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) diff --git a/fasthttpadaptor/request.go b/fasthttpadaptor/request.go index 62a85234ae..35266664db 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,6 +59,9 @@ func ConvertRequest(ctx *fasthttp.RequestCtx, r *http.Request, forServer bool) e case "Transfer-Encoding": r.TransferEncoding = append(r.TransferEncoding, sv) default: + if sk == fasthttp.HeaderCookie { + sv = strings.Clone(sv) + } r.Header.Set(sk, sv) } })