From 2583673c8b0bb156e52e81484d17739d99fd5dc8 Mon Sep 17 00:00:00 2001 From: Kevin Wan Date: Sat, 9 Aug 2025 13:07:17 +0800 Subject: [PATCH] chore: always ignore unknown fields for gateway requests (#5072) Signed-off-by: kevin --- gateway/config.go | 4 --- gateway/internal/requestparser.go | 30 ++++++---------- gateway/internal/requestparser_test.go | 48 +++++++++++++------------- gateway/server.go | 18 +++++----- 4 files changed, 43 insertions(+), 57 deletions(-) diff --git a/gateway/config.go b/gateway/config.go index b70408e14..29e5f254d 100644 --- a/gateway/config.go +++ b/gateway/config.go @@ -10,10 +10,6 @@ type ( GatewayConf struct { rest.RestConf Upstreams []Upstream - // IgnoreUnknownFields specifies whether to ignore unknown fields during proto unmarshaling. - // When set to true, extra query parameters or JSON fields that don't exist in the gRPC message - // will be ignored instead of causing an error. - IgnoreUnknownFields bool `json:",default=false"` } // HttpClientConf is the configuration for an HTTP client. diff --git a/gateway/internal/requestparser.go b/gateway/internal/requestparser.go index bcd0d4729..91737c76a 100644 --- a/gateway/internal/requestparser.go +++ b/gateway/internal/requestparser.go @@ -13,7 +13,7 @@ import ( ) // NewRequestParser creates a new request parser from the given http.Request and resolver. -func NewRequestParser(r *http.Request, resolver jsonpb.AnyResolver, ignoreUnknownFields bool) (grpcurl.RequestParser, error) { +func NewRequestParser(r *http.Request, resolver jsonpb.AnyResolver) (grpcurl.RequestParser, error) { vars := pathvar.Vars(r) params, err := httpx.GetFormValues(r) if err != nil { @@ -26,14 +26,11 @@ func NewRequestParser(r *http.Request, resolver jsonpb.AnyResolver, ignoreUnknow body, ok := getBody(r) if !ok { - return buildJsonRequestParserFromMap(params, resolver, ignoreUnknownFields) + return buildJsonRequestParserFromMap(params, resolver) } if len(params) == 0 { - if ignoreUnknownFields { - return buildJsonRequestParserWithUnknownFields(body, resolver) - } - return buildJsonRequestParser(body, resolver) + return buildJsonRequestParserFromReader(body, resolver) } m := make(map[string]any) @@ -45,32 +42,27 @@ func NewRequestParser(r *http.Request, resolver jsonpb.AnyResolver, ignoreUnknow m[k] = v } - return buildJsonRequestParserFromMap(m, resolver, ignoreUnknownFields) + return buildJsonRequestParserFromMap(m, resolver) } -func buildJsonRequestParserFromMap(data map[string]any, resolver jsonpb.AnyResolver, ignoreUnknownFields bool) (grpcurl.RequestParser, error) { +func buildJsonRequestParserFromMap(data map[string]any, resolver jsonpb.AnyResolver) ( + grpcurl.RequestParser, error) { var buf bytes.Buffer if err := json.NewEncoder(&buf).Encode(data); err != nil { return nil, err } - if ignoreUnknownFields { - return buildJsonRequestParserWithUnknownFields(&buf, resolver) - } - return buildJsonRequestParser(&buf, resolver) + return buildJsonRequestParserFromReader(&buf, resolver) } -// buildJsonRequestParser creates a JSON request parser with default settings -func buildJsonRequestParser(data io.Reader, resolver jsonpb.AnyResolver) (grpcurl.RequestParser, error) { - return grpcurl.NewJSONRequestParser(data, resolver), nil -} - -// buildJsonRequestParserWithUnknownFields creates a JSON request parser that ignores unknown fields -func buildJsonRequestParserWithUnknownFields(data io.Reader, resolver jsonpb.AnyResolver) (grpcurl.RequestParser, error) { +// buildJsonRequestParserFromReader creates a JSON request parser with ignoring unknown fields. +func buildJsonRequestParserFromReader(data io.Reader, resolver jsonpb.AnyResolver) ( + grpcurl.RequestParser, error) { unmarshaler := jsonpb.Unmarshaler{ AllowUnknownFields: true, AnyResolver: resolver, } + return grpcurl.NewJSONRequestParserWithUnmarshaler(data, unmarshaler), nil } diff --git a/gateway/internal/requestparser_test.go b/gateway/internal/requestparser_test.go index 5ae0de654..dadd4d52f 100644 --- a/gateway/internal/requestparser_test.go +++ b/gateway/internal/requestparser_test.go @@ -14,7 +14,7 @@ import ( func TestNewRequestParserNoVar(t *testing.T) { req := httptest.NewRequest("GET", "/", http.NoBody) - parser, err := NewRequestParser(req, nil, false) + parser, err := NewRequestParser(req, nil) assert.Nil(t, err) assert.NotNil(t, parser) } @@ -22,14 +22,14 @@ func TestNewRequestParserNoVar(t *testing.T) { func TestNewRequestParserWithVars(t *testing.T) { req := httptest.NewRequest("GET", "/", http.NoBody) req = pathvar.WithVars(req, map[string]string{"a": "b"}) - parser, err := NewRequestParser(req, nil, false) + parser, err := NewRequestParser(req, nil) assert.Nil(t, err) assert.NotNil(t, parser) } func TestNewRequestParserNoVarWithBody(t *testing.T) { req := httptest.NewRequest("GET", "/", strings.NewReader(`{"a": "b"}`)) - parser, err := NewRequestParser(req, nil, false) + parser, err := NewRequestParser(req, nil) assert.Nil(t, err) assert.NotNil(t, parser) } @@ -37,7 +37,7 @@ func TestNewRequestParserNoVarWithBody(t *testing.T) { func TestNewRequestParserWithNegativeContentLength(t *testing.T) { req := httptest.NewRequest("GET", "/", strings.NewReader(`{"a": "b"}`)) req.ContentLength = -1 - parser, err := NewRequestParser(req, nil, false) + parser, err := NewRequestParser(req, nil) assert.Nil(t, err) assert.NotNil(t, parser) } @@ -45,7 +45,7 @@ func TestNewRequestParserWithNegativeContentLength(t *testing.T) { func TestNewRequestParserWithVarsWithBody(t *testing.T) { req := httptest.NewRequest("GET", "/", strings.NewReader(`{"a": "b"}`)) req = pathvar.WithVars(req, map[string]string{"c": "d"}) - parser, err := NewRequestParser(req, nil, false) + parser, err := NewRequestParser(req, nil) assert.Nil(t, err) assert.NotNil(t, parser) } @@ -53,14 +53,14 @@ func TestNewRequestParserWithVarsWithBody(t *testing.T) { func TestNewRequestParserWithVarsWithWrongBody(t *testing.T) { req := httptest.NewRequest("GET", "/", strings.NewReader(`{"a": "b"`)) req = pathvar.WithVars(req, map[string]string{"c": "d"}) - parser, err := NewRequestParser(req, nil, false) + parser, err := NewRequestParser(req, nil) assert.NotNil(t, err) assert.Nil(t, parser) } func TestNewRequestParserWithForm(t *testing.T) { req := httptest.NewRequest("GET", "/val?a=b", nil) - parser, err := NewRequestParser(req, nil, false) + parser, err := NewRequestParser(req, nil) assert.Nil(t, err) assert.NotNil(t, parser) } @@ -68,7 +68,7 @@ func TestNewRequestParserWithForm(t *testing.T) { func TestNewRequestParserWithNilBody(t *testing.T) { req := httptest.NewRequest("GET", "/val?a=b", http.NoBody) req.Body = nil - parser, err := NewRequestParser(req, nil, false) + parser, err := NewRequestParser(req, nil) assert.Nil(t, err) assert.NotNil(t, parser) } @@ -76,20 +76,20 @@ func TestNewRequestParserWithNilBody(t *testing.T) { func TestNewRequestParserWithBadBody(t *testing.T) { req := httptest.NewRequest("GET", "/val?a=b", badBody{}) req.Body = badBody{} - parser, err := NewRequestParser(req, nil, false) + parser, err := NewRequestParser(req, nil) assert.Nil(t, err) assert.NotNil(t, parser) } func TestNewRequestParserWithBadForm(t *testing.T) { req := httptest.NewRequest("GET", "/val?a%1=b", http.NoBody) - parser, err := NewRequestParser(req, nil, false) + parser, err := NewRequestParser(req, nil) assert.NotNil(t, err) assert.Nil(t, parser) } func TestRequestParser_buildJsonRequestParserFromMap(t *testing.T) { - parser, err := buildJsonRequestParserFromMap(map[string]any{"a": make(chan int)}, nil, false) + parser, err := buildJsonRequestParserFromMap(map[string]any{"a": make(chan int)}, nil) assert.NotNil(t, err) assert.Nil(t, parser) } @@ -107,23 +107,23 @@ func TestNewRequestParserWithIgnoreUnknownFields(t *testing.T) { // Test case 1: No body, no vars - should work with both true and false req1 := httptest.NewRequest("GET", "/", http.NoBody) - parser1, err1 := NewRequestParser(req1, resolver, true) + parser1, err1 := NewRequestParser(req1, resolver) assert.Nil(t, err1) assert.NotNil(t, parser1) req2 := httptest.NewRequest("GET", "/", http.NoBody) - parser2, err2 := NewRequestParser(req2, resolver, false) + parser2, err2 := NewRequestParser(req2, resolver) assert.Nil(t, err2) assert.NotNil(t, parser2) // Test case 2: With JSON body - tests the body parsing path req3 := httptest.NewRequest("POST", "/", strings.NewReader(`{"field": "value"}`)) - parser3, err3 := NewRequestParser(req3, resolver, true) + parser3, err3 := NewRequestParser(req3, resolver) assert.Nil(t, err3) assert.NotNil(t, parser3) req4 := httptest.NewRequest("POST", "/", strings.NewReader(`{"field": "value"}`)) - parser4, err4 := NewRequestParser(req4, resolver, false) + parser4, err4 := NewRequestParser(req4, resolver) assert.Nil(t, err4) assert.NotNil(t, parser4) } @@ -134,14 +134,14 @@ func TestNewRequestParserWithVarsAndIgnoreUnknownFields(t *testing.T) { // Test with path variables and ignoreUnknownFields = true req := httptest.NewRequest("GET", "/", http.NoBody) req = pathvar.WithVars(req, map[string]string{"a": "b"}) - parser, err := NewRequestParser(req, resolver, true) + parser, err := NewRequestParser(req, resolver) assert.Nil(t, err) assert.NotNil(t, parser) // Test with path variables and ignoreUnknownFields = false req2 := httptest.NewRequest("GET", "/", http.NoBody) req2 = pathvar.WithVars(req2, map[string]string{"c": "d"}) - parser2, err2 := NewRequestParser(req2, resolver, false) + parser2, err2 := NewRequestParser(req2, resolver) assert.Nil(t, err2) assert.NotNil(t, parser2) } @@ -151,13 +151,13 @@ func TestNewRequestParserWithBodyAndIgnoreUnknownFields(t *testing.T) { // Test with body and ignoreUnknownFields = true req := httptest.NewRequest("POST", "/", strings.NewReader(`{"a": "b"}`)) - parser, err := NewRequestParser(req, resolver, true) + parser, err := NewRequestParser(req, resolver) assert.Nil(t, err) assert.NotNil(t, parser) // Test with body and ignoreUnknownFields = false req2 := httptest.NewRequest("POST", "/", strings.NewReader(`{"c": "d"}`)) - parser2, err2 := NewRequestParser(req2, resolver, false) + parser2, err2 := NewRequestParser(req2, resolver) assert.Nil(t, err2) assert.NotNil(t, parser2) } @@ -168,14 +168,14 @@ func TestNewRequestParserWithVarsBodyAndIgnoreUnknownFields(t *testing.T) { // Test with both path variables and body, ignoreUnknownFields = true req := httptest.NewRequest("POST", "/", strings.NewReader(`{"a": "b"}`)) req = pathvar.WithVars(req, map[string]string{"c": "d"}) - parser, err := NewRequestParser(req, resolver, true) + parser, err := NewRequestParser(req, resolver) assert.Nil(t, err) assert.NotNil(t, parser) // Test with both path variables and body, ignoreUnknownFields = false req2 := httptest.NewRequest("POST", "/", strings.NewReader(`{"e": "f"}`)) req2 = pathvar.WithVars(req2, map[string]string{"g": "h"}) - parser2, err2 := NewRequestParser(req2, resolver, false) + parser2, err2 := NewRequestParser(req2, resolver) assert.Nil(t, err2) assert.NotNil(t, parser2) } @@ -185,12 +185,12 @@ func TestBuildJsonRequestParserFromMapWithIgnoreUnknownFields(t *testing.T) { // Test buildJsonRequestParserFromMap with ignoreUnknownFields = true data := map[string]any{"key": "value"} - parser, err := buildJsonRequestParserFromMap(data, resolver, true) + parser, err := buildJsonRequestParserFromMap(data, resolver) assert.Nil(t, err) assert.NotNil(t, parser) // Test buildJsonRequestParserFromMap with ignoreUnknownFields = false - parser2, err2 := buildJsonRequestParserFromMap(data, resolver, false) + parser2, err2 := buildJsonRequestParserFromMap(data, resolver) assert.Nil(t, err2) assert.NotNil(t, parser2) } @@ -200,7 +200,7 @@ func TestBuildJsonRequestParserWithUnknownFields(t *testing.T) { // Test buildJsonRequestParserWithUnknownFields data := strings.NewReader(`{"test": "value"}`) - parser, err := buildJsonRequestParserWithUnknownFields(data, resolver) + parser, err := buildJsonRequestParserFromReader(data, resolver) assert.Nil(t, err) assert.NotNil(t, parser) } diff --git a/gateway/server.go b/gateway/server.go index 3f60da9f4..4a5578ff4 100644 --- a/gateway/server.go +++ b/gateway/server.go @@ -30,12 +30,11 @@ type ( // Server is a gateway server. Server struct { *rest.Server - upstreams []Upstream - conns []zrpc.Client - processHeader func(http.Header) []string - dialer func(conf zrpc.RpcClientConf) zrpc.Client - middlewares []rest.Middleware - ignoreUnknownFields bool + upstreams []Upstream + conns []zrpc.Client + processHeader func(http.Header) []string + dialer func(conf zrpc.RpcClientConf) zrpc.Client + middlewares []rest.Middleware } // Option defines the method to customize Server. @@ -45,9 +44,8 @@ type ( // MustNewServer creates a new gateway server. func MustNewServer(c GatewayConf, opts ...Option) *Server { svr := &Server{ - upstreams: c.Upstreams, - Server: rest.MustNewServer(c.RestConf), - ignoreUnknownFields: c.IgnoreUnknownFields, + upstreams: c.Upstreams, + Server: rest.MustNewServer(c.RestConf), } for _, opt := range opts { opt(svr) @@ -116,7 +114,7 @@ func (s *Server) buildChainHandler(handler http.HandlerFunc) http.HandlerFunc { func (s *Server) buildGrpcHandler(source grpcurl.DescriptorSource, resolver jsonpb.AnyResolver, cli zrpc.Client, rpcPath string) func(http.ResponseWriter, *http.Request) { handler := func(w http.ResponseWriter, r *http.Request) { - parser, err := internal.NewRequestParser(r, resolver, s.ignoreUnknownFields) + parser, err := internal.NewRequestParser(r, resolver) if err != nil { httpx.ErrorCtx(r.Context(), w, err) return