diff --git a/java/src/org/openqa/selenium/json/JsonInput.java b/java/src/org/openqa/selenium/json/JsonInput.java index d40fb51f2b7b2..10e891e87eb17 100644 --- a/java/src/org/openqa/selenium/json/JsonInput.java +++ b/java/src/org/openqa/selenium/json/JsonInput.java @@ -133,7 +133,6 @@ public JsonType peek() { return JsonType.NULL; case '-': - case '+': case '0': case '1': case '2': @@ -223,52 +222,88 @@ public String nextName() { */ public Number nextNumber() { expect(JsonType.NUMBER); - boolean mightBeDecimal = 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((char) input.read()); - break; - case '.': - case 'e': - case 'E': - mightBeDecimal = true; - builder.append((char) input.read()); - break; - default: - read = false; + boolean isDecimal = false; + + // Optional leading minus. (Per RFC 8259 §6, a leading '+' is not allowed.) + if (input.peek() == '-') { + builder.append((char) input.read()); + } + + // Integer part: either "0" or [1-9] [0-9]*. + int first = input.peek(); + if (first == '0') { + builder.append((char) input.read()); + // Leading zeros ("00", "01", ...) are not allowed. + if (isDigit(input.peek())) { + throw new JsonException("Leading zeros are not permitted in JSON numbers. " + input); + } + } else if (first >= '1' && first <= '9') { + while (isDigit(input.peek())) { + builder.append((char) input.read()); } - } while (read); + } else { + throw new JsonException("Expected digit but saw " + describeChar(first) + ". " + input); + } + + // Optional fractional part: '.' 1*DIGIT + if (input.peek() == '.') { + isDecimal = true; + builder.append((char) input.read()); + if (!isDigit(input.peek())) { + throw new JsonException( + "Expected at least one digit after '.' but saw " + + describeChar(input.peek()) + + ". " + + input); + } + while (isDigit(input.peek())) { + builder.append((char) input.read()); + } + } + + // Optional exponent part: ('e' | 'E') ('+' | '-')? 1*DIGIT + if (input.peek() == 'e' || input.peek() == 'E') { + isDecimal = true; + builder.append((char) input.read()); + if (input.peek() == '+' || input.peek() == '-') { + builder.append((char) input.read()); + } + if (!isDigit(input.peek())) { + throw new JsonException( + "Expected at least one digit in exponent but saw " + + describeChar(input.peek()) + + ". " + + input); + } + while (isDigit(input.peek())) { + builder.append((char) input.read()); + } + } try { - // The JSON Schema does state the decimal point should not be used distinguish between - // integers and floating point values. - // Therefore, using a Long is only a fast path here, but we should not rely on the `double` - // value below is a real floating point. - if (!mightBeDecimal) { + // Fast path for integers: Long-valued when no fraction/exponent was present. + if (!isDecimal) { return Long.valueOf(builder.toString()); } - - return new BigDecimal(builder.toString()).doubleValue(); + double value = new BigDecimal(builder.toString()).doubleValue(); + if (Double.isInfinite(value) || Double.isNaN(value)) { + throw new JsonException("Number is out of range for a double: " + builder + ". " + input); + } + return value; } catch (NumberFormatException e) { throw new JsonException("Unable to parse to a number: " + builder + ". " + input, e); } } + private static boolean isDigit(int c) { + return c >= '0' && c <= '9'; + } + + private static String describeChar(int c) { + return c == Input.EOF ? "" : "'" + (char) c + "'"; + } + /** * Read the next element of the JSON input stream as a string. * diff --git a/java/test/org/openqa/selenium/json/JsonInputTest.java b/java/test/org/openqa/selenium/json/JsonInputTest.java index 99737c49b8618..037c5714c5ca7 100644 --- a/java/test/org/openqa/selenium/json/JsonInputTest.java +++ b/java/test/org/openqa/selenium/json/JsonInputTest.java @@ -325,6 +325,65 @@ void shouldRejectUnescapedControlCharactersInStrings() { } } + @Test + void shouldRejectSpecInvalidNumbers() { + // See RFC 8259 §6. Each of these was previously accepted by nextNumber(). + for (String bad : + new String[] { + "+5", // leading plus not allowed + "01", // leading zero not allowed + "007", // leading zero not allowed + ".5", // no digit before decimal + "5.", // no digit after decimal + "1e", // exponent without digits + "-", // sign without digits + }) { + try (JsonInput input = newInput(bad)) { + assertThatExceptionOfType(JsonException.class) + .describedAs("Input %s should be rejected", bad) + .isThrownBy(input::nextNumber); + } + } + } + + @Test + void shouldStillAcceptSpecValidNumbers() { + assertThat(parseNumber("0")).isEqualTo(0L); + assertThat(parseNumber("-0")).isEqualTo(0L); + assertThat(parseNumber("42")).isEqualTo(42L); + assertThat(parseNumber("-17")).isEqualTo(-17L); + assertThat(parseNumber("3.14")).isEqualTo(3.14d); + assertThat(parseNumber("-2.5e10")).isEqualTo(-2.5e10d); + assertThat(parseNumber("1E+2")).isEqualTo(100.0d); + assertThat(parseNumber("0.0")).isEqualTo(0.0d); + } + + @Test + void shouldRejectDoubleOverflow() { + try (JsonInput input = newInput("1e9999")) { + assertThatExceptionOfType(JsonException.class) + .isThrownBy(input::nextNumber) + .withMessageContaining("out of range"); + } + } + + private Number parseNumber(String raw) { + try (JsonInput input = newInput(raw)) { + return input.nextNumber(); + } + } + + @Test + void shouldReportEndOfInputAsEofNotAsAUnicodeReplacement() { + // If the number ends prematurely, the diagnostic should say "" rather than "'￿'" + // (which is a valid literal character elsewhere). + try (JsonInput input = newInput("5.")) { + assertThatExceptionOfType(JsonException.class) + .isThrownBy(input::nextNumber) + .withMessageContaining(""); + } + } + @Test void nullInputsShouldCoerceAsNullValues() throws IOException { try (InputStream is = new ByteArrayInputStream(new byte[0]);