Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

node, rpc: add configurable HTTP request limit #28948

Merged
merged 2 commits into from
Feb 7, 2024
Merged

node, rpc: add configurable HTTP request limit #28948

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8fe5ace
node, rpc: add configurable HTTP request limit
fjl Feb 7, 2024
ce8883c
rpc: fix tests
fjl Feb 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions node/defaults.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ const (
// needs of all CLs.
engineAPIBatchItemLimit = 2000
engineAPIBatchResponseSizeLimit = 250 * 1000 * 1000
engineAPIBodyLimit = 128 * 1024 * 1024
)

var (
Expand Down
6 changes: 4 additions & 2 deletions node/node.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -453,14 +453,16 @@ func (n *Node) startRPC() error {
jwtSecret: secret,
batchItemLimit: engineAPIBatchItemLimit,
batchResponseSizeLimit: engineAPIBatchResponseSizeLimit,
httpBodyLimit: engineAPIBodyLimit,
}
if err := server.enableRPC(allAPIs, httpConfig{
err := server.enableRPC(allAPIs, httpConfig{
CorsAllowedOrigins: DefaultAuthCors,
Vhosts: n.config.AuthVirtualHosts,
Modules: DefaultAuthModules,
prefix: DefaultAuthPrefix,
rpcEndpointConfig: sharedConfig,
}); err != nil {
})
if err != nil {
return err
}
servers = append(servers, server)
Expand Down
7 changes: 7 additions & 0 deletions node/rpcstack.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ type rpcEndpointConfig struct {
jwtSecret []byte // optional JWT secret
batchItemLimit int
batchResponseSizeLimit int
httpBodyLimit int
}

type rpcHandler struct {
Expand Down Expand Up @@ -304,6 +305,9 @@ func (h *httpServer) enableRPC(apis []rpc.API, config httpConfig) error {
// Create RPC server and handler.
srv := rpc.NewServer()
srv.SetBatchLimits(config.batchItemLimit, config.batchResponseSizeLimit)
if config.httpBodyLimit > 0 {
srv.SetHTTPBodyLimit(config.httpBodyLimit)
}
if err := RegisterApis(apis, config.Modules, srv); err != nil {
return err
}
Expand Down Expand Up @@ -336,6 +340,9 @@ func (h *httpServer) enableWS(apis []rpc.API, config wsConfig) error {
// Create RPC server and handler.
srv := rpc.NewServer()
srv.SetBatchLimits(config.batchItemLimit, config.batchResponseSizeLimit)
if config.httpBodyLimit > 0 {
srv.SetHTTPBodyLimit(config.httpBodyLimit)
}
if err := RegisterApis(apis, config.Modules, srv); err != nil {
return err
}
Expand Down
18 changes: 9 additions & 9 deletions rpc/http.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ import (
)

const (
maxRequestContentLength = 1024 * 1024 * 5
contentType = "application/json"
defaultBodyLimit = 5 * 1024 * 1024
contentType = "application/json"
)

// https://www.jsonrpc.org/historical/json-rpc-over-http.html#id13
Expand Down Expand Up @@ -253,8 +253,8 @@ type httpServerConn struct {
r *http.Request
}

func newHTTPServerConn(r *http.Request, w http.ResponseWriter) ServerCodec {
body := io.LimitReader(r.Body, maxRequestContentLength)
func (s *Server) newHTTPServerConn(r *http.Request, w http.ResponseWriter) ServerCodec {
body := io.LimitReader(r.Body, int64(s.httpBodyLimit))
conn := &httpServerConn{Reader: body, Writer: w, r: r}

encoder := func(v any, isErrorResponse bool) error {
Expand Down Expand Up @@ -312,7 +312,7 @@ func (s *Server) ServeHTTP(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusOK)
return
}
if code, err := validateRequest(r); err != nil {
if code, err := s.validateRequest(r); err != nil {
http.Error(w, err.Error(), code)
return
}
Expand All @@ -330,19 +330,19 @@ func (s *Server) ServeHTTP(w http.ResponseWriter, r *http.Request) {
// until EOF, writes the response to w, and orders the server to process a
// single request.
w.Header().Set("content-type", contentType)
codec := newHTTPServerConn(r, w)
codec := s.newHTTPServerConn(r, w)
defer codec.close()
s.serveSingleRequest(ctx, codec)
}

// validateRequest returns a non-zero response code and error message if the
// request is invalid.
func validateRequest(r *http.Request) (int, error) {
func (s *Server) validateRequest(r *http.Request) (int, error) {
if r.Method == http.MethodPut || r.Method == http.MethodDelete {
return http.StatusMethodNotAllowed, errors.New("method not allowed")
}
if r.ContentLength > maxRequestContentLength {
err := fmt.Errorf("content length too large (%d>%d)", r.ContentLength, maxRequestContentLength)
if r.ContentLength > int64(s.httpBodyLimit) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the s.httpBodyLimit is an int, so the default-value is 0. Perhaps 0 should mean "not set" ?

Suggested change
if r.ContentLength > int64(s.httpBodyLimit) {
if s.httpBodyLimit != 0 && r.ContentLength > int64(s.httpBodyLimit) {

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I see now that you set it to the default value, and then do not invoke SetHTTPBodyLimit unless it's non-zero.
Ok then, I guess that works too. If a library user explicitly invokes SetHTTPBodyLimit(0) they have themselves to blame.

err := fmt.Errorf("content length too large (%d>%d)", r.ContentLength, s.httpBodyLimit)
return http.StatusRequestEntityTooLarge, err
}
// Allow OPTIONS (regardless of content-type)
Expand Down
8 changes: 5 additions & 3 deletions rpc/http_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,13 @@ func confirmStatusCode(t *testing.T, got, want int) {

func confirmRequestValidationCode(t *testing.T, method, contentType, body string, expectedStatusCode int) {
t.Helper()

s := NewServer()
request := httptest.NewRequest(method, "http://url.com", strings.NewReader(body))
if len(contentType) > 0 {
request.Header.Set("Content-Type", contentType)
}
code, err := validateRequest(request)
code, err := s.validateRequest(request)
if code == 0 {
if err != nil {
t.Errorf("validation: got error %v, expected nil", err)
Expand All @@ -64,7 +66,7 @@ func TestHTTPErrorResponseWithPut(t *testing.T) {
}

func TestHTTPErrorResponseWithMaxContentLength(t *testing.T) {
body := make([]rune, maxRequestContentLength+1)
body := make([]rune, defaultBodyLimit+1)
confirmRequestValidationCode(t,
http.MethodPost, contentType, string(body), http.StatusRequestEntityTooLarge)
}
Expand Down Expand Up @@ -104,7 +106,7 @@ func TestHTTPResponseWithEmptyGet(t *testing.T) {

// This checks that maxRequestContentLength is not applied to the response of a request.
func TestHTTPRespBodyUnlimited(t *testing.T) {
const respLength = maxRequestContentLength * 3
const respLength = defaultBodyLimit * 3

s := NewServer()
defer s.Stop()
Expand Down
13 changes: 11 additions & 2 deletions rpc/server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,13 +51,15 @@ type Server struct {
run atomic.Bool
batchItemLimit int
batchResponseLimit int
httpBodyLimit int
}

// NewServer creates a new server instance with no registered handlers.
func NewServer() *Server {
server := &Server{
idgen: randomIDGenerator(),
codecs: make(map[ServerCodec]struct{}),
idgen: randomIDGenerator(),
codecs: make(map[ServerCodec]struct{}),
httpBodyLimit: defaultBodyLimit,
}
server.run.Store(true)
// Register the default service providing meta information about the RPC service such
Expand All @@ -78,6 +80,13 @@ func (s *Server) SetBatchLimits(itemLimit, maxResponseSize int) {
s.batchResponseLimit = maxResponseSize
}

// SetHTTPBodyLimit sets the size limit for HTTP requests.
//
// This method should be called before processing any requests via ServeHTTP.
func (s *Server) SetHTTPBodyLimit(limit int) {
s.httpBodyLimit = limit
}

// RegisterName creates a service for the given receiver type under the given name. When no
// methods on the given receiver match the criteria to be either a RPC method or a
// subscription an error is returned. Otherwise a new service is created and added to the
Expand Down
4 changes: 2 additions & 2 deletions rpc/websocket_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ func TestWebsocketLargeCall(t *testing.T) {

// This call sends slightly less than the limit and should work.
var result echoResult
arg := strings.Repeat("x", maxRequestContentLength-200)
arg := strings.Repeat("x", defaultBodyLimit-200)
if err := client.Call(&result, "test_echo", arg, 1); err != nil {
t.Fatalf("valid call didn't work: %v", err)
}
Expand All @@ -106,7 +106,7 @@ func TestWebsocketLargeCall(t *testing.T) {
}

// This call sends twice the allowed size and shouldn't work.
arg = strings.Repeat("x", maxRequestContentLength*2)
arg = strings.Repeat("x", defaultBodyLimit*2)
err = client.Call(&result, "test_echo", arg)
if err == nil {
t.Fatal("no error for too large call")
Expand Down
Loading