📝 [Proposal]: Cache Middleware Mechanism & Other Middleware Mechanism that uses storage

H0llyW00dzZ commented 3 months ago

Feature Proposal Description

It would be better if the key generator could use UUID instead of directly using the value from c.Patch() as the key in the current implementation (e.g, in cache). When implementing a custom key generator mechanism bound to a UUID with a group key, for example, frontend:uuid, it keeps getting cache misses because it is trying to get Key: frontend:uuid_GET & Key: frontend:uuid_GET_Body.

Storage Used: Redis/Valkey

Alignment with Express API

This proposal does not align with Express.js.

HTTP RFC Standards Compliance

This proposal complies with HTTP caching standards.

API Stability

It would be great if UUIDs are bound instead of directly using the value from c.Patch(). This would ensure better stability and reduce the likelihood of changes or deprecations in the future.

Feature Examples

The feature should allow for a custom key generator instead of directly using the value from c.Patch(). In Redis/Valkey, it is possible to implement a group key, for example: `frontend:uuid`, where `frontend` is the group and `uuid` is a randomly generated key to get the value.

Checklist:

[X] I agree to follow Fiber's Code of Conduct.
[X] I have searched for existing issues that describe my proposal before opening this one.
[X] I understand that a proposal that does not meet these guidelines may be closed without explanation.

welcome[bot] commented 3 months ago

Thanks for opening your first issue here! 🎉 Be sure to follow the issue template! If you need help or want to chat with us, join us on Discord https://gofiber.io/discord

gaby commented 3 months ago

@H0llyW00dzZ The middleware uses c.Path(). Can you provide an example of what your current KeyGenerator is doing and how your requests to the server look like?

H0llyW00dzZ commented 3 months ago

@H0llyW00dzZ The middleware uses c.Path(). Can you provide an example of what your current KeyGenerator is doing and how your requests to the server look like?

@gaby I've already implemented it; however, it only works with hashes.

For example:

// GenerateCacheKey generates a cache key based on the request method, URL path, and query parameters.
//
// It takes the following parameter:
//   - c: The Fiber context.
//
// It returns the generated cache key as a string.
//
// This function generates a cache key by concatenating the request method, URL path, and sorted query parameters.
// It then computes the SHA-256 hash of the concatenated string and returns the hexadecimal representation of the hash.
//
// Example usage:
//
//  cacheKey := k.GenerateCacheKey(c)
//
// Note: This is now suitable and secure to use with the Fiber cache middleware because it computes the SHA-256 hash of the key instead of using c.Patch() (Default Fiber).
// For example, "frontend:44658f661a1a27cf94e51bf48947525e5dfcfb6f95050b52800300f2554b7f99_GET_body",
// where 44658f661a1a27cf94e51bf48947525e5dfcfb6f95050b52800300f2554b7f99_GET_body is the actual key to get the value.
// Previously, it was not secure because the key directly used c.Path(), which could leak sensitive information to the public, for example, in Redis/Valkey logs, commander panels, cloud.
func (k *KeyIdentifier) GenerateCacheKey(c *fiber.Ctx) string {
    // Get the request method
    method := c.Method()

    // Get the URL path
    path := c.Path()

    // Get the sorted query parameters
    queryParams := getSortedQueryParams(c.Request().URI().QueryArgs())

    // Concatenate the method, path, and query parameters
    key := fmt.Sprintf("%s:%s?%s", method, path, queryParams)

    // Compute the SHA-256 hash of the key
    digest := sha256.Sum256([]byte(key))

    // Convert the hash to a hexadecimal string
    cacheKey := hex.EncodeToString(digest[:])

    // No need to copy; this is already an immutable, built-in, secure cryptographic hash.
    return k.config.Prefix + cacheKey
}

// getSortedQueryParams returns the sorted query parameters as a string.
//
// It takes the following parameter:
//   - queryParams: The query parameters as a *fasthttp.Args.
//
// It returns the sorted query parameters as a string.
//
// This function sorts the query parameter keys, concatenates the key-value pairs, and returns the resulting string.
// The purpose of sorting the query parameters is to ensure that the order of the parameters does not affect the generated cache key.
func getSortedQueryParams(queryParams *fasthttp.Args) string {
    var params []string

    // Iterate over the query parameters
    queryParams.VisitAll(func(key, value []byte) {
        params = append(params, fmt.Sprintf("%s=%s", string(key), string(value)))
    })

    // Sort the key-value pairs
    slices.Sort(params)

    // Join the sorted key-value pairs with "&"
    return strings.Join(params, "&")
}

Now it has become organized and secure. Previously, it was not secure because it directly used c.Path().

H0llyW00dzZ commented 3 months ago

So, the proposal I think would be better (and probably possible) is to use UUID as the default and remove the "_GET_body" suffix for the Cache KeyGenerator.

For example, the UUID used in the session middleware works well.

H0llyW00dzZ commented 3 months ago

Additionally, if it's not possible to use UUID as the default instead of c.Path(), I propose removing the _GET_body _GET suffix for the Cache KeyGenerator when custom key generators are provided.

gaby commented 3 months ago

@H0llyW00dzZ We can't remove the suffix, because you can register the same route with multiple methods. The whole purpose of having KeyGenerator as a config, is to allow the developer to implement that as they see fit on their application. The default of using c.Path() is just a basic use-case.

While your hashed approach is probably very good, is also means EACH request to the server will have to be hashed before even checking the cache.

H0llyW00dzZ commented 3 months ago

@H0llyW00dzZ We can't remove the suffix, because you can register the same route with multiple methods. The whole purpose of having KeyGenerator as a config, is to allow the developer to implement that as they see fit on their application. The default of using c.Path() is just a basic use-case.

Wait, so it's not possible to remove the suffix?

While your hashed approach is probably very good, is also means EACH request to the server will have to be hashed before even checking the cache.

Yeah, it's to make it always unique as well, so the result of the hash (digest) will never be the same (e.g, when cache TTL expired in redis/valkey).

gofiber / fiber