Protobuf.js: Constructing map<int64, ...> using longHash as keys doesn't yield correct proto on the wire

Created on 16 Apr 2019  路  8Comments  路  Source: protobufjs/protobuf.js

protobuf.js version: 6.8.8 (fetched through grpc 1.20-pre1)

Long 'hash' keys are not interpreted correctly when constructing map

For messages like that

message Obj {}
message MyMessage {
  map<int64, Obj> MyMap = 1;
}

creating message with

proto.MyMessage.create({ 
   MyMap: {
     ['\u0002\u0000\u0000\u0000\u0000\u0000\u0000\u0000']: {}
  }
})

creates seemingly valid object, prints out as

MyMessage { MyMap: { '\u0002\u0000\u0000\u0000\u0000\u0000\u0000\u0000': {} }

but after encoding and decoding it, the key is changed to 0:

proto.MyMessage.decode(proto.MyMessage.encode(req).finish());

prints

 MyMessage {
     MyMap:
      { '\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000': Obj {} } } 

All 8 comments

Note: I'm using typescript generated static code and it runs in NodeJs context (main process of Electron)

the map implementation at the moment is just the javascript object, to be able to use typed keys, the implementation needs to be changed to use proper map:
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Map

the proper map implementation might be a good idea, but it is a major change of type. old code will have to be migrated for this version upgrade.

I'm not sure if the implementation of map or being able to use typed keys is really important here. The type of key in the problematic example is string, but what matters is that proto field is a map with int64 keys.
The bug is in the way keys themselves are parsed when encoding such map field.

For example, a workaround that I use now is to use decimal number in string as keys, e.g.

proto.MyMessage.create({ 
   MyMap: {
     ['2']: {}
  }
})

works correctly and I guess converting '\u0002\u0000\u0000\u0000\u0000\u0000\u0000\u0000' somewhere out there simply fails and 0 is used.

I imagine there should be some code in encode that will check for a case of strings with 8 characters and parse them with Long.hashToLong
Of course this raises ambiguity of how to treat '12345678': should it be a 12345678 number or parsed hash '\u0049\u0050\u0051...'

I am facing the same issue. Any idea when it will be resolved? Or, any other solution?

same issue here, have to turn back to official impl

Same here

I fix it temporarily use patch-package锛宼his my code

diff --git a/node_modules/protobufjs/src/decoder.js b/node_modules/protobufjs/src/decoder.js
index 491dd30..cfaa927 100644
--- a/node_modules/protobufjs/src/decoder.js
+++ b/node_modules/protobufjs/src/decoder.js
@@ -72,7 +72,7 @@ function decoder(mtype) {
                 ("}");

             if (types.long[field.keyType] !== undefined) gen
-                ("%s[typeof k===\"object\"?util.longToHash(k):k]=value", ref);
+                ("%s[util.Long.isLong(k)?k.toString():typeof k===\"object\"?util.longToHash(k):k]=value", ref);
             else gen
                 ("%s[k]=value", ref);
Was this page helpful?
0 / 5 - 0 ratings