From 5a7e5784824fb9a06e6892c3b8ab80aed9dd1121 Mon Sep 17 00:00:00 2001 From: Charlie Gordon Date: Tue, 14 May 2024 20:36:10 +0200 Subject: [PATCH] Improve parsing error messages (#405) - 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 --- quickjs.c | 42 ++++++++++++++++++++++++++++++++++-------- run-test262.c | 12 ++++++++---- tests/test_language.js | 32 ++++++++++++++++++++++++++++++-- 3 files changed, 72 insertions(+), 14 deletions(-) diff --git a/quickjs.c b/quickjs.c index 84c4448..57e907e 100644 --- a/quickjs.c +++ b/quickjs.c @@ -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], @@ -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); @@ -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) diff --git a/run-test262.c b/run-test262.c index 61be6cd..be4ed4d 100644 --- a/run-test262.c +++ b/run-test262.c @@ -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; diff --git a/tests/test_language.js b/tests/test_language.js index 083ae43..92cd03c 100644 --- a/tests/test_language.js +++ b/tests/test_language.js @@ -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)) { @@ -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}`); } @@ -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(); @@ -629,3 +656,4 @@ test_argument_scope(); test_function_expr_name(); test_reserved_names(); test_number_literals(); +test_syntax();