From 6b428371e6d0f6caf366aea8ecf3312c17d6bf70 Mon Sep 17 00:00:00 2001 From: Matthias Schiffer Date: Mon, 20 Jan 2020 10:27:54 +0100 Subject: [PATCH] Fix handling of map entries with omitted key or value According to [1], map encoding must be compatible with a repeated message using indices 1 and 2 for key and value. In particular this implies that both key and value may be omitted when they are equal to the default value - which some protobuf implementations like protobuf-c actually do. The comments in the added testcase are based on the output of protobuf-inspector [2]. [1] https://developers.google.com/protocol-buffers/docs/proto3#backwards-compatibility [2] https://github.com/jmendeth/protobuf-inspector Based-on-patch-by: Shrimpz --- src/decoder.js | 52 +++++++++++++++++++++++++++++++++------------- tests/comp_maps.js | 44 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 81 insertions(+), 15 deletions(-) diff --git a/src/decoder.js b/src/decoder.js index 6dde6f82e..491dd3059 100644 --- a/src/decoder.js +++ b/src/decoder.js @@ -19,7 +19,7 @@ function decoder(mtype) { var gen = util.codegen(["r", "l"], mtype.name + "$decode") ("if(!(r instanceof Reader))") ("r=Reader.create(r)") - ("var c=l===undefined?r.len:r.pos+l,m=new this.ctor" + (mtype.fieldsArray.filter(function(field) { return field.map; }).length ? ",k" : "")) + ("var c=l===undefined?r.len:r.pos+l,m=new this.ctor" + (mtype.fieldsArray.filter(function(field) { return field.map; }).length ? ",k,value" : "")) ("while(r.pos>>3){") + ("case 1: k=r.%s(); break", field.keyType) + ("case 2:"); + + if (types.basic[type] === undefined) gen + ("value=types[%i].decode(r,r.uint32())", i); // can't be groups + else gen + ("value=r.%s()", type); + + gen + ("break") + ("default:") + ("r.skipType(tag2&7)") + ("break") + ("}") + ("}"); + + if (types.long[field.keyType] !== undefined) gen + ("%s[typeof k===\"object\"?util.longToHash(k):k]=value", ref); + else gen + ("%s[k]=value", ref); // Repeated fields } else if (field.repeated) { gen diff --git a/tests/comp_maps.js b/tests/comp_maps.js index 7c5768a17..37051f6b0 100644 --- a/tests/comp_maps.js +++ b/tests/comp_maps.js @@ -92,6 +92,50 @@ tape.test("maps", function(test) { test.end(); }); + test.test(test.name + " - omitted fields", function(test) { + + var mapRoot = protobuf.Root.fromJSON({ + nested: { + MapMessage: { + fields: { + value: { + keyType: "int32", + type: "string", + id: 1 + } + } + } + } + }); + + var MapMessage = mapRoot.lookup("MapMessage"); + + var value = { + value: { + 0: '' + } + }; + var dec; + + // 1 = message(1 = 0, 2 = empty chunk) + dec = MapMessage.decode(Uint8Array.of(0x0a, 0x04, 0x08, 0x00, 0x12, 0x00)); + test.deepEqual(dec, value, "should correct decode the buffer without omitted fields"); + + // 1 = message(1 = 0) + dec = MapMessage.decode(Uint8Array.of(0x0a, 0x02, 0x08, 0x00)); + test.deepEqual(dec, value, "should correct decode the buffer with omitted value"); + + // 1 = message(2 = empty chunk) + dec = MapMessage.decode(Uint8Array.of(0x0a, 0x02, 0x12, 0x00)); + test.deepEqual(dec, value, "should correct decode the buffer with omitted key"); + + // 1 = empty chunk + dec = MapMessage.decode(Uint8Array.of(0x0a, 0x00)); + test.deepEqual(dec, value, "should correct decode the buffer with both key and value omitted"); + + test.end(); + }); + test.end(); });