From 91755e99493521062908a603349c11dd283a75dd Mon Sep 17 00:00:00 2001
From: Marko Lahma <marko.lahma@gmail.com>
Date: Mon, 22 Jul 2024 22:57:58 +0300
Subject: [PATCH] Ensure Acornima ParseErrorException is exposed as
 JavasScriptException (#1924)

---
 Jint.Tests/Parser/JavascriptParserTests.cs    |  7 +++-
 Jint.Tests/Runtime/ClassTests.cs              | 30 +++++++++++++
 Jint.Tests/Runtime/EngineTests.cs             | 14 +++----
 Jint/Engine.cs                                |  8 ++--
 Jint/Extensions/AcornimaExtensions.cs         | 42 +++++++++++++++++++
 Jint/Native/Function/ClassDefinition.cs       |  2 +-
 Jint/Native/Function/EvalFunction.cs          | 16 +------
 .../Function/FunctionInstance.Dynamic.cs      |  2 +-
 Jint/Native/ShadowRealm/ShadowRealm.cs        |  2 +-
 Jint/Runtime/Modules/ModuleFactory.cs         | 16 +------
 10 files changed, 93 insertions(+), 46 deletions(-)
 create mode 100644 Jint/Extensions/AcornimaExtensions.cs

diff --git a/Jint.Tests/Parser/JavascriptParserTests.cs b/Jint.Tests/Parser/JavascriptParserTests.cs
index 0a2659895b..e224b61316 100644
--- a/Jint.Tests/Parser/JavascriptParserTests.cs
+++ b/Jint.Tests/Parser/JavascriptParserTests.cs
@@ -1,4 +1,6 @@
-namespace Jint.Tests.Parsing;
+using Jint.Runtime;
+
+namespace Jint.Tests.Parsing;
 
 public class JavascriptParserTests
 {
@@ -163,7 +165,8 @@ public void ShouldProvideLocationForMultiLinesStringLiterals()
     [Fact]
     public void ShouldThrowErrorForInvalidLeftHandOperation()
     {
-        Assert.Throws<SyntaxErrorException>(() => new Engine().Execute("~ (WE0=1)--- l('1');"));
+        var ex = Assert.Throws<JavaScriptException>(() => new Engine().Execute("~ (WE0=1)--- l('1');"));
+        Assert.Equal("Invalid left-hand side expression in postfix operation (<anonymous>:1:4)", ex.Message);
     }
 
 
diff --git a/Jint.Tests/Runtime/ClassTests.cs b/Jint.Tests/Runtime/ClassTests.cs
index b644451425..b3714f11cb 100644
--- a/Jint.Tests/Runtime/ClassTests.cs
+++ b/Jint.Tests/Runtime/ClassTests.cs
@@ -1,3 +1,5 @@
+using Jint.Runtime;
+
 namespace Jint.Tests.Runtime;
 
 public class ClassTests
@@ -45,4 +47,32 @@ get doubleWidth () {
         Assert.Equal(10, engine.Evaluate("board.width"));
         Assert.Equal(20, engine.Evaluate("board.doubleWidth "));
     }
+
+    [Fact]
+    public void PrivateMemberAccessOutsideOfClass()
+    {
+        var ex = Assert.Throws<JavaScriptException>(() => new Engine().Evaluate
+        (
+            """
+            class A { }
+            new A().#nonexistent = 1;
+            """
+        ));
+
+        Assert.Equal("Private field '#nonexistent' must be declared in an enclosing class (<anonymous>:2:9)", ex.Message);
+    }
+
+    [Fact]
+    public void PrivateMemberAccessAgainstUnknownMemberInConstructor()
+    {
+        var ex = Assert.Throws<JavaScriptException>(() => new Engine().Evaluate
+        (
+            """
+            class A { constructor() { #nonexistent = 2; } }
+            new A();
+            """
+        ));
+
+        Assert.Equal("Unexpected identifier '#nonexistent' (<anonymous>:1:27)", ex.Message);
+    }
 }
diff --git a/Jint.Tests/Runtime/EngineTests.cs b/Jint.Tests/Runtime/EngineTests.cs
index fcb75c010c..b403722d0c 100644
--- a/Jint.Tests/Runtime/EngineTests.cs
+++ b/Jint.Tests/Runtime/EngineTests.cs
@@ -1046,11 +1046,11 @@ public void ShouldGetParseErrorLocation()
             {
                 engine.Evaluate("1.2+ new", "jQuery.js");
             }
-            catch (SyntaxErrorException e)
+            catch (JavaScriptException e)
             {
-                Assert.Equal(1, e.LineNumber);
-                Assert.Equal(8, e.Column);
-                Assert.Equal("jQuery.js", e.SourceFile);
+                Assert.Equal(1, e.Location.Start.Line);
+                Assert.Equal(8, e.Location.Start.Column);
+                Assert.Equal("jQuery.js", e.Location.SourceFile);
             }
         }
         #region DateParsingAndStrings
@@ -1314,7 +1314,7 @@ public void ShouldNotAllowDuplicateProtoProperty()
         {
             var code = "if({ __proto__: [], __proto__:[] } instanceof Array) {}";
 
-            Exception ex = Assert.Throws<SyntaxErrorException>(() => _engine.Execute(code, new ScriptParsingOptions { Tolerant = false }));
+            Exception ex = Assert.Throws<JavaScriptException>(() => _engine.Execute(code, new ScriptParsingOptions { Tolerant = false }));
             Assert.Contains("Duplicate __proto__ fields are not allowed in object literals", ex.Message);
 
             ex = Assert.Throws<JavaScriptException>(() => _engine.Execute($"eval('{code}')"));
@@ -2865,8 +2865,8 @@ public void ShouldObeyScriptLevelStrictModeInFunctions()
             Assert.Equal("Cannot delete property 'prototype' of function Boolean() { [native code] }", ex.Message);
 
             const string source2 = "'use strict'; delete foobar;";
-            var ex2 = Assert.Throws<SyntaxErrorException>(() => engine.Evaluate(source2));
-            Assert.Equal("Delete of an unqualified identifier in strict mode", ex2.Description);
+            ex = Assert.Throws<JavaScriptException>(() => engine.Evaluate(source2));
+            Assert.Equal("Delete of an unqualified identifier in strict mode (<anonymous>:1:22)", ex.Message);
         }
 
         [Fact]
diff --git a/Jint/Engine.cs b/Jint/Engine.cs
index 2ba6a12429..eae724f7b6 100644
--- a/Jint/Engine.cs
+++ b/Jint/Engine.cs
@@ -341,7 +341,7 @@ internal void ResetCallStack()
         /// </summary>
         public JsValue Evaluate(string code, string? source = null)
         {
-            var script = _defaultParser.ParseScript(code, source ?? "<anonymous>", _isStrict);
+            var script = _defaultParser.ParseScriptGuarded(Realm, code, source ?? "<anonymous>", _isStrict);
             return Evaluate(new Prepared<Script>(script, _defaultParser.Options));
         }
 
@@ -357,7 +357,7 @@ public JsValue Evaluate(string code, ScriptParsingOptions parsingOptions)
         public JsValue Evaluate(string code, string source, ScriptParsingOptions parsingOptions)
         {
             var parser = GetParserFor(parsingOptions);
-            var script = parser.ParseScript(code, source, _isStrict);
+            var script = parser.ParseScriptGuarded(Realm, code, source, _isStrict);
             return Evaluate(new Prepared<Script>(script, parser.Options));
         }
 
@@ -372,7 +372,7 @@ public JsValue Evaluate(in Prepared<Script> preparedScript)
         /// </summary>
         public Engine Execute(string code, string? source = null)
         {
-            var script = _defaultParser.ParseScript(code, source ?? "<anonymous>", _isStrict);
+            var script = _defaultParser.ParseScriptGuarded(Realm, code, source ?? "<anonymous>", _isStrict);
             return Execute(new Prepared<Script>(script, _defaultParser.Options));
         }
 
@@ -388,7 +388,7 @@ public Engine Execute(string code, ScriptParsingOptions parsingOptions)
         public Engine Execute(string code, string source, ScriptParsingOptions parsingOptions)
         {
             var parser = GetParserFor(parsingOptions);
-            var script = parser.ParseScript(code, source, _isStrict);
+            var script = parser.ParseScriptGuarded(Realm, code, source, _isStrict);
             return Execute(new Prepared<Script>(script, parser.Options));
         }
 
diff --git a/Jint/Extensions/AcornimaExtensions.cs b/Jint/Extensions/AcornimaExtensions.cs
new file mode 100644
index 0000000000..8450e741ff
--- /dev/null
+++ b/Jint/Extensions/AcornimaExtensions.cs
@@ -0,0 +1,42 @@
+using Jint.Runtime;
+
+namespace Jint;
+
+internal static class AcornimaExtensions
+{
+    public static Script ParseScriptGuarded(this Parser parser, Realm realm, string code, string? source = null, bool strict = false)
+    {
+        try
+        {
+            return parser.ParseScript(code, source, strict);
+        }
+        catch (ParseErrorException e)
+        {
+            ExceptionHelper.ThrowSyntaxError(realm, e.Message, ToLocation(e, source));
+            return default;
+        }
+    }
+
+    public static Module ParseModuleGuarded(this Parser parser, Engine engine, string code, string? source = null)
+    {
+        try
+        {
+            return parser.ParseModule(code, source);
+        }
+        catch (ParseErrorException ex)
+        {
+            ExceptionHelper.ThrowSyntaxError(engine.Realm, $"Error while loading module: error in module '{source}': {ex.Error}", ToLocation(ex, source));
+            return default;
+        }
+        catch (Exception)
+        {
+            ExceptionHelper.ThrowJavaScriptException(engine, $"Could not load module {source}", AstExtensions.DefaultLocation);
+            return default;
+        }
+    }
+
+    private static SourceLocation ToLocation(ParseErrorException ex, string? source)
+    {
+        return SourceLocation.From(Position.From(ex.LineNumber, ex.Column), Position.From(ex.LineNumber, ex.Column), source);
+    }
+}
diff --git a/Jint/Native/Function/ClassDefinition.cs b/Jint/Native/Function/ClassDefinition.cs
index 6a919e7348..2c5d34281a 100644
--- a/Jint/Native/Function/ClassDefinition.cs
+++ b/Jint/Native/Function/ClassDefinition.cs
@@ -26,7 +26,7 @@ static ClassDefinition()
         // generate missing constructor AST only once
         static MethodDefinition CreateConstructorMethodDefinition(Parser parser, string source)
         {
-            var script = parser.ParseScript(source);
+            var script = parser.ParseScriptGuarded(new Engine().Realm, source);
             var classDeclaration = (ClassDeclaration) script.Body[0];
             return (MethodDefinition) classDeclaration.Body.Body[0];
         }
diff --git a/Jint/Native/Function/EvalFunction.cs b/Jint/Native/Function/EvalFunction.cs
index 099067c488..0298d9a150 100644
--- a/Jint/Native/Function/EvalFunction.cs
+++ b/Jint/Native/Function/EvalFunction.cs
@@ -84,21 +84,7 @@ internal JsValue PerformEval(JsValue x, Realm callerRealm, bool strictCaller, bo
             CheckPrivateFields = false
         };
         var parser = _engine.GetParserFor(adjustedParserOptions);
-        try
-        {
-            script = parser.ParseScript(x.ToString(), strict: strictCaller);
-        }
-        catch (ParseErrorException e)
-        {
-            if (string.Equals(e.Error.Code, "InvalidLhsInAssignment", StringComparison.Ordinal))
-            {
-                ExceptionHelper.ThrowReferenceError(callerRealm, (string?) null);
-            }
-            else
-            {
-                ExceptionHelper.ThrowSyntaxError(callerRealm, e.Message);
-            }
-        }
+        script = parser.ParseScriptGuarded(_engine.Realm, x.ToString(), strict: strictCaller);
 
         var body = script.Body;
         if (body.Count == 0)
diff --git a/Jint/Native/Function/FunctionInstance.Dynamic.cs b/Jint/Native/Function/FunctionInstance.Dynamic.cs
index 5576bdd76f..3a4545b0d9 100644
--- a/Jint/Native/Function/FunctionInstance.Dynamic.cs
+++ b/Jint/Native/Function/FunctionInstance.Dynamic.cs
@@ -150,7 +150,7 @@ internal Function CreateDynamicFunction(
                 parserOptions = parserOptions with { AllowReturnOutsideFunction = true };
             }
             Parser parser = new(parserOptions);
-            function = (IFunction) parser.ParseScript(functionExpression, strict: _engine._isStrict).Body[0];
+            function = (IFunction) parser.ParseScriptGuarded(callerRealm, functionExpression, strict: _engine._isStrict).Body[0];
         }
         catch (ParseErrorException ex)
         {
diff --git a/Jint/Native/ShadowRealm/ShadowRealm.cs b/Jint/Native/ShadowRealm/ShadowRealm.cs
index 56ba725d37..1411ace357 100644
--- a/Jint/Native/ShadowRealm/ShadowRealm.cs
+++ b/Jint/Native/ShadowRealm/ShadowRealm.cs
@@ -110,7 +110,7 @@ internal JsValue PerformShadowRealmEval(string sourceText, ParserOptions parserO
         Script script;
         try
         {
-            script = parser.ParseScript(sourceText, strict: _engine._isStrict);
+            script = parser.ParseScriptGuarded(callerRealm, sourceText, strict: _engine._isStrict);
         }
         catch (ParseErrorException e)
         {
diff --git a/Jint/Runtime/Modules/ModuleFactory.cs b/Jint/Runtime/Modules/ModuleFactory.cs
index 0cd7f8b072..2196c6c315 100644
--- a/Jint/Runtime/Modules/ModuleFactory.cs
+++ b/Jint/Runtime/Modules/ModuleFactory.cs
@@ -22,23 +22,9 @@ public static class ModuleFactory
     public static Module BuildSourceTextModule(Engine engine, ResolvedSpecifier resolved, string code, ModuleParsingOptions? parsingOptions = null)
     {
         var source = resolved.Uri?.LocalPath ?? resolved.Key;
-        AstModule module;
         var parserOptions = (parsingOptions ?? ModuleParsingOptions.Default).GetParserOptions();
         var parser = new Parser(parserOptions);
-        try
-        {
-            module = parser.ParseModule(code, source);
-        }
-        catch (ParseErrorException ex)
-        {
-            ExceptionHelper.ThrowSyntaxError(engine.Realm, $"Error while loading module: error in module '{source}': {ex.Error}");
-            module = null;
-        }
-        catch (Exception)
-        {
-            ExceptionHelper.ThrowJavaScriptException(engine, $"Could not load module {source}", AstExtensions.DefaultLocation);
-            module = null;
-        }
+        var module = parser.ParseModuleGuarded(engine, code, source);
 
         return BuildSourceTextModule(engine, new Prepared<AstModule>(module, parserOptions));
     }