[java] fix JSON parsing of numbers with exponent

[joerg1985]

User description

🔗 Related Issues

I noticed this when reading the z-index of an element via Javascript.
All numbers with exponent and no fraction part are broken.
e.g. 42e-1 should be parsed as double 4.2 but instead was parsed as long 4

💥 What does this PR do?

Fix the JSON parsing of numbers with exponent and no fraction part.

🔧 Implementation Notes

💡 Additional Considerations

🔄 Types of changes

• Bug fix (backwards compatible)

---

PR Type

Bug fix

---

Description

• Fix JSON parsing of numbers with exponent notation

• Correctly parse exponents without fractional parts as doubles

• Refactor number parsing logic using switch statement

• Add test cases for exponent notation variants

---

Diagram Walkthrough

flowchart LR
  A["JSON Number Input"] --> B["Check for decimal point or exponent"]
  B --> C{Contains decimal or exponent?}
  C -->|Yes| D["Parse as Double via BigDecimal"]
  C -->|No| E["Parse as Long"]
  D --> F["Return Number"]
  E --> F

Loading

File Walkthrough

	Relevant files
Bug fix	JsonInput.java Refactor number parsing with exponent support                        java/src/org/openqa/selenium/json/JsonInput.java Refactored number parsing logic from character-by-character checks to switch statement Changed tracking from fractionalPart boolean to decimal boolean to detect both decimal points and exponents Fixed bug where numbers with exponents but no fractional part were incorrectly parsed as Long instead of Double Simplified parsing logic to return Long for integers and Double for numbers with decimal or exponent notation +30/-18
Tests	JsonTest.java Add exponent notation test cases                                                  java/test/org/openqa/selenium/json/JsonTest.java Added test case for exponent with decimal fraction: 4.2e+1 should parse as 42.0 Added test case for positive exponent without fraction: 42e+1 should parse as 420.0 Added test case for negative exponent without fraction: 42e-1 should parse as 4.2 Added test case for negative exponent with fraction: 4.2e-1 should parse as 0.42 +4/-0

---

[qodo-code-review]

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢	No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
⚪	🎫 No ticket provided Create ticket/issue
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
🟢	Generic: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Meaningful Naming and Self-Documenting Code Objective: Ensure all identifiers clearly express their purpose and intent, making code self-documenting Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
⚪	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Number grammar too loose: The updated loop accepts +/- anywhere in the number token (not only at the start or after e/E), which may over-consume invalid JSON and make recovery/continued parsing less robust. Referred Code switch (input.peek()) { case '-': case '+': case '0': case '1': case '2': case '3': case '4': case '5': case '6': case '7': case '8': case '9': builder.append(input.read()); break; case '.': case 'e': case 'E': decimal = true; builder.append(input.read()); break; ... (clipped 4 lines) Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: Exception leaks context: The thrown JsonException embeds both the parsed token (builder) and input state in the message, which could expose unexpected input contents depending on how/where the exception is surfaced. Referred Code } catch (NumberFormatException e) { throw new JsonException("Unable to parse to a number: " + builder + ". " + input); } Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: Permissive number parsing: By allowing +/- characters unconditionally during tokenization, the parser may accept/consume malformed JSON number inputs in a way that could affect downstream parsing or error localization. Referred Code switch (input.peek()) { case '-': case '+': case '0': case '1': case '2': case '3': case '4': case '5': case '6': case '7': case '8': case '9': builder.append(input.read()); break; case '.': case 'e': case 'E': decimal = true; builder.append(input.read()); break; ... (clipped 4 lines) Learn more about managing compliance generic rules or creating your own custom rules
Update

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

[qodo-code-review]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Implement robust stateful number parsing Implement a more robust, stateful number parser to correctly validate the structure of JSON numbers and reject invalid formats, preventing NumberFormatException from malformed inputs. java/src/org/openqa/selenium/json/JsonInput.java [221-263] public Number nextNumber() { expect(JsonType.NUMBER); - boolean decimal = false; StringBuilder builder = new StringBuilder(); - // We know it's safe to use a do/while loop since the first character was a number - boolean read = true; - do { - switch (input.peek()) { - case '-': - case '+': - case '0': - case '1': - case '2': - case '3': - case '4': - case '5': - case '6': - case '7': - case '8': - case '9': - builder.append(input.read()); - break; - case '.': - case 'e': - case 'E': - decimal = true; - builder.append(input.read()); - break; - default: - read = false; + boolean isDecimal = false; + + // The first character has been peeked at, and it's either a digit or a '-' + char next = input.peek(); + if (next == '-') { + builder.append(input.read()); + } + + // Read the integer part + while (Character.isDigit(input.peek())) { + builder.append(input.read()); + } + + // Read the fractional part + if (input.peek() == '.') { + isDecimal = true; + builder.append(input.read()); + while (Character.isDigit(input.peek())) { + builder.append(input.read()); } - } while (read); + } - try { - if (!decimal) { - return Long.valueOf(builder.toString()); + // Read the exponent part + next = input.peek(); + if (next == 'e' || next == 'E') { + isDecimal = true; + builder.append(input.read()); + + next = input.peek(); + if (next == '+' || next == '-') { + builder.append(input.read()); } - return new BigDecimal(builder.toString()).doubleValue(); + while (Character.isDigit(input.peek())) { + builder.append(input.read()); + } + } + + String raw = builder.toString(); + try { + if (isDecimal) { + return new BigDecimal(raw).doubleValue(); + } + return Long.valueOf(raw); } catch (NumberFormatException e) { - throw new JsonException("Unable to parse to a number: " + builder + ". " + input); + throw new JsonException("Unable to parse to a number: " + raw + ". " + input, e); } } Apply / Chat Suggestion importance[1-10]: 9 __ Why: The suggestion correctly identifies a significant robustness issue where the parser accepts malformed numbers (e.g., with multiple decimal points), leading to exceptions. The proposed stateful parsing logic is a major improvement that correctly validates the number's structure according to JSON specifications.	High
Learned best practice	Parameterize repeated test cases Collapse the exponent parsing cases into a small table-driven loop so adding more numeric formats doesn’t require more repetitive assertions. java/test/org/openqa/selenium/json/JsonTest.java [63-69] assertThat((Number) new Json().toType("42", Number.class)).isEqualTo(42L); assertThat((Integer) new Json().toType("42", Integer.class)).isEqualTo(42); assertThat((Double) new Json().toType("42", Double.class)).isEqualTo(42.0); -assertThat((Double) new Json().toType("4.2e+1", Double.class)).isEqualTo(42.0); -assertThat((Double) new Json().toType("42e+1", Double.class)).isEqualTo(420.0); -assertThat((Double) new Json().toType("42e-1", Double.class)).isEqualTo(4.2); -assertThat((Double) new Json().toType("4.2e-1", Double.class)).isEqualTo(0.42); +for (List<Object> tc : + List.of( + List.of("4.2e+1", 42.0), + List.of("42e+1", 420.0), + List.of("42e-1", 4.2), + List.of("4.2e-1", 0.42))) { + assertThat((Double) new Json().toType((String) tc.get(0), Double.class)).isEqualTo((Double) tc.get(1)); +} + Apply / Chat Suggestion importance[1-10]: 6 __ Why: Relevant best practice - Reduce duplication by centralizing repeated logic (e.g., repeated test assertions) into shared structures or loops.	Low
Update

[VietND96]

This is fine to me, can you also check the code suggestion Implement robust stateful number parsing is valid or not

[asolntsev]

I found a bug:

assertThat((Double) new Json().toType("42e+1", Integer.class)).isEqualTo(420);

throws java.lang.ClassCastException: class java.lang.Integer cannot be cast to class java.lang.Double :)

[joerg1985]

I found a bug:

assertThat((Double) new Json().toType("42e+1", Integer.class)).isEqualTo(420);

throws java.lang.ClassCastException: class java.lang.Integer cannot be cast to class java.lang.Double :)

The line above does try to cast an Integer to a Double, there is no valid change to the Json parser that could fix this.

[asolntsev]

@joerg1985 My bad, sorry! :)
LOL I was too rushing with findings, like a young cowboy... :)

Anyway, I have one small suggestion how to simplify the code.

[asolntsev]

A general question is: why Selenium re-invented the wheel instead of using some read-made JSON parser, e.g. Jackson?

This bug shows why it was a bad idea...

[asolntsev]

@joerg1985 Wait, this time I found a bug. I think. Maybe. Probably. :)

  @Test
  void shouldParseNonDecimalNumbersAsLongs_e() {
    try (JsonInput input = newInput("42e+1")) {
      assertThat(input.peek()).isEqualTo(NUMBER);
      assertThat(input.nextNumber()).isEqualTo(420L);
    }
  }

expected: 420L
 but was: 420.0

[joerg1985]

A general question is: why Selenium re-invented the wheel instead of using some read-made JSON parser, e.g. Jackson?

This bug shows why it was a bad idea...

I don't know, but in the past GSON has been used and i guess they had good reasons for this.

Into the exponent without fraction i have to look closer the next days.

[asolntsev]

Gson is not maintained anymore. So yes, it was a good idea to stop using Gson. But it doesn't mean we had to re-invent the wheel. ;)

This bug is not critical at all, indeed. I think we can merge this PR.

[asolntsev]

@joerg1985 I found another problem. At least inconsistency.

The case is when JsonInput is parsing a number followed by text. For two different texts, it shows a different behaviour:

  @Test
  void bug() {
    try (JsonInput input = newInput("12345altruistic")) {
      assertThat(input.peek()).isEqualTo(NUMBER);
      assertThat(input.nextNumber()).isEqualTo(12345L);
    }
    try (JsonInput input = newInput("12345egoistic")) { // throws JsonException: Unable to parse to a number: 12345e
      assertThat(input.peek()).isEqualTo(NUMBER);
      assertThat(input.nextNumber()).isEqualTo(12345L);
    }
  }

[asolntsev]

@joerg1985 I tried a bit different fix using BigDecimal.getScale().
Perhaps a bit simpler in the end.

#16988

[joerg1985]

@joerg1985 I found another problem. At least inconsistency.

The case is when JsonInput is parsing a number followed by text. For two different texts, it shows a different behaviour:

  @Test
  void bug() {
    try (JsonInput input = newInput("12345altruistic")) {
      assertThat(input.peek()).isEqualTo(NUMBER);
      assertThat(input.nextNumber()).isEqualTo(12345L);
    }
    try (JsonInput input = newInput("12345egoistic")) { // throws JsonException: Unable to parse to a number: 12345e
      assertThat(input.peek()).isEqualTo(NUMBER);
      assertThat(input.nextNumber()).isEqualTo(12345L);
    }
  }

The JsonInput does kind of stream the data, so the stream must be read to the end to be sure the complete input was valid.
So this is more or less by design and usually there should be a following next..., begin... or end... call, which must throw in this case.

LGTM, we could still change it before the next release.