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

Map fields with empty string as key are handled incorrectly #843

Closed
mkosieradzki opened this issue Jun 24, 2017 · 3 comments · Fixed by #1348 · May be fixed by #845
Closed

Map fields with empty string as key are handled incorrectly #843

mkosieradzki opened this issue Jun 24, 2017 · 3 comments · Fixed by #1348 · May be fixed by #845

Comments

@mkosieradzki
Copy link

mkosieradzki commented Jun 24, 2017

protobuf.js version: 6.8.0

UPDATED:
Current codegen for map-fields when string key is empty is incorrect.

ORIGINAL:
Current codegen for map-fields is invalid. It assumes that map keys are non empty (!) strings (!!).

  1. Map CAN have empty string as indices.
  2. Map CAN be indexed by something different than string...

Both parsing and JS representation (Object instead of Map) are broken.

@dcodeIO
Copy link
Member

dcodeIO commented Jun 24, 2017

It does not assume that protobuf map keys are strings, it just makes any map keys strings on the JS-side because that's how it works in JavaScript. For example, long ints are converted to an 8 bytes hash string and there are utility functions to convert / generate these keys. Other values are simply converted to strings to be usable as a JS object key.

@mkosieradzki
Copy link
Author

mkosieradzki commented Jun 24, 2017

@dcodeIO Thanks and sorry, I have jumped too quick to conclusions. So only empty strings are mishandled.

@mkosieradzki mkosieradzki changed the title Map fields are implemented incorrectly Map fields with empty string as key are handled incorrectly Jun 24, 2017
@mkosieradzki
Copy link
Author

I have fixed the codegen in my fork and I will create a PR soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants