twitchtv / twirp

A simple RPC framework with protobuf service definitions
https://twitchtv.github.io/twirp/docs/intro.html
Apache License 2.0
7.2k stars 326 forks source link

Send empty request body returns error #42

Closed jhaynie closed 6 years ago

jhaynie commented 6 years ago

Some of my Services have empty request bodies.

For example:

message BarRequest {
}

message BarResponse {
   string hey = 1;
}

service Foo {
    // ReleaseList will return a list of releases
    rpc Bar(BarRequest) returns (BarResponse);
}

In this case, the client should be able to send a POST with an empty body (which is accept based on HTTP spec). However, this generates an error such as:

{"code":"internal","msg":"failed to parse request json: EOF","meta":{"cause":"*v1.wrappedError"}}

I believe the server should check the Content-Length header before trying to unmarshal the request (and let the implementation handle if required).

(Happy to provide a PR that would fix this if you're OK with this)

spenczar commented 6 years ago

Closing as a duplicate of #21. An empty JSON request is {}, not the empty string; an empty string is not valid JSON.

jhaynie commented 6 years ago

according to HTTP spec you don't have to provide a body for a POST request. wouldn't a simple content length check before trying to unmarshal data be the right fix??

spenczar commented 6 years ago

I agree that that would be simple to implement, but it adds unnecessary complexity to Twirp's specification. It goes from "we parse the body according to the content-type" to "we parse the body according to the content-type, unless the content-length header claims the body has length 0, in which case we assume a body with zero values for all fields." I don't think it's worth special-casing, especially since we don't have any other handling based on Content-Length right now.