Incorrect error message when execute protoc-gen-grpc-gateway to HTTP GET method with BODY

[budougumi0617]

I try to compile by protoc-gen-grpc-gateway the below definition to generate reverse-proxy file.

service TaskManager {
  rpc GetTask (GetTaskRequest) returns (Task) {
    option (google.api.http) = {
       get: "/v1/tasklist-gateway/task/{id}" // "id" is defined in GetTaskRequest
       body: "*"
    };
  }
}

message GetTaskRequest {
  int32 id = 1;
}

protoc returns error with below text.

needs request body even though http method is GET: GetTask

protoc -I/usr/local/include -I. -I$GOPATH/src -I$GOPATH/src/github.com/grpc-ecosystem/grpc-gateway/third_party/googleapis --grpc-gateway_out=logtostderr=true:. proto/task_list.proto
--grpc-gateway_out: needs request body even though http method is GET: GetTask

However, solution for the error is remove body statement.

  rpc GetTask (GetTaskRequest) returns (Task) {
    option (google.api.http) = {
       get: "/v1/tasklist-gateway/task/{id}" // "id" is defined in GetTaskRequest
    };
  }

I think HTTP GET request should not include BODY. It it correct error text?

Detail

Related implementation codes are below.

grpc-gateway/protoc-gen-grpc-gateway/descriptor/services.go

Lines 75 to 79 in 5e029d0

	httpMethod = "GET"
	pathTemplate = opts.GetGet()
	if opts.Body != "" {
	return nil, fmt.Errorf("needs request body even though http method is GET: %s", md.GetName())
	}

		case opts.GetGet() != "":
			httpMethod = "GET"
			pathTemplate = opts.GetGet()
			if opts.Body != "" {
				return nil, fmt.Errorf("needs request body even though http method is GET: %s", md.GetName())
			}

It seems "if statement" validates that request body has no string, but error text is recommending set the request body. Also, I think DELETE method has same issue.

grpc-gateway/protoc-gen-grpc-gateway/descriptor/services.go

Lines 89 to 94 in 5e029d0

	case opts.GetDelete() != "":
	httpMethod = "DELETE"
	pathTemplate = opts.GetDelete()
	if opts.Body != "" && !r.allowDeleteBody {
	return nil, fmt.Errorf("needs request body even though http method is DELETE: %s", md.GetName())
	}

		case opts.GetDelete() != "":
			httpMethod = "DELETE"
			pathTemplate = opts.GetDelete()
			if opts.Body != "" && !r.allowDeleteBody {
				return nil, fmt.Errorf("needs request body even though http method is DELETE: %s", md.GetName())
			}

And, I created below pull request for this issue.
#532

I would appreciate it if you could check this issue, and PR.

[achew22]

Thank you for your thorough issue report and pull request. The behavior you specified is indeed incorrect. Sorry about that. I've merged your PR and you shouldn't have that sort of problem again.

Thanks so much for contributing!

[lixin9311]

I think we should reconsider this issue.

FYI, https://stackoverflow.com/questions/978061/http-get-with-request-body

Any HTTP request message is allowed to contain a message body.

Although it's not a common way, at least provide an option for it, like DELETE.

[johanbrandhorst]

Maybe I can help you fix this issue without adding GET body parsing. What problem are you having? Feel free to drop by #grpc-gateway on the Gophers slack https://invite.slack.golangbridge.org/.