Invalid memory address when using google.longrunning.Operation without operation_info

[arjca]

Hi,

One method of my Protobuf service is expected to return early, and proceed to computations after the response. Google Cloud guidelines say this method must return a google.longrunning.Operation.

When following this rule, api-linter returns an error: runtime error: invalid memory address or nil pointer dereference. There is no error if I replace google.longrunning.Operation by google.protobuf.Empty, for instance.

Stack trace with --debug is the following:

goroutine 1 [running]:
runtime/debug.Stack()
        /usr/lib/go/src/runtime/debug/stack.go:24 +0x5e
runtime/debug.PrintStack()
        /usr/lib/go/src/runtime/debug/stack.go:16 +0x13
github.com/googleapis/api-linter/lint.(*Linter).runAndRecoverFromPanics.func1()
       [...]/api-linter/lint/lint.go:127 +0x52
panic({0xa6c9a0?, 0x10a5ab0?})
        /usr/lib/go/src/runtime/panic.go:770 +0x132
github.com/jhump/protoreflect/desc.(*MessageDescriptor).GetName(...)
        [...]/go/pkg/mod/github.com/jhump/protoreflect@v1.16.0/desc/descriptor.go:398
github.com/googleapis/api-linter/rules/internal/utils.LintMethodHasMatchingResponseName(0xc0002fe140)
       [...]/api-linter/rules/internal/utils/common_lints.go:191 +0x115
github.com/googleapis/api-linter/rules/aip0136.init.func9(0xc0002fe140)
        [...]/api-linter/rules/aip0136/response_message_name.go:44 +0x27
github.com/googleapis/api-linter/lint.(*MethodRule).Lint(0x10b44c0, 0x0?)
       [...]/api-linter/lint/rule.go:213 +0x11b
github.com/googleapis/api-linter/lint.(*Linter).runAndRecoverFromPanics(0xc0001db080?, {0xc26a80?, 0x10b44c0?}, 0xc0001372c0?)
        [...]/api-linter/lint/lint.go:137 +0x85
github.com/googleapis/api-linter/lint.(*Linter).lintFileDescriptor(0xc000690210, 0xc0001990a0)
        [...]/api-linter/lint/lint.go:97 +0x1e7
github.com/googleapis/api-linter/lint.(*Linter).LintProtos(0xc000690210, {0xc00011a920, 0x1, 0x0?})
        [...]/api-linter/lint/lint.go:71 +0xbd
main.(*cli).lint(0xc000163bc0, 0xc0002f92f0, {0x112e0a0?, 0x0?, 0x0?})
        [...]/api-linter/cmd/api-linter/cli.go:195 +0xa98
main.runCLI({0xc00012e010?, 0xc00010e058?, 0x104bfe0?})
        [...]/api-linter/cmd/api-linter/main.go:46 +0x39
main.main()
        [...]/api-linter/cmd/api-linter/main.go:39 +0x49

The error is produced in rule AIP-136 when using a method from utils: using GetResponseType on the desc.MethodDescriptor of the method returning google.longrunning.Operation returns nil.

Environment details

• Programming language: N/A
• OS: Arch Linux x86_64
• Language runtime version: go1.22.4 linux/amd64
• Package version: 1.66.1

Steps to reproduce

api-linter returns the error with the following code:

syntax = "proto3";

package arjca.prj.v1;

import "google/api/annotations.proto";
import "google/longrunning/operations.proto";

service AwesomeService {
  rpc Process(ProcessRequest) returns (google.longrunning.Operation);
}

message ProcessRequest {}

I uploaded a repository with this code and dependencies (googleapis Protobuf files): https://github.com/arjca/api-linter-bug-longrunning. To reproduce: clone it, fetch the submodule (make init) and finally run api-linter (make lint).

[arjca]

I investigated a bit and found something odd.

In the function GetOperationInfo of file rules/internal/utils/extension.go, there is a check for nil value at line 47. If the value is not nil, it is then cast as a pointer to lrpb.OperationInfo. However, after the cast, value is now nil!

See:

opts := m.GetMethodOptions()
if x := proto.GetExtension(opts, lrpb.E_OperationInfo); x != nil {
	fmt.Println(x == nil) // => false
	fmt.Println(x.(*lrpb.OperationInfo) == nil) // => true (!)

	return x.(*lrpb.OperationInfo)
}

I got this case because I did not put the option operation_info in my method. I then modified my example:

syntax = "proto3";

package arjca.prj.v1;

import "google/api/annotations.proto";
import "google/longrunning/operations.proto";

service AwesomeService {
  rpc Process(ProcessRequest) returns (google.longrunning.Operation) {
    option (google.longrunning.operation_info) = {
      response_type: "ProcessResponse"
    };
  };
}

message ProcessRequest {
}

message ProcessResponse {
}

And it works!

Still, I think that the behavior with the missing google.longrunning.operation_info option is not desired. The error message should be more compelling, so that users understand what they have to modify. Or perhaps the linter should not crash at all, as the absence of this option should be covered by AIP-151.

[noahdietz]

as the absence of this option should be covered by AIP-151.

Indeed - if we cannot derive the LRO response type, we shouldn't crash, we should just exit. Thanks for the report (and for doing all of that debugging). I'll send a patch