Skip to content

Faulty duplicate detection for request and response types at app initialization #894

@Mcklmo

Description

@Mcklmo

This code causes a runtime panic:

r := mux.NewRouter()
api := New(r, huma.DefaultConfig("Test", "1.0.0"))

var (
	op1 = huma.Operation{Method: http.MethodGet, Path: "/1"} // no OperationID
	op2 = huma.Operation{Method: http.MethodGet, Path: "/2"} // no OperationID
)

huma.Register(api, op1, func(ctx context.Context, i *struct { // inline request input type definition
	Body struct {
		Test1 string // field name varying
	}
},
) (*struct{}, error,
) {
	return nil, nil
})

huma.Register(api, op2, func(ctx context.Context, i *struct { // inline request input type definition
	Body struct {
		Test2 string // field name varying
	}
},
) (*struct{}, error,
) {
	return nil, nil
})

I added a fix in this PR. I used the above code in a unit test in the PR. The output of the test prior to my fix can be seen below

The panic

--- FAIL: TestOperationWithoutIDAndInlineRequestTypeDefinitionNotPanics (0.00s)
panic: duplicate name: Request, new type: struct { Test2 string }, existing type: struct { Test1 string } [recovered, repanicked]

goroutine 22 [running]:
testing.tRunner.func1.2({0x10510b440, 0x1400009fd00})
	/usr/local/go/src/testing/testing.go:1872 +0x190
testing.tRunner.func1()
	/usr/local/go/src/testing/testing.go:1875 +0x31c
panic({0x10510b440?, 0x1400009fd00?})
	/usr/local/go/src/runtime/panic.go:783 +0x120
github.com/danielgtaylor/huma/v2.(*mapRegistry).Schema(0x140000c6f00, {0x10516fda0, 0x1051166a0}, 0x1, {0x105064408, 0x7})
	/Users/moritzmarcushonscheidt/Projects/opensource/huma/registry.go:109 +0x4b0
github.com/danielgtaylor/huma/v2.SchemaFromField({0x10516c700, 0x140000c6f00}, {{0x1050d8719, 0x4}, {0x0, 0x0}, {0x10516fda0, 0x1051166a0}, {0x0, 0x0}, ...}, ...)
	/Users/moritzmarcushonscheidt/Projects/opensource/huma/schema.go:534 +0x64
github.com/danielgtaylor/huma/v2.setRequestBodyFromBody(0x1400018e640, {0x10516c700, 0x140000c6f00}, {{0x1050d8719, 0x4}, {0x0, 0x0}, {0x10516fda0, 0x1051166a0}, {0x0, ...}, ...}, ...)
	/Users/moritzmarcushonscheidt/Projects/opensource/huma/huma.go:1228 +0x228
github.com/danielgtaylor/huma/v2.processInputType({0x10516fda0, 0x105116720}, 0x1400018e640, {0x10516c700, 0x140000c6f00})
	/Users/moritzmarcushonscheidt/Projects/opensource/huma/huma.go:1165 +0x12c
github.com/danielgtaylor/huma/v2.Register[...]({_, _}, {{0x1050635cd, 0x3}, {0x10506347d, 0x2}, 0x0, 0x0, 0x0, {0x0, ...}, ...}, ...)
	/Users/moritzmarcushonscheidt/Projects/opensource/huma/huma.go:645 +0x1a0
github.com/danielgtaylor/huma/v2/adapters/humamux.TestOperationWithoutIDAndInlineRequestTypeDefinitionNotPanics(0x14000093340?)
	/Users/moritzmarcushonscheidt/Projects/opensource/huma/adapters/humamux/humagmux_test.go:156 +0x1d0
testing.tRunner(0x14000093340, 0x105166cb0)
	/usr/local/go/src/testing/testing.go:1934 +0xc8
created by testing.(*T).Run in goroutine 1
	/usr/local/go/src/testing/testing.go:1997 +0x364
FAIL	github.com/danielgtaylor/huma/v2/adapters/humamux	0.252s
FAIL

Almost semantically equivalent cases that do not cause a panic

The bug does not occur, if the type is defined as an actual type instead of being inlined in the call to Register. E.g., this works fine:

r := mux.NewRouter()
api := New(r, huma.DefaultConfig("Test", "1.0.0"))

var (
	op1 = huma.Operation{Method: http.MethodGet, Path: "/1"} // no OperationID
	op2 = huma.Operation{Method: http.MethodGet, Path: "/2"} // no OperationID
)

type (
	i1 struct {
		Body struct {
			Test1 string // field name varying
		}
	}
	i2 struct {
		Body struct {
			Test2 string // field name varying
		}
	}
)

huma.Register(api, op1, func(ctx context.Context, I *i1) (*struct{}, error,
) {
	return nil, nil
})

huma.Register(api, op2, func(ctx context.Context, i *i2) (*struct{}, error,
) {
	return nil, nil
})

Also, the bug is not triggered, if a unique OperationID is provided:

r := mux.NewRouter()
api := New(r, huma.DefaultConfig("Test", "1.0.0"))

var (
	op1 = huma.Operation{Method: http.MethodGet, Path: "/1", OperationID: "op1"}
	op2 = huma.Operation{Method: http.MethodGet, Path: "/2", OperationID: "op2"}
)

huma.Register(api, op1, func(ctx context.Context, i *struct { // inline request input type definition
	Body struct {
		Test1 string // field name varying
	}
},
) (*struct{}, error,
) {
	return nil, nil
})

huma.Register(api, op2, func(ctx context.Context, i *struct { // inline request input type definition
	Body struct {
		Test2 string // field name varying
	}
},
) (*struct{}, error,
) {
	return nil, nil
})

Reflection on alternative solutions

I think that an alternative to the fix in my PR would be to provide an improved panic message. But in my opinion, the problem stems from the fact that operations with an empty OperationID are tolerated in the first place. This is, however, more of an architectural issue and I felt like at least I can propose some kind of solution that definitely not breaks any existing codebases.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions