protobuf.js icon indicating copy to clipboard operation
protobuf.js copied to clipboard

Empty strings read as undefined in Map fields

Open paambaati opened this issue 6 years ago • 5 comments

protobuf.js version: 5.0.3 and 6.8.8

A Go-written message has an empty string as a map value, but the Node version fails when deserializing it because it sees it as undefined.

Proto file — https://github.com/liftbridge-io/liftbridge-grpc/blob/5694b15f251d2ff16d7d4c3e8d944aab327d3ef0/api.proto#L93-L104

The value for a Message on the Go side looks like this —

offset:69 key:"some-key" value:"some-vlaue" timestamp:1568177809419877000 subject:"test11" headers:<key:"reply" value:"" > headers:<key:"subject" value:"test11" >

Reproduction steps

  1. docker run -p 4222:4222 -ti nats:latest --debug --trace in a window.
  2. go get github.com/liftbridge-io/go-liftbridge and then $GOPATH/bin/liftbridge --raft-bootstrap-seed --nats-servers nats://localhost:4222 --level debug in another window.
  3. Clone my repo from https://github.com/paambaati/node-liftbridge.git in yet another window.
  4. yarn install or npm install
  5. yarn run debug or npm run debug

When trying to read this message on the Node.js side, I get this error —

Error [AssertionError]: Assertion failed
    at new goog.asserts.AssertionError (~/node-liftbridge/node_modules/google-protobuf/google-protobuf.js:1166:22)
    at Object.goog.asserts.doAssertFailure_ (~/node-liftbridge/node_modules/google-protobuf/google-protobuf.js:1186:9)
    at Object.goog.asserts.assert [as assert] (~/node-liftbridge/node_modules/google-protobuf/google-protobuf.js:1193:55)
    at Function.jspb.Map.deserializeBinary (~/node-liftbridge/node_modules/google-protobuf/google-protobuf.js:3881:18)
    at ~/node-liftbridge/grpc/generated/api_pb.js:2259:18
    at jspb.BinaryReader.readMessage (~/node-liftbridge/node_modules/google-protobuf/google-protobuf.js:3517:5)
    at Function.proto.proto.Message.deserializeBinaryFromReader (~/node-liftbridge/grpc/generated/api_pb.js:2258:14)
    at Function.proto.proto.Message.deserializeBinary (~/node-liftbridge/grpc/generated/api_pb.js:2214:30)
    at deserialize_proto_Message (~/node-liftbridge/grpc/generated/api_grpc_pb.js:59:25)
    at ~/node-liftbridge/node_modules/grpc/src/common.js:38:12
    at ~/node-liftbridge/node_modules/grpc/src/client_interceptors.js:689:22 {
  message: 'Assertion failed',
  reportErrorToServer: true,
  messagePattern: 'Assertion failed'
}

Specifically, this happens when the header value for key "reply" is read. Printing the values f and g in the below snippet prints "reply" for f and undefined for g, when one would expect it to be "" (an empty string).

jspb.Map.deserializeBinary = function(a, b, c, d, e, f) {
    for (var g = void 0; b.nextField() && !b.isEndGroup();) {
        var h = b.getFieldNumber();
        1 == h ? f = c.call(b) : 2 == h && (a.valueCtor_ ? (goog.asserts.assert(e), g = new a.valueCtor_, d.call(b, g, e)) : g = d.call(b))
    }
    goog.asserts.assert(void 0 != f);
    goog.asserts.assert(void 0 != g);
    a.set(f, g)
};

Related issues

  1. https://github.com/grpc/grpc-node/issues/1022

paambaati avatar Sep 11 '19 05:09 paambaati

This might be related to https://github.com/protobufjs/protobuf.js/issues/843 and or https://github.com/protobufjs/protobuf.js/issues/960. @rom1504 @GaloisGirl @dcodeIO @mkosieradzki Thoughts?

paambaati avatar Sep 11 '19 09:09 paambaati

Quite possible. There is no solution to this problem yet though afaik

rom1504 avatar Sep 11 '19 10:09 rom1504

I'm not sure where exactly the issue lies, so I've also created a new issue on protocolbuffers/protobuf-javascript#43.

paambaati avatar Sep 13 '19 06:09 paambaati

https://github.com/protobufjs/protobuf.js/pull/1348 should fix this.

paambaati avatar Feb 13 '20 07:02 paambaati

protocolbuffers/protobuf#1348 was merged. Can this be closed, @paambaati?

webmaster128 avatar Aug 07 '20 20:08 webmaster128