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

[kskalski]

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

Long 'hash' keys are not interpreted correctly when constructing map<int64, Obj>

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 {} } } 

[kskalski]

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

[zdevwu]

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

[zdevwu]

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.

[kskalski]

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...'

[bkak]

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

[Notmeor]

same issue here, have to turn back to official impl

[chuganzy]

Same here

[SzHeJason]

I fix it temporarily use patch-package，this 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);

[ankisme]

Same issue here. The issue is opened at 2019.4, now is 2021.8 the issue is still not fixed?

[antonilol]

SzHeJason's patch fixed it for me, can #1669 be merged? this issue has been open for more that 3.5 years...