search

goadesign / goa

🌟 Goa: Elevate Go API development! 🚀 Streamlined design, automatic code generation, and seamless HTTP/gRPC support. ✨
https://goa.design
MIT License
5.62k stars 557 forks source link

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

Closed paprikati closed 6 months ago

paprikati commented 6 months ago

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 atfoo.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 commented 6 months ago

This is fixed by https://github.com/goadesign/goa/pull/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 commented 6 months ago

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