fix: avoid faulty duplicate detection

[Mcklmo]

The bug is caused by registering operations with inline struct definitions with varying field names, combined with an empty OperationID.

I noticed this causing a runtime panic on app start. It is no actual security vulnerability, because it is not possible to trigger during app uptime - it only triggers during the initialisation phase. But it is an irritating error to me as a user, as the panic message does not really make sense to me.

I added a test for reproducability. To see the problem in action, just run the tests on your current version, without my fix to the code.

I fixed the error by allowing multiple inline struct definitions with varying field names. What would have caused a duplicate panic before now increments the would be duplicate by one. E.g., the first type will be called "Request", the second "Request1", and so on. That way the behavior is still deterministic and humanly readable in a generated spec. This change does not affect existing code bases because the condition the change has effect on was previously guarded by the runtime panic. I.e., no configuration can have existed that was able to initialise the app with this condition before. Hence, the change will be entirely backwards compatible.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.20%. Comparing base (4e53fef) to head (4a1230a).

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #893      +/-   ##
==========================================
+ Coverage   93.15%   93.20%   +0.04%     
==========================================
  Files          23       23              
  Lines        4955     4961       +6     
==========================================
+ Hits         4616     4624       +8     
+ Misses        275      274       -1     
+ Partials       64       63       -1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[wolveix]

Hey @Mcklmo! Sorry this has sat for so long.

I agree that the underlying issue could be improved; however, I'm not sure this is the best approach. We generally avoid adding anything to our external interface definitions (see #925). Would you be open to reworking this to try to avoid this?

[Mcklmo]

Hi, I see your point. How about we pivot on no fix, but a more developer friendly error message. I could take a shot at that. Let me know what you think.

[wolveix]

Honestly while an improved error message might help, I think fixing the underlying issue would still be better if we can find an elegant way to implement it :D

[Mcklmo]

The underlying issue stems from the fact that empty OperationIDs are legal in the first place and the hint creation logic creates exact copies of the hints. The bug is quite rare, also because it only occurs if the operations are registered with input or output types defined as inline structs.

IMO, OperationIDs should be required and unique: that would fix the issue entirely. However, that would of course change the external API substantially, which I agree is a bad idea.

What I propose in the PR is a deterministic hint creation logic that only has effect for apps that currently don't exist, because they would panic on start. So I actually think that this a fix with an absolute minimal impact on the external API.

Do you have another idea how this could be approached?

[wolveix]

It is worth mentioning that since #937, operation IDs must be unique. Perhaps I'm slightly misunderstanding (just getting back from being away from a few weeks); would you mind providing a code sample that triggers this issue, please? Here's a base example I use for testing:

package main

import (
	"context"
	"fmt"
	"net/http"

	"github.com/danielgtaylor/huma/v2"
	"github.com/danielgtaylor/huma/v2/adapters/humachi"
	"github.com/danielgtaylor/huma/v2/humacli"
	"github.com/go-chi/chi/v5"

	_ "github.com/danielgtaylor/huma/v2/formats/cbor"
)

// Options for the CLI.
type Options struct {
	Port int `help:"Port to listen on" short:"p" default:"8888"`
}

func main() {
	// Create a CLI app which takes a port option.
	cli := humacli.New(func(hooks humacli.Hooks, options *Options) {
		// Create a new router & API
		router := chi.NewMux()
		cfg := huma.DefaultConfig("My API", "1.0.0")
		cfg.DocsRenderer = huma.DocsRendererScalar
		api := humachi.New(router, cfg)

		// Register GET /greeting/{name}.
		huma.Register(api, huma.Operation{
			OperationID: "get-greeting",
			Summary:     "Get a greeting",
			Method:      http.MethodGet,
			Path:        "/greeting/{name}",
		}, func(ctx context.Context, input *struct {
			Name string `path:"name" minLength:"1" maxLength:"255" pattern:"^(?:[^%]|%[0-9A-Fa-f]{2})+$" example:"lol" doc:"The path of the directory that is URL encoded"`
		}) (*struct{}, error) {
			return nil, nil
		})

		// Register GET /greeting2/{name}.
		huma.Register(api, huma.Operation{
			OperationID: "get-greeting-2",
			Summary:     "Get a greeting",
			Method:      http.MethodPost,
			Path:        "/greeting2/{name}",
		}, func(ctx context.Context, input *struct {
			Name string `path:"name" minLength:"1" maxLength:"255" pattern:"^(?:[^%]|%[0-9A-Fa-f]{2})+$" example:"lol" doc:"The path of the directory that is URL encoded"`
		}) (*struct{}, error) {
			return nil, nil
		})

		// Tell the CLI how to start your router.
		hooks.OnStart(func() {
			http.ListenAndServe(fmt.Sprintf(":%d", options.Port), router)
		})
	})

	// Run the CLI. When passed no commands, it starts the server.
	cli.Run()
}

[Mcklmo]

That actually sounds promising, I didn't know that. You can try running the unit tests I added to the PR. Back when I created the PR, they triggered the bug, before my fix.

[wolveix]

Sorry for leaving this so long, and thanks for the fix and the repro tests. I reworked the implementation a bit to address a couple of issues:

• The name-only lookup didn't account for type identity, so two operations sharing an identical inline body (same reflect.Type) got split into Request and Request1 instead of deduplicating like before.
• NameExistsInSchema on the Registry interface is a breaking change for custom implementations.

I moved the collision handling into mapRegistry.Schema, where the seen set already tracks type identity: identical types reuse the existing ref, distinct anonymous types get a numeric suffix, and named-type collisions still panic. No interface change. Kept your repro tests and added registry-level ones.

Will give this a final pass tomorrow when I'm not so tired, and then merge and include in our next release 🎉

[Mcklmo]

Awesome @wolveix, thank you for following through on this! 😊

I'm glad I could contribute with identifying the bug and adding tests.