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

Call grpc.Invoke with nil "opts ...CallOption" throw a "invalid memory address or nil pointer dereference" and panics #7000

Closed
brunoluizkatz-NETZSCH opened this issue Feb 23, 2024 · 3 comments

Comments

@brunoluizkatz-NETZSCH
Copy link

brunoluizkatz-NETZSCH commented Feb 23, 2024

What version of gRPC are you using?

v1.58.2

What version of Go are you using (go version)?

$ go version
go version go1.20.2 darwin/arm64

$ protoc --version                                                                                                                                                                                                                                                                                                                                    !10024
libprotoc 24.4

What operating system (Linux, Windows, …) and version?

Linux (docker build image: golang:1.20-alpine) and Mac Sonoma 14.0 (23A344)

What did you do?

Call to Invoke at call.go#L29 function with a nil opts ...CallOption parameter will panic the application.

This invoke with nil value at opts ...CallOption throw the invalid memory address or nil pointer dereference error. This "error" happens at v1.58.2 grpc package version or the latest.

E.g:

s.myGrpcApi.GetSomething(ctx, &service.GetSomethingRequest{
		Somethin: something.GetId(),
	}, nil, nil)

will call

func (cc *ClientConn) Invoke(ctx context.Context, method string, args, reply any, opts ...CallOption) error // ...

call.go#L29

To fix this, we simple remove the nil, nil option parameters to cc *ClientConn, opts ...CallOption values.

I don't know if this is a bug or a feature request.

What did you expect to see?

Ignoring the options if is []nil or one of than is nil. Apparently variadic parameters consider nil as []nil (see https://go.dev/ref/spec#Passing_arguments_to_..._parameters)

What did you see instead?

An nil pointer error at stream.go#L281 triggering the defer function above at stream.go#L274 who eventually finish the application with a panic.

@arvindbr8
Copy link
Member

arvindbr8 commented Feb 26, 2024

go.dev/ref/spec#Passing_arguments_to_..._parameters

I'm not seeing where it exactly talks about the nil handling there. I don't think Invoke() should handle nil being passed along opts.

For an example like https://go.dev/play/p/HxapK0toPq8 -

package main

import "fmt"

type CallOption interface {
	before(*string) error
	after(*string, *string)
}

type EmptyCallOption struct{}

func (EmptyCallOption) before(*string) error   { return nil }
func (EmptyCallOption) after(*string, *string) {}

func f(a ...CallOption) {
	fmt.Printf("%+v\n", a)
}

func main() {
	f()
	f(nil)
	f(EmptyCallOption{})
}

the output is:

[]
[<nil>]
[{}]

i.e., f() != f(nil)

@brunoluizkatz-NETZSCH
Copy link
Author

I don't remember the gRPC package version we used before updating to Go v1.20, but before this update calling the Invoke method with the options set with nil doesn't panic, but after this update invoking the method results in a panic.

If the opts ...CallOption is nil. The panic occurs after the defer at https://github.com/grpc/grpc-go/blob/master/stream.go#L274 who is triggered by the calling one [nil] as a function without validation if the value is nil or not, at https://github.com/grpc/grpc-go/blob/master/stream.go#L280

@dfawley
Copy link
Member

dfawley commented Feb 27, 2024

If you check the blame layer:

https://github.com/grpc/grpc-go/blame/master/stream.go#L280

None of this code has been touched at all for many years, so I don't think there is any regression here.

It's a programmer error to pass nil as a CallOption. It's not a situation that is difficult to avoid, and in fact it's very unusual to encounter it.

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

No branches or pull requests

3 participants