docs/whats_new.md (1)

1130-1134: Doc-code mismatch: clarify when ETag is recalculated.

You state that strong ETags are recomputed “for compressed payloads.” In code, ETags are recomputed unconditionally after invoking the compressor, even if no compression actually occurs (e.g., small bodies). Please either clarify the doc or gate recomputation in code to only when Content-Encoding is set.

middleware/etag/etag.go (2)

45-45: Use canonical header casing for ETag.

[]byte("Etag") is non‑canonical. Prefer "ETag" to match fiber.HeaderETag conventions and avoid accidental mismatches with other code using canonical names.

Apply this diff:

-	normalizedHeaderETag := []byte("Etag")
+	normalizedHeaderETag := []byte("ETag")

---

78-84: Minor: avoid shadow naming with package name.

Local var etag shadows the package name etag in middleware/compress. Not a bug here, but renaming to tag/value reduces confusion in call sites and stack traces.

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Learnt from: sixcolors
PR: gofiber/fiber#3016
File: middleware/session/store.go:164-167
Timestamp: 2024-10-02T23:03:31.727Z
Learning: Unit tests in this project use testify require.

Learnt from: sixcolors
PR: gofiber/fiber#2922
File: middleware/cors/utils.go:63-71
Timestamp: 2024-07-26T21:00:12.902Z
Learning: The project uses the testify/assert package for assertions in unit tests.

middleware/compress/compress.go (2)

12-19: Fix: hasToken won’t compile and misiterates slice (duplicate).

Replace SplitSeq and index iteration; trim spaces; prefer stdlib.

Apply:

@@
-func hasToken(header, token string) bool {
-	for part := range strings.SplitSeq(header, ",") {
-		if strings.EqualFold(utils.Trim(part, ' '), token) {
-			return true
-		}
-	}
-	return false
-}
+func hasToken(header, token string) bool {
+	for _, part := range strings.Split(header, ",") {
+		if strings.EqualFold(strings.TrimSpace(part), token) {
+			return true
+		}
+	}
+	return false
+}

Also drop the now‑unused utils import:

@@
-import (
-	"strings"
-
-	"github.com/gofiber/fiber/v3"
-	"github.com/gofiber/fiber/v3/middleware/etag"
-	"github.com/gofiber/utils/v2"
-	"github.com/valyala/fasthttp"
-)
+import (
+	"strings"
+
+	"github.com/gofiber/fiber/v3"
+	"github.com/gofiber/fiber/v3/middleware/etag"
+	"github.com/valyala/fasthttp"
+)

---

111-116: Only recompute strong ETag when compression actually happened (duplicate).

Gate on Content-Encoding and avoid clobbering app ETags; also guard nil.

Apply:

@@
-		if tag := c.GetRespHeader(fiber.HeaderETag); tag != "" && !strings.HasPrefix(tag, "W/") {
-			c.Set(fiber.HeaderETag, string(etag.Generate(c.Response().Body())))
-		}
+		if ce := c.GetRespHeader(fiber.HeaderContentEncoding); ce != "" {
+			if tag := c.GetRespHeader(fiber.HeaderETag); tag != "" && !strings.HasPrefix(tag, "W/") {
+				if newTag := etag.Generate(c.Response().Body()); len(newTag) != 0 {
+					c.Set(fiber.HeaderETag, string(newTag))
+				}
+			}
+		}

middleware/compress/compress_test.go (1)

352-374: Strong ETag recomputation test LGTM. Add guard for non-compress case (duplicate).

Add a test ensuring strong ETag stays unchanged when compressor skips (tiny body).

Apply:

+func Test_Compress_Strong_ETag_Unchanged_When_Not_Compressed(t *testing.T) {
+	t.Parallel()
+	app := fiber.New()
+	app.Use(New())
+	app.Get("/", func(c fiber.Ctx) error {
+		c.Set(fiber.HeaderETag, "\"abc\"")
+		return c.SendString("tiny") // below compression threshold
+	})
+	req := httptest.NewRequest(fiber.MethodGet, "/", nil)
+	req.Header.Set("Accept-Encoding", "gzip")
+	resp, err := app.Test(req, testConfig)
+	require.NoError(t, err)
+	require.Equal(t, "", resp.Header.Get(fiber.HeaderContentEncoding))
+	require.Equal(t, "\"abc\"", resp.Header.Get(fiber.HeaderETag))
+	require.Equal(t, "Accept-Encoding", resp.Header.Get(fiber.HeaderVary))
+}

middleware/compress/compress_test.go (2)

397-434: Add HEAD coverage for early-return branches.

Please add HEAD tests for (a) pre‑encoded responses and (b) no-transform skip to ensure body is empty and Content-Length equals would‑be length.

Example additions:

+func Test_Compress_Head_Metadata_Preencoded(t *testing.T) {
+	t.Parallel()
+	app := fiber.New()
+	app.Use(New())
+	handler := func(c fiber.Ctx) error {
+		c.Set(fiber.HeaderContentEncoding, "gzip")
+		return c.SendString("hello")
+	}
+	app.Get("/", handler)
+	app.Head("/", handler)
+	getReq := httptest.NewRequest(fiber.MethodGet, "/", nil)
+	getResp, _ := app.Test(getReq, testConfig)
+	headReq := httptest.NewRequest(fiber.MethodHead, "/", nil)
+	headResp, _ := app.Test(headReq, testConfig)
+	require.Empty(t, mustReadAll(t, headResp.Body))
+	require.Equal(t, getResp.Header.Get(fiber.HeaderContentLength), headResp.Header.Get(fiber.HeaderContentLength))
+}
+
+func Test_Compress_Head_Metadata_NoTransform(t *testing.T) {
+	t.Parallel()
+	app := fiber.New()
+	app.Use(New())
+	handler := func(c fiber.Ctx) error {
+		c.Set(fiber.HeaderCacheControl, "no-transform")
+		return c.SendString("hello")
+	}
+	app.Get("/", handler)
+	app.Head("/", handler)
+	req := httptest.NewRequest(fiber.MethodHead, "/", nil)
+	req.Header.Set("Accept-Encoding", "gzip")
+	resp, _ := app.Test(req, testConfig)
+	require.Empty(t, mustReadAll(t, resp.Body))
+	require.NotEmpty(t, resp.Header.Get(fiber.HeaderContentLength))
+}

Helper:

func mustReadAll(t *testing.T, r io.Reader) []byte { b, err := io.ReadAll(r); require.NoError(t, err); return b }

---

580-614: Also assert Vary on no-transform skips.

Spec requires emitting Vary: Accept-Encoding even when skipping.

Apply:

@@
-			resp, err := app.Test(req, testConfig)
+			resp, err := app.Test(req, testConfig)
 			require.NoError(t, err, "app.Test(req)")
 			require.Equal(t, "", resp.Header.Get(fiber.HeaderContentEncoding))
+			require.Equal(t, "Accept-Encoding", resp.Header.Get(fiber.HeaderVary))

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Learnt from: sixcolors
PR: gofiber/fiber#3016
File: middleware/session/store.go:164-167
Timestamp: 2024-10-02T23:03:31.727Z
Learning: Unit tests in this project use testify require.

Learnt from: sixcolors
PR: gofiber/fiber#2922
File: middleware/cors/utils.go:63-71
Timestamp: 2024-07-26T21:00:12.902Z
Learning: The project uses the testify/assert package for assertions in unit tests.

middleware/etag/etag.go (1)

17-31: Generate: pooling + defensive copy look correct.

Using bytebufferpool with an immediate defer and returning a fresh copy prevents use-after-put; nice.

middleware/etag/etag.go (2)

33-40: Micro: avoid an extra allocation in GenerateWeak.

Pre-size the result to len("W/")+len(tag) to cut one growth step.

Apply:

 func GenerateWeak(body []byte) []byte {
   tag := Generate(body)
   if tag == nil {
     return nil
   }
-  return append(weakPrefix, tag...)
+  out := make([]byte, 0, len(weakPrefix)+len(tag))
+  out = append(out, weakPrefix...)
+  out = append(out, tag...)
+  return out
 }

---

47-47: Naming: consider a package-level canonical key.

normalizedHeaderETag is effectively a constant. Hoist to package scope to reuse across handlers.

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you open a pull request for review, mark a draft as ready, or comment "@codex review". If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex fix this CI failure" or "@codex address that feedback".

middleware/compress/compress_test.go (1)

396-416: Thanks for adding the “not compressed” strong ETag guard.

middleware/compress/compress_test.go (3)

191-208: Good: validates Vary addition on compress path.

You can DRY response-body handling and ensure bodies are closed by adding a tiny helper and using t.Cleanup across tests.

+// helper near the top of the file
+func readAllAndClose(t *testing.T, rc io.ReadCloser) []byte {
+	t.Helper()
+	b, err := io.ReadAll(rc)
+	require.NoError(t, err)
+	t.Cleanup(func() { _ = rc.Close() })
+	return b
+}

---

417-453: HEAD metadata propagation validated; body omitted as intended.

Consider asserting the 200 status on both GET and HEAD for completeness.

-	getResp, err := app.Test(getReq, testConfig)
+	getResp, err := app.Test(getReq, testConfig)
 	require.NoError(t, err, "app.Test(getReq)")
+	require.Equal(t, fiber.StatusOK, getResp.StatusCode)
@@
-	headResp, err := app.Test(headReq, testConfig)
+	headResp, err := app.Test(headReq, testConfig)
 	require.NoError(t, err, "app.Test(headReq)")
+	require.Equal(t, fiber.StatusOK, headResp.StatusCode)

---

598-634: Add Vary assertion to no‑transform skip for consistency with other skip paths.

Apply this diff inside the subtest assertions:

-			resp, err := app.Test(req, testConfig)
+			resp, err := app.Test(req, testConfig)
 			require.NoError(t, err, "app.Test(req)")
 			require.Equal(t, "", resp.Header.Get(fiber.HeaderContentEncoding))
+			require.Equal(t, "Accept-Encoding", resp.Header.Get(fiber.HeaderVary))

Also consider using the readAllAndClose helper across tests to avoid leaking bodies.

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro