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

Fix the issue with int64 value type in a map #2006

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

bobhfut
Copy link

@bobhfut bobhfut commented Jun 26, 2024

As an employee from ByteDance, we have discovered a bug in our codebase. The issue occurs when a property in a protocol buffer (PB) structure is of type map<int32, int64>. When the value in the map is 0, the corresponding JavaScript value is represented as the number 0 instead of Long.ZERO.

We are working to fix this bug.

@dcodeIO
Copy link
Member

dcodeIO commented Jun 26, 2024

Isn't the issue rather the value in, say, types.defaults["int64"]? Already does a lookup there, whereas the new isLongType seems redundant on this basis.

@bobhfut
Copy link
Author

bobhfut commented Jun 26, 2024

Isn't the issue rather the value in, say, types.defaults["int64"]? Already does a lookup there, whereas the new isLongType seems redundant on this basis.

The value stored in types.defaults["int64"] is the numerical value 0, but in reality we need to generate a string like "$util.Long ? $util.Long.fromValue(0) : 0" for all xx64 related types.

@bobhfut
Copy link
Author

bobhfut commented Jun 26, 2024

Isn't the issue rather the value in, say, types.defaults["int64"]? Already does a lookup there, whereas the new isLongType seems redundant on this basis.

maybe we just set types.defaults["xxx64"] to string "$util.Long ? $util.Long.fromValue(0) : 0" ?
The problem is that there are places where default values are being used to perform numeric comparisons.

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

Successfully merging this pull request may close these issues.

2 participants