From 94fff6fe360e18684342be37a80597a547c83f47 Mon Sep 17 00:00:00 2001 From: Jeremy Mouton Date: Mon, 19 Oct 2026 19:59:18 +0200 Subject: [PATCH 1/4] fix: never fill path, query or header fields from the body v3.3.0 decodes the body before binding the path, query and headers, but the binders only overwrite the parameters present in the request. A header-only field absent from the request kept the value the JSON decoder took from a body key with the same name: Tenant `header:"X-Tenant"` became "evil" with {"tenant":"evil"} and no header. That defeats an application trusting a header set by a proxy, such as X-User-Id. After decoding the body, parseInput now resets the uri, query and header fields that declare no body tag (json, form, xml, msgpack, cbor; "-" counts as none), then binds them from the request. A field that also declares a body tag keeps accepting either. TestHeaderNotInRequestBody pinned the old behavior (a required header satisfied from the body); it now expects 422. Co-Authored-By: Claude Opus 5.5 --- binding_order_test.go | 62 ++++++++++++++++++++++++++++++++++++++ common.go | 69 +++++++++++++++++++++++++++++++++++++++++-- header_params_test.go | 13 ++++---- 3 files changed, 134 insertions(+), 10 deletions(-) diff --git a/binding_order_test.go b/binding_order_test.go index 6371115..94ef945 100644 --- a/binding_order_test.go +++ b/binding_order_test.go @@ -4,6 +4,7 @@ import ( "encoding/json" "io" "net/http/httptest" + "reflect" "strings" "testing" @@ -68,3 +69,64 @@ func TestBindingOrder_BodySchemaExcludesPathParameters(t *testing.T) { assert.Contains(t, props, "name") assert.NotContains(t, schema, "required", "the only required field is the path parameter") } + +// proxyInput mimics a handler that trusts identity headers set by a proxy. +type proxyInput struct { + ID string `uri:"id"` + UserID string `header:"X-User-Id"` + Filter string `query:"filter"` + Name string `json:"name"` + // Sort declares both: it is accepted from the query or from the body. + Sort string `query:"sort" json:"sort"` +} + +func TestBindingOrder_AbsentParametersAreNotFilledFromTheBody(t *testing.T) { + var got proxyInput + app := fiber.New() + oapi := New(app) + Put(oapi, "/items/:id", func(c fiber.Ctx, in proxyInput) (fiber.Map, struct{}) { + got = in + return fiber.Map{}, struct{}{} + }, OpenAPIOptions{}) + + send := func(contentType, body, query string) { + t.Helper() + got = proxyInput{} + req := httptest.NewRequest("PUT", "/items/from-path"+query, strings.NewReader(body)) + req.Header.Set("Content-Type", contentType) + resp, err := app.Test(req) + require.NoError(t, err) + require.Equal(t, 200, resp.StatusCode) + } + + send("application/json", `{"id":"evil","userId":"evil","x-user-id":"evil","filter":"evil","name":"widget","sort":"from-body"}`, "") + assert.Equal(t, "from-path", got.ID) + assert.Empty(t, got.UserID, "a header absent from the request must not come from the body") + assert.Empty(t, got.Filter, "a query parameter absent from the request must not come from the body") + assert.Equal(t, "widget", got.Name) + assert.Equal(t, "from-body", got.Sort, "a field that also declares a json tag accepts the body") + + send("application/json", `{"sort":"from-body"}`, "?sort=from-query") + assert.Equal(t, "from-query", got.Sort, "the query still wins when present") + + send("application/x-www-form-urlencoded", "UserID=evil&Filter=evil&ID=evil", "") + assert.Equal(t, "from-path", got.ID) + assert.Empty(t, got.UserID, "the same holds for form bodies") + assert.Empty(t, got.Filter) +} + +func TestParamOnlyFields(t *testing.T) { + type input struct { + Path string `uri:"id"` + Header string `header:"X-A"` + Hidden string `header:"X-B" json:"-"` + Both string `query:"q" json:"q,omitempty"` + FormBoth string `query:"f" form:"f"` + Body string `json:"body"` + private string `header:"X-C"` + } + assert.Equal(t, []int{0, 1, 2}, paramOnlyFields(reflect.TypeFor[input]())) + assert.Equal(t, []int{0, 1, 2}, paramOnlyFields(reflect.TypeFor[*input]())) + assert.Nil(t, paramOnlyFields(reflect.TypeFor[string]())) + assert.Nil(t, paramOnlyFields(nil)) +} diff --git a/common.go b/common.go index f4dd056..f242529 100644 --- a/common.go +++ b/common.go @@ -27,6 +27,9 @@ type inputShape struct { // rawBody is the index of the []byte field tagged body:"raw", which // receives the request body as-is, or -1. rawBody int + // paramOnly lists the fields bound from the path, the query or the + // headers that declare no body tag (see paramOnlyFields). + paramOnly []int } var shapeCache sync.Map // map[reflect.Type]*inputShape @@ -40,8 +43,9 @@ func shapeFor[TInput any]() *inputShape { } rawBody, _ := rawBodyField(t) // an invalid declaration panics in Method s := &inputShape{ - isStruct: t != nil && dereferenceType(t).Kind() == reflect.Struct, - rawBody: rawBody, + isStruct: t != nil && dereferenceType(t).Kind() == reflect.Struct, + rawBody: rawBody, + paramOnly: paramOnlyFields(t), } actual, _ := shapeCache.LoadOrStore(t, s) return actual.(*inputShape) @@ -86,6 +90,11 @@ func parseInput[TInput any](app *OApiApp, c fiber.Ctx, path string, options *Ope } } } + // The decoder may have filled a path, query or header field from a + // body key with the same name. Binding below only overwrites the + // parameters present in the request, so reset those fields first: a + // header absent from the request must not keep a value from the body. + resetFields(&input, shape.paramOnly) } if shape.isStruct { @@ -158,6 +167,62 @@ func rawBodyField(t reflect.Type) (int, error) { var bytesType = reflect.TypeFor[[]byte]() +// bodyTags are the struct tags through which a body decoder fills a field. +var bodyTags = []string{"json", "form", "xml", "msgpack", "cbor"} + +// paramOnlyFields returns the indexes of the fields of the input struct t +// that are bound from the path, the query or the headers and declare no body +// tag (a "-" tag counts as none). Body decoders still fill such a field when a +// key matches its name, so parseInput resets them after decoding the body. A +// field that declares both, e.g. query:"name" json:"name", accepts either. +func paramOnlyFields(t reflect.Type) []int { + if t == nil { + return nil + } + t = dereferenceType(t) + if t.Kind() != reflect.Struct { + return nil + } + var fields []int + for i := 0; i < t.NumField(); i++ { + field := t.Field(i) + if !field.IsExported() { + continue + } + if field.Tag.Get("uri") == "" && field.Tag.Get("query") == "" && field.Tag.Get("header") == "" { + continue + } + fromBody := false + for _, tag := range bodyTags { + if name, _, _ := strings.Cut(field.Tag.Get(tag), ","); name != "" && name != "-" { + fromBody = true + break + } + } + if !fromBody { + fields = append(fields, i) + } + } + return fields +} + +// resetFields sets the given fields of input to their zero value. +func resetFields[TInput any](input *TInput, fields []int) { + if len(fields) == 0 { + return + } + v := reflect.ValueOf(input).Elem() + if v.Kind() == reflect.Ptr { + if v.IsNil() { + return + } + v = v.Elem() + } + for _, i := range fields { + v.Field(i).SetZero() + } +} + // setRawBody stores body in the raw body field of input, allocating the // struct when the input is a nil pointer. func setRawBody[TInput any](input *TInput, field int, body []byte) { diff --git a/header_params_test.go b/header_params_test.go index 878545a..b1e0879 100644 --- a/header_params_test.go +++ b/header_params_test.go @@ -266,20 +266,17 @@ func TestHeaderNotInRequestBody(t *testing.T) { _, hasName := properties["name"] assert.True(t, hasName, "JSON body field should appear in request body schema") - // Verify that sending the header field in JSON body without the actual header - // does NOT satisfy the header requirement when using json:"-". - // Without json:"-", c.BodyParser can populate exported fields by Go name. - // Header parsing runs after body parsing, so real headers always take priority. + // Sending the header field in the JSON body without the actual header does + // NOT satisfy the header requirement: the JSON decoder fills RequestID by + // Go field name, but a header-only field (no json tag) is reset after the + // body is decoded, so validate:"required" fails. bodyReq := httptest.NewRequest(http.MethodPost, "/items", strings.NewReader(`{"RequestID":"from-body","name":"test"}`)) bodyReq.Header.Set("Content-Type", "application/json") - // No x-request-id header set — body sets RequestID via Go field name match - // but header parser doesn't overwrite (no header present), so body value persists. - // Use json:"-" on header fields to prevent body injection. bodyResp, err := app.Test(bodyReq) require.NoError(t, err) - assert.Equal(t, 200, bodyResp.StatusCode) + assert.Equal(t, 422, bodyResp.StatusCode) } func TestHeaderTakesPriorityOverBody(t *testing.T) { From 405572c697a3963f03519ff11cacbd527b8820c3 Mon Sep 17 00:00:00 2001 From: Jeremy Mouton Date: Mon, 19 Oct 2026 19:59:18 +0200 Subject: [PATCH 2/4] fix: give Problem and RawResponse value receivers HTTPStatus, ContentType and Error were declared on *Problem, so a Problem passed by value (as TError or in OpenAPIOptions.Errors) implemented none of them: the library fell back to a Code int field, found a string, and served and documented a 500 in application/json. RawResponse had the same flaw: by value it was not a Responder and got encoded as JSON. The methods now have value receivers, so both work by value as well as by pointer. Co-Authored-By: Claude Opus 5.5 --- problem.go | 9 ++++++--- problem_test.go | 24 ++++++++++++++++++++++++ responder.go | 5 +++-- responder_test.go | 16 ++++++++++++++++ 4 files changed, 49 insertions(+), 5 deletions(-) diff --git a/problem.go b/problem.go index 038c682..72d60d5 100644 --- a/problem.go +++ b/problem.go @@ -27,14 +27,17 @@ type Problem struct { Errors []ValidationErrorEntry `json:"errors,omitempty" yaml:"errors,omitempty"` } +// The methods of Problem have value receivers, so that a Problem works the +// same whether it is passed by value or by pointer. + // HTTPStatus returns the status of the problem. -func (p *Problem) HTTPStatus() int { return p.Status } +func (p Problem) HTTPStatus() int { return p.Status } // ContentType returns application/problem+json. -func (p *Problem) ContentType() string { return MIMEApplicationProblemJSON } +func (p Problem) ContentType() string { return MIMEApplicationProblemJSON } // Error returns the title of the problem. -func (p *Problem) Error() string { return p.Title } +func (p Problem) Error() string { return p.Title } // ContentTyper is an optional interface an error value (TError, an entry of // OpenAPIOptions.Errors) can implement to choose the Content-Type it is sent diff --git a/problem_test.go b/problem_test.go index a87231f..857a521 100644 --- a/problem_test.go +++ b/problem_test.go @@ -242,3 +242,27 @@ func operationResponses(oapi *OApiApp, path, method string) map[string]any { spec := oapi.GenerateOpenAPISpec() return spec["paths"].(map[string]any)[path].(map[string]any)[method].(map[string]any)["responses"].(map[string]any) } + +func TestProblem_ByValue(t *testing.T) { + app := fiber.New() + oapi := New(app) + Get(oapi, "/value", func(c fiber.Ctx, _ struct{}) (fiber.Map, Problem) { + return nil, Problem{Status: 409, Title: "Conflict", Code: "already_exists"} + }, OpenAPIOptions{}) + Get(oapi, "/declared", func(c fiber.Ctx, _ struct{}) (fiber.Map, error) { + return nil, Problem{Status: 409, Title: "Conflict"} + }, OpenAPIOptions{Errors: []any{Problem{Status: 409, Title: "Conflict"}}}) + + status, p := readProblem(t, app, httptest.NewRequest("GET", "/value", nil)) + assert.Equal(t, 409, status) + assert.Equal(t, "already_exists", p.Code) + status, _ = readProblem(t, app, httptest.NewRequest("GET", "/declared", nil)) + assert.Equal(t, 409, status) + + catchAll := operationResponses(oapi, "/value", "get")["4XX"].(map[string]any)["content"].(map[string]any) + assert.Contains(t, catchAll, MIMEApplicationProblemJSON) + declared := operationResponses(oapi, "/declared", "get") + require.Contains(t, declared, "409", "the status comes from HTTPStatus(), not from the Code string") + assert.Contains(t, declared["409"].(map[string]any)["content"], MIMEApplicationProblemJSON) + assert.NotContains(t, declared, "500") +} diff --git a/responder.go b/responder.go index 4548a1c..459f4fc 100644 --- a/responder.go +++ b/responder.go @@ -28,8 +28,9 @@ type RawResponse struct { Body []byte } -// Respond writes the status, the Content-Type and the body. -func (r *RawResponse) Respond(c fiber.Ctx) error { +// Respond writes the status, the Content-Type and the body. Its receiver is a +// value, so a RawResponse works by value as well as by pointer. +func (r RawResponse) Respond(c fiber.Ctx) error { if r.Status != 0 { c.Status(r.Status) } diff --git a/responder_test.go b/responder_test.go index 56f5baa..8984eae 100644 --- a/responder_test.go +++ b/responder_test.go @@ -162,3 +162,19 @@ func TestResponseContentType_JSONOutput(t *testing.T) { assert.Equal(t, "application/vnd.api+json", resp.Header.Get("Content-Type")) assert.Contains(t, operationResponses(oapi, "/doc", "get")["200"].(map[string]any)["content"], "application/vnd.api+json") } + +func TestRawResponse_ByValue(t *testing.T) { + app := fiber.New() + oapi := New(app) + Get(oapi, "/value", func(c fiber.Ctx, _ struct{}) (RawResponse, struct{}) { + return RawResponse{ContentType: "image/png", Body: []byte("png")}, struct{}{} + }, OpenAPIOptions{}) + + resp, err := app.Test(httptest.NewRequest("GET", "/value", nil)) + require.NoError(t, err) + raw, _ := io.ReadAll(resp.Body) + assert.Equal(t, 200, resp.StatusCode) + assert.Equal(t, "image/png", resp.Header.Get("Content-Type")) + assert.Equal(t, "png", string(raw), "a RawResponse by value is written as-is, not encoded as JSON") + assert.Contains(t, operationResponses(oapi, "/value", "get")["200"].(map[string]any)["content"], fiber.MIMEOctetStream) +} From 32fb3521134328e641f52643219b7e1a5198bf09 Mon Sep 17 00:00:00 2001 From: Jeremy Mouton Date: Mon, 19 Oct 2026 19:59:18 +0200 Subject: [PATCH 3/4] fix: treat a validator returning no AuthContext as a 401 A validator returning (nil, nil) made validateSecurityRequirement dereference a nil AuthContext and panic when security schemes are configured; with Bearer-only validation the request went on without a context. Every scheme validator (Bearer, Basic, API key, AWS SigV4) now turns such a result into an authentication failure, in routes and in the standalone middlewares alike. Co-Authored-By: Claude Opus 5.5 --- auth_schemes.go | 6 ++--- token_validator.go | 18 ++++++++++++-- token_validator_test.go | 55 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 74 insertions(+), 5 deletions(-) diff --git a/auth_schemes.go b/auth_schemes.go index 110bf66..dc0c13a 100644 --- a/auth_schemes.go +++ b/auth_schemes.go @@ -98,7 +98,7 @@ func validateBasicAuth(c fiber.Ctx, authService AuthorizationService) (*AuthCont return nil, fmt.Errorf("invalid Basic auth format: expected username:password") } - return basicValidator.ValidateBasicAuth(parts[0], parts[1]) + return requireAuthContext(basicValidator.ValidateBasicAuth(parts[0], parts[1])) } // validateAPIKey validates an API key from header, query, or cookie. @@ -124,7 +124,7 @@ func validateAPIKey(c fiber.Ctx, scheme SecurityScheme, authService Authorizatio return nil, fmt.Errorf("API key not found in %s parameter '%s'", scheme.In, scheme.Name) } - return apiKeyValidator.ValidateAPIKey(key, scheme.In, scheme.Name) + return requireAuthContext(apiKeyValidator.ValidateAPIKey(key, scheme.In, scheme.Name)) } // validateAWSSigV4 validates an AWS Signature V4 Authorization header. @@ -161,7 +161,7 @@ func validateAWSSigV4(c fiber.Ctx, authService AuthorizationService) (*AuthConte params.Headers[headerName] = c.Get(headerName) } - return awsValidator.ValidateAWSSignature(params) + return requireAuthContext(awsValidator.ValidateAWSSignature(params)) } // parseAWSSigV4Header parses an AWS SigV4 Authorization header into its components. diff --git a/token_validator.go b/token_validator.go index ef29c49..f507573 100644 --- a/token_validator.go +++ b/token_validator.go @@ -53,7 +53,21 @@ type ContextTokenValidator interface { // service implements ContextTokenValidator. func validateToken(c fiber.Ctx, authService AuthorizationService, token string) (*AuthContext, error) { if ctxValidator, ok := authService.(ContextTokenValidator); ok { - return ctxValidator.ValidateTokenCtx(c, token) + return requireAuthContext(ctxValidator.ValidateTokenCtx(c, token)) } - return authService.ValidateToken(token) + return requireAuthContext(authService.ValidateToken(token)) +} + +// errNoAuthContext reports a validator that accepted the credentials without +// returning an AuthContext. The request is treated as unauthenticated (401). +var errNoAuthContext = errors.New("authentication failed: the validator returned no authentication context") + +// requireAuthContext turns a (nil, nil) result of a validator into an +// authentication failure, so that nothing downstream dereferences a nil +// AuthContext. +func requireAuthContext(authCtx *AuthContext, err error) (*AuthContext, error) { + if err == nil && authCtx == nil { + return nil, errNoAuthContext + } + return authCtx, err } diff --git a/token_validator_test.go b/token_validator_test.go index f05b329..fd9c1c6 100644 --- a/token_validator_test.go +++ b/token_validator_test.go @@ -2,10 +2,12 @@ package fiberoapi import ( "errors" + "strings" "testing" "github.com/gofiber/fiber/v3" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func newTokenValidatorApp(t *testing.T, svc AuthorizationService) (*fiber.App, *OApiApp) { @@ -128,3 +130,56 @@ func TestContextTokenValidator_IsPreferred(t *testing.T) { assert.NoError(t, err) assert.Equal(t, 200, resp.StatusCode, "BearerTokenMiddleware prefers ValidateTokenCtx too") } + +// nilContextService accepts every credential without returning a context. +type nilContextService struct{ causeAuthService } + +func (nilContextService) ValidateToken(string) (*AuthContext, error) { return nil, nil } +func (nilContextService) ValidateBasicAuth(string, string) (*AuthContext, error) { return nil, nil } + +func TestNilAuthContext_IsAnAuthenticationFailure(t *testing.T) { + for name, schemes := range map[string]map[string]SecurityScheme{ + "bearer only": nil, + "security schemes": {"bearerAuth": {Type: "http", Scheme: "bearer"}}, + } { + t.Run(name, func(t *testing.T) { + app, decodes := newSpyApp() + oapi := New(app, Config{ + EnableValidation: true, + EnableAuthorization: true, + AuthService: nilContextService{}, + SecuritySchemes: schemes, + }) + registerAuthFirstRoute(oapi) + + assert.Equal(t, 401, postJSON(t, app, "/items", "any", `{"name": "widget"}`)) + assert.Zero(t, decodes.Load()) + }) + } + + basic := SecurityScheme{Type: "http", Scheme: "basic"} + for name, mw := range map[string]fiber.Handler{ + "BearerTokenMiddleware": BearerTokenMiddleware(nilContextService{}), + "SmartAuthMiddleware": SmartAuthMiddleware(nilContextService{}, Config{ + SecuritySchemes: map[string]SecurityScheme{"bearerAuth": {Type: "http", Scheme: "bearer"}}, + }), + "BasicAuthMiddleware": BasicAuthMiddleware(nilContextService{}), + "MultiSchemeAuthMiddleware basic": MultiSchemeAuthMiddleware(nilContextService{}, Config{ + SecuritySchemes: map[string]SecurityScheme{"basicAuth": basic}, + }), + } { + t.Run(name, func(t *testing.T) { + app := fiber.New() + app.Use(mw) + app.Get("/me", func(c fiber.Ctx) error { return c.SendString("ok") }) + + req := newBearerRequest("GET", "/me", "any") + if strings.Contains(name, "Basic") || strings.Contains(name, "basic") { + req.SetBasicAuth("user", "pass") + } + resp, err := app.Test(req) + require.NoError(t, err) + assert.Equal(t, 401, resp.StatusCode) + }) + } +} From 16e20b17cdfbcc0e2bca0dc8a950bdab62621503 Mon Sep 17 00:00:00 2001 From: Jeremy Mouton Date: Mon, 19 Oct 2026 19:59:18 +0200 Subject: [PATCH 4/4] docs: document the v3.3.1 fixes and fix the Method signature Path, query and header fields never come from the body, Problem works by value, and a validator returning no context is a 401. The Method example had its first two arguments swapped: the signature is Method(router, method, path, handler, options). Co-Authored-By: Claude Opus 5.5 --- README.md | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 9de2727..6df9b69 100644 --- a/README.md +++ b/README.md @@ -36,7 +36,7 @@ go get github.com/labbs/fiber-oapi/v3 > > Everything above is opt-in, except **three changes of behavior**: > 1. **[Authentication runs first.](#authentication-runs-first)** A protected route checks the credentials before reading its input: a request that fails authentication gets its 401 without its body being decoded, even if that body is malformed or invalid (it used to get a 400 or 422). -> 2. **Path, query and header values now win over the body.** The body used to be decoded after the path and query, and the JSON decoder matches keys case-insensitively: a field tagged `uri:"id"` without `json:"-"` could be overwritten by `{"id": "..."}` in the body. The body is now decoded first. +> 2. **Path, query and header values now win over the body.** The body used to be decoded after the path and query, and the JSON decoder matches keys case-insensitively: a field tagged `uri:"id"` without `json:"-"` could be overwritten by `{"id": "..."}` in the body. The body is now decoded first, and path, query and header fields are reset after it, so they never take a value from the body — not even when the parameter is absent from the request (fixed in v3.3.1). > 3. **`uri` fields no longer appear in the documented request body.** They were already documented as path parameters. > > Also: `SmartAuthMiddleware` and `MultiSchemeAuthMiddleware` now go through `Config.AuthErrorHandler` when one is set. @@ -136,7 +136,7 @@ fiberoapi.Put(router, path, handler, options) fiberoapi.Patch(router, path, handler, options) fiberoapi.Delete(router, path, handler, options) fiberoapi.Head(router, path, handler, options) -fiberoapi.Method(method, router, path, handler, options) // Custom HTTP method +fiberoapi.Method(router, method, path, handler, options) // Custom HTTP method ``` ## Parameter Types @@ -150,8 +150,11 @@ type MyInput struct { } ``` -The body is decoded first, then the path, query and header values are bound: -they always win over a body key with the same name. +Path, query and header fields never take their value from the body, even when +the parameter is absent from the request: the body is decoded first, those +fields are reset, then bound from the request. A field that also declares a +body tag (e.g. `query:"sort" json:"sort"`) accepts either, and the request +parameter wins when present. Special tags: - `body:"raw"` — Receive the request body as-is in a `[]byte` field (see [Raw request body](#raw-request-body-binary)) @@ -524,8 +527,8 @@ AuthErrorHandler: func(c fiber.Ctx, err *fiberoapi.AuthError) error { Any error value — a `TError`, an entry of `OpenAPIOptions.Errors` — can choose its `Content-Type` by implementing `ContentTyper`; `Problem` answers -`application/problem+json`. A handler can therefore return a `*Problem` as its -`TError`: +`application/problem+json`. A handler can therefore return a `Problem` or a +`*Problem` as its `TError`: ```go type ContentTyper interface{ ContentType() string } @@ -791,6 +794,9 @@ oapi := fiberoapi.New(app, fiberoapi.Config{ answers 500 (`ErrResourceAuthorizationNotConfigured`), never an access granted by default. +A validator that returns `(nil, nil)` — no error, but no `AuthContext` either — +is treated as a failed authentication (401), whatever the scheme. + A service that needs the request to validate a Bearer token (client IP, request context...) can implement `ContextTokenValidator`; it is then used instead of `ValidateToken`: