Skip to content

Commit 740b692

Browse files
authored
[java] Tighten JSON number lexer to match RFC 8259 (#17739)
The previous nextNumber() collected any subset of '- + 0-9 . e E' and then handed the string to Long.valueOf or BigDecimal. That accepted several spec-invalid forms: '+5' (leading plus), '01' (leading zero), '.5' (no digit before decimal), '5.' (no digit after decimal), '1e' (exponent without digits), '-' alone. Very large exponents also silently produced Double.POSITIVE_INFINITY, for which JSON has no representation. Replace the collector with a proper state machine that follows the grammar 'number = [minus] int [frac] [exp]', and reject non-finite doubles. Also drop '+' from the peek() classifier so it no longer claims to see a JSON number.
1 parent 5b842c1 commit 740b692

2 files changed

Lines changed: 130 additions & 36 deletions

File tree

java/src/org/openqa/selenium/json/JsonInput.java

Lines changed: 71 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,6 @@ public JsonType peek() {
133133
return JsonType.NULL;
134134

135135
case '-':
136-
case '+':
137136
case '0':
138137
case '1':
139138
case '2':
@@ -223,52 +222,88 @@ public String nextName() {
223222
*/
224223
public Number nextNumber() {
225224
expect(JsonType.NUMBER);
226-
boolean mightBeDecimal = false;
227225
StringBuilder builder = new StringBuilder();
228-
// We know it's safe to use a do/while loop since the first character was a number
229-
boolean read = true;
230-
do {
231-
switch (input.peek()) {
232-
case '-':
233-
case '+':
234-
case '0':
235-
case '1':
236-
case '2':
237-
case '3':
238-
case '4':
239-
case '5':
240-
case '6':
241-
case '7':
242-
case '8':
243-
case '9':
244-
builder.append((char) input.read());
245-
break;
246-
case '.':
247-
case 'e':
248-
case 'E':
249-
mightBeDecimal = true;
250-
builder.append((char) input.read());
251-
break;
252-
default:
253-
read = false;
226+
boolean isDecimal = false;
227+
228+
// Optional leading minus. (Per RFC 8259 §6, a leading '+' is not allowed.)
229+
if (input.peek() == '-') {
230+
builder.append((char) input.read());
231+
}
232+
233+
// Integer part: either "0" or [1-9] [0-9]*.
234+
int first = input.peek();
235+
if (first == '0') {
236+
builder.append((char) input.read());
237+
// Leading zeros ("00", "01", ...) are not allowed.
238+
if (isDigit(input.peek())) {
239+
throw new JsonException("Leading zeros are not permitted in JSON numbers. " + input);
240+
}
241+
} else if (first >= '1' && first <= '9') {
242+
while (isDigit(input.peek())) {
243+
builder.append((char) input.read());
254244
}
255-
} while (read);
245+
} else {
246+
throw new JsonException("Expected digit but saw " + describeChar(first) + ". " + input);
247+
}
248+
249+
// Optional fractional part: '.' 1*DIGIT
250+
if (input.peek() == '.') {
251+
isDecimal = true;
252+
builder.append((char) input.read());
253+
if (!isDigit(input.peek())) {
254+
throw new JsonException(
255+
"Expected at least one digit after '.' but saw "
256+
+ describeChar(input.peek())
257+
+ ". "
258+
+ input);
259+
}
260+
while (isDigit(input.peek())) {
261+
builder.append((char) input.read());
262+
}
263+
}
264+
265+
// Optional exponent part: ('e' | 'E') ('+' | '-')? 1*DIGIT
266+
if (input.peek() == 'e' || input.peek() == 'E') {
267+
isDecimal = true;
268+
builder.append((char) input.read());
269+
if (input.peek() == '+' || input.peek() == '-') {
270+
builder.append((char) input.read());
271+
}
272+
if (!isDigit(input.peek())) {
273+
throw new JsonException(
274+
"Expected at least one digit in exponent but saw "
275+
+ describeChar(input.peek())
276+
+ ". "
277+
+ input);
278+
}
279+
while (isDigit(input.peek())) {
280+
builder.append((char) input.read());
281+
}
282+
}
256283

257284
try {
258-
// The JSON Schema does state the decimal point should not be used distinguish between
259-
// integers and floating point values.
260-
// Therefore, using a Long is only a fast path here, but we should not rely on the `double`
261-
// value below is a real floating point.
262-
if (!mightBeDecimal) {
285+
// Fast path for integers: Long-valued when no fraction/exponent was present.
286+
if (!isDecimal) {
263287
return Long.valueOf(builder.toString());
264288
}
265-
266-
return new BigDecimal(builder.toString()).doubleValue();
289+
double value = new BigDecimal(builder.toString()).doubleValue();
290+
if (Double.isInfinite(value) || Double.isNaN(value)) {
291+
throw new JsonException("Number is out of range for a double: " + builder + ". " + input);
292+
}
293+
return value;
267294
} catch (NumberFormatException e) {
268295
throw new JsonException("Unable to parse to a number: " + builder + ". " + input, e);
269296
}
270297
}
271298

299+
private static boolean isDigit(int c) {
300+
return c >= '0' && c <= '9';
301+
}
302+
303+
private static String describeChar(int c) {
304+
return c == Input.EOF ? "<EOF>" : "'" + (char) c + "'";
305+
}
306+
272307
/**
273308
* Read the next element of the JSON input stream as a string.
274309
*

java/test/org/openqa/selenium/json/JsonInputTest.java

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -325,6 +325,65 @@ void shouldRejectUnescapedControlCharactersInStrings() {
325325
}
326326
}
327327

328+
@Test
329+
void shouldRejectSpecInvalidNumbers() {
330+
// See RFC 8259 §6. Each of these was previously accepted by nextNumber().
331+
for (String bad :
332+
new String[] {
333+
"+5", // leading plus not allowed
334+
"01", // leading zero not allowed
335+
"007", // leading zero not allowed
336+
".5", // no digit before decimal
337+
"5.", // no digit after decimal
338+
"1e", // exponent without digits
339+
"-", // sign without digits
340+
}) {
341+
try (JsonInput input = newInput(bad)) {
342+
assertThatExceptionOfType(JsonException.class)
343+
.describedAs("Input %s should be rejected", bad)
344+
.isThrownBy(input::nextNumber);
345+
}
346+
}
347+
}
348+
349+
@Test
350+
void shouldStillAcceptSpecValidNumbers() {
351+
assertThat(parseNumber("0")).isEqualTo(0L);
352+
assertThat(parseNumber("-0")).isEqualTo(0L);
353+
assertThat(parseNumber("42")).isEqualTo(42L);
354+
assertThat(parseNumber("-17")).isEqualTo(-17L);
355+
assertThat(parseNumber("3.14")).isEqualTo(3.14d);
356+
assertThat(parseNumber("-2.5e10")).isEqualTo(-2.5e10d);
357+
assertThat(parseNumber("1E+2")).isEqualTo(100.0d);
358+
assertThat(parseNumber("0.0")).isEqualTo(0.0d);
359+
}
360+
361+
@Test
362+
void shouldRejectDoubleOverflow() {
363+
try (JsonInput input = newInput("1e9999")) {
364+
assertThatExceptionOfType(JsonException.class)
365+
.isThrownBy(input::nextNumber)
366+
.withMessageContaining("out of range");
367+
}
368+
}
369+
370+
private Number parseNumber(String raw) {
371+
try (JsonInput input = newInput(raw)) {
372+
return input.nextNumber();
373+
}
374+
}
375+
376+
@Test
377+
void shouldReportEndOfInputAsEofNotAsAUnicodeReplacement() {
378+
// If the number ends prematurely, the diagnostic should say "<EOF>" rather than "'￿'"
379+
// (which is a valid literal character elsewhere).
380+
try (JsonInput input = newInput("5.")) {
381+
assertThatExceptionOfType(JsonException.class)
382+
.isThrownBy(input::nextNumber)
383+
.withMessageContaining("<EOF>");
384+
}
385+
}
386+
328387
@Test
329388
void nullInputsShouldCoerceAsNullValues() throws IOException {
330389
try (InputStream is = new ByteArrayInputStream(new byte[0]);

0 commit comments

Comments
 (0)