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.

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

Gateway server panics on invalid Grpc-Timeout value #2822

Closed
alkapov opened this issue Jul 29, 2022 · 2 comments · Fixed by #2823
Closed

Gateway server panics on invalid Grpc-Timeout value #2822

alkapov opened this issue Jul 29, 2022 · 2 comments · Fixed by #2823

Comments

@alkapov
Copy link

alkapov commented Jul 29, 2022

🐛 Bug Report

Server panics when receives request with invalid Grpc-Timeout value.

Let's consider following code from example:

ctx, err = runtime.AnnotateIncomingContext(ctx, mux, req, "/grpc.gateway.examples.internal.proto.examplepb.EchoService/EchoBody", runtime.WithHTTPPathPattern("/v1/example/echo_body"))
if err != nil {
runtime.HTTPError(ctx, mux, outboundMarshaler, w, req, err)
return
}

runtime.AnnotateIncomingContext may return ctx == nil if err != nil.
But then ctx is passed into runtime.HTTPError(ctx, ...), which in default case calls runtime.DefaultHTTPErrorHandler(ctx, ...), which calls ServerMetadataFromContext(ctx).

The latter tries to get value from that context and panics, given that the context is nil.

func ServerMetadataFromContext(ctx context.Context) (md ServerMetadata, ok bool) {
md, ok = ctx.Value(serverMetadataKey{}).(ServerMetadata)
return
}

To Reproduce

  1. Clone repo at last revision (tested rev: 1911ac0).
  2. Run servers:
    • go run examples/internal/cmd/example-grpc-server/main.go
    • go run examples/internal/cmd/example-gateway-server/main.go
  3. Ensure that everything is OK.
    $ curl --location --request POST 'http://127.0.0.1:8080/v1/example/echo_body' \
      --header 'Grpc-Timeout: 1S' \
      --header 'Content-Type: application/json' \
      --data-raw '{"num": 1}'
    # returns
    {"id":"", "num":"1", "status":null}
  4. Invalidate Grpc-Timeout and call again.
    $ curl --location --request POST 'http://127.0.0.1:8080/v1/example/echo_body' \
      --header 'Grpc-Timeout: INVALID' \
      --header 'Content-Type: application/json' \
      --data-raw '{"num": 1}'
    # fails
    curl: (52) Empty reply from server

Server log:

2022/07/29 17:34:44 http: panic serving 127.0.0.1:58784: runtime error: invalid memory address or nil pointer dereference
goroutine 30 [running]:
net/http.(*conn).serve.func1()
	/opt/homebrew/Cellar/go/1.18.4/libexec/src/net/http/server.go:1825 +0xb0
panic({0x102d7c8e0, 0x1031ad1a0})
	/opt/homebrew/Cellar/go/1.18.4/libexec/src/runtime/panic.go:844 +0x258
github.com/grpc-ecosystem/grpc-gateway/v2/runtime.ServerMetadataFromContext(...)
	/dev/src/github.com/grpc-ecosystem/grpc-gateway/runtime/context.go:186
github.com/grpc-ecosystem/grpc-gateway/v2/runtime.DefaultHTTPErrorHandler({0x0, 0x0}, 0x14000147508?, {0x102e2c6e0, 0x1031acfa0}, {0x102e2b920?, 0x14000276700}, 0x2?, {0x102e27a28, 0x14000132418})
	/dev/src/github.com/grpc-ecosystem/grpc-gateway/runtime/errors.go:125 +0x50c
github.com/grpc-ecosystem/grpc-gateway/v2/runtime.HTTPError(...)
	/dev/src/github.com/grpc-ecosystem/grpc-gateway/runtime/errors.go:81
github.com/grpc-ecosystem/grpc-gateway/v2/examples/internal/proto/examplepb.RegisterEchoServiceHandlerClient.func6({0x102e2b920, 0x14000276700}, 0x14000240400, 0x14000123000?)
	/dev/src/github.com/grpc-ecosystem/grpc-gateway/examples/internal/proto/examplepb/echo_service.pb.gw.go:1171 +0x278
github.com/grpc-ecosystem/grpc-gateway/v2/runtime.(*ServeMux).ServeHTTP(0x140001b4600, {0x102e2b920, 0x14000276700}, 0x14000240400)
	/dev/src/github.com/grpc-ecosystem/grpc-gateway/runtime/mux.go:386 +0xd8c
net/http.(*ServeMux).ServeHTTP(0x14000251770?, {0x102e2b920, 0x14000276700}, 0x14000240400)
	/opt/homebrew/Cellar/go/1.18.4/libexec/src/net/http/server.go:2462 +0x144
github.com/grpc-ecosystem/grpc-gateway/v2/examples/internal/gateway.allowCORS.func1({0x102e2b920, 0x14000276700}, 0x14000240400)
	/dev/src/github.com/grpc-ecosystem/grpc-gateway/examples/internal/gateway/handlers.go:41 +0x1b8
net/http.HandlerFunc.ServeHTTP(0x0?, {0x102e2b920?, 0x14000276700?}, 0x10277e0d0?)
	/opt/homebrew/Cellar/go/1.18.4/libexec/src/net/http/server.go:2084 +0x3c
net/http.serverHandler.ServeHTTP({0x102e2a758?}, {0x102e2b920, 0x14000276700}, 0x14000240400)
	/opt/homebrew/Cellar/go/1.18.4/libexec/src/net/http/server.go:2916 +0x3fc
net/http.(*conn).serve(0x14000274280, {0x102e2be48, 0x14000250870})
	/opt/homebrew/Cellar/go/1.18.4/libexec/src/net/http/server.go:1966 +0x56c
created by net/http.(*Server).Serve
	/opt/homebrew/Cellar/go/1.18.4/libexec/src/net/http/server.go:3071 +0x450

Expected behavior

Server should return error like that:

{
    "code": 3,
    "message": "invalid grpc-timeout: INVALID",
    "details": []
}

Actual Behavior

Server panics, no response returned.

Your Environment

macOS 12.4
go version go1.18.4 darwin/arm64
repo revision 1911ac0

johanbrandhorst added a commit that referenced this issue Jul 30, 2022
This will fix #2822 for users who only update their runtime
johanbrandhorst added a commit that referenced this issue Jul 30, 2022
This will fix #2822 for users who only update their generator
@johanbrandhorst
Copy link
Collaborator

Thank you for this bug report. I consider this a very bad bug so I've gone ahead and submitted a double fix myself in #2823.

johanbrandhorst added a commit that referenced this issue Jul 30, 2022
This will fix #2822 for users who only update their generator
johanbrandhorst added a commit that referenced this issue Jul 30, 2022
* Handle nil contexts in Context methods

This will fix #2822 for users who only update their runtime

* Avoid passing nil context to runtime.HTTPError

This will fix #2822 for users who only update their generator
@johanbrandhorst
Copy link
Collaborator

We've released a fix for this in https://github.com/grpc-ecosystem/grpc-gateway/releases/tag/v2.11.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants