Skip to content

Commit

Permalink
Prevent users from passing arbitrary parser options to Jint (#1831)
Browse files Browse the repository at this point in the history
* Reduce parsing options for end users (replace ParserOptions with Script/ModuleParseOptions in the public API)
* Enforce that pre-parsed scripts can be passed to Jint only if they were created using Engine.PrepareScript/PrepareModule
* Avoid copying beefy ExecutionContext struct where possible
* Propagate ParserOptions via ExecutionContext
* Introduce alias for Esprima.Ast.Module
  • Loading branch information
adams85 authored Apr 9, 2024
1 parent b6f99b9 commit e1e2d6d
Show file tree
Hide file tree
Showing 45 changed files with 754 additions and 229 deletions.
1 change: 1 addition & 0 deletions Directory.Build.props
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
<Using Include="Esprima.Utils" />

<Using Include="Esprima.Ast.BindingPattern" Alias="DestructuringPattern" />
<Using Include="Esprima.Ast.Module" Alias="AstModule" />
<Using Include="Esprima.Ast.Nodes" Alias="NodeType" />
<Using Include="Esprima.JavaScriptParser" Alias="Parser" />
<Using Include="Esprima.Location" Alias="SourceLocation" />
Expand Down
2 changes: 1 addition & 1 deletion Directory.Packages.props
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
<ItemGroup>
<PackageVersion Include="BenchmarkDotNet" Version="0.13.12" />
<PackageVersion Include="BenchmarkDotNet.TestAdapter" Version="0.13.12" />
<PackageVersion Include="Esprima" Version="3.0.4" />
<PackageVersion Include="Esprima" Version="3.0.5" />
<PackageVersion Include="FluentAssertions" Version="6.12.0" />
<PackageVersion Include="Flurl.Http.Signed" Version="3.2.4" />
<PackageVersion Include="Jurassic" Version="3.2.7" />
Expand Down
2 changes: 1 addition & 1 deletion Jint.Benchmark/DromaeoBenchmark.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ public class DromaeoBenchmark
{"dromaeo-string-base64", null}
};

private readonly Dictionary<string, Script> _prepared = new();
private readonly Dictionary<string, Prepared<Script>> _prepared = new();

private Engine engine;

Expand Down
5 changes: 2 additions & 3 deletions Jint.Benchmark/EngineComparisonBenchmark.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ namespace Jint.Benchmark;
[BenchmarkCategory("EngineComparison")]
public class EngineComparisonBenchmark
{
private static readonly Dictionary<string, Script> _parsedScripts = new();
private static readonly Dictionary<string, Prepared<Script>> _parsedScripts = new();

private static readonly Dictionary<string, string> _files = new()
{
Expand All @@ -38,7 +38,6 @@ public class EngineComparisonBenchmark
[GlobalSetup]
public void Setup()
{
var parser = new Parser();
foreach (var fileName in _files.Keys.ToList())
{
var script = File.ReadAllText($"Scripts/{fileName}.js");
Expand All @@ -47,7 +46,7 @@ public void Setup()
script = _dromaeoHelpers + Environment.NewLine + Environment.NewLine + script;
}
_files[fileName] = script;
_parsedScripts[fileName] = parser.ParseScript(script, strict: true);
_parsedScripts[fileName] = Engine.PrepareScript(script, strict: true);
}
}

Expand Down
9 changes: 4 additions & 5 deletions Jint.Benchmark/EngineConstructionBenchmark.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,14 @@ namespace Jint.Benchmark;
[MemoryDiagnoser]
public class EngineConstructionBenchmark
{
private Script _program;
private Script _simple;
private Prepared<Script> _program;
private Prepared<Script> _simple;

[GlobalSetup]
public void GlobalSetup()
{
var parser = new Parser();
_program = parser.ParseScript("([].length + ''.length)");
_simple = parser.ParseScript("1");
_program = Engine.PrepareScript("([].length + ''.length)");
_simple = Engine.PrepareScript("1");
new Engine().Evaluate(_program);
}

Expand Down
2 changes: 1 addition & 1 deletion Jint.Benchmark/ObjectAccessBenchmark.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ namespace Jint.Benchmark;
[MemoryDiagnoser]
public class ObjectAccessBenchmark
{
private readonly Script _script;
private readonly Prepared<Script> _script;

public ObjectAccessBenchmark()
{
Expand Down
2 changes: 1 addition & 1 deletion Jint.Benchmark/ShadowRealmBenchmark.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ public class ShadowRealmBenchmark
";

private Engine engine;
private Script parsedScript;
private Prepared<Script> parsedScript;

[GlobalSetup]
public void Setup()
Expand Down
2 changes: 1 addition & 1 deletion Jint.Benchmark/SingleScriptBenchmark.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ namespace Jint.Benchmark;
public abstract class SingleScriptBenchmark
{
private string _script;
private Script _parsedScript;
private Prepared<Script> _parsedScript;

protected abstract string FileName { get; }

Expand Down
6 changes: 3 additions & 3 deletions Jint.Repl/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,10 @@
Console.WriteLine();

var defaultColor = Console.ForegroundColor;
var parserOptions = new ParserOptions
var parsingOptions = new ScriptParsingOptions
{
Tolerant = true,
RegExpParseMode = RegExpParseMode.AdaptToInterpreted
CompileRegex = false,
};

var serializer = new JsonSerializer(engine);
Expand All @@ -60,7 +60,7 @@

try
{
var result = engine.Evaluate(input, parserOptions);
var result = engine.Evaluate(input, parsingOptions);
JsValue str = result;
if (!result.IsPrimitive() && result is not IJsPrimitive)
{
Expand Down
8 changes: 2 additions & 6 deletions Jint.Tests.CommonScripts/ConcurrencyTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,10 @@ namespace Jint.Tests.CommonScripts;
public class ConcurrencyTest
{
[Test]
[TestCase(true)]
[TestCase(false)]
public void ConcurrentEnginesCanUseSameAst(bool prepared)
public void ConcurrentEnginesCanUseSameAst()
{
var scriptContents = SunSpiderTests.GetEmbeddedFile("babel-standalone.js");
var script = prepared
? Engine.PrepareScript(scriptContents)
: new Parser().ParseScript(scriptContents);
var script = Engine.PrepareScript(scriptContents);

Parallel.ForEach(Enumerable.Range(0, 3), x =>
{
Expand Down
20 changes: 12 additions & 8 deletions Jint.Tests.PublicInterface/ModuleLoaderTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ ModuleScript RunModule(string code)
public void CustomModuleLoaderWithCachingSupport()
{
// Different engines use the same module loader.
// The module loader caches the parsed Esprima.Ast.Module
// The module loader caches the parsed Module
// which allows to re-use these for different engine runs.
var store = new ModuleStore(new Dictionary<string, string>()
{
Expand Down Expand Up @@ -177,7 +177,7 @@ Module IModuleLoader.LoadModule(Engine engine, ResolvedSpecifier resolved)
/// <summary>
/// <para>
/// A simple <see cref="IModuleLoader"/> implementation which will
/// re-use prepared <see cref="Esprima.Ast.Module"/> or <see cref="JsValue"/> modules to
/// re-use prepared <see cref="AstModule"/> or <see cref="JsValue"/> modules to
/// produce <see cref="Jint.Runtime.Modules.Module"/>.
/// </para>
/// <para>
Expand Down Expand Up @@ -231,20 +231,24 @@ private ParsedModule GetParsedModule(Uri uri, ResolvedSpecifier resolved)

private sealed class ParsedModule
{
private readonly Esprima.Ast.Module? _textModule;
private readonly Prepared<AstModule>? _textModule;
private readonly (JsValue Json, string Location)? _jsonModule;

private ParsedModule(Esprima.Ast.Module? textModule, (JsValue Json, string Location)? jsonModule)
private ParsedModule(in Prepared<AstModule> textModule)
{
_textModule = textModule;
_jsonModule = jsonModule;
}

private ParsedModule(JsValue json, string location)
{
_jsonModule = (json, location);
}

public static ParsedModule TextModule(string script, string location)
=> new(Engine.PrepareModule(script, location), null);
=> new(Engine.PrepareModule(script, location));

public static ParsedModule JsonModule(string json, string location)
=> new(null, (ParseJson(json), location));
=> new(ParseJson(json), location);

private static JsValue ParseJson(string json)
{
Expand All @@ -258,7 +262,7 @@ public Module ToModule(Engine engine)
if (_jsonModule is not null)
return ModuleFactory.BuildJsonModule(engine, _jsonModule.Value.Json, _jsonModule.Value.Location);
if (_textModule is not null)
return ModuleFactory.BuildSourceTextModule(engine, _textModule);
return ModuleFactory.BuildSourceTextModule(engine, _textModule.Value);
throw new InvalidOperationException("Unexpected state - no module type available");
}
}
Expand Down
3 changes: 1 addition & 2 deletions Jint.Tests.PublicInterface/ShadowRealmTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,7 @@ public void CanReuseScriptWithShadowRealm()
var shadowRealm2 = engine.Intrinsics.ShadowRealm.Construct();
shadowRealm2.SetValue("message", "realm 2");

var parser = new Parser();
var script = parser.ParseScript("(function hello() {return \"hello \" + message})();");
var script = Engine.PrepareScript("(function hello() {return \"hello \" + message})();");

// Act & Assert
Assert.Equal("hello engine", engine.Evaluate(script));
Expand Down
2 changes: 1 addition & 1 deletion Jint.Tests.Test262/State.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,5 @@ public static partial class State
/// <summary>
/// Pre-compiled scripts for faster execution.
/// </summary>
public static readonly Dictionary<string, Script> Sources = new(StringComparer.OrdinalIgnoreCase);
public static readonly Dictionary<string, Prepared<Script>> Sources = new(StringComparer.OrdinalIgnoreCase);
}
14 changes: 10 additions & 4 deletions Jint.Tests.Test262/Test262Test.cs
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,10 @@ private Engine BuildTestExecutor(Test262File file)
throw new Exception("only script parsing supported");
}
var options = new ParserOptions { RegExpParseMode = RegExpParseMode.AdaptToInterpreted, Tolerant = false };
var parser = new Parser(options);
var script = parser.ParseScript(args.At(0).AsString());
var script = Engine.PrepareScript(args.At(0).AsString(), options: new ScriptPreparationOptions
{
ParsingOptions = ScriptParsingOptions.Default with { CompileRegex = false, Tolerant = false },
});
return engine.Evaluate(script);
}), true, true, true));
Expand Down Expand Up @@ -93,7 +94,12 @@ private static void ExecuteTest(Engine engine, Test262File file)
}
else
{
engine.Execute(new Parser().ParseScript(file.Program, source: file.FileName));
var script = Engine.PrepareScript(file.Program, source: file.FileName, options: new ScriptPreparationOptions
{
ParsingOptions = ScriptParsingOptions.Default with { CompileRegex = false, Tolerant = false },
});

engine.Execute(script);
}
}

Expand Down
12 changes: 6 additions & 6 deletions Jint.Tests/Runtime/EngineTests.ScriptPreparation.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,16 @@ public partial class EngineTests
public void ScriptPreparationAcceptsReturnOutsideOfFunctions()
{
var preparedScript = Engine.PrepareScript("return 1;");
Assert.IsType<ReturnStatement>(preparedScript.Body[0]);
Assert.IsType<ReturnStatement>(preparedScript.Program.Body[0]);
}

[Fact]
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.Body[0]);
var declaration = Assert.IsType<VariableDeclaration>(script.Program.Body[0]);
var init = Assert.IsType<RegExpLiteral>(declaration.Declarations[0].Init);
var regex = Assert.IsType<Regex>(init.AssociatedData);
var regex = Assert.IsType<Regex>(init.Value);
Assert.Equal("[cgt]", regex.ToString());
Assert.Equal(RegexOptions.Compiled, regex.Options & RegexOptions.Compiled);

Expand All @@ -32,7 +32,7 @@ public void CanPreCompileRegex()
public void ScriptPreparationFoldsConstants()
{
var preparedScript = Engine.PrepareScript("return 1 + 2;");
var returnStatement = Assert.IsType<ReturnStatement>(preparedScript.Body[0]);
var returnStatement = Assert.IsType<ReturnStatement>(preparedScript.Program.Body[0]);
var constant = Assert.IsType<JintConstantExpression>(returnStatement.Argument?.AssociatedData);
Assert.Equal(3, constant.GetValue(null!));

Expand All @@ -43,7 +43,7 @@ public void ScriptPreparationFoldsConstants()
public void ScriptPreparationOptimizesNegatingUnaryExpression()
{
var preparedScript = Engine.PrepareScript("-1");
var expression = Assert.IsType<ExpressionStatement>(preparedScript.Body[0]);
var expression = Assert.IsType<ExpressionStatement>(preparedScript.Program.Body[0]);
var unaryExpression = Assert.IsType<UnaryExpression>(expression.Expression);
var constant = Assert.IsType<JintConstantExpression>(unaryExpression.AssociatedData);

Expand All @@ -55,7 +55,7 @@ public void ScriptPreparationOptimizesNegatingUnaryExpression()
public void ScriptPreparationOptimizesConstantReturn()
{
var preparedScript = Engine.PrepareScript("return false;");
var statement = Assert.IsType<ReturnStatement>(preparedScript.Body[0]);
var statement = Assert.IsType<ReturnStatement>(preparedScript.Program.Body[0]);
var returnStatement = Assert.IsType<ConstantStatement>(statement.AssociatedData);

var builtStatement = JintStatement.Build(statement);
Expand Down
14 changes: 7 additions & 7 deletions Jint.Tests/Runtime/EngineTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1296,8 +1296,8 @@ public void ShouldExecuteDromaeoBase64()
public void ShouldExecuteKnockoutWithoutErrorWhetherTolerantOrIntolerant()
{
var content = GetEmbeddedFile("knockout-3.4.0.js");
_engine.Execute(content, new ParserOptions { Tolerant = true });
_engine.Execute(content, new ParserOptions { Tolerant = false });
_engine.Execute(content, new ScriptParsingOptions { Tolerant = true });
_engine.Execute(content, new ScriptParsingOptions { Tolerant = false });
}

[Fact]
Expand All @@ -1314,7 +1314,7 @@ public void ShouldNotAllowDuplicateProtoProperty()
{
var code = "if({ __proto__: [], __proto__:[] } instanceof Array) {}";

Exception ex = Assert.Throws<ParseErrorException>(() => _engine.Execute(code, new ParserOptions { Tolerant = false }));
Exception ex = Assert.Throws<ParseErrorException>(() => _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}')"));
Expand Down Expand Up @@ -2944,7 +2944,7 @@ public void ExecuteWithSourceShouldTriggerBeforeEvaluateEvent()
public void ExecuteWithParserOptionsShouldTriggerBeforeEvaluateEvent()
{
TestBeforeEvaluateEvent(
(engine, code) => engine.Execute(code, ParserOptions.Default),
(engine, code) => engine.Execute(code, ScriptParsingOptions.Default),
expectedSource: "<anonymous>"
);
}
Expand All @@ -2953,7 +2953,7 @@ public void ExecuteWithParserOptionsShouldTriggerBeforeEvaluateEvent()
public void ExecuteWithSourceAndParserOptionsShouldTriggerBeforeEvaluateEvent()
{
TestBeforeEvaluateEvent(
(engine, code) => engine.Execute(code, "mysource", ParserOptions.Default),
(engine, code) => engine.Execute(code, "mysource", ScriptParsingOptions.Default),
expectedSource: "mysource"
);
}
Expand All @@ -2980,7 +2980,7 @@ public void EvaluateWithSourceShouldTriggerBeforeEvaluateEvent()
public void EvaluateWithParserOptionsShouldTriggerBeforeEvaluateEvent()
{
TestBeforeEvaluateEvent(
(engine, code) => engine.Evaluate(code, ParserOptions.Default),
(engine, code) => engine.Evaluate(code, ScriptParsingOptions.Default),
expectedSource: "<anonymous>"
);
}
Expand All @@ -2989,7 +2989,7 @@ public void EvaluateWithParserOptionsShouldTriggerBeforeEvaluateEvent()
public void EvaluateWithSourceAndParserOptionsShouldTriggerBeforeEvaluateEvent()
{
TestBeforeEvaluateEvent(
(engine, code) => engine.Evaluate(code, "mysource", ParserOptions.Default),
(engine, code) => engine.Evaluate(code, "mysource", ScriptParsingOptions.Default),
expectedSource: "mysource"
);
}
Expand Down
16 changes: 8 additions & 8 deletions Jint.Tests/Runtime/ErrorTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -294,12 +294,12 @@ public void StackTraceCollectedForImmediatelyInvokedFunctionExpression()
return item;
})(getItem);";

var parserOptions = new ParserOptions
var parsingOptions = new ScriptParsingOptions
{
RegExpParseMode = RegExpParseMode.AdaptToInterpreted,
CompileRegex = false,
Tolerant = true
};
var ex = Assert.Throws<JavaScriptException>(() => engine.Execute(script, "get-item.js", parserOptions));
var ex = Assert.Throws<JavaScriptException>(() => engine.Execute(script, "get-item.js", parsingOptions));

const string expected = @"Error: Cannot read property '5' of null
at getItem (items, itemIndex) get-item.js:2:22
Expand Down Expand Up @@ -383,11 +383,11 @@ public void ErrorsHaveCorrectConstructor(string type)
[Fact]
public void CallStackWorksWithRecursiveCalls()
{
static ParserOptions CreateParserOptions()
static ScriptParsingOptions CreateParsingOptions()
{
return new ParserOptions
return new ScriptParsingOptions
{
RegExpParseMode = RegExpParseMode.AdaptToInterpreted,
CompileRegex = false,
Tolerant = true
};
}
Expand All @@ -406,13 +406,13 @@ static ParserOptions CreateParserOptions()
nuм -= 3;",
_ => throw new FileNotFoundException($"File '{path}' not exist.", path)
};
engine.Execute(content, path, CreateParserOptions());
engine.Execute(content, path, CreateParsingOptions());
}));
engine.Execute(
@"var num = 5;
executeFile(""first-file.js"");",
"main-file.js",
CreateParserOptions()
CreateParsingOptions()
);
});

Expand Down
Loading

0 comments on commit e1e2d6d

Please sign in to comment.