Skip to content

🩹 fix: Cache middleware hasDirective misses directives with trailing space/tab/= #4144

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
mango766 wants to merge 2 commits into
base: main
Choose a base branch
from
+61 −2
Open

🩹 fix: Cache middleware hasDirective misses directives with trailing space/tab/= #4144

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2b0017d
🩹 Fix: cache middleware hasDirective misses directives with trailing …
Mar 16, 2026
ad54cd8
Update middleware/cache/cache_test.go
gaby Mar 19, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions middleware/cache/cache.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -905,7 +905,11 @@ func New(config ...Config) fiber.Handler {
}
}

// hasDirective checks if a cache-control header contains a directive (case-insensitive)
// hasDirective checks if a cache-control header contains a directive (case-insensitive).
// A directive is considered matched when it appears as a standalone token, meaning
// it must be preceded by the start of string, a comma, or whitespace, and followed
// by the end of string, a comma, whitespace, or '=' (for directives with arguments
// like no-cache="field").
func hasDirective(cc, directive string) bool {
ccLen := len(cc)
dirLen := len(directive)
Expand All @@ -919,7 +923,8 @@ func hasDirective(cc, directive string) bool {
continue
}
}
if i+dirLen == ccLen || cc[i+dirLen] == ',' {
end := i + dirLen
if end == ccLen || cc[end] == ',' || cc[end] == ' ' || cc[end] == '=' || cc[end] == '\t' {
return true
}
}
Expand Down
54 changes: 54 additions & 0 deletions middleware/cache/cache_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5091,3 +5091,57 @@ func Test_Cache_ConfigurationAndResponseHandling(t *testing.T) {
require.Equal(t, cacheMiss, rsp2.Header.Get("X-Cache"))
})
}

// go test -run Test_hasDirective
func Test_hasDirective(t *testing.T) {
t.Parallel()

tests := []struct {
name string
cc string
directive string
expected bool
}{
// Basic matching
{name: "exact match", cc: "no-cache", directive: "no-cache", expected: true},
{name: "not found", cc: "public", directive: "no-cache", expected: false},
{name: "empty header", cc: "", directive: "no-cache", expected: false},

// Comma-separated directives
{name: "first in list", cc: "no-cache, no-store", directive: "no-cache", expected: true},
{name: "last in list", cc: "public, no-cache", directive: "no-cache", expected: true},
{name: "middle in list", cc: "public, no-cache, no-store", directive: "no-cache", expected: true},
{name: "no spaces", cc: "public,no-cache,no-store", directive: "no-cache", expected: true},
{name: "tab before directive", cc: "public,\tno-cache", directive: "no-cache", expected: true},
Copy link
Contributor

@coderabbitai coderabbitai bot Mar 19, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify if the implementation supports tab as a preceding character
rg -n "prev != " middleware/cache/cache.go -A2 -B2

Repository: gofiber/fiber

Length of output: 161

🏁 Script executed:

# Get the complete hasDirective function
rg -n "func.*hasDirective" middleware/cache/cache.go -A 50

Repository: gofiber/fiber

Length of output: 1377

🏁 Script executed:

# Get the complete test case context around line 5115
sed -n '5110,5120p' middleware/cache/cache_test.go

Repository: gofiber/fiber

Length of output: 859

🏁 Script executed:

# Check if there are any updates to the preceding character validation logic
rg -n "prev.*==" middleware/cache/cache.go

Repository: gofiber/fiber

Length of output: 39

Test case "tab before directive" will fail against the current implementation.

The hasDirective implementation (line 922) only allows space ' ' or comma ',' as valid preceding characters:

if prev != ' ' && prev != ',' {
    continue
}

For the test case cc: "public,\tno-cache" with directive: "no-cache", the tab '\t' at position 7 (before the directive) will not match this condition, causing the function to return false instead of the expected true.

While the implementation does support tab as a following character (line 927: cc[end] == '\t'), tabs are not accepted as preceding characters. Either:

  1. Add '\t' to the preceding character check (line 922) to accept tabs before directives, or
  2. Update the test expectation to false
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@middleware/cache/cache_test.go` at line 5115, The test fails because
hasDirective only treats space and comma as valid preceding characters; modify
the preceding-character check inside hasDirective to also accept tab ('\t') so
strings like "public,\tno-cache" match; locate the condition that currently
reads "if prev != ' ' && prev != ',' { continue }" and add '\t' to that allowed
set (or equivalently accept unicode.IsSpace(prev)), then run tests to confirm
the "tab before directive" case passes.


// Trailing/leading whitespace
{name: "trailing space", cc: "no-cache ", directive: "no-cache", expected: true},
{name: "trailing tab", cc: "no-cache\t", directive: "no-cache", expected: true},
{name: "space before comma", cc: "no-cache , public", directive: "no-cache", expected: true},

// Directives with arguments (=)
{name: "directive with value", cc: "no-cache=\"Set-Cookie\"", directive: "no-cache", expected: true},
{name: "directive with value in list", cc: "public, no-cache=\"Set-Cookie\", no-store", directive: "no-cache", expected: true},

// Partial matches should not match
{name: "prefix match only", cc: "no-cache-extended", directive: "no-cache", expected: false},
{name: "suffix match only", cc: "xno-cache", directive: "no-cache", expected: false},

// Case insensitivity
{name: "uppercase", cc: "NO-CACHE", directive: "no-cache", expected: true},
{name: "mixed case", cc: "No-Cache", directive: "no-cache", expected: true},

// Private directive
{name: "private standalone", cc: "private", directive: "private", expected: true},
{name: "private with trailing space", cc: "private ", directive: "private", expected: true},
{name: "private in list with spaces", cc: "max-age=3600, private ", directive: "private", expected: true},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
result := hasDirective(tt.cc, tt.directive)
require.Equal(t, tt.expected, result)
})
}
}
Loading