Repeated fields should support packed by default in proto3 decoding

[zachd]

Hi there,

We found an issue where we were unable to decode a repeated ENUM field. The fix was to identify the repeated field as [packed=true] on the Ruby decoder side to allow successful decoding. This goes against the protobuf standard as packed should be assumed as true.

Further details

The structure of the proto file is as follows

syntax = "proto3";

enum Numerical {
    ZERO = 0;
    ONE = 1;
    TWO = 2;
}

message Data {
    repeated Numerical numericals = 0;
}

And when this is encoded using the C framework, but then decoded via ruby-protobuf in Rails, we get an error undefined method&' for "\x01":String` which I assume is the decoder attempting to decode incorrectly.

Our fix was to update the proto file defining the data as packed for the Ruby decoder.

message Data {
    repeated Numerical numericals = 0 [packed=true];
}

According to https://developers.google.com/protocol-buffers/docs/encoding#packed this is in fact incorrect and should not be required as a fix. Something should change in the decoding logic of the Ruby package to detect packed repeated fields for enum types in proto3

"In proto3, repeated fields of scalar numeric types are packed by default."

[film42]

@zachd Which version of the protobuf gem are you using? Is there a chance you're using the google-protobuf gem?

[zachd]

@zachd Which version of the protobuf gem are you using? Is there a chance you're using the google-protobuf gem?

Hi @film42, this is a copy of my Gemfile.lock for protobuf which I'm quite certain is the protobuf gem here. I see I am 2 bugfix versions behind but I can't see anything that could be related since my release.

GEM
  remote: https://rubygems.org/
  specs:
    protobuf (3.10.1)
      activesupport (>= 3.2)
      middleware
      thor
      thread_safe