Tom Sepez (Gerrit)

unread,

Jun 22, 2026, 5:11:05 PM (3 days ago) Jun 22

to dan sinclair, pdfium...@luci-project-accounts.iam.gserviceaccount.com, pdfium-...@googlegroups.com

Attention needed from dan sinclair

Tom Sepez voted and added 1 comment

Votes added by Tom Sepez

Commit-Queue

+1

1 comment

File xfa/fgas/crt/cfgas_stringformatter.cpp

Line 1394, Patchset 1: if (!safeExponent.IsValid()) {

Tom Sepez . resolved

Please fix this WARNING reported by autoreview issue finding: This check is redundant because `safeExponent` is initialized to 0 (which is valid), and any mutations to it inside the loop immediately return `false` if it becomes invalid. You can safely remove lines 1394-1396 and directly assign the value on line 1397.

Open in Gerrit

Related details

Attention is currently required from:

dan sinclair

Submit Requirements:

Code-Owners
Code-Review
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Lei Zhang (Gerrit)

unread,

Jun 22, 2026, 7:11:32 PM (3 days ago) Jun 22

to Tom Sepez, Lei Zhang, dan sinclair, pdfium...@luci-project-accounts.iam.gserviceaccount.com, pdfium-...@googlegroups.com

Attention needed from Tom Sepez and dan sinclair

Lei Zhang added 1 comment

File xfa/fgas/crt/cfgas_stringformatter.cpp

Line 1362, Patchset 2 (Latest): pdfium::CheckedNumeric<int32_t> safeExponent = 0;

Lei Zhang . unresolved

style

Open in Gerrit

Related details

Attention is currently required from:

Tom Sepez
dan sinclair

Submit Requirements:

Code-Owners
Code-Review

No-Unresolved-Comments

Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Lei Zhang (Gerrit)

unread,

Jun 22, 2026, 7:26:33 PM (3 days ago) Jun 22

to Tom Sepez, Lei Zhang, dan sinclair, pdfium...@luci-project-accounts.iam.gserviceaccount.com, pdfium-...@googlegroups.com

Attention needed from Tom Sepez and dan sinclair

Lei Zhang voted and added 2 comments

Votes added by Lei Zhang

Code-Review

+1

2 comments

File xfa/fgas/crt/cfgas_stringformatter.cpp

Line 13, Patchset 2 (Latest):#include <limits>

Lei Zhang . unresolved

No longer used?

Line 1364, Patchset 2 (Latest): pdfium::CheckedNumeric<int32_t> safeMultiplier = 1;

Lei Zhang . unresolved

style x2

Open in Gerrit

Related details

Attention is currently required from:

Tom Sepez
dan sinclair

Submit Requirements:

Code-Owners
Code-Review

No-Unresolved-Comments

Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Tom Sepez (Gerrit)

unread,

Jun 22, 2026, 8:05:40 PM (2 days ago) Jun 22

to Lei Zhang, dan sinclair, pdfium...@luci-project-accounts.iam.gserviceaccount.com, pdfium-...@googlegroups.com

Attention needed from dan sinclair

Tom Sepez voted and added 3 comments

Votes added by Tom Sepez

Commit-Queue

+2

3 comments

File xfa/fgas/crt/cfgas_stringformatter.cpp

Line 13, Patchset 2:#include <limits>

Lei Zhang . resolved

No longer used?

Tom Sepez

Used at 1583.

Line 1362, Patchset 2: pdfium::CheckedNumeric<int32_t> safeExponent = 0;

Lei Zhang . resolved

style

Tom Sepez

Done

Line 1364, Patchset 2: pdfium::CheckedNumeric<int32_t> safeMultiplier = 1;

Lei Zhang . resolved

style x2

Tom Sepez

Done

Open in Gerrit

Related details

Attention is currently required from:

dan sinclair

Submit Requirements:

Code-Owners
Code-Review

Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

open

diffy

pdfium-scoped@luci-project-accounts.iam.gserviceaccount.com (Gerrit)

unread,

Jun 22, 2026, 8:50:11 PM (2 days ago) Jun 22

to Tom Sepez, Lei Zhang, dan sinclair, pdfium-...@googlegroups.com

pdfium...@luci-project-accounts.iam.gserviceaccount.com submitted the change with unreviewed changes

Unreviewed changes

2 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:

```
The name of the file: xfa/fgas/crt/cfgas_stringformatter.cpp
Insertions: 12, Deletions: 12.

@@ -1359,18 +1359,18 @@
         ccf--;
         break;
       case 'E': {
-        pdfium::CheckedNumeric<int32_t> safeExponent = 0;
-        bool bExpSign = false;
-        pdfium::CheckedNumeric<int32_t> safeMultiplier = 1;
+        bool has_exp_sign = false;
+        pdfium::CheckedNumeric<int32_t> safe_exponent = 0;
+        pdfium::CheckedNumeric<int32_t> safe_multiplier = 1;
         while (cc < spSrcNum.size()) {
           if (spSrcNum[cc] == 'E' || spSrcNum[cc] == 'e') {
             break;
           }
           if (FXSYS_IsDecimalDigit(spSrcNum[cc])) {
             int32_t digit = FXSYS_DecimalCharToInt(spSrcNum[cc]);
-            safeExponent += safeMultiplier * digit;
-            safeMultiplier *= 10;
-            if (!safeExponent.IsValid() || !safeMultiplier.IsValid()) {
+            safe_exponent += safe_multiplier * digit;
+            safe_multiplier *= 10;
+            if (!safe_exponent.IsValid() || !safe_multiplier.IsValid()) {
               return false;
             }
             cc--;
@@ -1383,7 +1383,7 @@
           if (cc - iMinusLen + 1 <= spSrcNum.size() &&
               UNSAFE_TODO(wcsncmp(spSrcNum.data() + (cc - iMinusLen + 1),
                                   wsMinus.c_str(), iMinusLen)) == 0) {
-            bExpSign = true;
+            has_exp_sign = true;
             cc -= iMinusLen;
             continue;
           }
@@ -1391,8 +1391,8 @@
           return false;
         }
         cc--;
-        iExponent = safeExponent.ValueOrDie();
-        iExponent = bExpSign ? -iExponent : iExponent;
+        iExponent = safe_exponent.ValueOrDie();
+        iExponent = has_exp_sign ? -iExponent : iExponent;
         ccf--;
         break;
       }
@@ -1565,13 +1565,13 @@
             return false;
           }
           iExponent = 0;
-          bool bExpSign = false;
+          bool has_exp_sign = false;
           cc++;
           if (cc < spSrcNum.size()) {
             if (spSrcNum[cc] == '+') {
               cc++;
             } else if (spSrcNum[cc] == '-') {
-              bExpSign = true;
+              has_exp_sign = true;
               cc++;
             }
           }
@@ -1586,7 +1586,7 @@
             iExponent = iExponent * 10 + digit;
             cc++;
           }
-          iExponent = bExpSign ? -iExponent : iExponent;
+          iExponent = has_exp_sign ? -iExponent : iExponent;
           break;
         }
         case '$': {
```

Change information

Commit message:

Fix exponent parsing in CFGAS_StringFormatter::ParseNum backward loop

When parsing numbers without a dot in the input but with an exponent
specified (e.g., "12e2"), the parser walks backward. The backward loop
incorrectly calculated the exponent by multiplying the current
accumulator by 10 and adding the new digit, which is incorrect for
decimal base when walking backward.

Fix this by using CheckedNumeric and a multiplier that increases by
factor of 10 at each step, ensuring correct positional value of digits
when processed from right to left.

Add unit tests to verify the fix for both positive and negative
exponents.

TAG=agy
CONV=32392532-2550-4531-8902-0f605362e1ca

Change-Id: If0cd346150150952cff1c40832c316f047aa9839

Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/150470

Commit-Queue: Tom Sepez <tse...@chromium.org>

Reviewed-by: Lei Zhang <the...@chromium.org>

Files:

M xfa/fgas/crt/cfgas_stringformatter.cpp
M xfa/fgas/crt/cfgas_stringformatter_unittest.cpp

Change size: S

Delta: 2 files changed, 15 insertions(+), 11 deletions(-)

Branch: refs/heads/main

Submit Requirements:

Code-Review: +1 by Lei Zhang

Open in Gerrit

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

open

diffy

satisfied_requirement

Reply all

Reply to author

Forward

Fix exponent parsing in CFGAS_StringFormatter::ParseNum backward loop [pdfium : main]

Tom Sepez (Gerrit)

Tom Sepez voted and added 1 comment

Votes added by Tom Sepez

1 comment

Related details

Lei Zhang (Gerrit)

Lei Zhang added 1 comment

Related details

Lei Zhang (Gerrit)

Lei Zhang voted and added 2 comments

Votes added by Lei Zhang

2 comments

Related details

Tom Sepez (Gerrit)

Tom Sepez voted and added 3 comments

Votes added by Tom Sepez

3 comments

Related details

pdfium-scoped@luci-project-accounts.iam.gserviceaccount.com (Gerrit)

pdfium...@luci-project-accounts.iam.gserviceaccount.com submitted the change with unreviewed changes

Unreviewed changes

Change information