Skip to content

reject negative fractional_count in FastFixedDtoa - #315

Merged
floitsch merged 1 commit into
google:masterfrom
jdymitarai:fastfixeddtoa-negative-fractional-count
Sep 10, 2026
Merged

reject negative fractional_count in FastFixedDtoa#315
floitsch merged 1 commit into
google:masterfrom
jdymitarai:fastfixeddtoa-negative-fractional-count

Conversation

@jdymitarai

@jdymitarai jdymitarai commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Problem

FastFixedDtoa only validated that f ractional_count <= 20, but did not check for negative values (f ractional_count < 0).
While DoubleToStringConverter::ToFixed guards against negative requested_digits (PR #298), direct callers of FastFixedDtoa passing a negative fr ractional_count could succeed and write integer digits into the buffer, potentially overrunning small buffers. In addition, when FastFixedDtoa returned false, *length was left uninitialized.

Solution

  1. Ensure *length = 0; is set at the start of FastFixedDtoa.
  2. Reject fractional_count < 0 alongside f ractional_count > 20 by returning false, matching the guard pattern in FastDtoa (PR guard non-positive requested_digits in DigitGenCounted #313).
  3. Add FastFixedDtoaNegativeFractionalCount in test/cctest/test-fixed-dtoa.cc to verify that negative fractional counts fail and do not touch the buffer.

FastFixedDtoa only validated that fractional_count <= 20, but did not reject negative values (fractional_count < 0). While DoubleToStringConverter::ToFixed rejects negative requested_digits before calling FastFixedDtoa (PR google#298), a direct caller of FastFixedDtoa passing a negative fractional_count could succeed and write integer digits into the output buffer, potentially overrunning smaller buffers.

Reject fractional_count < 0 by returning false and ensure *length is reset to 0, matching the behavior in FastDtoa (PR google#313). Add a test in test-fixed-dtoa.cc.

@floitsch floitsch left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@floitsch
floitsch merged commit 147ea7c into google:master Sep 10, 2026
9 checks passed
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