CrowdStrike / csproto

CrowdStrike's Protocol Buffers library
MIT License
124 stars 13 forks source link

`protoc-gen-fastmarshal` generated code fails to unmarshal empty protov2 message #89

Closed wtrep closed 1 year ago

wtrep commented 1 year ago

Title

protoc-gen-fastmarshal generated code fails to unmarshal empty protov2 message

Version

0.14.0

Description

Reflection-free generated code (*.pb.fm.go) doesn't seem to support "unmarshalling" empty messages.

The Unmarshal method first validates that the buffer isn't empty:

// Unmarshal decodes a binary encoded Protobuf message from p and populates m with the result.
func (m *BogusRequest) Unmarshal(p []byte) error {
    if len(p) == 0 {
        return fmt.Errorf("cannot unmarshal from an empty buffer")
    }
        // ...
}

This seems to not support empty messages.

Repro steps

  1. Create a simple proto:
    
    syntax = "proto3";

package bogus.v1alpha1;

option go_package = "github.com/bogus/api/proto/v1alpha1";

message BogusRequest {}

message BogusResponse { string response = 1; }

service BogusService { rpc BogusEmptyBody(BogusRequest) returns (BogusResponse) {} }


2. Generate the `*.pb.fm.go` files with `--fastmarshal_opt=apiversion=v2,paths=source_relative`
3. Register the codec with `encoding.RegisterCodec(csproto.GrpcCodec{})`
4. Implement the server:
```go
package bogus

import (
    "context"

    "github.com/bogus/api/proto/v1alpha1"
)

type Server struct {
    v1alpha1.UnimplementedBogusServiceServer
}

func (s *Server) BogusEmptyBody(_ context.Context, _ *v1alpha1.BogusRequest) (*v1alpha1.BogusResponse, error) {
    return &v1alpha1.BogusResponse{Response: "bogus"}, nil
}
  1. Make a simple request:

    ❯ grpcurl -plaintext localhost:8080 bogus.v1alpha1.BogusService/BogusEmptyBody
    ERROR:
    Code: Internal
    Message: grpc: error unmarshalling request: cannot unmarshal from an empty buffer
  2. Comment out encoding.RegisterCodec(csproto.GrpcCodec{})

  3. Validate that it works now:

    ❯ grpcurl -plaintext localhost:8080  bogus.v1alpha1.BogusService/BogusEmptyBody
    {
    "response": "bogus"
    }
dylan-bourque commented 1 year ago

@wtrep this was a miss on my part. I wrongly assumed that trying to unmarshal no data should be an error.

That snippet in the generated code should just return nil instead of an error and leave the message unmodified. Happy to review a PR if you want to submit one. If not, I'll fix this as soon as I finish up some other in-flight work I have.

The templates are here and you'd need to update both singlefile.go.tmpl and permessage.go.tmpl. Go text templates aren't great at sharing code. 😢

wtrep commented 1 year ago

No problem ! I'll submit a PR

dylan-bourque commented 1 year ago

@wtrep I had some spare bandwidth so I went ahead and knocked this out. This should resolve your issue.

wtrep commented 1 year ago

Thanks @dylan-bourque !

dylan-bourque commented 1 year ago

@wtrep I just tagged v0.18.0. Let me know if that doesn't work for you.