protocolbuffers / protocolbuffers/protobuf-javascript

map.js triggers assertion when encountering an integer-keyed map with key 0

Open
#27 2 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

bug help wanted javascript port-fix triaged
Dominant language
JavaScript
Stars
471
Forks
91
Avg merge
3h 57m
Merged PRs (30d)
2

Description

Version: 3.12.2
Language: Javascript

Following is a base64 string of bytes that has been coded from C# side. I could immediately decode at C# side just fine, but it failed to decode at JS library.

CiRlZjlhNTJlNC0wZDRmLTQyNzEtOGQzYi02ODZjYWE3N2M4ZWUSClBMQVlFUjc2MTYiCUxZWERQQkFDRkocaEpRQnFhT0t3SmZVbWtyTzF5UjZ5WVBXSWMxM1oCEgBaBggBEgIIAbIBAggBwgEMCIaVuPgFEOjNpLcB

It is coming from this proto definition. Please ignore references to other unknown messages, because the problem is on just field index 11.

message PlayerData {

    option (private_message) = true;
    reserved 3,5,7,8, 13 to 16;

    string player_id = 1;
    string display_name = 2;
    string short_player_id = 4;
    string start_playing = 6;
    string firebase_uid = 9;

    map<string,MinigameProgress> minigame_progresses = 10;

    //Problematic field on JS side
    map<int32,CharacterConfiguration> character_configurations = 11;

    int32 chilli = 12;

    int32 match_count_two_players = 17;
    int32 selected_character_configuration = 18;

    repeated UnlockedSet unlocked_sets = 19;

    repeated PermanentEvent permanent_events = 20; 

    map<string, google.protobuf.Timestamp> trial_timestamps = 21;

    SavedConfigurations saved_configurations = 22;

    repeated PurchasableFeatureId purchasable_features = 23;

    google.protobuf.Timestamp start_playing_20 = 24;
}

message CharacterConfiguration{

    E7.DuelOtters.Core.CharacterKind kind = 1;
    string headgear_system_name = 2;
    string clothing_system_name = 3;
    int32 winning_dance_id = 4;
    ColorMutation color_mutation = 5;
    EyeMutation eye_mutation = 6;
    CheekMutation cheek_mutation = 7;

    ClothingKind clothing_kind = 8;
    HeadgearKind headgear_kind = 9;
}

This is a ToString of real data coming from C#. Many were default, so it seems they had been efficiently stripped off.

{ "playerId": "ef9a52e4-0d4f-4271-8d3b-686caa77c8ee", "displayName": "PLAYER7616", "shortPlayerId": "LYXDPBACF", "firebaseUid": "hJQBqaOKwJfUmkrO1yR6yYPWIc13", "characterConfigurations": { "0": { }, "1": { "kind": "CharacterKind_Bomberjack" } }, "savedConfigurations": { "autoGameCenter": true }, "startPlaying20": "2020-07-14T19:41:58.384378600Z" }

When trying to deserialize in JS :

const b64 = "CiRlZjlhNTJlNC0wZDRmLTQyNzEtOGQzYi02ODZjYWE3N2M4ZWUSClBMQVlFUjc2MTYiCUxZWERQQkFDRkocaEpRQnFhT0t3SmZVbWtyTzF5UjZ5WVBXSWMxM1oCEgBaBggBEgIIAbIBAggBwgEMCIaVuPgFEOjNpLcB"
const buf = Buffer.from(b64, "base64")
PlayerData.deserializeBinary(buf)

It produces assertion failure :

Error [AssertionError]: Assertion failed
    at new goog.asserts.AssertionError (/Users/Sargon/Documents/Unity Projects/Main Projects/DuelOtters/Firebase/functions/node_modules/google-protobuf/google-protobuf.js:81:876)
    at Object.goog.asserts.doAssertFailure_ (/Users/Sargon/Documents/Unity Projects/Main Projects/DuelOtters/Firebase/functions/node_modules/google-protobuf/google-protobuf.js:82:257)
    at Object.goog.asserts.assert [as assert] (/Users/Sargon/Documents/Unity Projects/Main Projects/DuelOtters/Firebase/functions/node_modules/google-protobuf/google-protobuf.js:83:83)
    at Function.jspb.Map.deserializeBinary (/Users/Sargon/Documents/Unity Projects/Main Projects/DuelOtters/Firebase/functions/node_modules/google-protobuf/google-protobuf.js:411:241)

When compared with original source code, this assertion should be line 507 in js/map.js, jspb.Map.deserializeBinary : goog.asserts.assert(key != undefined); The key and value is going to be used in the map.set that follows so two asserts are there.

The problematic part in the data is the field index 11, map<int32,CharacterConfiguration> character_configurations = 11;

It is an int mapped to an another message. In the bytes, it has 2 members, int 0 mapped to an empty message, and int 1 mapped to a message with a bit of string data. Again, this is what they are at C#'s ToString debug :

 "characterConfigurations": { "0": { }, "1": { "kind": "CharacterKind_Bomberjack" } }

By simply removing the 0 keyed entry with default message value, and add it back to be keyed with 2 instead :

 "characterConfigurations": { "1": { "kind": "CharacterKind_Bomberjack" },  "2": { } }

JS is able to deserialize without triggering the assertion error. It seems like there is something with integer key at number 0. (C# could deserialize 0-keyed map in the protobuf bytes just fine)

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in js/map.js at jspb.Map.deserializeBinary, around line 507, and reproduce the failure with the base64 input and PlayerData definition from the issue. Compare decoding of the integer-keyed map entries for keys 0 and 1. Done means the provided data deserializes without an assertion while preserving the key-0 entry.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript
Domain
backend-api-design
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.