[PATCH 0/2] MR6322: jscript: eval and JSON fixes
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> They're global in scope. Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/jscript/engine.c | 4 ++-- dlls/mshtml/tests/documentmode.js | 4 ++++ 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/dlls/jscript/engine.c b/dlls/jscript/engine.c index ca662ae99ff..92ef45bbec0 100644 --- a/dlls/jscript/engine.c +++ b/dlls/jscript/engine.c @@ -3461,7 +3461,7 @@ HRESULT exec_source(script_ctx_t *ctx, DWORD flags, bytecode_t *bytecode, functi return hres; } - if((flags & EXEC_EVAL) && ctx->call_ctx) { + if((flags & EXEC_EVAL) && ctx->call_ctx && scope) { variable_obj = jsdisp_addref(ctx->call_ctx->variable_obj); }else if(!(flags & (EXEC_GLOBAL | EXEC_EVAL))) { hres = create_dispex(ctx, NULL, NULL, &variable_obj); @@ -3516,7 +3516,7 @@ HRESULT exec_source(script_ctx_t *ctx, DWORD flags, bytecode_t *bytecode, functi this_obj = NULL; } - if(ctx->call_ctx && (flags & EXEC_EVAL)) { + if(ctx->call_ctx && scope && (flags & EXEC_EVAL)) { hres = detach_variable_object(ctx, ctx->call_ctx, FALSE); if(FAILED(hres)) goto fail; diff --git a/dlls/mshtml/tests/documentmode.js b/dlls/mshtml/tests/documentmode.js index 7fdf510ddd7..73f54cd708b 100644 --- a/dlls/mshtml/tests/documentmode.js +++ b/dlls/mshtml/tests/documentmode.js @@ -862,6 +862,10 @@ sync_test("eval", function() { context = {}; (function(eval) { eval(code); })(function() { context.barfoo = 4321; }); ok(context.barfoo === 4321, "context.barfoo = " + context.barfoo); + + (0,eval)("var foobar = 'wine';"); + ok(foobar === "wine", "global foobar = " + foobar); + delete foobar; }); sync_test("for..in", function() { -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6322
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/jscript/json.c | 6 +++--- dlls/jscript/lex.c | 12 ++++++++++-- dlls/jscript/parser.h | 2 +- dlls/jscript/tests/api.js | 2 +- dlls/mshtml/tests/documentmode.js | 23 +++++++++++++++++++++++ 5 files changed, 38 insertions(+), 7 deletions(-) diff --git a/dlls/jscript/json.c b/dlls/jscript/json.c index b56d75d3570..d3177ab56a1 100644 --- a/dlls/jscript/json.c +++ b/dlls/jscript/json.c @@ -80,10 +80,10 @@ static HRESULT parse_json_string(json_parse_ctx_t *ctx, WCHAR **r) if(len) memcpy(buf, ptr, len*sizeof(WCHAR)); - if(!unescape(buf, &len)) { - FIXME("unescape failed\n"); + if(!unescape(buf, &len, ctx->ctx->html_mode)) { + WARN("unescape failed\n"); free(buf); - return E_FAIL; + return JS_E_INVALID_CHAR; } buf[len] = 0; diff --git a/dlls/jscript/lex.c b/dlls/jscript/lex.c index 3bb2040803d..99eb0bc4c77 100644 --- a/dlls/jscript/lex.c +++ b/dlls/jscript/lex.c @@ -224,7 +224,7 @@ static BOOL skip_spaces(parser_ctx_t *ctx) return ctx->ptr != ctx->end; } -BOOL unescape(WCHAR *str, size_t *len) +BOOL unescape(WCHAR *str, size_t *len, BOOL json) { WCHAR *pd, *p, c, *end = str + *len; int i; @@ -241,8 +241,12 @@ BOOL unescape(WCHAR *str, size_t *len) switch(*p) { case '\'': + if(json) + return FALSE; + /* fall through */ case '\"': case '\\': + case '/': c = *p; break; case 'b': @@ -261,6 +265,8 @@ BOOL unescape(WCHAR *str, size_t *len) c = '\r'; break; case 'x': + if(json) + return FALSE; if(p + 2 >= end) return FALSE; i = hex_to_int(*++p); @@ -297,6 +303,8 @@ BOOL unescape(WCHAR *str, size_t *len) c += i; break; default: + if(json) + return FALSE; if(is_digit(*p)) { c = *p++ - '0'; if(p < end && is_digit(*p)) { @@ -362,7 +370,7 @@ static int parse_string_literal(parser_ctx_t *ctx, jsstr_t **ret, WCHAR endch) if(!unescape_str) return lex_error(ctx, E_OUTOFMEMORY); memcpy(unescape_str, ptr, len * sizeof(WCHAR)); - if(!unescape(unescape_str, &len)) { + if(!unescape(unescape_str, &len, FALSE)) { WARN("unescape failed\n"); return lex_error(ctx, E_FAIL); } diff --git a/dlls/jscript/parser.h b/dlls/jscript/parser.h index 3b616b43b07..f10dea93101 100644 --- a/dlls/jscript/parser.h +++ b/dlls/jscript/parser.h @@ -66,7 +66,7 @@ static inline void *parser_alloc_tmp(parser_ctx_t *ctx, DWORD size) } BOOL is_identifier_char(WCHAR); -BOOL unescape(WCHAR*,size_t*); +BOOL unescape(WCHAR*,size_t*,BOOL); HRESULT parse_decimal(const WCHAR**,const WCHAR*,double*); typedef enum { diff --git a/dlls/jscript/tests/api.js b/dlls/jscript/tests/api.js index a82e4756c24..255418a6744 100644 --- a/dlls/jscript/tests/api.js +++ b/dlls/jscript/tests/api.js @@ -2010,7 +2010,7 @@ ok(isNaN(tmp), "Math.tan(-Infinity) is not NaN"); ["true", true], [" \nnull ", null], ["{}", {}], - ["\"\\r\\n test\\u1111\"", "\r\n test\u1111"], + ["\"\\r\\n test\\u1111\\/\\x20\\45\\'\"", "\r\n test\u1111/ %'"], ["{\"x\" :\n true}", {x:true}], ["{\"x y\": {}, \"z\": {\"x\":null}}", {"x y":{}, z:{x:null}}], ["[]", []], diff --git a/dlls/mshtml/tests/documentmode.js b/dlls/mshtml/tests/documentmode.js index 73f54cd708b..a80a74f12ae 100644 --- a/dlls/mshtml/tests/documentmode.js +++ b/dlls/mshtml/tests/documentmode.js @@ -1838,6 +1838,29 @@ async_test("weakmap_obj", function() { window.setTimeout(retry); }); +sync_test("JSON.parse escapes", function() { + if(!window.JSON) + return; + + var i, valid = [ "b", "t", "n", "f", "r", "u1111", '"', "/" ]; + + for(i = 0; i < valid.length; i++) { + var a = JSON.parse('"\\' + valid[i] + '"'), b = eval('"\\' + valid[i] + '"'); + ok(a === b, "JSON.parse with \\" + valid[i] + " returned " + a); + } + + var invalid = [ "0", "00", "05", "x20", "i", "'" ]; + + for(i = 0; i < invalid.length; i++) { + try { + JSON.parse('"\\' + invalid[i] + '"'); + ok(false, "expected exception calling JSON.parse with \\" + invalid[i]); + } catch(e) { + ok(e.number === 0xa03f6 - 0x80000000, "calling JSON.parse with \\" + invalid[i] + " threw " + e.number); + } + } +}); + sync_test("storage", function() { var v = document.documentMode, i, r, list; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6322
Jacek Caban (@jacek) commented about dlls/jscript/engine.c:
return hres; }
- if((flags & EXEC_EVAL) && ctx->call_ctx) { + if((flags & EXEC_EVAL) && ctx->call_ctx && scope) { variable_obj = jsdisp_addref(ctx->call_ctx->variable_obj);
Do we still need to check for `ctx->call_ctx` with this change? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6322#note_79316
Jacek Caban (@jacek) commented about dlls/jscript/lex.c:
return ctx->ptr != ctx->end; }
-BOOL unescape(WCHAR *str, size_t *len) +BOOL unescape(WCHAR *str, size_t *len, BOOL json)
Spec defines a separate rules for that and as far as I can tell only `UnicodeEscapeSequence` is shared with JS lexer (which matches your implementation). Maybe it makes sense to just use a separate implementation? Your disabling most of cases for JSON anyway. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6322#note_79317
Jacek Caban (@jacek) commented about dlls/mshtml/tests/documentmode.js:
window.setTimeout(retry); });
+sync_test("JSON.parse escapes", function() {
Please don't use `documentmode.js` for things that don't need change between compat modes. We have `es5.js` for that. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6322#note_79318
On Tue Aug 20 18:56:09 2024 +0000, Jacek Caban wrote:
Please don't use `documentmode.js` for things that don't need change between compat modes. We have `es5.js` for that. Are you sure about that? That doesn't run the tests to show it's for html mode in general, even pre ES5. For instance, if I change "html_mode" to a version check for ES5 the tests will still pass, even though it's wrong?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6322#note_79446
On Tue Aug 20 18:56:07 2024 +0000, Jacek Caban wrote:
Do we still need to check for `ctx->call_ctx` with this change? You're right, for eval (since we test flag anyway) it's either NULL or call_ctx's scope.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6322#note_79451
On Wed Aug 21 14:17:19 2024 +0000, Gabriel Ivăncescu wrote:
Are you sure about that? That doesn't run the tests to show it's for html mode in general, even pre ES5. For instance, if I change "html_mode" to a version check for ES5 the tests will still pass, even though it's wrong? Following that logic, we'd move everything to `documentmode.js`. Otherwise even if there is no version check, tests wouldn't fail if you added one.
"HTML" mode is really a distinction between classic JScript and JS inside MSHTML (that are separate things on Windows, even in quirks mode). So from MSHTML point of view, all its compatibility modes behave the same. If you intend to show a difference, you'd need to add a separate test to jscript/tests. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6322#note_79491
participants (2)
-
Gabriel Ivăncescu -
Jacek Caban (＠jacek)