diff --git a/cache.go b/cache.go index ce6c335..4840c1e 100644 --- a/cache.go +++ b/cache.go @@ -7,7 +7,6 @@ import ( "net/http" "net/url" "sort" - "strings" "time" "github.com/chenyahui/gin-cache/persist" @@ -191,27 +190,20 @@ func getRequestUriIgnoreQueryOrder(requestURI string) (string, error) { return "", err } - values := parsedUrl.Query() + values, err := url.ParseQuery(parsedUrl.RawQuery) + if err != nil { + return "", err + } if len(values) == 0 { return requestURI, nil } - queryKeys := make([]string, 0, len(values)) - for queryKey := range values { - queryKeys = append(queryKeys, queryKey) - } - sort.Strings(queryKeys) - - queryVals := make([]string, 0, len(values)) - for _, queryKey := range queryKeys { - sort.Strings(values[queryKey]) - for _, val := range values[queryKey] { - queryVals = append(queryVals, queryKey+"="+val) - } + for _, queryValues := range values { + sort.Strings(queryValues) } - return parsedUrl.Path + "?" + strings.Join(queryVals, "&"), nil + return parsedUrl.EscapedPath() + "?" + values.Encode(), nil } // CacheByRequestPath a shortcut function for caching response by url path, means will discard the query params diff --git a/cache_test.go b/cache_test.go index 3f97d41..2ac6607 100644 --- a/cache_test.go +++ b/cache_test.go @@ -5,6 +5,7 @@ import ( "math/rand" "net/http" "net/http/httptest" + "net/url" "sync" "sync/atomic" "testing" @@ -199,6 +200,112 @@ func TestGetRequestUriIgnoreQueryOrder(t *testing.T) { assert.Equal(t, "/test?d=4&e=5", val) } +func TestGetRequestUriIgnoreQueryOrderEscaping(t *testing.T) { + for _, requestURI := range []string{ + "/cache?x=a%26y%3Db", + "/cache?a%3Db=c", + "/cache?x=a%2Bb", + "/cache?x=a+b", + "/cache?x=%23value", + "/cache?%26key=%3Dvalue", + "/cache?x=%25value", + "/cache?x=%E9%9B%AA", + "/cache%2Fitem?x=1", + "/cache%3Fitem?x=1", + "/cache%23item?x=1", + } { + t.Run(requestURI, func(t *testing.T) { + key, err := getRequestUriIgnoreQueryOrder(requestURI) + require.NoError(t, err) + original, err := url.ParseRequestURI(requestURI) + require.NoError(t, err) + normalized, err := url.ParseRequestURI(key) + require.NoError(t, err) + assert.Equal(t, original.EscapedPath(), normalized.EscapedPath()) + assert.Equal(t, original.Query(), normalized.Query()) + }) + } +} + +func TestIgnoreQueryOrderDistinctResponses(t *testing.T) { + middlewares := []struct { + name string + new func() gin.HandlerFunc + }{ + {"option", func() gin.HandlerFunc { + return CacheByRequestURI(persist.NewMemoryStore(time.Minute), time.Minute, IgnoreQueryOrder()) + }}, + {"strategy", func() gin.HandlerFunc { + return Cache(persist.NewMemoryStore(time.Minute), time.Minute, + WithCacheStrategyByRequest(CacheStrategyRequestURIIgnoreQueryOrder)) + }}, + } + pairs := []struct { + name string + first string + second string + }{ + {"value separator", "/cache?x=a%26y%3Db", "/cache?x=a&y=b"}, + {"key separator", "/cache?a%3Db=c", "/cache?a=b%3Dc"}, + {"literal plus", "/cache?x=a%2Bb", "/cache?x=a+b"}, + {"path separator", "/cache%2Fitem?x=1", "/cache/item?x=1"}, + {"path query delimiter", "/cache%3Fitem?x=1", "/cache?item%3Fx=1"}, + {"invalid field", "/cache?x=1&y=%ZZ", "/cache?x=1"}, + {"distinct invalid fields", "/cache?x=1&y=%", "/cache?x=1&y=%ZZ"}, + } + for _, middleware := range middlewares { + t.Run(middleware.name, func(t *testing.T) { + for _, pair := range pairs { + t.Run(pair.name, func(t *testing.T) { + engine := gin.New() + engine.Use(middleware.new()) + calls := 0 + engine.GET("/*path", func(c *gin.Context) { + calls++ + c.String(http.StatusOK, c.Request.RequestURI) + }) + for _, requestURI := range []string{pair.first, pair.second, pair.first, pair.second} { + writer := httptest.NewRecorder() + engine.ServeHTTP(writer, httptest.NewRequest(http.MethodGet, requestURI, nil)) + assert.Equal(t, http.StatusOK, writer.Code) + assert.Equal(t, requestURI, writer.Body.String()) + } + assert.Equal(t, 2, calls) + }) + } + }) + } +} + +func TestGetRequestUriIgnoreQueryOrderMalformedQuery(t *testing.T) { + for _, requestURI := range []string{"/cache?x=1&y=%ZZ", "/cache?x=1&y=%"} { + t.Run(requestURI, func(t *testing.T) { + _, err := getRequestUriIgnoreQueryOrder(requestURI) + require.Error(t, err) + }) + } +} + +func TestIgnoreQueryOrderEquivalentResponses(t *testing.T) { + engine := gin.New() + engine.Use(CacheByRequestURI(persist.NewMemoryStore(time.Minute), time.Minute, IgnoreQueryOrder())) + calls := 0 + engine.GET("/cache", func(c *gin.Context) { + calls++ + c.String(http.StatusOK, c.Request.RequestURI) + }) + for _, requestURI := range []string{ + "/cache?x=b%26c&x=a%2Bb&a=%E9%9B%AA", + "/cache?a=%E9%9B%AA&x=a%2Bb&x=b%26c", + } { + writer := httptest.NewRecorder() + engine.ServeHTTP(writer, httptest.NewRequest(http.MethodGet, requestURI, nil)) + assert.Equal(t, http.StatusOK, writer.Code) + assert.Equal(t, "/cache?x=b%26c&x=a%2Bb&a=%E9%9B%AA", writer.Body.String()) + } + assert.Equal(t, 1, calls) +} + func TestCacheByRequestURIIgnoreOrder(t *testing.T) { memoryStore := persist.NewMemoryStore(1 * time.Minute) cacheURIMiddleware := CacheByRequestURI(memoryStore, 3*time.Second, IgnoreQueryOrder())