Skip to content

fix(cache): recognize space, tab, and = as directive terminators in hasDirective #4211

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
Yanhu007 wants to merge 2 commits into
base: main
Choose a base branch
from
+53 −3
Open

fix(cache): recognize space, tab, and = as directive terminators in hasDirective #4211

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1eefc14
fix(cache): recognize space, tab, and = as directive terminators in h…
Yanhu007 Apr 12, 2026
05391d5
Update middleware/cache/cache.go
gaby Apr 12, 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
12 changes: 9 additions & 3 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,9 @@ 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 followed by end-of-string, ',', ' ', '\t', or '='
// per RFC 9111 §5.2.
func hasDirective(cc, directive string) bool {
ccLen := len(cc)
dirLen := len(directive)
Expand All @@ -915,11 +917,15 @@ func hasDirective(cc, directive string) bool {
}
if i > 0 {
prev := cc[i-1]
if prev != ' ' && prev != ',' {
if prev != ' ' && prev != ',' && prev != '\t' {
continue
}
}
Comment on lines 918 to 923
Copy link

Copilot AI Apr 12, 2026

Choose a reason for hiding this comment

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

hasDirective now treats '\t' as a valid separator/terminator, but parseCacheControlDirectives (used by parseRequestCacheControl/parseResponseCacheControl) still only skips/trims spaces. This means Cache-Control directives preceded/followed by tabs (valid OWS) can still be missed in the main parsing path, potentially reintroducing incorrect caching decisions. Consider extending parseCacheControlDirectives to treat '\t' like ' ' when skipping leading separators and trimming key/value.

Copilot uses AI. Check for mistakes.
if i+dirLen == ccLen || cc[i+dirLen] == ',' {
if i+dirLen == ccLen {
return true
}
next := cc[i+dirLen]
if next == ',' || next == ' ' || next == '\t' || next == '=' {
return true
}
}
Expand Down
44 changes: 44 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,47 @@
require.Equal(t, cacheMiss, rsp2.Header.Get("X-Cache"))
})
}

func Test_hasDirective(t *testing.T) {
t.Parallel()

tests := []struct {
name string
cc string
directive string
want bool
}{
// Basic matches
Comment on lines +5098 to +5104
Copy link

Copilot AI Apr 12, 2026

Choose a reason for hiding this comment

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

PR description mentions 16 hasDirective test cases, but this table currently defines 15. Either add the missing edge case (if intended) or update the PR description/test output snippet to match the actual count.

Copilot uses AI. Check for mistakes.
{"exact match", "no-cache", "no-cache", true},
{"comma separated", "public, no-cache, max-age=0", "no-cache", true},
{"at start", "no-cache, max-age=0", "no-cache", true},
{"at end", "public, no-cache", "no-cache", true},
{"not present", "public, max-age=0", "no-cache", false},
{"partial match", "no-cach", "no-cache", false},

Check warning on line 5110 in middleware/cache/cache_test.go

View workflow job for this annotation

GitHub Actions / cspell 

Misspelled word (cach) Suggestions: (catch*, cache*)
{"substring of longer token", "no-cache-extended", "no-cache", false},

// Trailing whitespace (#4143)
{"trailing space", "no-cache ", "no-cache", true},
{"trailing tab", "no-cache\t", "no-cache", true},
{"private trailing space", "private ", "private", true},

// Directive with value (#4143)
{"directive with equals", `no-cache="Set-Cookie"`, "no-cache", true},
{"max-age with value", "max-age=3600", "max-age", true},
{"s-maxage with value in list", "public, s-maxage=600, max-age=3600", "s-maxage", true},

// Tab as separator before directive
{"tab before directive", "public,\tno-cache", "no-cache", true},

// Case insensitive
{"case insensitive", "No-Cache", "no-cache", true},
}

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