Skip to content

Commit

Permalink
revert force-set of CORS header on cache misses
Browse files Browse the repository at this point in the history
CORs headers are expected to be managed by the backend. However, for
cached requests, we want to ensure that CORs is being set properly.
This commit reverts setting the CORs header on cache misses.
Additionally, it removes the tests that expect a CORs header on cache misses.
  • Loading branch information
pirtleshell committed Feb 21, 2024
1 parent d7f1fa5 commit 8dca990
Show file tree
Hide file tree
Showing 6 changed files with 18 additions and 14 deletions.
4 changes: 2 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,8 @@ up:
.PHONY: down
# stop the service and it's dependencies
down:
rm docker/shared/genesis.json
rm docker/shared/VALIDATOR_NODE_ID
rm docker/shared/genesis.json || echo no shared genesis present
rm docker/shared/VALIDATOR_NODE_ID || echo no shared validator node id present
docker compose down

.PHONY: restart
Expand Down
2 changes: 1 addition & 1 deletion ci.docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ services:
dockerfile: ci.Dockerfile
env_file: .env
environment:
PROXY_HEIGHT_BASED_ROUTING_ENABLED: true
PROXY_HEIGHT_BASED_ROUTING_ENABLED: "true"
# use public testnet as backend origin server to avoid having
# to self-host a beefy Github Action runner
# to build and run a kava node each execution
Expand Down
6 changes: 4 additions & 2 deletions main_batch_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -225,8 +225,10 @@ func TestE2ETest_ValidBatchEvmRequests(t *testing.T) {
// check expected cache status header
require.Equal(t, tc.expectedCacheHeader, resp.Header.Get(cachemdw.CacheHeaderKey))

// verify CORS header
require.Equal(t, resp.Header[accessControlAllowOriginHeaderName], []string{"*"})
// verify CORS header (only on cache hits. cache misses return the backend value, if set)
if tc.expectedCacheHeader == cachemdw.CacheHitHeaderValue {
require.Equal(t, resp.Header[accessControlAllowOriginHeaderName], []string{"*"})
}

// wait for all metrics to be created.
// besides verification, waiting for the metrics ensures future tests don't fail b/c metrics are being processed
Expand Down
2 changes: 1 addition & 1 deletion main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -525,7 +525,7 @@ func TestE2ETestCachingMdwWithBlockNumberParam(t *testing.T) {
expectKeysNum(t, redisClient, tc.keysNum)
expectedKey := "local-chain:evm-request:eth_getBlockByNumber:sha256:d08b426164eacf6646fb1817403ec0af5d37869a0f32a01ebfab3096fa4999be"
containsKey(t, redisClient, expectedKey)
require.Equal(t, cacheMissResp.Header[accessControlAllowOriginHeaderName], []string{"*"})
// don't check CORs because proxy only force-sets header for cache hits.

// eth_getBlockByNumber - cache HIT
cacheHitResp := mkJsonRpcRequest(t, proxyServiceURL, 1, tc.method, tc.params)
Expand Down
2 changes: 1 addition & 1 deletion service/batchmdw/batch_processor.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ func (bp *BatchProcessor) applyHeaders(h http.Header) {
// clear content length, will be set by actual Write to client
// must be cleared in order to prevent premature end of client read
bp.header.Del("Content-Length")
// clear cache hit header, will be set by flush()
// clear cache hit header, will be set by RequestAndServe()
bp.header.Del(cachemdw.CacheHeaderKey)
}

Expand Down
16 changes: 9 additions & 7 deletions service/middleware.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ func createDecodeRequestMiddleware(next http.HandlerFunc, batchProcessingMiddlew

// skip processing if there is no body content
if r.Body == nil {
serviceLogger.Trace().Msg("no data in request")
serviceLogger.Trace().Msg("no data in request body")
next.ServeHTTP(w, r)
return
}
Expand All @@ -119,6 +119,13 @@ func createDecodeRequestMiddleware(next http.HandlerFunc, batchProcessingMiddlew
// Repopulate the request body for the ultimate consumer of this request
r.Body = io.NopCloser(&rawBodyBuffer)

// skip processing if body is empty
if len(rawBody) == 0 {
serviceLogger.Trace().Msg("no data in request body")
next.ServeHTTP(w, r)
return
}

// attempt to decode as single EVM request
decodedRequest, err := decode.DecodeEVMRPCRequest(rawBody)
if err == nil {
Expand Down Expand Up @@ -263,7 +270,7 @@ func createProxyRequestMiddleware(next http.Handler, config config.Config, servi
w.Header().Set(headerName, headerValue)
}
}
// add CORS headers (if not already added)
// cached requests should have CORs set, but if they don't, default to configured value.
accessControlAllowOriginValue := config.GetAccessControlAllowOriginValue(r.Host)
if w.Header().Get("Access-Control-Allow-Origin") == "" && accessControlAllowOriginValue != "" {
w.Header().Set("Access-Control-Allow-Origin", accessControlAllowOriginValue)
Expand All @@ -281,11 +288,6 @@ func createProxyRequestMiddleware(next http.Handler, config config.Config, servi
Msg("cache miss")

w.Header().Add(cachemdw.CacheHeaderKey, cachemdw.CacheMissHeaderValue)
// add CORS headers (if not already added)
accessControlAllowOriginValue := config.GetAccessControlAllowOriginValue(r.Host)
if w.Header().Get("Access-Control-Allow-Origin") == "" && accessControlAllowOriginValue != "" {
w.Header().Set("Access-Control-Allow-Origin", accessControlAllowOriginValue)
}
proxy.ServeHTTP(lrw, r)
}

Expand Down

0 comments on commit 8dca990

Please sign in to comment.