Skip to content

Commit

Permalink
🩹Fix: Adaptor middleware duplicates cookies (#3151)
Browse files Browse the repository at this point in the history
* 🩹Fix: Adaptor middleware duplicates cookies

* 🩹Fix: add extra cases for Test_HTTPMiddlewareWithCookies

---------

Co-authored-by: Juan Calderon-Perez <[email protected]>
  • Loading branch information
sigmundxia and gaby authored Oct 3, 2024
1 parent 44cd700 commit 85a5fb8
Show file tree
Hide file tree
Showing 2 changed files with 78 additions and 0 deletions.
2 changes: 2 additions & 0 deletions middleware/adaptor/adaptor.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,8 @@ func HTTPMiddleware(mw func(http.Handler) http.Handler) fiber.Handler {
c.Request().SetHost(r.Host)
c.Request().Header.SetHost(r.Host)

// Remove all cookies before setting, see https://github.com/valyala/fasthttp/pull/1864
c.Request().Header.DelAllCookies()
for key, val := range r.Header {
for _, v := range val {
c.Request().Header.Set(key, v)
Expand Down
76 changes: 76 additions & 0 deletions middleware/adaptor/adaptor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"net/http"
"net/http/httptest"
"net/url"
"strings"
"testing"

"github.com/gofiber/fiber/v3"
Expand Down Expand Up @@ -200,6 +201,81 @@ func Test_HTTPMiddleware(t *testing.T) {
require.Equal(t, "okay", resp.Header.Get("context_second_okay"))
}

func Test_HTTPMiddlewareWithCookies(t *testing.T) {
const (
cookieHeader = "Cookie"
setCookieHeader = "Set-Cookie"
cookieOneName = "cookieOne"
cookieTwoName = "cookieTwo"
cookieOneValue = "valueCookieOne"
cookieTwoValue = "valueCookieTwo"
)
nethttpMW := func(next http.Handler) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if r.Method != http.MethodPost {
w.WriteHeader(http.StatusMethodNotAllowed)
return
}
next.ServeHTTP(w, r)
})
}

app := fiber.New()
app.Use(HTTPMiddleware(nethttpMW))
app.Post("/", func(c fiber.Ctx) error {
// RETURNING CURRENT COOKIES TO RESPONSE
var cookies []string = strings.Split(c.Get(cookieHeader), "; ")
for _, cookie := range cookies {
c.Set(setCookieHeader, cookie)
}
return c.SendStatus(fiber.StatusOK)
})

// Test case for POST request with cookies
t.Run("POST request with cookies", func(t *testing.T) {
req, err := http.NewRequestWithContext(context.Background(), fiber.MethodPost, "/", nil)
require.NoError(t, err)
req.AddCookie(&http.Cookie{Name: cookieOneName, Value: cookieOneValue})
req.AddCookie(&http.Cookie{Name: cookieTwoName, Value: cookieTwoValue})

resp, err := app.Test(req)
require.NoError(t, err)
cookies := resp.Cookies()
require.Len(t, cookies, 2)
for _, cookie := range cookies {
switch cookie.Name {
case cookieOneName:
require.Equal(t, cookieOneValue, cookie.Value)
case cookieTwoName:
require.Equal(t, cookieTwoValue, cookie.Value)
default:
t.Error("unexpected cookie key")
}
}
})

// New test case for GET request
t.Run("GET request", func(t *testing.T) {
req, err := http.NewRequestWithContext(context.Background(), fiber.MethodGet, "/", nil)
require.NoError(t, err)

resp, err := app.Test(req)
require.NoError(t, err)
require.Equal(t, http.StatusMethodNotAllowed, resp.StatusCode)
})

// New test case for request without cookies
t.Run("POST request without cookies", func(t *testing.T) {
req, err := http.NewRequestWithContext(context.Background(), fiber.MethodPost, "/", nil)
require.NoError(t, err)

resp, err := app.Test(req)
require.NoError(t, err)
require.Equal(t, http.StatusOK, resp.StatusCode)
require.Empty(t, resp.Cookies())
})
}

func Test_FiberHandler(t *testing.T) {
testFiberToHandlerFunc(t, false)
}
Expand Down

1 comment on commit 85a5fb8

@github-actions
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.50.

Benchmark suite Current: 85a5fb8 Previous: 6e74114 Ratio
Benchmark_Utils_GetOffer/1_parameter 222.5 ns/op 0 B/op 0 allocs/op 131.1 ns/op 0 B/op 0 allocs/op 1.70
Benchmark_Utils_GetOffer/1_parameter - ns/op 222.5 ns/op 131.1 ns/op 1.70
Benchmark_Middleware_BasicAuth - B/op 80 B/op 48 B/op 1.67
Benchmark_Middleware_BasicAuth - allocs/op 5 allocs/op 3 allocs/op 1.67
Benchmark_Middleware_BasicAuth_Upper - B/op 80 B/op 48 B/op 1.67
Benchmark_Middleware_BasicAuth_Upper - allocs/op 5 allocs/op 3 allocs/op 1.67
Benchmark_CORS_NewHandler - B/op 16 B/op 0 B/op +∞
Benchmark_CORS_NewHandler - allocs/op 1 allocs/op 0 allocs/op +∞
Benchmark_CORS_NewHandlerSingleOrigin - B/op 16 B/op 0 B/op +∞
Benchmark_CORS_NewHandlerSingleOrigin - allocs/op 1 allocs/op 0 allocs/op +∞
Benchmark_CORS_NewHandlerPreflight - B/op 104 B/op 0 B/op +∞
Benchmark_CORS_NewHandlerPreflight - allocs/op 5 allocs/op 0 allocs/op +∞
Benchmark_CORS_NewHandlerPreflightSingleOrigin - B/op 104 B/op 0 B/op +∞
Benchmark_CORS_NewHandlerPreflightSingleOrigin - allocs/op 5 allocs/op 0 allocs/op +∞
Benchmark_CORS_NewHandlerPreflightWildcard - B/op 104 B/op 0 B/op +∞
Benchmark_CORS_NewHandlerPreflightWildcard - allocs/op 5 allocs/op 0 allocs/op +∞
Benchmark_Middleware_CSRF_GenerateToken - B/op 517 B/op 334 B/op 1.55
Benchmark_Middleware_CSRF_GenerateToken - allocs/op 10 allocs/op 6 allocs/op 1.67

This comment was automatically generated by workflow using github-action-benchmark.

Please sign in to comment.