map doesn't work for empty values

[rom1504]

protobuf.js version: 6.8.3

protobuf.js throws an error when trying to decode any map encoded using google's protobuf for c#

message= Message.decode(buffer);

Error: invalid wire type 6 at offset 896
    at n.skipType (reader.js:375)

[dcodeIO]

Usually, when a wild wire type 6 appears, the binary data has been corrupted somehow. Are you sure this is not related to this?

[rom1504]

Yeah I'm already using arraybuffer as advised.
It works as long as I'm not using any map.
Whenever I use a map (for example map<string, int32> ints = 4;), I'm getting that kind of errors.

[dcodeIO]

Do you have a (hex) dump of such an erroring buffer? Ideally a minimal one, just showing the issue.

[rom1504]

I tested with a few minimal examples. a map<int32,int32> seems to work. However a map<string,string> with an empty value is getting me an error (it's a different error, but I am not sure if the error means anything, it just seems to be doing something wrong before)

[50, 9, 10, 7, 116, 101, 115, 116, 75, 101, 121]
should be read with :

message Test
{
map<string,string> test = 6;
}

I just put key="testKey" with value="" in the map for testing.

I'm getting

RangeError: index out of range: 11 + 10 > 11
    at r (reader.js:13)
    at n.uint32 (reader.js:94)
    at n.bytes (reader.js:300)
    at n.string (reader.js:321)
    at n.Test$decode [as decode] (eval at i (index.js:50), <anonymous>:35:15)

I guess it might be related to #845

[rom1504]

I'm getting the same result with a map<string,int32> and an empty value (0)

[dcodeIO]

From a first look it seems the encoder is omitting the empty value string in C#, while the decoder expects both key and value to be present. As you said, most likely related to #845

Do you also get an error when all keys and values are non-empty?

[rom1504]

No I'm not getting the error when keys and values are non-empty.

[GaloisGirl]

@dcodeIO I keep encountering this bug.

A fix is ready to merge - did you get a chance to look at it?

[rom1504]

What is the fix ? Do you mean #1087 ?

…

On Tue, Jul 17, 2018, 19:01 Anna Kosieradzka ***@***.***> wrote: @dcodeIO <https://github.com/dcodeIO> I keep encountering this bug. A fix is ready to merge - did you get a chance to look at it? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#960 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ACPN_sHQ1leKD4f0gHYqqfsmP5zxR4Qiks5uHhiAgaJpZM4Q7Ho8> .

[GaloisGirl]

I was thinking of #845 , but whichever does the job is fine by me.

[rom1504]

Ah yeah ok. I'm also still interested by a fix for this. This is a blocker to use protobuf generated by c# backends.

…

On Wed, Jul 18, 2018, 12:46 Anna Kosieradzka ***@***.***> wrote: I was thinking of #845 <#845> , but whichever does the job is fine by me. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#960 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ACPN_rnAxLxteLP4DzQvWZHhXeeIRi2Eks5uHxIJgaJpZM4Q7Ho8> .

[pnpetrov]

Up! Any plans to have a fix for this? It is two years after it was found and it is still reproducible. Both pull requests seem to be absolutely ignored

[billyplus]

check these data, when the value is 0 in the map, that value is skipped.
online decoder explained: https://protogen.marcgravell.com/decode?hex=1A-04-08-08-10-01-1A-02-08-03-1A-04-08-07-10-01-1A-02-08-0B-1A-04-08-0A-10-01-1A-02-08-02

    26, // 0001 1010  filed=3, tag=2
    4, // len=4
    8, // 0000 1000 field=1
    8, // 8
    16, // 0001 0000  field=2
    1, // end 8=1
    26, // 0001 1010  filed=3, tag=2
    2, // len=2, key = 3, value = 0 <----------value is zero
    8, // 0000 1000 field=1
    3, // 3  3=0
    26, // 0001 1010  filed=3, tag=2-
    4, // len=4, key=7, value=1 <---------- value is not zero
    8, // field=1 tag=0
    7, // 7
    16, // field=2 
    1, // end 7=1
    26, // 0001 1010  filed=3, tag=2
    2, // len=2
    8, // field=1 tag = 0
    11, // 11=0
    26, // 0001 1010  filed=3, tag=2
    4, // len=4
    8, // field = 1
    10, // 10
    16, //field=2
    1, // 10=1
    26, // 0001 1010  filed=3, tag=2
    2, // len=2
    8, // field=1
    2, // 8=2 end

And I check the static js generated. The decoder moves the pointer forward when the value is zero, which makes mistake.

                case 2:
                    reader.skip().pos++;
                    if (message.strData === $util.emptyObject)
                        message.strData = {};
                    key = reader.int32(); 
                    reader.pos++; // <- what if value is zero?????????????
                    message.strData[key] = reader.string();
                    break;
                case 3:
                    reader.skip().pos++;
                    if (message.intData === $util.emptyObject)
                        message.intData = {};
                    key = reader.int32();
                    reader.pos++;
                    message.intData[key] = reader.int32();
                    break;

[adriancable]

All - what is happening with this? I am affected by the issue too. It doesn't appear that a fix for it has been committed - can anyone clarify?

[tq-schifferm]

I can confirm that #1087 fixes this issue, and I would be very happy about a merge.

[alexandervain]

@dcodeIO @alexander-fenster any plans to merge the fix for this issue?

[adriancable]

+1 for this - it is quite a serious bug and I would be delighted to see it fixed. I can confirm that #1348 does indeed fix this.

@dcodeIO / @nicolasnoble?

[hugebdu]

IMHO it's about time to merge the fix

[adriancable]

@bcoe / @dcodeIO / @alexander-fenster - would you kindly consider this? I would be absolutely delighted beyond belief to see this merged. Thank you so much!