Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Goa panics when a user provides an explicit null where a Map is expected, if it's a nested structure #3483

Closed
paprikati opened this issue Feb 27, 2024 · 2 comments · Fixed by #3490
Closed

Goa panics when a user provides an explicit null where a Map is expected, if it's a nested structure #3483

paprikati opened this issue Feb 27, 2024 · 2 comments · Fixed by #3490

Comments

@paprikati
Copy link

Repro steps

design

Method("Create", func() {
	Payload(func() {
		Attribute("foo", MapOf(String, TestStruct))
		Required("foo")
	})

	HTTP(func() {
		POST("/")
		Response(StatusCreated)
	})
})


var TestStruct = CommonType("TestStruct", func() {
	Attribute("hello", TestStructTwo)
})

var TestStructTwo = CommonType("TestStructTwo", func() {
	Attribute("again", String)
})

payload

{"foo": {"bar": null } }

Expected behaviour

Some kind of human readable error, like struct at foo.bar` cannot be null

Resulting error:

             	/usr/local/go/src/runtime/panic.go:770 +0x124
             github.com/incident-io/core/server/api/public/gen/http/catalog_v2/server.unmarshalTestStructRequestBodyToCommonTestStruct(...)
             	/Users/lcurtis/repos/core/server/api/public/gen/http/catalog_v2/server/encode_decode.go:524
             github.com/incident-io/core/server/api/public/gen/http/catalog_v2/server.NewCreateEntryPayload(0x14004b19db0)
             	/Users/lcurtis/repos/core/server/api/public/gen/http/catalog_v2/server/types.go:590 +0x3a8
             github.com/incident-io/core/server/api/public/gen/http/catalog_v2/server.NewCreateEntryHandler.DecodeCreateEntryRequest.func2(0x14005218d80)
             	/Users/lcurtis/repos/core/server/api/public/gen/http/catalog_v2/server/encode_decode.go:256 +0x1d8
             github.com/incident-io/core/server/api/public/gen/http/catalog_v2/server.NewCreateEntryHandler.func1({0x10d9c85b0, 0x14001468a80}, 0x14005218d80)
             	/Users/lcurtis/repos/core/server/api/public/gen/http/catalog_v2/server/server.go:508 +0x1a4
             net/http.HandlerFunc.ServeHTTP(0x1400539db90, {0x10d9c85b0, 0x14001468a80}, 0x14005218d80)
             	/usr/local/go/src/net/http/server.go:2166 +0x40
             github.com/go-chi/chi/v5.(*Mux).routeHTTP(0x14004d79aa0, {0x10d9c85b0, 0x14001468a80}, 0x14005218d80)
             	/Users/lcurtis/go/pkg/mod/github.com/go-chi/chi/v5@v5.0.11/mux.go:443 +0x2e0
             net/http.HandlerFunc.ServeHTTP(0x1400538e5d0, {0x10d9c85b0, 0x14001468a80}, 0x14005218d80)
             	/usr/local/go/src/net/http/server.go:2166 +0x40
             github.com/go-chi/chi/v5.(*Mux).ServeHTTP(0x14004d79aa0, {0x10d9c85b0, 0x14001468a80}, 0x14005218d80)
             	/Users/lcurtis/go/pkg/mod/github.com/go-chi/chi/v5@v5.0.11/mux.go:90 +0x5d4
             github.com/incident-io/core/server/api.BuildHTTP.func6.AuthenticationSourceProvider.1({0x10d9c85b0, 0x14001468a80}, 0x14005218b40)
             	/Users/lcurtis/repos/core/server/api/mw/authentication.go:82 +0x18c
             net/http.HandlerFunc.ServeHTTP(0x1400544bd10, {0x10d9c85b0, 0x14001468a80}, 0x14005218b40)
             	/usr/local/go/src/net/http/server.go:2166 +0x40
             github.com/incident-io/core/server/cmd/app/cmd.Run.func61.Run.func61.ObserveZendeskHTTP.1.2({0x10d9c85b0, 0x14001468a80}, 0x14005218b40)
             	/Users/lcurtis/repos/core/server/api/mw/observe.go:196 +0x98
             net/http.HandlerFunc.ServeHTTP(0x14005458750, {0x10d9c85b0, 0x14001468a80}, 0x14005218b40)
             	/usr/local/go/src/net/http/server.go:2166 +0x40
             github.com/incident-io/core/server/cmd/app/cmd.Run.func62.Run.func62.ObserveMobileHTTP.1.2({0x10d9c85b0, 0x14001468a80}, 0x14005218b40)
             	/Users/lcurtis/repos/core/server/api/mw/observe.go:174 +0xc8
             net/http.HandlerFunc.ServeHTTP(0x1400544bd28, {0x10d9c85b0, 0x14001468a80}, 0x14005218b40)
             	/usr/local/go/src/net/http/server.go:2166 +0x40
             github.com/incident-io/core/server/cmd/app/cmd.Run.func63.AuthenticationFromStore.1({0x10d9c85b0, 0x14001468a80}, 0x14005218a20)
             	/Users/lcurtis/repos/core/server/api/mw/authentication.go:133 +0x124
             net/http.HandlerFunc.ServeHTTP(0x1400544bd40, {0x10d9c85b0, 0x14001468a80}, 0x14005218a20)
             	/usr/local/go/src/net/http/server.go:2166 +0x40
             github.com/incident-io/core/server/cmd/app/cmd.Run.func64.Run.func64.WithSession.1.2({0x10d9c85b0, 0x14001468a80}, 0x140052187e0)
             	/Users/lcurtis/repos/core/server/api/mw/sessions.go:283 +0x3e0
             net/http.HandlerFunc.ServeHTTP(0x14005458780, {0x10d9c85b0, 0x14001468a80}, 0x140052187e0)
             	/usr/local/go/src/net/http/server.go:2166 +0x40
             github.com/incident-io/core/server/cmd/app/cmd.Run.func65.Run.func65.AuthenticationFromToken.1.2({0x10d9c85b0, 0x14001468a80}, 0x14005041d40)
             	/Users/lcurtis/repos/core/server/api/mw/authentication.go:105 +0x224
             net/http.HandlerFunc.ServeHTTP(0x140054587b0, {0x10d9c85b0, 0x14001468a80}, 0x14005041d40)
             	/usr/local/go/src/net/http/server.go:2166 +0x40
             github.com/incident-io/core/server/api/mw.SinglePageApp.func1.1({0x10d9c85b0, 0x14001468a80}, 0x14005041d40)
             	/Users/lcurtis/repos/core/server/api/mw/single_page_app.go:23 +0xb0
             net/http.HandlerFunc.ServeHTTP(0x14005387860, {0x10d9c85b0, 0x14001468a80}, 0x14005041d40)
             	/usr/local/go/src/net/http/server.go:2166 +0x40
             github.com/incident-io/core/server/cmd/app/cmd.Run.func67.APIRewrite.1({0x10d9c85b0, 0x14001468a80}, 0x14005041d40)
             	/Users/lcurtis/repos/core/server/api/mw/api_rewrite.go:33 +0x334
             net/http.HandlerFunc.ServeHTTP(0x1400544bd58, {0x10d9c85b0, 0x14001468a80}, 0x14005041d40)
             	/usr/local/go/src/net/http/server.go:2166 +0x40
             github.com/incident-io/core/server/api/mw.ApplySecurity.(*Secure).Handler.func1({0x10d9c85b0, 0x14001468a80}, 0x14005041d40)
             	/Users/lcurtis/go/pkg/mod/github.com/unrolled/secure@v1.13.0/secure.go:203 +0xf8
             net/http.HandlerFunc.ServeHTTP(0x1400544eba0, {0x10d9c85b0, 0x14001468a80}, 0x14005041d40)
             	/usr/local/go/src/net/http/server.go:2166 +0x40
             github.com/NYTimes/gziphandler.GzipHandlerWithOpts.func1.1({0x10d9d35f0, 0x1400161ea80}, 0x14005041d40)
             	/Users/lcurtis/go/pkg/mod/github.com/!n!y!times/gziphandler@v1.1.1/gzip.go:338 +0x424
             net/http.HandlerFunc.ServeHTTP(0x14005458930, {0x10d9d35f0, 0x1400161ea80}, 0x14005041d40)
             	/usr/local/go/src/net/http/server.go:2166 +0x40
             github.com/incident-io/core/server/cmd/app/cmd.Run.func70.CacheResponses.1({0x10d9d35f0, 0x1400161ea80}, 0x14005041d40)
             	/Users/lcurtis/repos/core/server/api/mw/cache_responses.go:58 +0x1a4
             net/http.HandlerFunc.ServeHTTP(0x14005454ac0, {0x10d9d35f0, 0x1400161ea80}, 0x14005041d40)
             	/usr/local/go/src/net/http/server.go:2166 +0x40
             github.com/incident-io/core/server/cmd/app/cmd.Run.func71.NoCache.1({0x10d9d35f0, 0x1400161ea80}, 0x14005041d40)
             	/Users/lcurtis/repos/core/server/api/mw/no_cache.go:25 +0x10c
             net/http.HandlerFunc.ServeHTTP(0x1400544bd70, {0x10d9d35f0, 0x1400161ea80}, 0x14005041d40)
             	/usr/local/go/src/net/http/server.go:2166 +0x40
             go.opencensus.io/plugin/ochttp.(*Handler).ServeHTTP(0x14005387950, {0x10d9d35f0, 0x1400161ea80}, 0x14005041d40)
             	/Users/lcurtis/go/pkg/mod/go.opencensus.io@v0.24.0/plugin/ochttp/server.go:92 +0x39c
             github.com/incident-io/core/server/api/mw.observeHTTP.func1.2({0x10d9c83a0, 0x14004c750a0}, 0x140050419e0)
             	/Users/lcurtis/repos/core/server/api/mw/observe.go:326 +0x1424
             net/http.HandlerFunc.ServeHTTP(0x14005458960, {0x10d9c83a0, 0x14004c750a0}, 0x140050410e0)
             	/usr/local/go/src/net/http/server.go:2166 +0x40
             github.com/getsentry/sentry-go/http.(*Handler).Handle.(*Handler).handle.func1({0x10d9c83a0, 0x14004c750a0}, 0x140050410e0)
             	/Users/lcurtis/go/pkg/mod/github.com/incident-io/sentry-go@v0.0.0-20230906150740-4d409645eb55/http/sentryhttp.go:116 +0x4bc
             net/http.HandlerFunc.ServeHTTP(0x1400544ebc0, {0x10d9c83a0, 0x14004c750a0}, 0x14005040a20)
             	/usr/local/go/src/net/http/server.go:2166 +0x40
             goa.design/goa/v3/http/middleware.RequestID.func1.1({0x10d9c83a0, 0x14004c750a0}, 0x14005040900)
             	/Users/lcurtis/go/pkg/mod/goa.design/goa/v3@v3.14.6/http/middleware/requestid.go:43 +0x29c
             net/http.HandlerFunc.ServeHTTP(0x14005454b00, {0x10d9c83a0, 0x14004c750a0}, 0x14005040900)
             	/usr/local/go/src/net/http/server.go:2166 +0x40
             net/http.serverHandler.ServeHTTP({0x14005212960}, {0x10d9c83a0, 0x14004c750a0}, 0x14005040900)
             	/usr/local/go/src/net/http/server.go:3137 +0x2a0
             net/http.(*conn).serve(0x140055463f0, {0x10d9dce40, 0x140052e0a50})
             	/usr/local/go/src/net/http/server.go:2039 +0x1180
             created by net/http.(*Server).Serve in goroutine 420
             	/usr/local/go/src/net/http/server.go:3285 +0xa84
@raphael raphael mentioned this issue Mar 10, 2024
Merged
@raphael
Copy link
Member

raphael commented Mar 10, 2024

This is fixed by #3490. Note that the fix does not return an error when an entry is nil as that could be a valid case. Instead the user code should check and return an error if that's not expected. Unfortunately there's no good way to define a validation for this specific case (not something that JSON schema or OpenAPI supports).

@paprikati
Copy link
Author

Yep that's all good, thanks so much for the fix!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Properly handle decoding of maps with nil entries goadesign/goa
2 participants
@raphael @paprikati