danielgtaylor / danielgtaylor/huma

Incorrect `allOf` validation.

Open
#787 2 comments 0 reactions 0 assignees View on GitHub
question
Dominant language
Go
Stars
4.4k
Forks
285
Avg merge
40m
Merged PRs (30d)
1

Description

Hi there,

I have the following types:

**main.go**

```go
type Base struct {
Name string `json:"name" doc:"The name to compare." minLength:"1" maxLength:"10"`
Age int `json:"age" doc:"The age to compare." minimum:"0" maximum:"100"`
}

type Address string
type Extended struct {
Base
Address Address `json:"address" doc:"The address to compare." minLength:"1" maxLength:"100"`
}

func (e Extended) TransformSchema(r huma.Registry, s *huma.Schema) *huma.Schema {
s.Properties = nil
s.Type = ""
s.Description = "Extended Schema."
s.AllOf = []*huma.Schema{
r.Schema(reflect.TypeOf(Base{}), true, ""),
{
Type: "object",
Properties: map[string]*huma.Schema{
"address": r.Schema(reflect.TypeOf(Address("")), true, ""),
},
},
}
return s
}

type Input struct {
Body struct {
Extended
}
}

type Output struct {
Body struct {
Message string `json:"message"`
}
}

func addRoutes(api huma.API) {
huma.Post(api, "/user", func(ctx context.Context, input *Input) (*Output, error) {
resp := &Output{}
resp.Body.Message = "It works!"
return resp, nil
})
}
```

Here's the full generated spec.

spec

```yaml
```yaml
components:
schemas:
Base:
additionalProperties: false
properties:
age:
description: The age to compare.
format: int64
maximum: 100
minimum: 0
type: integer
name:
description: The name to compare.
maxLength: 10
minLength: 1
type: string
required:
- name
- age
type: object
ErrorDetail:
additionalProperties: false
properties:
location:
description: Where the error occurred, e.g. 'body.items[3].tags' or 'path.thing-id'
type: string
message:
description: Error message text
type: string
value:
description: The value at the given location
type: object
ErrorModel:
additionalProperties: false
properties:
$schema:
description: A URL to the JSON Schema for this object.
examples:
- https://example.com/schemas/ErrorModel.json
format: uri
readOnly: true
type: string
detail:
description: A human-readable explanation specific to this occurrence of the problem.
examples:
- Property foo is required but is missing.
type: string
errors:
description: Optional list of individual error details
items:
$ref: "#/components/schemas/ErrorDetail"
type:
- array
- "null"
instance:
description: A URI reference that identifies the specific occurrence of the problem.
examples:
- https://example.com/error-log/abc123
format: uri
type: string
status:
description: HTTP status code
examples:
- 400
format: int64
type: integer
title:
description: A short, human-readable summary of the problem type. This value should not change between occurrences of the error.
examples:
- Bad Request
type: string
type:
default: about:blank
description: A URI reference to human-readable documentation for the error.
examples:
- https://example.com/errors/example
format: uri
type: string
type: object
InputBody:
additionalProperties: false
allOf:
- $ref: "#/components/schemas/Base"
- properties:
address:
type: string
type: object
description: Extended Schema.
required:
- address
- name
- age
OutputBody:
additionalProperties: false
properties:
$schema:
description: A URL to the JSON Schema for this object.
examples:
- https://example.com/schemas/OutputBody.json
format: uri
readOnly: true
type: string
message:
type: string
required:
- message
type: object
info:
title: My API
version: 1.0.0
openapi: 3.1.0
paths:
/user:
post:
operationId: post-user
requestBody:
content:
application/json:
schema:
$ref: "#/components/schemas/InputBody"
required: true
responses:
"200":
content:
application/json:
schema:
$ref: "#/components/schemas/OutputBody"
description: OK
default:
content:
application/problem+json:
schema:
$ref: "#/components/schemas/ErrorModel"
description: Error
summary: Post user

```

What I'm trying to achieve is a types' structure that generates OAS with `allOf` that reuses a shared schema.

```yaml
InputBody:
additionalProperties: false
allOf:
- $ref: "#/components/schemas/Base"
- properties:
address:
type: string
type: object
description: Schema for the DummyFilter.
required:
- address
- name
- age

Base:
additionalProperties: false
properties:
age:
description: The age to compare.
format: int64
maximum: 100
minimum: 0
type: integer
name:
description: The name to compare.
maxLength: 10
minLength: 1
type: string
required:
- name
- age
type: object
```

I need such OAS in the following pipeline steps.

**The issue**

main_test.go

```go
func TestUserCreate(t *testing.T) {
_, api := humatest.New(t, huma.DefaultConfig("Test API", "1.0.0"))
addRoutes(api)

_ = api.Post("/user", map[string]any{
"name": "ValidName",
"age": 1,
"address": "ValidAddress",
})
}
```

When I send a valid request ( with all three fields ) I get a validation error.

```sh
$> go test ./...

=== RUN TestUserCreate
main_test.go:14: Making request:
POST /user HTTP/1.1
Content-Type: application/json

{
"address": "ValidAddress",
"age": 1,
"name": "ValidName"
}
main_test.go:14: Got response:
HTTP/1.1 422 Unprocessable Entity
Connection: close
Content-Type: application/problem+json
Link: ; rel="describedBy"

{
"$schema": "https:///schemas/ErrorModel.json",
"title": "Unprocessable Entity",
"status": 422,
"detail": "validation failed",
"errors": [
{
"message": "unexpected property",
"location": "body.address",
"value": {
"address": "ValidAddress",
"age": 1,
"name": "ValidName"
}
}
]
}
```

The `address` field is unknown.

My internal research showed that internally Huma iterates over all `allOf` schemas by validating input against each of them ( [ref](https://github.com/danielgtaylor/huma/blob/main/validate.go#L378-L382) ) which doesn't look correctly.

In my opinion, the correct behavior is to merge all `allOf` schemas into a single one ( at least `properties` ) and run `Validate.`

This is how I fixed it internally (a quite naive approach, not well-tested yet )
```diff
diff --git a/validate.go b/validate.go
index 336817c..13dbf4f 100644
--- a/validate.go
+++ b/validate.go
@@ -376,9 +376,7 @@ func Validate(r Registry, s *Schema, path *PathBuffer, mode ValidateMode, v any,
}

if s.AllOf != nil {
- for _, sub := range s.AllOf {
- Validate(r, sub, path, mode, v, res)
- }
+ Validate(r, mergeAllOf(s.AllOf, r), path, mode, v, res)
}

if s.Not != nil {
@@ -560,6 +558,30 @@ func Validate(r Registry, s *Schema, path *PathBuffer, mode ValidateMode, v any,
}
}

+func mergeAllOf(allOf []*Schema, r Registry) *Schema {
+ if len(allOf) == 0 {
+ return nil
+ }
+
+ var merged *Schema
+ for _, s := range allOf {
+ if s.Ref != "" {
+ s = r.SchemaFromRef(s.Ref)
+ }
+
+ if merged == nil {
+ merged = s
+ continue
+ }
+
+ for k, v := range s.Properties {
+ merged.Properties[k] = v
+ }
+ }
+
+ return merged
+}
+
func handleArray[T any](r Registry, s *Schema, path *PathBuffer, mode ValidateMode, res *ValidateResult, arr []T) {
if s.MinItems != nil {
if len(arr) < *s.MinItems {

```

What do you think? Is there a better approach for handling such cases?

Thank you.

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.