Skip to content

Commit

Permalink
Improve parsing error messages (#405)
Browse files Browse the repository at this point in the history
- output more informative error messages in `js_parse_expect`.

The previous code was bogus:
```
    return js_parse_error(s, "expecting '%c'", tok);
```
this was causing a bug on `eval("do;")` where `tok` is `TOK_WHILE` (-70, 0xBA)
creating an invalid UTF-8 encoding (lone trailing byte).
This would ultimately have caused a failure in `JS_ThrowError2` if `JS_NewString`
failed when converting the error message to a string if the conversion detected the invalid
UTF-8 encoding and throwed an error (it currently does not, but should).

- test for `JS_NewString` failure in `JS_ThrowError2`
- test for `JS_FreeCString` failure in run-test262.c
- add more test cases
  • Loading branch information
chqrlie committed May 14, 2024
1 parent 99c6719 commit 5a7e578
Show file tree
Hide file tree
Showing 3 changed files with 72 additions and 14 deletions.
42 changes: 34 additions & 8 deletions quickjs.c
Original file line number Diff line number Diff line change
Expand Up @@ -6635,7 +6635,7 @@ static JSValue JS_ThrowError2(JSContext *ctx, JSErrorEnum error_num,
const char *fmt, va_list ap, BOOL add_backtrace)
{
char buf[256];
JSValue obj, ret;
JSValue obj, ret, msg;

vsnprintf(buf, sizeof(buf), fmt, ap);
obj = JS_NewObjectProtoClass(ctx, ctx->native_error_proto[error_num],
Expand All @@ -6644,9 +6644,13 @@ static JSValue JS_ThrowError2(JSContext *ctx, JSErrorEnum error_num,
/* out of memory: throw JS_NULL to avoid recursing */
obj = JS_NULL;
} else {
JS_DefinePropertyValue(ctx, obj, JS_ATOM_message,
JS_NewString(ctx, buf),
JS_PROP_WRITABLE | JS_PROP_CONFIGURABLE);
msg = JS_NewString(ctx, buf);
if (JS_IsException(msg))
msg = JS_NewString(ctx, "Invalid error message");
if (!JS_IsException(msg)) {
JS_DefinePropertyValue(ctx, obj, JS_ATOM_message, msg,
JS_PROP_WRITABLE | JS_PROP_CONFIGURABLE);
}
}
if (add_backtrace) {
build_backtrace(ctx, obj, NULL, 0, 0, 0);
Expand Down Expand Up @@ -18633,11 +18637,33 @@ int __attribute__((format(printf, 2, 3))) js_parse_error(JSParseState *s, const

static int js_parse_expect(JSParseState *s, int tok)
{
if (s->token.val != tok) {
/* XXX: dump token correctly in all cases */
return js_parse_error(s, "expecting '%c'", tok);
char buf[ATOM_GET_STR_BUF_SIZE];

if (s->token.val == tok)
return next_token(s);

switch(s->token.val) {
case TOK_EOF:
return js_parse_error(s, "Unexpected end of input");
case TOK_NUMBER:
return js_parse_error(s, "Unexpected number");
case TOK_STRING:
return js_parse_error(s, "Unexpected string");
case TOK_TEMPLATE:
return js_parse_error(s, "Unexpected string template");
case TOK_REGEXP:
return js_parse_error(s, "Unexpected regexp");
case TOK_IDENT:
return js_parse_error(s, "Unexpected identifier '%s'",
JS_AtomGetStr(s->ctx, buf, sizeof(buf),
s->token.u.ident.atom));
case TOK_ERROR:
return js_parse_error(s, "Invalid or unexpected token");
default:
return js_parse_error(s, "Unexpected token '%.*s'",
(int)(s->buf_ptr - s->token.ptr),
(const char *)s->token.ptr);
}
return next_token(s);
}

static int js_parse_expect_semi(JSParseState *s)
Expand Down
12 changes: 8 additions & 4 deletions run-test262.c
Original file line number Diff line number Diff line change
Expand Up @@ -1299,11 +1299,15 @@ static int eval_buf(JSContext *ctx, const char *buf, size_t buf_len,
const char *msg;

msg = JS_ToCString(ctx, exception_val);
error_class = strdup_len(msg, strcspn(msg, ":"));
if (!str_equal(error_class, error_type))
if (msg == NULL) {
ret = -1;
free(error_class);
JS_FreeCString(ctx, msg);
} else {
error_class = strdup_len(msg, strcspn(msg, ":"));
if (!str_equal(error_class, error_type))
ret = -1;
free(error_class);
JS_FreeCString(ctx, msg);
}
}
} else {
ret = -1;
Expand Down
32 changes: 30 additions & 2 deletions tests/test_language.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,14 @@ function assert_throws(expected_error, func, message)
var err = false;
var msg = message ? " (" + message + ")" : "";
try {
func();
switch (typeof func) {
case 'string':
eval(func);
break;
case 'function':
func();
break;
}
} catch(e) {
err = true;
if (!(e instanceof expected_error)) {
Expand Down Expand Up @@ -546,7 +553,7 @@ function test_function_expr_name()

function test_expr(expr, err) {
if (err)
assert_throws(err, () => eval(expr), `for ${expr}`);
assert_throws(err, expr, `for ${expr}`);
else
assert(1, eval(expr), `for ${expr}`);
}
Expand Down Expand Up @@ -608,6 +615,26 @@ function test_number_literals()
test_expr('0.a', SyntaxError);
}

function test_syntax()
{
assert_throws(SyntaxError, "do");
assert_throws(SyntaxError, "do;");
assert_throws(SyntaxError, "do{}");
assert_throws(SyntaxError, "if");
assert_throws(SyntaxError, "if\n");
assert_throws(SyntaxError, "if 1");
assert_throws(SyntaxError, "if \0");
assert_throws(SyntaxError, "if ;");
assert_throws(SyntaxError, "if 'abc'");
assert_throws(SyntaxError, "if `abc`");
assert_throws(SyntaxError, "if /abc/");
assert_throws(SyntaxError, "if abc");
assert_throws(SyntaxError, "if abc\u0064");
assert_throws(SyntaxError, "if abc\\u0064");
assert_throws(SyntaxError, "if \u0123");
assert_throws(SyntaxError, "if \\u0123");
}

test_op1();
test_cvt();
test_eq();
Expand All @@ -629,3 +656,4 @@ test_argument_scope();
test_function_expr_name();
test_reserved_names();
test_number_literals();
test_syntax();

0 comments on commit 5a7e578

Please sign in to comment.