Skip to content

Commit

Permalink
Fix bug 686806:
Browse files Browse the repository at this point in the history 
- trailing commas aren't allowed in object/array literals per JSON spec
- avoid using Integer.parseInt() to parse unicode escape sequences since parseInt() also accepts non-ASCII digits
- also avoid using Character.isDigit() in readNumber() for the very same reason
- readString() always created a StringBuilder instance to collect the input data, obviously this is actually only necessary when the input contains escaped characters. Therefore I've changed readString() to take the same approach as used in jsonparser.cpp
- the JSON number specification is stricter than Double.parseDouble(), for example Double.parseDouble() accepts the input string '10.'. To ensure only valid JSON number literals are passed to Double.parseDouble(), readNumber() was refactored to perform the necessary input validation.
  • Loading branch information
@anba
anba committed Jul 11, 2012
1 parent 5ef60f6 commit 2cf79b0
Show file tree
Hide file tree
Showing 2 changed files with 217 additions and 83 deletions.
240 changes: 157 additions & 83 deletions src/org/mozilla/javascript/json/JsonParser.java
View file
Expand Up @@ -86,15 +86,23 @@ private Object readValue() throws ParseException {
}

private Object readObject() throws ParseException {
consumeWhitespace();
Scriptable object = cx.newObject(scope);
// handle empty object literal case early
if (pos < length && src.charAt(pos) == '}') {
pos += 1;
return object;
}
String id;
Object value;
boolean needsComma = false;
consumeWhitespace();
while (pos < length) {
char c = src.charAt(pos++);
switch(c) {
case '}':
if (!needsComma) {
throw new ParseException("Unexpected comma in object literal");
}
return object;
case ',':
if (!needsComma) {
Expand Down Expand Up @@ -128,13 +136,21 @@ private Object readObject() throws ParseException {
}

private Object readArray() throws ParseException {
consumeWhitespace();
// handle empty array literal case early
if (pos < length && src.charAt(pos) == ']') {
pos += 1;
return cx.newArray(scope, 0);
}
List<Object> list = new ArrayList<Object>();
boolean needsComma = false;
consumeWhitespace();
while (pos < length) {
char c = src.charAt(pos);
switch(c) {
case ']':
if (!needsComma) {
throw new ParseException("Unexpected comma in array literal");
}
pos += 1;
return cx.newArray(scope, list.toArray());
case ',':
Expand All @@ -157,108 +173,166 @@ private Object readArray() throws ParseException {
}

private String readString() throws ParseException {
StringBuilder b = new StringBuilder();
/*
* Optimization: if the source contains no escaped characters, create the
* string directly from the source text.
*/
int stringStart = pos;
while (pos < length) {
char c = src.charAt(pos++);
if (c <= '\u001F') {
throw new ParseException("String contains control character");
} else if (c == '\\') {
break;
} else if (c == '"') {
return src.substring(stringStart, pos - 1);
}
switch(c) {
}

/*
* Slow case: string contains escaped characters. Copy a maximal sequence
* of unescaped characters into a temporary buffer, then an escaped
* character, and repeat until the entire string is consumed.
*/
StringBuilder b = new StringBuilder();
while (pos < length) {
assert src.charAt(pos - 1) == '\\';
b.append(src, stringStart, pos - 1);
if (pos >= length) {
throw new ParseException("Unterminated string");
}
char c = src.charAt(pos++);
switch (c) {
case '"':
b.append('"');
break;
case '\\':
if (pos >= length) {
throw new ParseException("Unterminated string");
b.append('\\');
break;
case '/':
b.append('/');
break;
case 'b':
b.append('\b');
break;
case 'f':
b.append('\f');
break;
case 'n':
b.append('\n');
break;
case 'r':
b.append('\r');
break;
case 't':
b.append('\t');
break;
case 'u':
if (length - pos < 5) {
throw new ParseException("Invalid character code: \\u" + src.substring(pos));
}
c = src.charAt(pos++);
switch (c) {
case '"':
b.append('"');
break;
case '\\':
b.append('\\');
break;
case '/':
b.append('/');
break;
case 'b':
b.append('\b');
break;
case 'f':
b.append('\f');
break;
case 'n':
b.append('\n');
break;
case 'r':
b.append('\r');
break;
case 't':
b.append('\t');
break;
case 'u':
if (length - pos < 5) {
throw new ParseException("Invalid character code: \\u" + src.substring(pos));
}
try {
b.append((char) Integer.parseInt(src.substring(pos, pos + 4), 16));
pos += 4;
} catch (NumberFormatException nfx) {
throw new ParseException("Invalid character code: " + src.substring(pos, pos + 4));
}
break;
default:
throw new ParseException("Unexcpected character in string: '\\" + c + "'");
int code = fromHex(src.charAt(pos + 0)) << 12
| fromHex(src.charAt(pos + 1)) << 8
| fromHex(src.charAt(pos + 2)) << 4
| fromHex(src.charAt(pos + 3));
if (code < 0) {
throw new ParseException("Invalid character code: " + src.substring(pos, pos + 4));
}
pos += 4;
b.append((char) code);
break;
case '"':
return b.toString();
default:
b.append(c);
throw new ParseException("Unexpected character in string: '\\" + c + "'");
}
stringStart = pos;
while (pos < length) {
c = src.charAt(pos++);
if (c <= '\u001F') {
throw new ParseException("String contains control character");
} else if (c == '\\') {
break;
} else if (c == '"') {
b.append(src, stringStart, pos - 1);
return b.toString();
}
}
}
throw new ParseException("Unterminated string literal");
}

private Number readNumber(char first) throws ParseException {
StringBuilder b = new StringBuilder();
b.append(first);
while (pos < length) {
char c = src.charAt(pos);
if (!Character.isDigit(c)
&& c != '-'
&& c != '+'
&& c != '.'
&& c != 'e'
&& c != 'E') {
break;
private int fromHex(char c) {
return c >= '0' && c <= '9' ? c - '0'
: c >= 'A' && c <= 'F' ? c - 'A' + 10
: c >= 'a' && c <= 'f' ? c - 'a' + 10
: -1;
}

private Number readNumber(char c) throws ParseException {
assert c == '-' || (c >= '0' && c <= '9');
final int numberStart = pos - 1;
if (c == '-') {
c = nextOrNumberError(numberStart);
if (!(c >= '0' && c <= '9')) {
throw numberError(numberStart, pos);
}
pos += 1;
b.append(c);
}
String num = b.toString();
int numLength = num.length();
try {
// check for leading zeroes
for (int i = 0; i < numLength; i++) {
char c = num.charAt(i);
if (Character.isDigit(c)) {
if (c == '0'
&& numLength > i + 1
&& Character.isDigit(num.charAt(i + 1))) {
throw new ParseException("Unsupported number format: " + num);
}
break;
if (c != '0') {
readDigits();
}
// read optional fraction part
if (pos < length) {
c = src.charAt(pos);
if (c == '.') {
pos += 1;
c = nextOrNumberError(numberStart);
if (!(c >= '0' && c <= '9')) {
throw numberError(numberStart, pos);
}
readDigits();
}
final double dval = Double.parseDouble(num);
final int ival = (int)dval;
if (ival == dval) {
return Integer.valueOf(ival);
} else {
return Double.valueOf(dval);
}
// read optional exponent part
if (pos < length) {
c = src.charAt(pos);
if (c == 'e' || c == 'E') {
pos += 1;
c = nextOrNumberError(numberStart);
if (c == '-' || c == '+') {
c = nextOrNumberError(numberStart);
}
if (!(c >= '0' && c <= '9')) {
throw numberError(numberStart, pos);
}
readDigits();
}
}
String num = src.substring(numberStart, pos);
final double dval = Double.parseDouble(num);
final int ival = (int)dval;
if (ival == dval) {
return Integer.valueOf(ival);
} else {
return Double.valueOf(dval);
}
}

private ParseException numberError(int start, int end) {
return new ParseException("Unsupported number format: " + src.substring(start, end));
}

private char nextOrNumberError(int numberStart) throws ParseException {
if (pos >= length) {
throw numberError(numberStart, length);
}
return src.charAt(pos++);
}

private void readDigits() {
for (; pos < length; ++pos) {
char c = src.charAt(pos);
if (!(c >= '0' && c <= '9')) {
break;
}
} catch (NumberFormatException nfe) {
throw new ParseException("Unsupported number format: " + num);
}
}

Expand Down
60 changes: 60 additions & 0 deletions testsrc/org/mozilla/javascript/tests/json/JsonParserTest.java
View file
Expand Up @@ -196,6 +196,66 @@ public void shouldThrowParseExceptionWhenIncompleteArray() throws Exception {
parser.parseValue("[1 ");
}

@Test(expected = ParseException.class)
public void shouldFailToParseIllegalUnicodeEscapeSeq() throws Exception {
parser.parseValue("\"\\u-123\"");
}

@Test(expected = ParseException.class)
public void shouldFailToParseIllegalUnicodeEscapeSeq2() throws Exception {
parser.parseValue("\"\\u006\u0661\"");
}

@Test(expected = ParseException.class)
public void shouldFailToParseIllegalUnicodeEscapeSeq3() throws Exception {
parser.parseValue("\"\\u006١\"");
}

@Test(expected = ParseException.class)
public void shouldFailToParseTrailingCommaInObject1() throws Exception {
parser.parseValue("{\"a\": 1,}");
}

@Test(expected = ParseException.class)
public void shouldFailToParseTrailingCommaInObject2() throws Exception {
parser.parseValue("{,\"a\": 1}");
}

@Test(expected = ParseException.class)
public void shouldFailToParseTrailingCommaInObject3() throws Exception {
parser.parseValue("{,}");
}

@Test
public void shouldParseEmptyObject() throws Exception {
parser.parseValue("{}");
}

@Test(expected = ParseException.class)
public void shouldFailToParseTrailingCommaInArray1() throws Exception {
parser.parseValue("[1,]");
}

@Test(expected = ParseException.class)
public void shouldFailToParseTrailingCommaInArray2() throws Exception {
parser.parseValue("[,1]");
}

@Test(expected = ParseException.class)
public void shouldFailToParseTrailingCommaInArray3() throws Exception {
parser.parseValue("[,]");
}

@Test
public void shouldParseEmptyArray() throws Exception {
parser.parseValue("[]");
}

@Test(expected = ParseException.class)
public void shouldFailToParseIllegalNumber() throws Exception {
parser.parseValue("1.");
}

private String str(char... chars) {
return new String(chars);
}
Expand Down

0 comments on commit 2cf79b0

Please sign in to comment.