Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Check cyclic references when evaluating ToObject #936

Merged
merged 1 commit into from
Jul 22, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions Jint.Tests/Jint.Tests.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
<AssemblyOriginatorKeyFile>..\Jint\Jint.snk</AssemblyOriginatorKeyFile>
<SignAssembly>true</SignAssembly>
<IsPackable>false</IsPackable>
<LangVersion>latest</LangVersion>
</PropertyGroup>
<ItemGroup>
<EmbeddedResource Include="Runtime\Scripts\*.*;Parser\Scripts\*.*" />
Expand Down
28 changes: 28 additions & 0 deletions Jint.Tests/Runtime/InteropTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2563,5 +2563,33 @@ public void ObjectWrapperWrappingDictionaryShouldNotBeArrayLike()
var wrapper = new ObjectWrapper(_engine, new Dictionary<string, object>());
Assert.False(wrapper.IsArrayLike);
}

[Fact]
public void ShouldHandleCyclicReferences()
{
var engine=new Engine();

static void Test(string message, object value) { Console.WriteLine(message); }

engine.Realm.GlobalObject.FastAddProperty("global", engine.Realm.GlobalObject, true, true, true);
engine.Realm.GlobalObject.FastAddProperty("test", new DelegateWrapper(engine, (Action<string, object>) Test), true, true, true);

var ex = Assert.Throws<JavaScriptException>(() => engine.Realm.GlobalObject.ToObject());
Assert.Equal("Cyclic reference detected.", ex.Message);

ex = Assert.Throws<JavaScriptException>(() =>
engine.Execute(@"
var demo={};
demo.value=1;
test('Test 1', demo.value===1);
test('Test 2', demo.value);
demo.demo=demo;
test('Test 3', demo);
test('Test 4', global);"
)
);

Assert.Equal("Cyclic reference detected.", ex.Message);
}
}
}
40 changes: 40 additions & 0 deletions Jint/Collections/ObjectTraverseStack.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
using System.Collections.Generic;
using Jint.Native;
using Jint.Runtime;

namespace Jint.Collections
{
/// <summary>
/// Helps traversing objects and checks for cyclic references.
/// </summary>
internal sealed class ObjectTraverseStack
{
private readonly Engine _engine;
private readonly Stack<object> _stack = new();

public ObjectTraverseStack(Engine engine)
{
_engine = engine;
}

public void Enter(JsValue value)
{
if (value is null)
{
ExceptionHelper.ThrowArgumentNullException(nameof(value));
}

if (_stack.Contains(value))
{
ExceptionHelper.ThrowTypeError(_engine.Realm, "Cyclic reference detected.");
}

_stack.Push(value);
}

public void Exit()
{
_stack.Pop();
}
}
}
3 changes: 2 additions & 1 deletion Jint/Native/Iterator/IteratorInstance.cs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@ public IteratorInstance(Engine engine)

public override object ToObject()
{
throw new System.NotImplementedException();
ExceptionHelper.ThrowNotImplementedException();
return null;
}

public override bool Equals(JsValue other)
Expand Down
30 changes: 8 additions & 22 deletions Jint/Native/Json/JsonSerializer.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using System.Collections.Generic;
using System.Linq;
using Jint.Collections;
using Jint.Native.Global;
using Jint.Native.Object;
using Jint.Runtime;
Expand All @@ -16,14 +17,14 @@ public JsonSerializer(Engine engine)
_engine = engine;
}

private Stack<object> _stack;
private ObjectTraverseStack _stack;
private string _indent, _gap;
private List<JsValue> _propertyList;
private JsValue _replacerFunction = Undefined.Instance;

public JsValue Serialize(JsValue value, JsValue replacer, JsValue space)
{
_stack = new Stack<object>();
_stack = new ObjectTraverseStack(_engine);

// for JSON.stringify(), any function passed as the first argument will return undefined
// if the replacer is not defined. The function is not called either.
Expand Down Expand Up @@ -247,8 +248,7 @@ private static string Quote(string value)

private string SerializeArray(JsValue value)
{
EnsureNonCyclicity(value);
_stack.Push(value);
_stack.Enter(value);
var stepback = _indent;
_indent = _indent + _gap;
var partial = new List<string>();
Expand All @@ -262,7 +262,7 @@ private string SerializeArray(JsValue value)
}
if (partial.Count == 0)
{
_stack.Pop();
_stack.Exit();
return "[]";
}

Expand All @@ -280,30 +280,16 @@ private string SerializeArray(JsValue value)
final = "[\n" + _indent + properties + "\n" + stepback + "]";
}

_stack.Pop();
_stack.Exit();
_indent = stepback;
return final;
}

private void EnsureNonCyclicity(object value)
{
if (value == null)
{
ExceptionHelper.ThrowArgumentNullException(nameof(value));
}

if (_stack.Contains(value))
{
ExceptionHelper.ThrowTypeError(_engine.Realm, "Cyclic reference detected.");
}
}

private string SerializeObject(ObjectInstance value)
{
string final;

EnsureNonCyclicity(value);
_stack.Push(value);
_stack.Enter(value);
var stepback = _indent;
_indent += _gap;

Expand Down Expand Up @@ -346,7 +332,7 @@ private string SerializeObject(ObjectInstance value)
final = "{\n" + _indent + properties + "\n" + stepback + "}";
}
}
_stack.Pop();
_stack.Exit();
_indent = stepback;
return final;
}
Expand Down
55 changes: 34 additions & 21 deletions Jint/Native/Object/ObjectInstance.cs
Original file line number Diff line number Diff line change
Expand Up @@ -891,11 +891,19 @@ public override string ToString()

public override object ToObject()
{
return ToObject(new ObjectTraverseStack(_engine));
}

private object ToObject(ObjectTraverseStack stack)
{
stack.Enter(this);
if (this is IObjectWrapper wrapper)
{
stack.Exit();
return wrapper.Target;
}

object converted = null;
switch (Class)
{
case ObjectClass.Array:
Expand All @@ -910,67 +918,63 @@ public override object ToObject()
if (kpresent)
{
var kvalue = arrayInstance.Get(k);
result[k] = kvalue.ToObject();
var value = kvalue is ObjectInstance oi
? oi.ToObject(stack)
: kvalue.ToObject();
result[k] = value;
}
else
{
result[k] = null;
}
}

return result;
converted = result;
}

break;

case ObjectClass.String:
if (this is StringInstance stringInstance)
{
return stringInstance.PrimitiveValue.ToString();
converted = stringInstance.PrimitiveValue.ToString();
}

break;

case ObjectClass.Date:
if (this is DateInstance dateInstance)
{
return dateInstance.ToDateTime();
converted = dateInstance.ToDateTime();
}

break;

case ObjectClass.Boolean:
if (this is BooleanInstance booleanInstance)
{
return ((JsBoolean) booleanInstance.PrimitiveValue)._value
? JsBoolean.BoxedTrue
: JsBoolean.BoxedFalse;
converted = ((JsBoolean) booleanInstance.PrimitiveValue)._value
? JsBoolean.BoxedTrue
: JsBoolean.BoxedFalse;
}

break;

case ObjectClass.Function:
if (this is FunctionInstance function)
{
return (Func<JsValue, JsValue[], JsValue>) function.Call;
converted = (Func<JsValue, JsValue[], JsValue>) function.Call;
}

break;

case ObjectClass.Number:
if (this is NumberInstance numberInstance)
{
return numberInstance.NumberData._value;
converted = numberInstance.NumberData._value;
}

break;

case ObjectClass.RegExp:
if (this is RegExpInstance regeExpInstance)
{
return regeExpInstance.Value;
converted = regeExpInstance.Value;
}

break;

case ObjectClass.Arguments:
Expand All @@ -983,14 +987,23 @@ public override object ToObject()
continue;
}

o.Add(p.Key.ToString(), Get(p.Key, this).ToObject());
var key = p.Key.ToString();
var propertyValue = Get(p.Key, this);
var value = propertyValue is ObjectInstance oi
? oi.ToObject(stack)
: propertyValue.ToObject();
o.Add(key, value);
}

return o;
converted = o;
break;
default:
converted = this;
break;
}


return this;
stack.Exit();
return converted;
}

/// <summary>
Expand Down