From 54544cad99c70f471eb44aa39300a44fc97d8ef7 Mon Sep 17 00:00:00 2001 From: Qiaozp Date: Tue, 9 May 2023 10:58:31 +0800 Subject: [PATCH 1/3] Fix: prevent caching js when http code isn't 200 Signed-off-by: Qiaozp --- pkg/server/server.go | 2 +- pkg/server/utils/filters/gzip.go | 1 + pkg/server/utils/filters/js-cache.go | 6 +++++- pkg/server/utils/filters/js-cache_test.go | 23 ++++++++++++++++++++--- 4 files changed, 27 insertions(+), 5 deletions(-) diff --git a/pkg/server/server.go b/pkg/server/server.go index 6badd913c..061b453ab 100644 --- a/pkg/server/server.go +++ b/pkg/server/server.go @@ -350,7 +350,7 @@ func enrichSwaggerObject(swo *spec.Swagger) { } func (s *restServer) ServeHTTP(res http.ResponseWriter, req *http.Request) { - staticFilters := []utils.FilterFunction{} + var staticFilters []utils.FilterFunction if features.APIServerFeatureGate.Enabled(features.APIServerEnableCacheJSFile) { staticFilters = append(staticFilters, filters.JSCache) } diff --git a/pkg/server/utils/filters/gzip.go b/pkg/server/utils/filters/gzip.go index 33e95fae1..ab71a750f 100644 --- a/pkg/server/utils/filters/gzip.go +++ b/pkg/server/utils/filters/gzip.go @@ -32,6 +32,7 @@ func Gzip(req *http.Request, res http.ResponseWriter, chain *utils.FilterChain) if doCompress { w, err := restful.NewCompressingResponseWriter(res, encoding) if err != nil { + klog.Errorf("failed to create the compressing writer, err: %s", err.Error()) res.WriteHeader(http.StatusInternalServerError) return } diff --git a/pkg/server/utils/filters/js-cache.go b/pkg/server/utils/filters/js-cache.go index 731d742ff..75b365827 100644 --- a/pkg/server/utils/filters/js-cache.go +++ b/pkg/server/utils/filters/js-cache.go @@ -51,7 +51,11 @@ func JSCache(req *http.Request, res http.ResponseWriter, chain *utils.FilterChai res.Header().Set(HeaderHitCache, "false") cacheWriter := &CacheWriter{writer: res, cacheData: &cacheData{}} chain.ProcessFilter(req, cacheWriter) - jsFileCache.Add(req.URL.String(), cacheWriter.cacheData) + if cacheWriter.cacheData.code == http.StatusOK { + jsFileCache.Add(req.URL.String(), cacheWriter.cacheData) + } else { + klog.Warningf("Skip cache the js file, code: %d, url: %s", cacheWriter.cacheData.code, req.URL.String()) + } return } chain.ProcessFilter(req, res) diff --git a/pkg/server/utils/filters/js-cache_test.go b/pkg/server/utils/filters/js-cache_test.go index a32e2d67a..23cafda78 100644 --- a/pkg/server/utils/filters/js-cache_test.go +++ b/pkg/server/utils/filters/js-cache_test.go @@ -37,7 +37,7 @@ func TestJSCache(t *testing.T) { chain.ProcessFilter(&http.Request{Method: "GET", URL: u}, res1) assert.Equal(t, res1.Code, 200) assert.Equal(t, res1.Body.String(), jsContent) - assert.Equal(t, res1.HeaderMap.Get(HeaderHitCache), "false") + assert.Equal(t, res1.Header().Get(HeaderHitCache), "false") assert.Equal(t, jsFileCache.Len(), 1) data, ok := jsFileCache.Get(u.String()) assert.Equal(t, ok, true) @@ -49,7 +49,7 @@ func TestJSCache(t *testing.T) { assert.Equal(t, res2.Code, 200) assert.Equal(t, res2.Body.String(), jsContent) - assert.Equal(t, res2.HeaderMap.Get(HeaderHitCache), "true") + assert.Equal(t, res2.Header().Get(HeaderHitCache), "true") u2, err := url.Parse("/test.js?v=2") assert.Equal(t, err, nil) @@ -58,7 +58,18 @@ func TestJSCache(t *testing.T) { chain.ProcessFilter(&http.Request{Method: "GET", URL: u2}, res3) assert.Equal(t, res3.Code, 200) assert.Equal(t, res3.Body.String(), jsContent) - assert.Equal(t, res3.HeaderMap.Get(HeaderHitCache), "false") + assert.Equal(t, res3.Header().Get(HeaderHitCache), "false") + assert.Equal(t, jsFileCache.Len(), 2) + + u3, err := url.Parse("/test.js?v=3") + assert.NoError(t, err) + res4 := httptest.NewRecorder() + chain = utils.NewFilterChain(loadJSNotOK, JSCache) + chain.ProcessFilter(&http.Request{Method: "GET", URL: u3}, res4) + assert.Equal(t, res4.Code, 304) + assert.Equal(t, res4.Body.String(), "") + assert.Equal(t, res4.Header().Get(HeaderHitCache), "false") + assert.Equal(t, jsFileCache.Len(), 2) } var jsContent = "console.log(\"hello\")" @@ -69,3 +80,9 @@ func loadJS(req *http.Request, res http.ResponseWriter) { res.Write([]byte(jsContent)) res.Header().Add(restful.HEADER_ContentType, "application/javascript") } + +func loadJSNotOK(req *http.Request, res http.ResponseWriter) { + fmt.Printf("return not 200,path:%s \n", req.URL.String()) + res.WriteHeader(304) + res.Header().Add(restful.HEADER_ContentType, "application/javascript") +} From b41ba47d96705c6eada3c42499c17b2416c26a90 Mon Sep 17 00:00:00 2001 From: Qiaozp Date: Tue, 9 May 2023 11:20:17 +0800 Subject: [PATCH 2/3] Strict cache output Signed-off-by: Qiaozp --- pkg/server/utils/filters/js-cache.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/pkg/server/utils/filters/js-cache.go b/pkg/server/utils/filters/js-cache.go index 75b365827..1b15bb8e6 100644 --- a/pkg/server/utils/filters/js-cache.go +++ b/pkg/server/utils/filters/js-cache.go @@ -41,8 +41,13 @@ func JSCache(req *http.Request, res http.ResponseWriter, chain *utils.FilterChai if matchCacheCondition(req) { if value, ok := jsFileCache.Get(req.URL.String()); ok { if cacheData, ok := value.(*cacheData); ok { - cacheData.Write(res) - return + if cacheData.data.Len() == 0 { + klog.Warningf("Cache data is empty, url: %s", req.URL.String()) + jsFileCache.Remove(req.URL.String()) + } else { + cacheData.Write(res) + return + } } } } From 3e3845147a98230b4b433e1d6dc8a28799ce160e Mon Sep 17 00:00:00 2001 From: Qiaozp Date: Tue, 9 May 2023 11:47:56 +0800 Subject: [PATCH 3/3] remove danger log Signed-off-by: Qiaozp --- pkg/server/utils/filters/js-cache.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/server/utils/filters/js-cache.go b/pkg/server/utils/filters/js-cache.go index 1b15bb8e6..b00e5737b 100644 --- a/pkg/server/utils/filters/js-cache.go +++ b/pkg/server/utils/filters/js-cache.go @@ -42,7 +42,7 @@ func JSCache(req *http.Request, res http.ResponseWriter, chain *utils.FilterChai if value, ok := jsFileCache.Get(req.URL.String()); ok { if cacheData, ok := value.(*cacheData); ok { if cacheData.data.Len() == 0 { - klog.Warningf("Cache data is empty, url: %s", req.URL.String()) + klog.Warningf("Cache data is empty") jsFileCache.Remove(req.URL.String()) } else { cacheData.Write(res) @@ -59,7 +59,7 @@ func JSCache(req *http.Request, res http.ResponseWriter, chain *utils.FilterChai if cacheWriter.cacheData.code == http.StatusOK { jsFileCache.Add(req.URL.String(), cacheWriter.cacheData) } else { - klog.Warningf("Skip cache the js file, code: %d, url: %s", cacheWriter.cacheData.code, req.URL.String()) + klog.Warningf("Skip cache the js file, code: %d", cacheWriter.cacheData.code) } return }