From 3d5c8720e3b0bcd06cf3e920cef64e9e1cf9d154 Mon Sep 17 00:00:00 2001
From: Marko Lahma <marko.lahma@gmail.com>
Date: Wed, 24 Jul 2024 12:05:56 +0300
Subject: [PATCH] Ensure script preparation exposes only Jint's exception type
 (#1927)

---
 ...dulePreparationTests.ScriptPreparation.cs} | 47 +++++++++++++------
 Jint/Engine.Ast.cs                            | 32 +++++++++----
 Jint/JintException.cs                         | 15 ++++++
 Jint/Runtime/JintException.cs                 | 20 --------
 Jint/ScriptPreparationException.cs            |  8 ++++
 5 files changed, 79 insertions(+), 43 deletions(-)
 rename Jint.Tests/Runtime/{EngineTests.ScriptPreparation.cs => ScriptModulePreparationTests.ScriptPreparation.cs} (54%)
 create mode 100644 Jint/JintException.cs
 delete mode 100644 Jint/Runtime/JintException.cs
 create mode 100644 Jint/ScriptPreparationException.cs

diff --git a/Jint.Tests/Runtime/EngineTests.ScriptPreparation.cs b/Jint.Tests/Runtime/ScriptModulePreparationTests.ScriptPreparation.cs
similarity index 54%
rename from Jint.Tests/Runtime/EngineTests.ScriptPreparation.cs
rename to Jint.Tests/Runtime/ScriptModulePreparationTests.ScriptPreparation.cs
index 16a37a13ef..5070bcb082 100644
--- a/Jint.Tests/Runtime/EngineTests.ScriptPreparation.cs
+++ b/Jint.Tests/Runtime/ScriptModulePreparationTests.ScriptPreparation.cs
@@ -6,13 +6,13 @@
 
 namespace Jint.Tests.Runtime;
 
-public partial class EngineTests
+public class ScriptModulePreparationTests
 {
     [Fact]
     public void ScriptPreparationAcceptsReturnOutsideOfFunctions()
     {
         var preparedScript = Engine.PrepareScript("return 1;");
-        Assert.IsType<ReturnStatement>(preparedScript.Program.Body[0]);
+        preparedScript.Program.Body[0].Should().BeOfType<ReturnStatement>();
     }
 
     [Fact]
@@ -21,10 +21,10 @@ public void CanPreCompileRegex()
         var script = Engine.PrepareScript("var x = /[cgt]/ig; var y = /[cgt]/ig; 'g'.match(x).length;");
         var declaration = Assert.IsType<VariableDeclaration>(script.Program.Body[0]);
         var init = Assert.IsType<RegExpLiteral>(declaration.Declarations[0].Init);
-        Assert.Equal("[cgt]", init.Value.ToString());
-        Assert.Equal(RegexOptions.Compiled, init.Value.Options & RegexOptions.Compiled);
 
-        Assert.Equal(1, _engine.Evaluate(script));
+        init.Value.ToString().Should().Be("[cgt]");
+        (init.Value.Options & RegexOptions.Compiled).Should().Be(RegexOptions.Compiled);
+        new Engine().Evaluate(script).AsNumber().Should().Be(1);
     }
 
     [Fact]
@@ -33,9 +33,9 @@ public void ScriptPreparationFoldsConstants()
         var preparedScript = Engine.PrepareScript("return 1 + 2;");
         var returnStatement = Assert.IsType<ReturnStatement>(preparedScript.Program.Body[0]);
         var constant = Assert.IsType<JintConstantExpression>(returnStatement.Argument?.UserData);
-        Assert.Equal(3, constant.GetValue(null!));
 
-        Assert.Equal(3, _engine.Evaluate(preparedScript));
+        constant.GetValue(null!).AsNumber().Should().Be(3);
+        new Engine().Evaluate(preparedScript).AsNumber().Should().Be(3);
     }
 
     [Fact]
@@ -46,8 +46,8 @@ public void ScriptPreparationOptimizesNegatingUnaryExpression()
         var unaryExpression = Assert.IsType<NonUpdateUnaryExpression>(expression.Expression);
         var constant = Assert.IsType<JintConstantExpression>(unaryExpression.UserData);
 
-        Assert.Equal(-1, constant.GetValue(null!));
-        Assert.Equal(-1, _engine.Evaluate(preparedScript));
+        constant.GetValue(null!).AsNumber().Should().Be(-1);
+        new Engine().Evaluate(preparedScript).AsNumber().Should().Be(-1);
     }
 
     [Fact]
@@ -58,10 +58,10 @@ public void ScriptPreparationOptimizesConstantReturn()
         var returnStatement = Assert.IsType<ConstantStatement>(statement.UserData);
 
         var builtStatement = JintStatement.Build(statement);
-        Assert.Same(returnStatement, builtStatement);
+        returnStatement.Should().BeSameAs(builtStatement);
 
-        var result = builtStatement.Execute(new EvaluationContext(_engine)).Value;
-        Assert.Equal(JsBoolean.False, result);
+        var result = builtStatement.Execute(new EvaluationContext( new Engine())).Value;
+        result.Should().Be(JsBoolean.False);
     }
 
     [Fact]
@@ -69,9 +69,26 @@ public void CompiledRegexShouldProduceSameResultAsNonCompiled()
     {
         const string Script = """JSON.stringify(/(.*?)a(?!(a+)b\2c)\2(.*)/.exec("baaabaac"))""";
 
-        var nonCompiled = _engine.Evaluate(Script);
-        var compiled = _engine.Evaluate(Engine.PrepareScript(Script));
+        var engine = new Engine();
+        var nonCompiledResult = engine.Evaluate(Script);
+        var compiledResult = engine.Evaluate(Engine.PrepareScript(Script));
 
-        Assert.Equal(nonCompiled, compiled);
+        nonCompiledResult.Should().Be(compiledResult);
+    }
+
+    [Fact]
+    public void PrepareScriptShouldNotLeakAcornimaException()
+    {
+        var ex = Assert.Throws<ScriptPreparationException>(() => Engine.PrepareScript("class A { } A().#nonexistent = 1;"));
+        ex.Message.Should().Be("Could not prepare script: Private field '#nonexistent' must be declared in an enclosing class (1:17)");
+        ex.InnerException.Should().BeOfType<SyntaxErrorException>();
+    }
+
+    [Fact]
+    public void PrepareModuleShouldNotLeakAcornimaException()
+    {
+        var ex = Assert.Throws<ScriptPreparationException>(() => Engine.PrepareModule("class A { } A().#nonexistent = 1;"));
+        ex.Message.Should().Be("Could not prepare script: Private field '#nonexistent' must be declared in an enclosing class (1:17)");
+        ex.InnerException.Should().BeOfType<SyntaxErrorException>();
     }
 }
diff --git a/Jint/Engine.Ast.cs b/Jint/Engine.Ast.cs
index 78b92ea6e6..0c6d7a703d 100644
--- a/Jint/Engine.Ast.cs
+++ b/Jint/Engine.Ast.cs
@@ -1,5 +1,4 @@
 using System.Runtime.InteropServices;
-using System.Text.RegularExpressions;
 using Jint.Native;
 using Jint.Runtime;
 using Jint.Runtime.Interpreter;
@@ -20,10 +19,20 @@ public partial class Engine
     public static Prepared<Script> PrepareScript(string code, string? source = null, bool strict = false, ScriptPreparationOptions? options = null)
     {
         options ??= ScriptPreparationOptions.Default;
+
         var astAnalyzer = new AstAnalyzer(options);
         var parserOptions = options.GetParserOptions();
-        var preparedScript = new Parser(parserOptions with { OnNode = astAnalyzer.NodeVisitor }).ParseScript(code, source, strict);
-        return new Prepared<Script>(preparedScript, parserOptions);
+        var parser = new Parser(parserOptions with { OnNode = astAnalyzer.NodeVisitor });
+
+        try
+        {
+            var preparedScript = parser.ParseScript(code, source, strict);
+            return new Prepared<Script>(preparedScript, parserOptions);
+        }
+        catch (Exception e)
+        {
+            throw new ScriptPreparationException("Could not prepare script: " + e.Message, e);
+        }
     }
 
     /// <summary>
@@ -35,19 +44,26 @@ public static Prepared<Script> PrepareScript(string code, string? source = null,
     public static Prepared<Module> PrepareModule(string code, string? source = null, ModulePreparationOptions? options = null)
     {
         options ??= ModulePreparationOptions.Default;
+
         var astAnalyzer = new AstAnalyzer(options);
         var parserOptions = options.GetParserOptions();
-        var preparedModule = new Parser(parserOptions with { OnNode = astAnalyzer.NodeVisitor }).ParseModule(code, source);
-        return new Prepared<Module>(preparedModule, parserOptions);
+        var parser = new Parser(parserOptions with { OnNode = astAnalyzer.NodeVisitor });
+
+        try
+        {
+            var preparedModule = parser.ParseModule(code, source);
+            return new Prepared<Module>(preparedModule, parserOptions);
+        }
+        catch (Exception e)
+        {
+            throw new ScriptPreparationException("Could not prepare script: " + e.Message, e);
+        }
     }
 
     private sealed class AstAnalyzer
     {
-        private static readonly bool _canCompileNegativeLookaroundAssertions = typeof(Regex).Assembly.GetName().Version?.Major is not (null or 7 or 8);
-
         private readonly IPreparationOptions<IParsingOptions> _preparationOptions;
         private readonly Dictionary<string, Environment.BindingName> _bindingNames = new(StringComparer.Ordinal);
-        private readonly Dictionary<string, Regex> _regexes = new(StringComparer.Ordinal);
 
         public AstAnalyzer(IPreparationOptions<IParsingOptions> preparationOptions)
         {
diff --git a/Jint/JintException.cs b/Jint/JintException.cs
new file mode 100644
index 0000000000..54c9500e34
--- /dev/null
+++ b/Jint/JintException.cs
@@ -0,0 +1,15 @@
+namespace Jint;
+
+/// <summary>
+/// Base class for exceptions thrown by Jint.
+/// </summary>
+public abstract class JintException : Exception
+{
+    internal JintException(string? message) : base(message)
+    {
+    }
+
+    internal JintException(string? message, Exception? innerException) : base(message, innerException)
+    {
+    }
+}
diff --git a/Jint/Runtime/JintException.cs b/Jint/Runtime/JintException.cs
deleted file mode 100644
index ce47317cf3..0000000000
--- a/Jint/Runtime/JintException.cs
+++ /dev/null
@@ -1,20 +0,0 @@
-namespace Jint.Runtime
-{
-    /// <summary>
-    /// Base class for exceptions thrown by Jint.
-    /// </summary>
-    public abstract class JintException : Exception
-    {
-        protected JintException()
-        {
-        }
-
-        protected JintException(string? message) : base(message)
-        {
-        }
-
-        protected JintException(string? message, Exception? innerException) : base(message, innerException)
-        {
-        }
-    }
-}
diff --git a/Jint/ScriptPreparationException.cs b/Jint/ScriptPreparationException.cs
new file mode 100644
index 0000000000..b8ad26e476
--- /dev/null
+++ b/Jint/ScriptPreparationException.cs
@@ -0,0 +1,8 @@
+namespace Jint;
+
+public sealed class ScriptPreparationException : JintException
+{
+    public ScriptPreparationException(string? message, Exception? innerException) : base(message, innerException)
+    {
+    }
+}