Enums should default to first defined value not the one with tag = 0

[robin-anil]

On the client side I am encoding

{
   operation: 'AND',
   valueSet: [{
      dimension: 'DIMENSION_1',
      value: [2]
   }]
}

On the server (in java) I am receiving

{
    valueSet: {
      value: [605]
    }
}

Here is the proto used. Stripped down for simplicity

message Filter {
  enum Operation {
    AND = 0;
  }

  optional Operation operation = 1;
  repeated ValueSet value_set = 2;
  repeated Filter filter = 5;
}

enum Dimension {
   DIMENSION_1 = 0;
}

message ValueSet {
  optional Dimension dimension = 1;
  repeated int64 value = 2;
}

[robin-anil]

On the client side converting to buffer and parsing the buffer back yields the correct proto/json (1st one). Since the server side is using Google's protobuf library, I suspect protobufJS to be having the bug, but we could be wrong.

[dcodeIO]

{
   "operation": "AND",
   "valueSet": [{
      "dimension": "DIMENSION_1",
      "value": [ 2 ]
   }]
}

Buffer:

08	id 1, wireType 0
00	value 0 -> Filter#operation = 0 (AND)
12	id 2, wireType 2
04	length 4 -> Filter#value_set[0] =
 08	id 1, wireType 0
 00	value 0 -> ValueSet#dimension = 0 (DIMENSION_1)
 10	id 2, wireType 0
 02	value 2 -> ValueSet#value = 2

Looks ok to me. The optional enum defaults are most likely encoded on the wire because these are set as strings without using Message.from to convert them properly. Encoders just silently convert them to numbers - either use Message.verify (returns "Filter.operation: enum value expected" here) to check or .from to coerce them.

See:

{
   "operation": 0,
   "valueSet": [{
      "dimension": 0,
      "value": [ 2 ]
   }]
}

Buffer:

12	id 2, wireType 2
02	length 2
 10	id 2, wireType 0
 02	value 2

Which is the same without optional enum defaults.

Correct usage example:

var protobuf = require("..");

var msg = {
   operation: "AND",
   valueSet: [{
      dimension: "DIMENSION_1",
      value: [2]
   }]
};

var root = protobuf.loadSync(require.resolve("./issue.proto"));

var Filter = root.lookup("Filter");

var reason = Filter.verify(msg);
if (reason !== null)
  console.log("message is invalid when calling just encode because: " + reason);

var buf = Filter.encode(Filter.from(msg)).finish();
console.log(buf);

[robin-anil]

ha. it turns out the enum is a bit weird. number 18 is defined at the top and on the server side it is being defaulted to DIMENSION_18 when you call valueSet.getDimension() as the parsed object does not seem to have the enum set.

  valueSet: {
     value: [605]
  }

enum Dimension {
   DIMENSION_18 = 18;
   DIMENSION_1 = 0;
}

[dcodeIO]

I see. Optional enums with default values are not encoded on the wire, so this seems to be the underlying issue.

[robin-anil]

Hmm yeah, in this case there is no [default = DIMENSION_1] specified. Could be a quirk in the format.

Or Google's internal java code could be tickling this bug, Oh dear!

[dcodeIO]

From Google's developer guide:

For enums, the default value is the first defined enum value, which must be 0.

https://developers.google.com/protocol-buffers/docs/proto3#default

Seems that the official implementation just takes the first defined, while protobuf.js uses 0. This still shouldn't result in an invalid wire format, though. Hmm.

[robin-anil]

The enum issue is fixed on the server side by sorting our enum defs for now, but yes you should use the first defined as the default in js as well. The int64 wire type issue is red-herring, that seems to be a bug in our code which was sending wrong data. I will rename the bug