Fix non-proto3 JSON format double decoding #1043
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When the field type is a proto `float` or `double` we always use a Dart `double` for the value. Trying to set the field an `int` causes an assertion failure in `FieldSet`. This code is also a bit inconsistent in itself: when the jspblite2 value is a string it decodes as `double`, but when it's a number it tries to convert to an `int`. So if the value is a string `"1"` it stores as `double` but if it's the number `1` it tries to convert to double. This isn't an issue with dart2js as with dart2js `int` and `double` are the same thing, but it causes assertion failures when this library is used with dart2wasm which distinguishes `int`s and `double`s. Fix by always storing proto `float`s and `double`s as Dart `double`s. cl/799924250
Package publishing
Documentation at https://github.com/dart-lang/ecosystem/wiki/Publishing-automation. |
sigurdm
approved these changes
Aug 28, 2025
copybara-service bot
pushed a commit
to dart-lang/sdk
that referenced
this pull request
Aug 28, 2025
Revisions updated by `dart tools/rev_sdk_deps.dart`. ecosystem (https://github.com/dart-lang/ecosystem/compare/68ff911..fd28be2): fd28be2 2025-08-27 Moritz Dont show warnings by default (dart-lang/ecosystem#365) http (https://github.com/dart-lang/http/compare/6656f15..ef05b37): ef05b37 2025-08-27 Brian Quinlan [cupertino_http]: Switch to ffigen 19.1.0 (dart-lang/http#1811) i18n (https://github.com/dart-lang/i18n/compare/6a8f9c7..9a211d1): 9a211d14 2025-08-27 Moritz [package:intl4x] `DateTime` formatting redesign (dart-lang/i18n#1003) protobuf (https://github.com/dart-lang/protobuf/compare/6e9c9f4..765ba8a): 765ba8a 2025-08-28 Ömer Sinan Ağacan Fix non-proto3 JSON format double decoding (google/protobuf.dart#1043) 703bf7c 2025-08-27 Ömer Sinan Ağacan Compile dart2wasm benchmarks with --no-strip-wasm (google/protobuf.dart#1044) 9939965 2025-08-26 Ömer Sinan Ağacan Fix JS interop annotations (google/protobuf.dart#1042) da354a2 2025-08-22 Ömer Sinan Ağacan Update benchmark builder to use -O2 with dart2wasm, add output dir to gitignore (google/protobuf.dart#1041) 41eba19 2025-08-22 Ömer Sinan Ağacan Add sinks to benchmarks to prevent smart tools eliminating benchmarked code (google/protobuf.dart#1040) 3ccc8ab 2025-08-19 dependabot[bot] Bump actions/checkout from 3.5.3 to 4.2.2 (google/protobuf.dart#1036) 6bce9d7 2025-08-19 Ömer Sinan Ağacan Fix unknown enum handling (google/protobuf.dart#853) 19e5a25 2025-08-15 Ömer Sinan Ağacan Implement deepCopy as copying, instead of merge (google/protobuf.dart#742) web (https://github.com/dart-lang/web/compare/ee9733f..e7895bd): e7895bd 2025-08-27 Nikechukwu [interop] Add Support for Declaration Merging and Overloading (dart-lang/web#449) webdev (https://github.com/dart-lang/webdev/compare/9724fea..a7d3d2f): a7d3d2fc 2025-08-27 Ben Konyi [ DWDS ] Fix issue where null was repeatedly sent to connected clients (dart-lang/webdev#2678) e3009dfb 2025-08-27 Nate Biggs Remove stale calls to dartSdkIsAtLeast. (dart-lang/webdev#2677) e0d58082 2025-08-26 Nate Biggs Fix expectation in test accessing private field. (dart-lang/webdev#2676) Change-Id: Ia98a8f065830865f6922ca032939380683645d72 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/447640 Reviewed-by: Konstantin Shcheglov <[email protected]> Commit-Queue: Konstantin Shcheglov <[email protected]> Auto-Submit: Devon Carew <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When the field type is a proto
floatordoublewe always use a Dartdoublefor the value. Trying to set the field anintcauses an assertion failure inFieldSet.This code is also a bit inconsistent in itself: when the jspblite2 value is a string it decodes as
double, but when it's a number it tries to convert to anint. So if the value is a string"1"it stores asdoublebut if it's the number1it tries to convert to double.This isn't an issue with dart2js as with dart2js
intanddoubleare the same thing, but it causes assertion failures when this library is used with dart2wasm which distinguishesints anddoubles.Fix by always storing proto
floats anddoubles as Dartdoubles.Note: this code is currently not used in dart2wasm, instead dart2wasm uses the VM's library. However if I start using this library instead (for benchmarking, to potentially switch to this library) then it breaks. So currently this change is not a user-visible change because dart2wasm doesn't use this library.
cl/799924250