Skip to content

Commit ce97757

Browse files
authored
Merge pull request #124 from lahma/stackoverflow
Stop a deeply recursive script from taking down the process
2 parents 0038d26 + f4dd372 commit ce97757

9 files changed

Lines changed: 188 additions & 10 deletions

File tree

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22

33
Released on ?.
44

5+
- Fixed `StackOverflowException` from deeply recursive scripts (#75)
6+
- Added `JsScriptingOptions` to tune the engine via `WithJs` (#75)
57
- Fixed usage of document ready state (#87) @Sebbs128
68
- Fixed `HasChildNodes` is now exposed as a method to DOM (#106) @arekdygas
79
- Updated to use AngleSharp v1

README.md

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,17 @@ var config = Configuration.Default
2121
.WithJs(); // from AngleSharp.Js
2222
```
2323

24-
This will register a scripting engine for JS files. The JS parsing options and more could be set with parameters of the `WithJs` method.
24+
This will register a scripting engine for JS files. The engine can be tuned by passing a `JsScriptingOptions` instance to `WithJs`:
25+
26+
```cs
27+
var config = Configuration.Default
28+
.WithJs(new JsScriptingOptions
29+
{
30+
// how deep a script may recurse before the engine reports
31+
// "Maximum call stack size exceeded" (10000 by default)
32+
MaxCallStackDepth = 5000,
33+
});
34+
```
2535

2636
You can also use this part with a console for logging. The call for this is `WithConsoleLogger`, e.g.,
2737

docs/general/01-Basics.md

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,17 @@ var config = Configuration.Default
3737
.WithJs(); // from AngleSharp.Js
3838
```
3939

40-
This will register a scripting engine for JS files. The JS parsing options and more could be set with parameters of the `WithJs` method.
40+
This will register a scripting engine for JS files. The engine can be tuned by passing a `JsScriptingOptions` instance to `WithJs`:
41+
42+
```cs
43+
var config = Configuration.Default
44+
.WithJs(new JsScriptingOptions
45+
{
46+
// how deep a script may recurse before the engine reports
47+
// "Maximum call stack size exceeded" (10000 by default)
48+
MaxCallStackDepth = 5000,
49+
});
50+
```
4151

4252
You can also use this part with a console for logging. The call for this is `WithConsoleLogger`, e.g.,
4353

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
namespace AngleSharp.Js.Tests
2+
{
3+
using AngleSharp.Dom;
4+
using Jint.Runtime;
5+
using NUnit.Framework;
6+
using System;
7+
using System.Threading.Tasks;
8+
9+
public class StackGuardTests
10+
{
11+
// A recursion that never ends. Jint implements no tail calls, so this really does
12+
// grow the call stack. Without a stack guard it exhausts the native one, and the
13+
// resulting StackOverflowException takes the whole process down - which is why the
14+
// tests below cannot assert anything weaker than "we got here at all".
15+
private const String RunawayRecursion = "function boom() { return boom(); } boom();";
16+
17+
// Deeper than a 1 MB stack holds, so the engine has to keep going on a fresh one
18+
// instead of reporting the depth as an error.
19+
private const String DeepRecursion = "function depth(n) { return n === 0 ? 0 : 1 + depth(n - 1); } depth(1500);";
20+
21+
[Test]
22+
public async Task RunawayRecursionInPageScriptDoesNotEscapeOpenAsync()
23+
{
24+
var config = Configuration.Default
25+
.WithJs()
26+
.WithEventLoop();
27+
28+
var content = $"<!doctype html><script>{RunawayRecursion}</script>";
29+
var document = await BrowsingContext.New(config).OpenAsync(m => m.Content(content));
30+
31+
Assert.IsNotNull(document);
32+
}
33+
34+
[Test]
35+
public async Task RunawayRecursionReportsMaximumCallStackSizeExceeded()
36+
{
37+
// A small limit keeps the failure quick and independent of how much native
38+
// stack the test runner happens to have left.
39+
var config = Configuration.Default
40+
.WithJs(new JsScriptingOptions { MaxCallStackDepth = 200 })
41+
.WithEventLoop();
42+
43+
var document = await BrowsingContext.New(config).OpenNewAsync();
44+
var error = Assert.Throws<JavaScriptException>(() => document.ExecuteScript(RunawayRecursion));
45+
46+
Assert.AreEqual("Maximum call stack size exceeded", error.Message);
47+
}
48+
49+
[Test]
50+
public async Task DeepButFiniteRecursionStillSucceeds()
51+
{
52+
var config = Configuration.Default
53+
.WithJs()
54+
.WithEventLoop();
55+
56+
var document = await BrowsingContext.New(config).OpenNewAsync();
57+
var result = document.ExecuteScript(DeepRecursion);
58+
59+
Assert.AreEqual(1500.0, result);
60+
}
61+
62+
[Test]
63+
public async Task EditingTheOptionsAfterwardsLeavesTheServiceAlone()
64+
{
65+
var options = new JsScriptingOptions();
66+
var config = Configuration.Default
67+
.WithJs(options)
68+
.WithEventLoop();
69+
70+
// The engine is only built once a document asks for it, so an edit landing
71+
// in between must not be the one deciding how that document behaves.
72+
options.MaxCallStackDepth = 200;
73+
74+
var document = await BrowsingContext.New(config).OpenNewAsync();
75+
var result = document.ExecuteScript(DeepRecursion);
76+
77+
Assert.AreEqual(1500.0, result);
78+
}
79+
}
80+
}

src/AngleSharp.Js/EngineInstance.cs

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,9 @@ sealed class EngineInstance
1616
{
1717
#region Fields
1818

19+
// Jint's StackGuard.Disabled, which is internal.
20+
private const Int32 StackGuardDisabled = -1;
21+
1922
private readonly Engine _engine;
2023
private readonly PrototypeCache _prototypes;
2124
private readonly ReferenceCache _references;
@@ -27,13 +30,18 @@ sealed class EngineInstance
2730

2831
#region ctor
2932

30-
public EngineInstance(IWindow window, IDictionary<String, Object> assignments, IEnumerable<Assembly> libs)
33+
public EngineInstance(IWindow window, IDictionary<String, Object> assignments, IEnumerable<Assembly> libs, JsScriptingOptions options)
3134
{
3235
_importMap = new JsImportMap();
3336

34-
_engine = new Engine((options) =>
37+
_engine = new Engine((o) =>
3538
{
36-
options.EnableModules(new JsModuleLoader(this, window.Document, false));
39+
o.EnableModules(new JsModuleLoader(this, window.Document, false));
40+
// Left alone, the JS call stack is the native one, and a script recursing
41+
// deeper than it holds takes the whole process down - a StackOverflowException
42+
// cannot be caught. Guarded, the engine continues on a fresh stack and finally
43+
// reports an ordinary "Maximum call stack size exceeded" error instead.
44+
o.Constraints.MaxExecutionStackCount = options.MaxCallStackDepth > 0 ? options.MaxCallStackDepth : StackGuardDisabled;
3745
});
3846
_libs = libs;
3947
_prototypes = new PrototypeCache(_engine, libs);

src/AngleSharp.Js/JsConfigurationExtensions.cs

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,9 +61,20 @@ public static IConfiguration WithEventLoop(this IConfiguration configuration, Fu
6161
/// </summary>
6262
/// <param name="configuration">The configuration to use.</param>
6363
/// <returns>The new configuration.</returns>
64-
public static IConfiguration WithJs(this IConfiguration configuration)
64+
public static IConfiguration WithJs(this IConfiguration configuration) =>
65+
configuration.WithJs(new JsScriptingOptions());
66+
67+
/// <summary>
68+
/// Sets scripting to true, registers the JavaScript engine with the
69+
/// given options and returns a new configuration with the scripting
70+
/// service and possible auxiliary services, if not yet registered.
71+
/// </summary>
72+
/// <param name="configuration">The configuration to use.</param>
73+
/// <param name="options">The options tuning the engine.</param>
74+
/// <returns>The new configuration.</returns>
75+
public static IConfiguration WithJs(this IConfiguration configuration, JsScriptingOptions options)
6576
{
66-
var service = new JsScriptingService();
77+
var service = new JsScriptingService(options);
6778
var observer = new EventAttributeObserver(service);
6879
var handler = new JsNavigationHandler(service);
6980

src/AngleSharp.Js/JsEventLoop.cs

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,14 @@ namespace AngleSharp.Js
1111
/// </summary>
1212
public sealed class JsEventLoop : IEventLoop, IDisposable
1313
{
14+
// Scripts run on this thread, and the JS call stack is the native one. The usual
15+
// 1 MB holds roughly a thousand JavaScript frames, well short of what a browser
16+
// offers, and every frame beyond it costs the engine a hop onto a fresh stack.
17+
// The size is reserved address space rather than memory, but a 32 bit process has
18+
// little of it to spare when it runs many loops, so only a 64 bit one is enlarged;
19+
// zero leaves the thread with the default of the process.
20+
private static readonly Int32 DefaultMaxStackSize = IntPtr.Size == 8 ? 16 * 1024 * 1024 : 0;
21+
1422
private readonly Dictionary<TaskPriority, Queue<LoopEntry>> _queues = new Dictionary<TaskPriority, Queue<LoopEntry>>();
1523
private readonly Object _lockObj = new Object();
1624
private CancellationTokenSource _cts;
@@ -19,8 +27,17 @@ public sealed class JsEventLoop : IEventLoop, IDisposable
1927
/// Creates a new event loop thread.
2028
/// </summary>
2129
public JsEventLoop()
30+
: this(DefaultMaxStackSize)
31+
{
32+
}
33+
34+
/// <summary>
35+
/// Creates a new event loop thread with the given stack size.
36+
/// </summary>
37+
/// <param name="maxStackSize">The stack size of the thread running the scripts.</param>
38+
public JsEventLoop(Int32 maxStackSize)
2239
{
23-
var thread = new Thread(Runner)
40+
var thread = new Thread(Runner, maxStackSize)
2441
{
2542
IsBackground = true,
2643
Name = "AngleSharpEventLoop",
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
namespace AngleSharp.Js
2+
{
3+
using System;
4+
5+
/// <summary>
6+
/// Options tuning the JavaScript engine.
7+
/// </summary>
8+
public sealed class JsScriptingOptions
9+
{
10+
/// <summary>
11+
/// Gets or sets the JavaScript call stack depth that has to be supported
12+
/// before the engine gives up with a "Maximum call stack size exceeded"
13+
/// error. Defaults to 10000, which is roughly what browsers allow. Values
14+
/// of zero or less remove the limit - the engine is then bounded by the
15+
/// native stack alone, so a runaway recursion terminates the process with
16+
/// an uncatchable StackOverflowException.
17+
/// </summary>
18+
public Int32 MaxCallStackDepth { get; set; } = 10000;
19+
20+
// An engine is built per window, long after the options were handed over, so
21+
// reading them then would let a later edit of the caller's object decide how
22+
// the next document behaves. The service takes this copy instead.
23+
internal JsScriptingOptions Clone() => new JsScriptingOptions
24+
{
25+
MaxCallStackDepth = MaxCallStackDepth,
26+
};
27+
}
28+
}

src/AngleSharp.Js/JsScriptingService.cs

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,18 +25,30 @@ public class JsScriptingService : IScriptingService
2525

2626
private readonly ConditionalWeakTable<IWindow, EngineInstance> _contexts;
2727
private readonly Dictionary<String, Object> _external;
28+
private readonly JsScriptingOptions _options;
2829

2930
#endregion
3031

3132
#region ctor
3233

3334
/// <summary>
34-
/// Creates a new JavaScript engine.
35+
/// Creates a new JavaScript engine using the default options.
3536
/// </summary>
3637
public JsScriptingService()
38+
: this(new JsScriptingOptions())
39+
{
40+
}
41+
42+
/// <summary>
43+
/// Creates a new JavaScript engine using the given options. The options
44+
/// are copied, so that editing them afterwards leaves this engine alone.
45+
/// </summary>
46+
/// <param name="options">The options tuning the engine.</param>
47+
public JsScriptingService(JsScriptingOptions options)
3748
{
3849
_contexts = new ConditionalWeakTable<IWindow, EngineInstance>();
3950
_external = new Dictionary<String, Object>();
51+
_options = (options ?? throw new ArgumentNullException(nameof(options))).Clone();
4052
}
4153

4254
#endregion
@@ -113,7 +125,7 @@ internal EngineInstance GetOrCreateInstance(IDocument document)
113125
if (!_contexts.TryGetValue(objectContext, out var instance))
114126
{
115127
var libs = GetAssemblies(document.Context).ToArray();
116-
instance = new EngineInstance(objectContext, _external, libs);
128+
instance = new EngineInstance(objectContext, _external, libs, _options);
117129
_contexts.Add(objectContext, instance);
118130
}
119131

0 commit comments

Comments
 (0)