Skip to content

[table-driven-branch] Pull over the concepts from main for strtod locale issues - #2193

Merged
thomasvl merged 4 commits into
apple:table-driven-branchfrom
thomasvl:update_table_driven
Oct 15, 2026
Merged

thomasvl merged 4 commits into
apple:table-driven-branchfrom
thomasvl:update_table_driven

Conversation

@thomasvl

@thomasvl thomasvl commented Oct 15, 2026 •

Copy link
Copy Markdown
Collaborator

Pulled the two commits over, but the branch has changed enough the fix doesn't really come directly at all, things are very different.

  • Reuse JSONReader/Tokenizer for quoted JSON numbers

    Instead of maintaining duplicate string-number parsing and calling strtod in JSONReader, parse quoted JSON numbers using a sub-JSONReader over the decoded UTF-8 bytes so that quoted and unquoted numbers share the exact same tokenizer validation and locale-independent conversion.

  • Avoid String creation when parsing unescaped quoted JSON numbers

    When a quoted JSON number has no backslash escapes (Token.Kind.string), slice tokenizer.current.text directly instead of decoding a String via consumeString(). Also defer the NaN/Infinity string comparison in parseQuotedDouble until after the leading/trailing digit check (and fail fast for input > 9 bytes) so numeric strings skip it entirely.

uchiha-bug-hunter and others added 4 commits October 15, 2026 12:11
In a process whose `LC_NUMERIC` locale uses a comma decimal separator,
every fractional float or double value fails to decode:

```swift
setlocale(LC_NUMERIC, "fr_FR")
try Google_Protobuf_DoubleValue(jsonString: "1.5")                // throws invalidUTF8
try Google_Protobuf_DoubleValue(textFormatString: "value: 3.75")  // throws malformedNumber
```

`DoubleParser.utf8ToDouble` parses the pre-validated token with
`strtod()`, which obeys the current C numeric locale: under a
comma-decimal locale it stops at the `.`, the consumed-length check
fails, and the value is rejected. Any code in the process calling
`setlocale(LC_ALL, "")` (common in GUI apps and localized CLI code)
breaks JSON and TextFormat decoding this way. The C++ implementation
parses numbers locale-independently in both codecs: `absl::SimpleAtod`
in json/internal/lexer.cc:266 and `NoLocaleStrtod` in
io/tokenizer.cc:1034, backed by `absl::from_chars` in io/strtod.cc.

The fix keeps the `strtod()` fast path and, only when it does not
consume the whole token, retries with Swift's `Double(_:)`, which the
stdlib pins to the C locale. In the C locale this changes nothing:
`Double(_:)` accepts exactly the fully consumed `strtod()` forms, so
anything rejected before is still rejected (checked against `1.2.3`,
`1e`, `1e+`, `1.5x`, and out-of-range `1e309`).

The new test switches `LC_NUMERIC` to a comma-decimal locale when one is
installed and skips otherwise, since minimal CI images often ship only
the C locale. It covers float and double in both codecs and that
malformed numbers still fail.

This fix was found and prepared with AI assistance and reviewed by me
before submitting.
Followup to 87cc8b1.

Also make the new test explicit that things like "1,2" won't get
accepted even though that covered in some other places too.
Instead of maintaining duplicate string-number parsing and calling strtod in JSONReader, parse quoted JSON numbers using a sub-JSONReader over the decoded UTF-8 bytes so that quoted and unquoted numbers share the exact same tokenizer validation and locale-independent conversion.
…oted JSON numbers

When a quoted JSON number has no backslash escapes (Token.Kind.string), slice tokenizer.current.text directly instead of decoding a String via consumeString(). Also defer the NaN/Infinity string comparison in parseQuotedDouble until after the leading/trailing digit check (and fail fast for input > 9 bytes) so numeric strings skip it entirely.
@thomasvl
thomasvl requested a review from allevato October 15, 2026 18:54
@thomasvl thomasvl added the semver/none No version bump required. label Oct 15, 2026
@thomasvl
thomasvl merged commit a6a9b05 into apple:table-driven-branch Oct 15, 2026
24 checks passed
@thomasvl
thomasvl deleted the update_table_driven branch October 15, 2026 19:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

semver/none No version bump required.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants