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

Version 5.5.2 adds new properties to the objects #60

Closed
HSyr opened this issue Jul 10, 2018 · 18 comments
Closed

Version 5.5.2 adds new properties to the objects #60

HSyr opened this issue Jul 10, 2018 · 18 comments
Assignees

Comments

@HSyr
Copy link

HSyr commented Jul 10, 2018

When an empty object is created and retured by:
function Test() { return {}; }

and then used at C# it contains the following properties:
__defineGetter__
__defineSetter__
__lookupGetter__
__lookupSetter__
__proto__
constructor
hasOwnProperty
isPrototypeOf
propertyIsEnumerable
toLocaleString
toString
valueOf

When such object is for instance serialized using Newtonsoft.Json.JsonConvert.SerializeObject and exception Process is terminated due to StackOverflowException occurs.

The same code at version 5.5.1 returned a real empty object without these properties and the serialization worked as expected.

@ClearScriptLib ClearScriptLib self-assigned this Jul 11, 2018
@BrknRobot
Copy link

BrknRobot commented Jul 12, 2018

As a workaround, you can use Object.create(null); to create objects without these properties, or use a serializer that supports loops. The exception occurs because __proto__ references the object being serialized.

@HSyr
Copy link
Author

HSyr commented Jul 16, 2018

Unfortunately using Object.create(null); is not feasible for me as I receive objects from the source code(s) that is out of my control. Also I have to stick with the serializer.
Question - will the future ClearScript releases keep creating objects with these properties?

@ClearScriptLib
Copy link
Collaborator

ClearScriptLib commented Jul 16, 2018

Hi @HSyr,

ClearScript doesn't add those properties; they're intrinsic to V8 JavaScript objects created in the normal manner (that is, with the standard prototype).

These properties are not enumerable on the JavaScript side, but ClearScript 5.5.2 exposes them to .NET dynamic enumeration in order to make them accessible in certain scenarios (see Issue #47).

ClearScript doesn't officially support script object serialization via Json.NET, but we're investigating. It may be possible to make it work without breaking property access in any supported scenarios.

Thanks!

@HSyr
Copy link
Author

HSyr commented Jul 16, 2018

Hi,
I understand the properties are intrinsic to V8 but I do not need them at the C# side at all. Not only because of failing JSON serialization.

I will keep using older version 5.5.1 or Microsoft.ClearScript.Windows that does not generate these properties.

@ClearScriptLib
Copy link
Collaborator

ClearScriptLib commented Jul 19, 2018

@HSyr, can you specify what problems, besides Json.NET serialization failures, these properties are causing? That would help us gauge the severity of this issue. Thanks!

@HSyr
Copy link
Author

HSyr commented Jul 19, 2018

The "problem" is, that they (the properties) are simply there but they are not expected, because as I pass the objects to the other components (that are out of my control) of our system and these components process all available object properties...

Let’s use analogy: when I call new object() at C# (or at many other languages), then all I get is really an empty object without any unwanted properties ,fields, ... I expect the same behavior in our scenario.

And this is what Microsoft.ClearScript.Windows.JScriptEngine does!

I thank you too!

@ClearScriptLib
Copy link
Collaborator

Hi @HSyr,

as I pass the objects to the other components (that are out of my control) of our system and these components process all available object properties...

Do you know how these other components enumerate the available object properties? Script objects support DynamicObject and IReflect. Exposing non-enumerable JavaScript properties through one but not the other may provide a regression-free fix.

Let’s use analogy: when I call new object() at C# (or at many other languages), then all I get is really an empty object without any unwanted properties ,fields, ...

Even empty .NET objects have members, as do V8's JavaScript objects. The difference is that, in JavaScript, members and contents are the same thing. Non-enumerable properties provide some mitigation, but in some .NET interop scenarios such properties are inaccessible.

We're still investigating, but an answer to the question above would be helpful. In any case, script object serialization via Json.NET would be nice, so we'll treat this a bug for now.

Thanks again!

@HSyr
Copy link
Author

HSyr commented Jul 25, 2018

These components are not .NET based. They receive the objects via JSON, so there is no DynamicObject and IReflect available. They use JSON schemas. I do not want to "dirty" the object passed to these components with unexpected and useless properties.

When I call new object() then from my perspective I do not see any public properties. There are some public methods only. So you are right, there are members, but we talk about properties that are to be serialized, not methods.

I am aware there might be private (and often undocumented) members in objects (I used them with reflection e.g. when making copy of Controls including events settings), but this is not case we are talking about.

I thank you too.

@ClearScriptLib
Copy link
Collaborator

These components are not .NET based. They receive the objects via JSON, so there is no DynamicObject and IReflect available.

Is the conversion of script objects to JSON always done via Json.NET?

@HSyr
Copy link
Author

HSyr commented Jul 26, 2018

Yes, always.
The conversion WAS ALWAYS available with version 5.5.1. The conversion IS NOT available with 5.5.2 for the reasons we discuss.
Thank you.

@ClearScriptLib
Copy link
Collaborator

Thanks for your help. Version 5.5.3 should have a fix for this issue.

@HSyr
Copy link
Author

HSyr commented Jul 31, 2018

I THANK YOU a lot. Looking forward to 5.5.3.

ClearScriptLib added a commit that referenced this issue Aug 20, 2018
…xed VB.NET access to nonexistent JavaScript properties (GitHub Issue #47, Take 2); fixed script object serialization via Json.NET (GitHub Issue #60); added host item invocability assessment and patched V8's typeof implementation to return "object" for all non-delegate host objects (GitHub Issue #62); added DocumentInfo and related APIs to address GitHub Issue #46; fixed property bag invocation; updated deployment and API documentation. Tested with V8 6.8.275.28.
@ClearScriptLib
Copy link
Collaborator

Version 5.5.3 has been posted.

@HSyr
Copy link
Author

HSyr commented Aug 21, 2018

Thank you very much.
When will the 5.5.3 be available via NuGet from VisualStudio?

@ClearScriptLib
Copy link
Collaborator

Hi @HSyr,

We don't release ClearScript binaries. If you're using a NuGet package, consider contacting its owners or building ClearScript yourself as outlined here.

Good luck!

@HSyr
Copy link
Author

HSyr commented Aug 21, 2018

The NuGet Package "ClearScript.V8 by Microsoft, Google" does not specify Owner(s), the field is empty. It says Author(s) is "Microsoft, Google" and project URL is https://github.com/Microsoft/ClearScript. So who do you recommend to contact?
Thank you.

@ClearScriptLib
Copy link
Collaborator

@HSyr
Copy link
Author

HSyr commented Aug 21, 2018

Thank you again. Hynek

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants