Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Empty strings read as undefined in Map fields #1293

Open
paambaati opened this issue Sep 11, 2019 · 5 comments
Open

Empty strings read as undefined in Map fields #1293

paambaati opened this issue Sep 11, 2019 · 5 comments

Comments

@paambaati
Copy link

paambaati commented Sep 11, 2019

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. Failed to parse server response when I try to read a stream grpc/grpc-node#1022
@paambaati paambaati changed the title Empty strings read Empty strings read as undefined Sep 11, 2019
@paambaati paambaati changed the title Empty strings read as undefined Empty strings read as undefined in Map fields Sep 11, 2019
@paambaati
Copy link
Author

paambaati commented Sep 11, 2019

This might be related to #843 and or #960. @rom1504 @GaloisGirl @dcodeIO @mkosieradzki Thoughts?

@rom1504
Copy link

rom1504 commented Sep 11, 2019

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

@paambaati
Copy link
Author

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

@paambaati
Copy link
Author

#1348 should fix this.

@webmaster128
Copy link
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants