mirror of
https://github.com/tendermint/tendermint.git
synced 2026-01-07 13:55:17 +00:00
rpc: remove cache control settings from the HTTP server (#7568)
We should not set cache-control headers on RPC responses. HTTP caching interacts poorly with resources that are expected to change frequently, or whose rate of change is unpredictable. More subtly, all calls to the POST endpoint use the same URL, which means a cacheable response from one call may actually "hide" an uncacheable response from a subsequent one. This is less of a problem for the GET endpoints, but that means the behaviour of RPCs varies depending on which HTTP method your client happens to use. Websocket requests were already marked statically uncacheable, adding yet a third combination. To address this: - Stop setting cache-control headers. - Update the tests that were checking for those headers. - Remove the flags to request cache-control. Apart from affecting the HTTP response headers, this change does not modify the behaviour of any of the RPC methods.
This commit is contained in:
@@ -53,11 +53,11 @@ type ResultEchoDataBytes struct {
|
||||
|
||||
// Define some routes
|
||||
var Routes = map[string]*server.RPCFunc{
|
||||
"echo": server.NewRPCFunc(EchoResult, "arg", false),
|
||||
"echo": server.NewRPCFunc(EchoResult, "arg"),
|
||||
"echo_ws": server.NewWSRPCFunc(EchoWSResult, "arg"),
|
||||
"echo_bytes": server.NewRPCFunc(EchoBytesResult, "arg", false),
|
||||
"echo_data_bytes": server.NewRPCFunc(EchoDataBytesResult, "arg", false),
|
||||
"echo_int": server.NewRPCFunc(EchoIntResult, "arg", false),
|
||||
"echo_bytes": server.NewRPCFunc(EchoBytesResult, "arg"),
|
||||
"echo_data_bytes": server.NewRPCFunc(EchoDataBytesResult, "arg"),
|
||||
"echo_int": server.NewRPCFunc(EchoIntResult, "arg"),
|
||||
}
|
||||
|
||||
func EchoResult(ctx context.Context, v string) (*ResultEcho, error) {
|
||||
|
||||
@@ -53,12 +53,7 @@ func makeJSONRPCHandler(funcMap map[string]*RPCFunc, logger log.Logger) http.Han
|
||||
return
|
||||
}
|
||||
|
||||
// Set the default response cache to true unless
|
||||
// 1. Any RPC request rrror.
|
||||
// 2. Any RPC request doesn't allow to be cached.
|
||||
// 3. Any RPC request has the height argument and the value is 0 (the default).
|
||||
var responses []rpctypes.RPCResponse
|
||||
mayCache := true
|
||||
for _, req := range requests {
|
||||
// Ignore notifications, which this service does not support.
|
||||
if req.ID == nil {
|
||||
@@ -69,25 +64,16 @@ func makeJSONRPCHandler(funcMap map[string]*RPCFunc, logger log.Logger) http.Han
|
||||
rpcFunc, ok := funcMap[req.Method]
|
||||
if !ok || rpcFunc.ws {
|
||||
responses = append(responses, rpctypes.RPCMethodNotFoundError(req.ID))
|
||||
mayCache = false
|
||||
continue
|
||||
}
|
||||
if !rpcFunc.cache {
|
||||
mayCache = false
|
||||
}
|
||||
|
||||
args, err := parseParams(rpcFunc, hreq, req)
|
||||
if err != nil {
|
||||
responses = append(responses, rpctypes.RPCInvalidParamsError(
|
||||
req.ID, fmt.Errorf("converting JSON parameters: %w", err)))
|
||||
mayCache = false
|
||||
continue
|
||||
}
|
||||
|
||||
if hasDefaultHeight(req, args) {
|
||||
mayCache = false
|
||||
}
|
||||
|
||||
returns := rpcFunc.f.Call(args)
|
||||
logger.Debug("HTTPJSONRPC", "method", req.Method, "args", args, "returns", returns)
|
||||
result, err := unreflectResult(returns)
|
||||
@@ -99,24 +85,21 @@ func makeJSONRPCHandler(funcMap map[string]*RPCFunc, logger log.Logger) http.Han
|
||||
// if this already of type RPC error then forward that error
|
||||
case *rpctypes.RPCError:
|
||||
responses = append(responses, rpctypes.NewRPCErrorResponse(req.ID, e.Code, e.Message, e.Data))
|
||||
mayCache = false
|
||||
default: // we need to unwrap the error and parse it accordingly
|
||||
switch errors.Unwrap(err) {
|
||||
// check if the error was due to an invald request
|
||||
case coretypes.ErrZeroOrNegativeHeight, coretypes.ErrZeroOrNegativePerPage,
|
||||
coretypes.ErrPageOutOfRange, coretypes.ErrInvalidRequest:
|
||||
responses = append(responses, rpctypes.RPCInvalidRequestError(req.ID, err))
|
||||
mayCache = false
|
||||
// lastly default all remaining errors as internal errors
|
||||
default: // includes ctypes.ErrHeightNotAvailable and ctypes.ErrHeightExceedsChainHead
|
||||
responses = append(responses, rpctypes.RPCInternalError(req.ID, err))
|
||||
mayCache = false
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if len(responses) > 0 {
|
||||
if wErr := WriteRPCResponseHTTP(w, mayCache, responses...); wErr != nil {
|
||||
if wErr := WriteRPCResponseHTTP(w, responses...); wErr != nil {
|
||||
logger.Error("failed to write responses", "err", wErr)
|
||||
}
|
||||
}
|
||||
@@ -288,12 +271,3 @@ func writeListOfEndpoints(w http.ResponseWriter, r *http.Request, funcMap map[st
|
||||
w.WriteHeader(200)
|
||||
w.Write(buf.Bytes()) // nolint: errcheck
|
||||
}
|
||||
|
||||
func hasDefaultHeight(r rpctypes.RPCRequest, h []reflect.Value) bool {
|
||||
switch r.Method {
|
||||
case "block", "block_results", "commit", "consensus_params", "validators":
|
||||
return len(h) < 2 || h[1].IsZero()
|
||||
default:
|
||||
return false
|
||||
}
|
||||
}
|
||||
|
||||
@@ -18,8 +18,8 @@ import (
|
||||
|
||||
func testMux() *http.ServeMux {
|
||||
funcMap := map[string]*RPCFunc{
|
||||
"c": NewRPCFunc(func(ctx context.Context, s string, i int) (string, error) { return "foo", nil }, "s,i", false),
|
||||
"block": NewRPCFunc(func(ctx context.Context, h int) (string, error) { return "block", nil }, "height", true),
|
||||
"c": NewRPCFunc(func(ctx context.Context, s string, i int) (string, error) { return "foo", nil }, "s,i"),
|
||||
"block": NewRPCFunc(func(ctx context.Context, h int) (string, error) { return "block", nil }, "height"),
|
||||
}
|
||||
mux := http.NewServeMux()
|
||||
logger := log.NewNopLogger()
|
||||
@@ -238,7 +238,7 @@ func TestRPCResponseCache(t *testing.T) {
|
||||
|
||||
// Always expecting back a JSONRPCResponse
|
||||
require.True(t, statusOK(res.StatusCode), "should always return 2XX")
|
||||
require.Equal(t, "max-age=31536000", res.Header.Get("Cache-control"))
|
||||
require.Equal(t, "", res.Header.Get("Cache-control"))
|
||||
|
||||
_, err := io.ReadAll(res.Body)
|
||||
res.Body.Close()
|
||||
|
||||
@@ -169,7 +169,7 @@ func WriteRPCResponseHTTPError(
|
||||
|
||||
// WriteRPCResponseHTTP marshals res as JSON (with indent) and writes it to w.
|
||||
// If the rpc response can be cached, add cache-control to the response header.
|
||||
func WriteRPCResponseHTTP(w http.ResponseWriter, c bool, res ...rpctypes.RPCResponse) error {
|
||||
func WriteRPCResponseHTTP(w http.ResponseWriter, res ...rpctypes.RPCResponse) error {
|
||||
var v interface{}
|
||||
if len(res) == 1 {
|
||||
v = res[0]
|
||||
@@ -182,10 +182,7 @@ func WriteRPCResponseHTTP(w http.ResponseWriter, c bool, res ...rpctypes.RPCResp
|
||||
return fmt.Errorf("json marshal: %w", err)
|
||||
}
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
if c {
|
||||
w.Header().Set("Cache-Control", "max-age=31536000") // expired after one year
|
||||
}
|
||||
w.WriteHeader(200)
|
||||
w.WriteHeader(http.StatusOK)
|
||||
_, err = w.Write(jsonBytes)
|
||||
return err
|
||||
}
|
||||
@@ -225,7 +222,7 @@ func RecoverAndLogHandler(handler http.Handler, logger log.Logger) http.Handler
|
||||
|
||||
// If RPCResponse
|
||||
if res, ok := e.(rpctypes.RPCResponse); ok {
|
||||
if wErr := WriteRPCResponseHTTP(rww, false, res); wErr != nil {
|
||||
if wErr := WriteRPCResponseHTTP(rww, res); wErr != nil {
|
||||
logger.Error("failed to write response", "res", res, "err", wErr)
|
||||
}
|
||||
} else {
|
||||
|
||||
@@ -122,7 +122,7 @@ func TestWriteRPCResponseHTTP(t *testing.T) {
|
||||
|
||||
// one argument
|
||||
w := httptest.NewRecorder()
|
||||
err := WriteRPCResponseHTTP(w, true, rpctypes.NewRPCSuccessResponse(id, &sampleResult{"hello"}))
|
||||
err := WriteRPCResponseHTTP(w, rpctypes.NewRPCSuccessResponse(id, &sampleResult{"hello"}))
|
||||
require.NoError(t, err)
|
||||
resp := w.Result()
|
||||
body, err := io.ReadAll(resp.Body)
|
||||
@@ -130,7 +130,7 @@ func TestWriteRPCResponseHTTP(t *testing.T) {
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, 200, resp.StatusCode)
|
||||
assert.Equal(t, "application/json", resp.Header.Get("Content-Type"))
|
||||
assert.Equal(t, "max-age=31536000", resp.Header.Get("Cache-control"))
|
||||
assert.Equal(t, "", resp.Header.Get("Cache-control"))
|
||||
assert.Equal(t, `{
|
||||
"jsonrpc": "2.0",
|
||||
"id": -1,
|
||||
@@ -142,9 +142,9 @@ func TestWriteRPCResponseHTTP(t *testing.T) {
|
||||
// multiple arguments
|
||||
w = httptest.NewRecorder()
|
||||
err = WriteRPCResponseHTTP(w,
|
||||
false,
|
||||
rpctypes.NewRPCSuccessResponse(id, &sampleResult{"hello"}),
|
||||
rpctypes.NewRPCSuccessResponse(id, &sampleResult{"world"}))
|
||||
rpctypes.NewRPCSuccessResponse(id, &sampleResult{"world"}),
|
||||
)
|
||||
require.NoError(t, err)
|
||||
resp = w.Result()
|
||||
body, err = io.ReadAll(resp.Body)
|
||||
|
||||
@@ -62,7 +62,7 @@ func makeHTTPHandler(rpcFunc *RPCFunc, logger log.Logger) func(http.ResponseWrit
|
||||
// if no error then return a success response
|
||||
case nil:
|
||||
res := rpctypes.NewRPCSuccessResponse(dummyID, result)
|
||||
if wErr := WriteRPCResponseHTTP(w, rpcFunc.cache, res); wErr != nil {
|
||||
if wErr := WriteRPCResponseHTTP(w, res); wErr != nil {
|
||||
logger.Error("failed to write response", "res", res, "err", wErr)
|
||||
}
|
||||
|
||||
|
||||
@@ -135,7 +135,7 @@ func TestParseJSONArray(t *testing.T) {
|
||||
|
||||
func TestParseJSONRPC(t *testing.T) {
|
||||
demo := func(ctx context.Context, height int, name string) {}
|
||||
call := NewRPCFunc(demo, "height,name", false)
|
||||
call := NewRPCFunc(demo, "height,name")
|
||||
|
||||
cases := []struct {
|
||||
raw string
|
||||
@@ -172,7 +172,7 @@ func TestParseJSONRPC(t *testing.T) {
|
||||
|
||||
func TestParseURI(t *testing.T) {
|
||||
demo := func(ctx context.Context, height int, name string) {}
|
||||
call := NewRPCFunc(demo, "height,name", false)
|
||||
call := NewRPCFunc(demo, "height,name")
|
||||
|
||||
cases := []struct {
|
||||
raw []string
|
||||
|
||||
@@ -31,22 +31,20 @@ type RPCFunc struct {
|
||||
returns []reflect.Type // type of each return arg
|
||||
argNames []string // name of each argument
|
||||
ws bool // websocket only
|
||||
cache bool // allow the RPC response can be cached by the proxy cache server
|
||||
}
|
||||
|
||||
// NewRPCFunc wraps a function for introspection.
|
||||
// f is the function, args are comma separated argument names
|
||||
// cache is a bool value to allow the client proxy server to cache the RPC results
|
||||
func NewRPCFunc(f interface{}, args string, cache bool) *RPCFunc {
|
||||
return newRPCFunc(f, args, false, cache)
|
||||
func NewRPCFunc(f interface{}, args string) *RPCFunc {
|
||||
return newRPCFunc(f, args, false)
|
||||
}
|
||||
|
||||
// NewWSRPCFunc wraps a function for introspection and use in the websockets.
|
||||
func NewWSRPCFunc(f interface{}, args string) *RPCFunc {
|
||||
return newRPCFunc(f, args, true, false)
|
||||
return newRPCFunc(f, args, true)
|
||||
}
|
||||
|
||||
func newRPCFunc(f interface{}, args string, ws bool, c bool) *RPCFunc {
|
||||
func newRPCFunc(f interface{}, args string, ws bool) *RPCFunc {
|
||||
var argNames []string
|
||||
if args != "" {
|
||||
argNames = strings.Split(args, ",")
|
||||
@@ -57,7 +55,6 @@ func newRPCFunc(f interface{}, args string, ws bool, c bool) *RPCFunc {
|
||||
returns: funcReturnTypes(f),
|
||||
argNames: argNames,
|
||||
ws: ws,
|
||||
cache: c,
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -13,7 +13,7 @@ import (
|
||||
)
|
||||
|
||||
var routes = map[string]*rpcserver.RPCFunc{
|
||||
"hello_world": rpcserver.NewRPCFunc(HelloWorld, "name,num", false),
|
||||
"hello_world": rpcserver.NewRPCFunc(HelloWorld, "name,num"),
|
||||
}
|
||||
|
||||
func HelloWorld(ctx context.Context, name string, num int) (Result, error) {
|
||||
|
||||
Reference in New Issue
Block a user