Skip to content

Commit

Permalink
πŸ› fix: Align cache middleware with RFC7231 (#3283)
Browse files Browse the repository at this point in the history
* 🩹 Fix(v3;middleware/cache): don't cache if status code is not cacheable

* allow 418 TeaPot

* fix test

* fix lint error

* check cacheability with map

* documentation

* fix: markdown lint

---------

Co-authored-by: Juan Calderon-Perez <[email protected]>
  • Loading branch information
miyamo2 and gaby authored Jan 20, 2025
1 parent aa245ae commit 8970f51
Show file tree
Hide file tree
Showing 4 changed files with 129 additions and 1 deletion.
25 changes: 25 additions & 0 deletions docs/middleware/cache.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,31 @@ Request Directives<br />
`Cache-Control: no-cache` will return the up-to-date response but still caches it. You will always get a `miss` cache status.<br />
`Cache-Control: no-store` will refrain from caching. You will always get the up-to-date response.

Cacheable Status Codes<br />

This middleware caches responses with the following status codes according to RFC7231:

- `200: OK`
- `203: Non-Authoritative Information`
- `204: No Content`
- `206: Partial Content`
- `300: Multiple Choices`
- `301: Moved Permanently`
- `404: Not Found`
- `405: Method Not Allowed`
- `410: Gone`
- `414: URI Too Long`
- `501: Not Implemented`

Additionally, `418: I'm a teapot` is not originally cacheable but is cached by this middleware.
If the status code is other than these, you will always get an `unreachable` cache status.

For more information about cacheable status codes or RFC7231, please refer to the following resources:

- [Cacheable - MDN Web Docs](https://developer.mozilla.org/en-US/docs/Glossary/Cacheable)

- [RFC7231 - Hypertext Transfer Protocol (HTTP/1.1): Semantics and Content](https://datatracker.ietf.org/doc/html/rfc7231)

## Signatures

```go
Expand Down
3 changes: 2 additions & 1 deletion docs/whats_new.md
Original file line number Diff line number Diff line change
Expand Up @@ -764,7 +764,8 @@ The adaptor middleware has been significantly optimized for performance and effi

### Cache

We are excited to introduce a new option in our caching middleware: Cache Invalidator. This feature provides greater control over cache management, allowing you to define a custom conditions for invalidating cache entries.
We are excited to introduce a new option in our caching middleware: Cache Invalidator. This feature provides greater control over cache management, allowing you to define a custom conditions for invalidating cache entries.
Additionally, the caching middleware has been optimized to avoid caching non-cacheable status codes, as defined by the [HTTP standards](https://datatracker.ietf.org/doc/html/rfc7231#section-6.1). This improvement enhances cache accuracy and reduces unnecessary cache storage usage.

### CORS

Expand Down
21 changes: 21 additions & 0 deletions middleware/cache/cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,21 @@ var ignoreHeaders = map[string]any{
"Content-Encoding": nil, // already stored explicitly by the cache manager
}

var cacheableStatusCodes = map[int]bool{
fiber.StatusOK: true,
fiber.StatusNonAuthoritativeInformation: true,
fiber.StatusNoContent: true,
fiber.StatusPartialContent: true,
fiber.StatusMultipleChoices: true,
fiber.StatusMovedPermanently: true,
fiber.StatusNotFound: true,
fiber.StatusMethodNotAllowed: true,
fiber.StatusGone: true,
fiber.StatusRequestURITooLong: true,
fiber.StatusTeapot: true,
fiber.StatusNotImplemented: true,
}

// New creates a new middleware handler
func New(config ...Config) fiber.Handler {
// Set default config
Expand Down Expand Up @@ -170,6 +185,12 @@ func New(config ...Config) fiber.Handler {
return err
}

// Don't cache response if status code is not cacheable
if !cacheableStatusCodes[c.Response().StatusCode()] {
c.Set(cfg.CacheHeader, cacheUnreachable)
return nil
}

// lock entry back and unlock on finish
mux.Lock()
defer mux.Unlock()
Expand Down
81 changes: 81 additions & 0 deletions middleware/cache/cache_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -918,6 +918,87 @@ func Test_Cache_MaxBytesSizes(t *testing.T) {
}
}

func Test_Cache_UncacheableStatusCodes(t *testing.T) {
t.Parallel()
app := fiber.New()
app.Use(New())

app.Get("/:statusCode", func(c fiber.Ctx) error {
statusCode, err := strconv.Atoi(c.Params("statusCode"))
require.NoError(t, err)
return c.Status(statusCode).SendString("foo")
})

uncacheableStatusCodes := []int{
// Informational responses
fiber.StatusContinue,
fiber.StatusSwitchingProtocols,
fiber.StatusProcessing,
fiber.StatusEarlyHints,

// Successful responses
fiber.StatusCreated,
fiber.StatusAccepted,
fiber.StatusResetContent,
fiber.StatusMultiStatus,
fiber.StatusAlreadyReported,
fiber.StatusIMUsed,

// Redirection responses
fiber.StatusFound,
fiber.StatusSeeOther,
fiber.StatusNotModified,
fiber.StatusUseProxy,
fiber.StatusSwitchProxy,
fiber.StatusTemporaryRedirect,
fiber.StatusPermanentRedirect,

// Client error responses
fiber.StatusBadRequest,
fiber.StatusUnauthorized,
fiber.StatusPaymentRequired,
fiber.StatusForbidden,
fiber.StatusNotAcceptable,
fiber.StatusProxyAuthRequired,
fiber.StatusRequestTimeout,
fiber.StatusConflict,
fiber.StatusLengthRequired,
fiber.StatusPreconditionFailed,
fiber.StatusRequestEntityTooLarge,
fiber.StatusUnsupportedMediaType,
fiber.StatusRequestedRangeNotSatisfiable,
fiber.StatusExpectationFailed,
fiber.StatusMisdirectedRequest,
fiber.StatusUnprocessableEntity,
fiber.StatusLocked,
fiber.StatusFailedDependency,
fiber.StatusTooEarly,
fiber.StatusUpgradeRequired,
fiber.StatusPreconditionRequired,
fiber.StatusTooManyRequests,
fiber.StatusRequestHeaderFieldsTooLarge,
fiber.StatusUnavailableForLegalReasons,

// Server error responses
fiber.StatusInternalServerError,
fiber.StatusBadGateway,
fiber.StatusServiceUnavailable,
fiber.StatusGatewayTimeout,
fiber.StatusHTTPVersionNotSupported,
fiber.StatusVariantAlsoNegotiates,
fiber.StatusInsufficientStorage,
fiber.StatusLoopDetected,
fiber.StatusNotExtended,
fiber.StatusNetworkAuthenticationRequired,
}
for _, v := range uncacheableStatusCodes {
resp, err := app.Test(httptest.NewRequest(fiber.MethodGet, fmt.Sprintf("/%d", v), nil))
require.NoError(t, err)
require.Equal(t, cacheUnreachable, resp.Header.Get("X-Cache"))
require.Equal(t, v, resp.StatusCode)
}
}

// go test -v -run=^$ -bench=Benchmark_Cache -benchmem -count=4
func Benchmark_Cache(b *testing.B) {
app := fiber.New()
Expand Down

1 comment on commit 8970f51

@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: 8970f51 Previous: e04f815 Ratio
Benchmark_Ctx_Send 6.546 ns/op 0 B/op 0 allocs/op 4.343 ns/op 0 B/op 0 allocs/op 1.51
Benchmark_Ctx_Send - ns/op 6.546 ns/op 4.343 ns/op 1.51
Benchmark_Utils_GetOffer/1_parameter 224.8 ns/op 0 B/op 0 allocs/op 136.5 ns/op 0 B/op 0 allocs/op 1.65
Benchmark_Utils_GetOffer/1_parameter - ns/op 224.8 ns/op 136.5 ns/op 1.65
`Benchmark_RoutePatternMatch//api/:param/fixedEnd_ not_match _/api/abc/def/fixedEnd - allocs/op` 14 allocs/op
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 515 B/op 341 B/op 1.51
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.