Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

toProto3JSON doesn't correctly handle map of bytes #103

Open
mjameswh opened this issue Sep 24, 2024 · 0 comments
Open

toProto3JSON doesn't correctly handle map of bytes #103

mjameswh opened this issue Sep 24, 2024 · 0 comments
Labels
priority: p2 Moderately-important priority. Fix may not be included in next release. size: l Pull request size is large. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Comments

@mjameswh
Copy link

We have some protobuf messages that contain fields of type map<string, bytes>. Both protobufjs and proto3-json-serializer's fromProto3JSON appears to work properly with those fields.

However, passing such messages through toProto3JSON results in incorrect behaviors while handling the bytes value.

  • In proto3-json-serializer v2.0.0, there is no error, but the Buffer object is passed unmodified to the output object;
  • In proto3-json-serializer v2.0.2, I get the following error message:
Error: toProto3JSON: don't know how to convert value my_buffer_content
 at convertSingleValue (/.../node_modules/proto3-json-serializer/typescript/src/toproto3json.ts:50:11)
 at toProto3JSON (/.../node_modules/proto3-json-serializer/typescript/src/toproto3json.ts:149:13)
 at /.../node_modules/proto3-json-serializer/typescript/src/toproto3json.ts:137:22
 at Array.map (<anonymous>)
 at toProto3JSON (/.../node_modules/proto3-json-serializer/typescript/src/toproto3json.ts:134:27)
 ... truncated ...

The bytes values are Buffer objects. I would expect toProto3JSON to convert them to base64-encoded strings, just like it does for a simple bytes field.

I haven't tested, but from looking at the code, I believe that repeated bytes likely suffer from the same incorrect behavior.

Environment details

  • Node.js version: 20.17.0
  • npm version: 10.8.2
  • proto3-json-serializer version: 2.0.0 - 2.0.2

Steps to reproduce

  1. Start with the official proto3-json-serializer repo's main branch;

  2. In test-fixture/proto/test.proto, modify the message MessageWithMap to:

      message MessageWithMap {
          map<string, MapValue> map_field = 3;
          map<string, string> string_map_field = 4;
          map<string, int64> long_map_field = 5;
    +     map<string, bytes> bytes_map_field = 6;
      }
  3. In typescript/test/unit/map.ts, add this to both message and json:

          'minus one': '-1',
          zero: '0',
        },
    +   bytesMapField: {
    +     key1: 'dmFsdWUgMQo=',
    +     key2: 'dmFsdWUgMgo=',
    +   },
      };
    
  4. Run tests: npm test

  5. Observe test failure

@mjameswh mjameswh added priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. labels Sep 24, 2024
@sofisl sofisl added the size: l Pull request size is large. label Feb 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: p2 Moderately-important priority. Fix may not be included in next release. size: l Pull request size is large. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
Development

No branches or pull requests

2 participants