diff --git a/CHANGELOG.md b/CHANGELOG.md index 26ffd1d..2d82e72 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ Released on ?. +- Fixed `StackOverflowException` from deeply recursive scripts (#75) +- Added `JsScriptingOptions` to tune the engine via `WithJs` (#75) - Fixed usage of document ready state (#87) @Sebbs128 - Fixed `HasChildNodes` is now exposed as a method to DOM (#106) @arekdygas - Updated to use AngleSharp v1 diff --git a/README.md b/README.md index 00bd372..79ce4bb 100644 --- a/README.md +++ b/README.md @@ -21,7 +21,17 @@ var config = Configuration.Default .WithJs(); // from AngleSharp.Js ``` -This will register a scripting engine for JS files. The JS parsing options and more could be set with parameters of the `WithJs` method. +This will register a scripting engine for JS files. The engine can be tuned by passing a `JsScriptingOptions` instance to `WithJs`: + +```cs +var config = Configuration.Default + .WithJs(new JsScriptingOptions + { + // how deep a script may recurse before the engine reports + // "Maximum call stack size exceeded" (10000 by default) + MaxCallStackDepth = 5000, + }); +``` You can also use this part with a console for logging. The call for this is `WithConsoleLogger`, e.g., diff --git a/docs/general/01-Basics.md b/docs/general/01-Basics.md index 2b37459..e498cb7 100644 --- a/docs/general/01-Basics.md +++ b/docs/general/01-Basics.md @@ -37,7 +37,17 @@ var config = Configuration.Default .WithJs(); // from AngleSharp.Js ``` -This will register a scripting engine for JS files. The JS parsing options and more could be set with parameters of the `WithJs` method. +This will register a scripting engine for JS files. The engine can be tuned by passing a `JsScriptingOptions` instance to `WithJs`: + +```cs +var config = Configuration.Default + .WithJs(new JsScriptingOptions + { + // how deep a script may recurse before the engine reports + // "Maximum call stack size exceeded" (10000 by default) + MaxCallStackDepth = 5000, + }); +``` You can also use this part with a console for logging. The call for this is `WithConsoleLogger`, e.g., diff --git a/src/AngleSharp.Js.Tests/StackGuardTests.cs b/src/AngleSharp.Js.Tests/StackGuardTests.cs new file mode 100644 index 0000000..f5dfd1a --- /dev/null +++ b/src/AngleSharp.Js.Tests/StackGuardTests.cs @@ -0,0 +1,80 @@ +namespace AngleSharp.Js.Tests +{ + using AngleSharp.Dom; + using Jint.Runtime; + using NUnit.Framework; + using System; + using System.Threading.Tasks; + + public class StackGuardTests + { + // A recursion that never ends. Jint implements no tail calls, so this really does + // grow the call stack. Without a stack guard it exhausts the native one, and the + // resulting StackOverflowException takes the whole process down - which is why the + // tests below cannot assert anything weaker than "we got here at all". + private const String RunawayRecursion = "function boom() { return boom(); } boom();"; + + // Deeper than a 1 MB stack holds, so the engine has to keep going on a fresh one + // instead of reporting the depth as an error. + private const String DeepRecursion = "function depth(n) { return n === 0 ? 0 : 1 + depth(n - 1); } depth(1500);"; + + [Test] + public async Task RunawayRecursionInPageScriptDoesNotEscapeOpenAsync() + { + var config = Configuration.Default + .WithJs() + .WithEventLoop(); + + var content = $""; + var document = await BrowsingContext.New(config).OpenAsync(m => m.Content(content)); + + Assert.IsNotNull(document); + } + + [Test] + public async Task RunawayRecursionReportsMaximumCallStackSizeExceeded() + { + // A small limit keeps the failure quick and independent of how much native + // stack the test runner happens to have left. + var config = Configuration.Default + .WithJs(new JsScriptingOptions { MaxCallStackDepth = 200 }) + .WithEventLoop(); + + var document = await BrowsingContext.New(config).OpenNewAsync(); + var error = Assert.Throws(() => document.ExecuteScript(RunawayRecursion)); + + Assert.AreEqual("Maximum call stack size exceeded", error.Message); + } + + [Test] + public async Task DeepButFiniteRecursionStillSucceeds() + { + var config = Configuration.Default + .WithJs() + .WithEventLoop(); + + var document = await BrowsingContext.New(config).OpenNewAsync(); + var result = document.ExecuteScript(DeepRecursion); + + Assert.AreEqual(1500.0, result); + } + + [Test] + public async Task EditingTheOptionsAfterwardsLeavesTheServiceAlone() + { + var options = new JsScriptingOptions(); + var config = Configuration.Default + .WithJs(options) + .WithEventLoop(); + + // The engine is only built once a document asks for it, so an edit landing + // in between must not be the one deciding how that document behaves. + options.MaxCallStackDepth = 200; + + var document = await BrowsingContext.New(config).OpenNewAsync(); + var result = document.ExecuteScript(DeepRecursion); + + Assert.AreEqual(1500.0, result); + } + } +} diff --git a/src/AngleSharp.Js/EngineInstance.cs b/src/AngleSharp.Js/EngineInstance.cs index 5f2033f..7edb1a1 100644 --- a/src/AngleSharp.Js/EngineInstance.cs +++ b/src/AngleSharp.Js/EngineInstance.cs @@ -16,6 +16,9 @@ sealed class EngineInstance { #region Fields + // Jint's StackGuard.Disabled, which is internal. + private const Int32 StackGuardDisabled = -1; + private readonly Engine _engine; private readonly PrototypeCache _prototypes; private readonly ReferenceCache _references; @@ -27,13 +30,18 @@ sealed class EngineInstance #region ctor - public EngineInstance(IWindow window, IDictionary assignments, IEnumerable libs) + public EngineInstance(IWindow window, IDictionary assignments, IEnumerable libs, JsScriptingOptions options) { _importMap = new JsImportMap(); - _engine = new Engine((options) => + _engine = new Engine((o) => { - options.EnableModules(new JsModuleLoader(this, window.Document, false)); + o.EnableModules(new JsModuleLoader(this, window.Document, false)); + // Left alone, the JS call stack is the native one, and a script recursing + // deeper than it holds takes the whole process down - a StackOverflowException + // cannot be caught. Guarded, the engine continues on a fresh stack and finally + // reports an ordinary "Maximum call stack size exceeded" error instead. + o.Constraints.MaxExecutionStackCount = options.MaxCallStackDepth > 0 ? options.MaxCallStackDepth : StackGuardDisabled; }); _libs = libs; _prototypes = new PrototypeCache(_engine, libs); diff --git a/src/AngleSharp.Js/JsConfigurationExtensions.cs b/src/AngleSharp.Js/JsConfigurationExtensions.cs index f6c93e7..5f06651 100644 --- a/src/AngleSharp.Js/JsConfigurationExtensions.cs +++ b/src/AngleSharp.Js/JsConfigurationExtensions.cs @@ -61,9 +61,20 @@ public static IConfiguration WithEventLoop(this IConfiguration configuration, Fu /// /// The configuration to use. /// The new configuration. - public static IConfiguration WithJs(this IConfiguration configuration) + public static IConfiguration WithJs(this IConfiguration configuration) => + configuration.WithJs(new JsScriptingOptions()); + + /// + /// Sets scripting to true, registers the JavaScript engine with the + /// given options and returns a new configuration with the scripting + /// service and possible auxiliary services, if not yet registered. + /// + /// The configuration to use. + /// The options tuning the engine. + /// The new configuration. + public static IConfiguration WithJs(this IConfiguration configuration, JsScriptingOptions options) { - var service = new JsScriptingService(); + var service = new JsScriptingService(options); var observer = new EventAttributeObserver(service); var handler = new JsNavigationHandler(service); diff --git a/src/AngleSharp.Js/JsEventLoop.cs b/src/AngleSharp.Js/JsEventLoop.cs index 9de88f7..ac94ac0 100644 --- a/src/AngleSharp.Js/JsEventLoop.cs +++ b/src/AngleSharp.Js/JsEventLoop.cs @@ -11,6 +11,14 @@ namespace AngleSharp.Js /// public sealed class JsEventLoop : IEventLoop, IDisposable { + // Scripts run on this thread, and the JS call stack is the native one. The usual + // 1 MB holds roughly a thousand JavaScript frames, well short of what a browser + // offers, and every frame beyond it costs the engine a hop onto a fresh stack. + // The size is reserved address space rather than memory, but a 32 bit process has + // little of it to spare when it runs many loops, so only a 64 bit one is enlarged; + // zero leaves the thread with the default of the process. + private static readonly Int32 DefaultMaxStackSize = IntPtr.Size == 8 ? 16 * 1024 * 1024 : 0; + private readonly Dictionary> _queues = new Dictionary>(); private readonly Object _lockObj = new Object(); private CancellationTokenSource _cts; @@ -19,8 +27,17 @@ public sealed class JsEventLoop : IEventLoop, IDisposable /// Creates a new event loop thread. /// public JsEventLoop() + : this(DefaultMaxStackSize) + { + } + + /// + /// Creates a new event loop thread with the given stack size. + /// + /// The stack size of the thread running the scripts. + public JsEventLoop(Int32 maxStackSize) { - var thread = new Thread(Runner) + var thread = new Thread(Runner, maxStackSize) { IsBackground = true, Name = "AngleSharpEventLoop", diff --git a/src/AngleSharp.Js/JsScriptingOptions.cs b/src/AngleSharp.Js/JsScriptingOptions.cs new file mode 100644 index 0000000..6499a2b --- /dev/null +++ b/src/AngleSharp.Js/JsScriptingOptions.cs @@ -0,0 +1,28 @@ +namespace AngleSharp.Js +{ + using System; + + /// + /// Options tuning the JavaScript engine. + /// + public sealed class JsScriptingOptions + { + /// + /// Gets or sets the JavaScript call stack depth that has to be supported + /// before the engine gives up with a "Maximum call stack size exceeded" + /// error. Defaults to 10000, which is roughly what browsers allow. Values + /// of zero or less remove the limit - the engine is then bounded by the + /// native stack alone, so a runaway recursion terminates the process with + /// an uncatchable StackOverflowException. + /// + public Int32 MaxCallStackDepth { get; set; } = 10000; + + // An engine is built per window, long after the options were handed over, so + // reading them then would let a later edit of the caller's object decide how + // the next document behaves. The service takes this copy instead. + internal JsScriptingOptions Clone() => new JsScriptingOptions + { + MaxCallStackDepth = MaxCallStackDepth, + }; + } +} diff --git a/src/AngleSharp.Js/JsScriptingService.cs b/src/AngleSharp.Js/JsScriptingService.cs index add873e..74a95ba 100644 --- a/src/AngleSharp.Js/JsScriptingService.cs +++ b/src/AngleSharp.Js/JsScriptingService.cs @@ -25,18 +25,30 @@ public class JsScriptingService : IScriptingService private readonly ConditionalWeakTable _contexts; private readonly Dictionary _external; + private readonly JsScriptingOptions _options; #endregion #region ctor /// - /// Creates a new JavaScript engine. + /// Creates a new JavaScript engine using the default options. /// public JsScriptingService() + : this(new JsScriptingOptions()) + { + } + + /// + /// Creates a new JavaScript engine using the given options. The options + /// are copied, so that editing them afterwards leaves this engine alone. + /// + /// The options tuning the engine. + public JsScriptingService(JsScriptingOptions options) { _contexts = new ConditionalWeakTable(); _external = new Dictionary(); + _options = (options ?? throw new ArgumentNullException(nameof(options))).Clone(); } #endregion @@ -113,7 +125,7 @@ internal EngineInstance GetOrCreateInstance(IDocument document) if (!_contexts.TryGetValue(objectContext, out var instance)) { var libs = GetAssemblies(document.Context).ToArray(); - instance = new EngineInstance(objectContext, _external, libs); + instance = new EngineInstance(objectContext, _external, libs, _options); _contexts.Add(objectContext, instance); }