Skip to content

Commit

Permalink
Fix improper memory reuse in NewFastHTTPHandler (#1860)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
sigmundxia authored Sep 10, 2024
1 parent 74864cb commit 65e989e
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 0 deletions.
47 changes: 47 additions & 0 deletions fasthttpadaptor/adaptor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 4 additions & 0 deletions fasthttpadaptor/request.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"io"
"net/http"
"net/url"
"strings"

"github.com/valyala/fasthttp"
)
Expand Down Expand Up @@ -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)
}
})
Expand Down

0 comments on commit 65e989e

Please sign in to comment.