Skip to content

Commit 29b1183

Browse files
dcodeIOdcodeIO
authored
fix: Consistently handle scalar map keys (#2186)
1 parent 1039194 commit 29b1183

7 files changed

Lines changed: 163 additions & 20 deletions

File tree

‎cli/package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
"pbts": "bin/pbts"
1919
},
2020
"peerDependencies": {
21-
"protobufjs": ">=8.0.2 <9"
21+
"protobufjs": ">=8.0.4 <9"
2222
},
2323
"dependencies": {
2424
"chalk": "^4.0.0",

‎index.d.ts‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2369,6 +2369,21 @@ export namespace util {
23692369
*/
23702370
function longFromHash(hash: string, unsigned?: boolean): (Long|number);
23712371

2372+
/**
2373+
* Converts a 64 bit key to a long or number if it is an 8 characters long hash string.
2374+
* @param key Map key
2375+
* @param [unsigned=false] Whether unsigned or not
2376+
* @returns Original value
2377+
*/
2378+
function longFromKey(key: string, unsigned?: boolean): (Long|number|string);
2379+
2380+
/**
2381+
* Converts a boolean key to a boolean value.
2382+
* @param key Map key
2383+
* @returns Boolean value
2384+
*/
2385+
function boolFromKey(key: string): boolean;
2386+
23722387
/**
23732388
* Merges the properties of the source object into the destination object.
23742389
* @param dst Destination object

‎src/converter.js‎

Lines changed: 23 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,9 @@
55
*/
66
var converter = exports;
77

8-
var Enum = require("./enum"),
9-
util = require("./util");
8+
var Enum = require("./enum"),
9+
types = require("./types"),
10+
util = require("./util");
1011

1112
/**
1213
* Generates a partial value fromObject conveter.
@@ -162,23 +163,26 @@ converter.fromObject = function fromObject(mtype) {
162163
* @param {Codegen} gen Codegen instance
163164
* @param {Field} field Reflected field
164165
* @param {number} fieldIndex Field index
165-
* @param {string} prop Property reference
166+
* @param {string} dstProp Destination property reference
167+
* @param {string} [srcProp] Source property reference
166168
* @returns {Codegen} Codegen instance
167169
* @ignore
168170
*/
169-
function genValuePartial_toObject(gen, field, fieldIndex, prop) {
171+
function genValuePartial_toObject(gen, field, fieldIndex, dstProp, srcProp) {
170172
/* eslint-disable no-unexpected-multiline, block-scoped-var, no-redeclare */
173+
if (!srcProp)
174+
srcProp = dstProp;
171175
if (field.resolvedType) {
172176
if (field.resolvedType instanceof Enum) gen
173-
("d%s=o.enums===String?(types[%i].values[m%s]===undefined?m%s:types[%i].values[m%s]):m%s", prop, fieldIndex, prop, prop, fieldIndex, prop, prop);
177+
("d%s=o.enums===String?(types[%i].values[m%s]===undefined?m%s:types[%i].values[m%s]):m%s", dstProp, fieldIndex, srcProp, srcProp, fieldIndex, srcProp, srcProp);
174178
else gen
175-
("d%s=types[%i].toObject(m%s,o)", prop, fieldIndex, prop);
179+
("d%s=types[%i].toObject(m%s,o)", dstProp, fieldIndex, srcProp);
176180
} else {
177181
var isUnsigned = false;
178182
switch (field.type) {
179183
case "double":
180184
case "float": gen
181-
("d%s=o.json&&!isFinite(m%s)?String(m%s):m%s", prop, prop, prop, prop);
185+
("d%s=o.json&&!isFinite(m%s)?String(m%s):m%s", dstProp, srcProp, srcProp, srcProp);
182186
break;
183187
case "uint64":
184188
isUnsigned = true;
@@ -187,16 +191,16 @@ function genValuePartial_toObject(gen, field, fieldIndex, prop) {
187191
case "sint64":
188192
case "fixed64":
189193
case "sfixed64": gen
190-
("if(typeof m%s===\"number\")", prop)
191-
("d%s=o.longs===String?String(m%s):m%s", prop, prop, prop)
194+
("if(typeof m%s===\"number\")", srcProp)
195+
("d%s=o.longs===String?String(m%s):m%s", dstProp, srcProp, srcProp)
192196
("else") // Long-like
193-
("d%s=o.longs===String?util.Long.prototype.toString.call(m%s):o.longs===Number?new util.LongBits(m%s.low>>>0,m%s.high>>>0).toNumber(%s):m%s", prop, prop, prop, prop, isUnsigned ? "true": "", prop);
197+
("d%s=o.longs===String?util.Long.prototype.toString.call(m%s):o.longs===Number?new util.LongBits(m%s.low>>>0,m%s.high>>>0).toNumber(%s):m%s", dstProp, srcProp, srcProp, srcProp, isUnsigned ? "true": "", srcProp);
194198
break;
195199
case "bytes": gen
196-
("d%s=o.bytes===String?util.base64.encode(m%s,0,m%s.length):o.bytes===Array?Array.prototype.slice.call(m%s):m%s", prop, prop, prop, prop, prop);
200+
("d%s=o.bytes===String?util.base64.encode(m%s,0,m%s.length):o.bytes===Array?Array.prototype.slice.call(m%s):m%s", dstProp, srcProp, srcProp, srcProp, srcProp);
197201
break;
198202
default: gen
199-
("d%s=m%s", prop, prop);
203+
("d%s=m%s", dstProp, srcProp);
200204
break;
201205
}
202206
}
@@ -281,12 +285,17 @@ converter.toObject = function toObject(mtype) {
281285
("var ks2");
282286
} gen
283287
("if(m%s&&(ks2=Object.keys(m%s)).length){", prop, prop)
284-
("d%s={}", prop)
288+
("d%s={}", prop);
289+
var longKey = types.long[field.keyType] !== undefined,
290+
srcProp = prop + "[ks2[j]]";
291+
gen
285292
("for(var j=0;j<ks2.length;++j){");
293+
if (longKey) gen
294+
("var k2=util.longFromKey(ks2[j],%j).toString()", field.keyType === "uint64" || field.keyType === "fixed64");
286295
gen
287296
("if(ks2[j]===\"__proto__\")")
288297
("util.makeProp(d%s,ks2[j])", prop);
289-
genValuePartial_toObject(gen, field, /* sorted */ index, prop + "[ks2[j]]")
298+
genValuePartial_toObject(gen, field, /* sorted */ index, longKey ? prop + "[k2]" : srcProp, srcProp)
290299
("}");
291300
} else if (field.repeated) { gen
292301
("if(m%s&&m%s.length){", prop, prop)

‎src/encoder.js‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,12 @@ function encoder(mtype) {
4747
if (field.map) {
4848
gen
4949
("if(%s!=null&&Object.hasOwnProperty.call(m,%j)){", ref, field.name) // !== undefined && !== null
50-
("for(var ks=Object.keys(%s),i=0;i<ks.length;++i){", ref)
50+
("for(var ks=Object.keys(%s),i=0;i<ks.length;++i){", ref);
51+
if (field.keyType === "bool") gen
52+
("w.uint32(%i).fork().uint32(%i).bool(util.boolFromKey(ks[i]))", (field.id << 3 | 2) >>> 0, 8 | types.mapKey[field.keyType]);
53+
else if (types.long[field.keyType] !== undefined) gen
54+
("w.uint32(%i).fork().uint32(%i).%s(util.longFromKey(ks[i],%j))", (field.id << 3 | 2) >>> 0, 8 | types.mapKey[field.keyType], field.keyType, field.keyType === "uint64" || field.keyType === "fixed64");
55+
else gen
5156
("w.uint32(%i).fork().uint32(%i).%s(ks[i])", (field.id << 3 | 2) >>> 0, 8 | types.mapKey[field.keyType], field.keyType);
5257
if (wireType === undefined) gen
5358
("types[%i].encode(%s[ks[i]],w.uint32(18).fork()).ldelim().ldelim()", index, ref); // can't be groups

‎src/util/minimal.js‎

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -200,7 +200,7 @@ util.key32Re = /^-?(?:0|[1-9][0-9]*)$/;
200200
* @type {RegExp}
201201
* @const
202202
*/
203-
util.key64Re = /^(?:[\\x00-\\xff]{8}|-?(?:0|[1-9][0-9]*))$/;
203+
util.key64Re = /^(?:[\x00-\xff]{8}|-?(?:0|[1-9][0-9]*))$/; // eslint-disable-line no-control-regex
204204

205205
/**
206206
* Converts a number or long to an 8 characters long hash string.
@@ -226,6 +226,27 @@ util.longFromHash = function longFromHash(hash, unsigned) {
226226
return bits.toNumber(Boolean(unsigned));
227227
};
228228

229+
/**
230+
* Converts a 64 bit key to a long or number if it is an 8 characters long hash string.
231+
* @param {string} key Map key
232+
* @param {boolean} [unsigned=false] Whether unsigned or not
233+
* @returns {Long|number|string} Original value
234+
*/
235+
util.longFromKey = function longFromKey(key, unsigned) {
236+
return util.key64Re.test(key) && !util.key32Re.test(key)
237+
? util.longFromHash(key, unsigned)
238+
: key;
239+
};
240+
241+
/**
242+
* Converts a boolean key to a boolean value.
243+
* @param {string} key Map key
244+
* @returns {boolean} Boolean value
245+
*/
246+
util.boolFromKey = function boolFromKey(key) {
247+
return key === "true" || key === "1";
248+
};
249+
229250
/**
230251
* Merges the properties of the source object into the destination object.
231252
* @memberof util

‎tests/comp_maps.js‎

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,98 @@ tape.test("maps", function(test) {
172172
test.end();
173173
});
174174

175+
test.test(test.name + " - scalar key roundtrip", function(test) {
176+
var mapRoot = protobuf.Root.fromJSON({
177+
nested: {
178+
MapMessage: {
179+
fields: {
180+
bools: {
181+
keyType: "bool",
182+
type: "bool",
183+
id: 1
184+
},
185+
ints: {
186+
keyType: "int64",
187+
type: "int64",
188+
id: 2
189+
},
190+
uints: {
191+
keyType: "uint64",
192+
type: "uint64",
193+
id: 3
194+
},
195+
sints: {
196+
keyType: "sint64",
197+
type: "string",
198+
id: 4
199+
},
200+
fixeds: {
201+
keyType: "fixed64",
202+
type: "string",
203+
id: 5
204+
},
205+
sfixeds: {
206+
keyType: "sfixed64",
207+
type: "string",
208+
id: 6
209+
}
210+
}
211+
}
212+
}
213+
});
214+
215+
var MapMessage = mapRoot.lookup("MapMessage");
216+
var uint64Max = "18446744073709551615";
217+
var uintMap = {};
218+
var fixedMap = {};
219+
uintMap[uint64Max] = "1";
220+
fixedMap[uint64Max] = "a";
221+
var boolBuf = Uint8Array.of(0x0a, 0x04, 0x08, 0x00, 0x10, 0x00);
222+
var intBuf = MapMessage.encode({ ints: { "-1": "-1" } }).finish();
223+
var uintBuf = MapMessage.encode({ uints: uintMap }).finish();
224+
var sintBuf = MapMessage.encode({ sints: { "-1": "a" } }).finish();
225+
var fixedBuf = MapMessage.encode({ fixeds: fixedMap }).finish();
226+
var sfixedBuf = MapMessage.encode({ sfixeds: { "-1": "a" } }).finish();
227+
var longCases = [
228+
[ "int64", intBuf, { ints: { "-1": "-1" } } ],
229+
[ "uint64", uintBuf, { uints: uintMap } ],
230+
[ "sint64", sintBuf, { sints: { "-1": "a" } } ],
231+
[ "fixed64", fixedBuf, { fixeds: fixedMap } ],
232+
[ "sfixed64", sfixedBuf, { sfixeds: { "-1": "a" } } ]
233+
];
234+
235+
function toArray(buf) {
236+
return Array.prototype.slice.call(buf);
237+
}
238+
239+
function assertReencode(buf, message) {
240+
test.deepEqual(toArray(MapMessage.encode(MapMessage.decode(buf)).finish()), toArray(buf), message);
241+
}
242+
243+
test.deepEqual(
244+
toArray(MapMessage.encode({ bools: { "false": false } }).finish()),
245+
toArray(boolBuf),
246+
"should encode false boolean keys"
247+
);
248+
assertReencode(boolBuf, "should re-encode false boolean keys");
249+
250+
longCases.forEach(function(testCase) {
251+
assertReencode(testCase[1], "should re-encode " + testCase[0] + " keys");
252+
test.equal(
253+
MapMessage.verify(MapMessage.decode(testCase[1])),
254+
null,
255+
"should verify decoded " + testCase[0] + " keys"
256+
);
257+
test.deepEqual(
258+
MapMessage.toObject(MapMessage.decode(testCase[1]), { longs: String }),
259+
testCase[2],
260+
"should output " + testCase[0] + " keys as decimal strings"
261+
);
262+
});
263+
264+
test.end();
265+
});
266+
175267
test.end();
176268
});
177269

‎tests/data/test.js‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8566,10 +8566,10 @@ $root.jspb = (function() {
85668566
writer.uint32(/* id 8, wireType 2 =*/66).fork().uint32(/* id 1, wireType 0 =*/8).int32(keys[i]).uint32(/* id 2, wireType 2 =*/18).string(message.mapInt32String[keys[i]]).ldelim();
85678567
if (message.mapInt64String != null && Object.hasOwnProperty.call(message, "mapInt64String"))
85688568
for (var keys = Object.keys(message.mapInt64String), i = 0; i < keys.length; ++i)
8569-
writer.uint32(/* id 9, wireType 2 =*/74).fork().uint32(/* id 1, wireType 0 =*/8).int64(keys[i]).uint32(/* id 2, wireType 2 =*/18).string(message.mapInt64String[keys[i]]).ldelim();
8569+
writer.uint32(/* id 9, wireType 2 =*/74).fork().uint32(/* id 1, wireType 0 =*/8).int64($util.longFromKey(keys[i], false)).uint32(/* id 2, wireType 2 =*/18).string(message.mapInt64String[keys[i]]).ldelim();
85708570
if (message.mapBoolString != null && Object.hasOwnProperty.call(message, "mapBoolString"))
85718571
for (var keys = Object.keys(message.mapBoolString), i = 0; i < keys.length; ++i)
8572-
writer.uint32(/* id 10, wireType 2 =*/82).fork().uint32(/* id 1, wireType 0 =*/8).bool(keys[i]).uint32(/* id 2, wireType 2 =*/18).string(message.mapBoolString[keys[i]]).ldelim();
8572+
writer.uint32(/* id 10, wireType 2 =*/82).fork().uint32(/* id 1, wireType 0 =*/8).bool($util.boolFromKey(keys[i])).uint32(/* id 2, wireType 2 =*/18).string(message.mapBoolString[keys[i]]).ldelim();
85738573
if (message.testMapFields != null && Object.hasOwnProperty.call(message, "testMapFields"))
85748574
$root.jspb.test.TestMapFieldsNoBinary.encode(message.testMapFields, writer.uint32(/* id 11, wireType 2 =*/90).fork()).ldelim();
85758575
if (message.mapStringTestmapfields != null && Object.hasOwnProperty.call(message, "mapStringTestmapfields"))
@@ -9307,9 +9307,10 @@ $root.jspb = (function() {
93079307
if (message.mapInt64String && (keys2 = Object.keys(message.mapInt64String)).length) {
93089308
object.mapInt64String = {};
93099309
for (var j = 0; j < keys2.length; ++j) {
9310+
var k2 = $util.longFromKey(keys2[j], false).toString();
93109311
if (keys2[j] === "__proto__")
93119312
$util.makeProp(object.mapInt64String, keys2[j]);
9312-
object.mapInt64String[keys2[j]] = message.mapInt64String[keys2[j]];
9313+
object.mapInt64String[k2] = message.mapInt64String[keys2[j]];
93139314
}
93149315
}
93159316
if (message.mapBoolString && (keys2 = Object.keys(message.mapBoolString)).length) {

0 commit comments

Comments
 (0)