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

Fix eval and function constructor failing for object literals with __proto__ prop #948

Merged
merged 4 commits into from Aug 30, 2021

Conversation

KurtGokhan
Copy link
Contributor

@KurtGokhan KurtGokhan commented Aug 23, 2021

Related to #598

Fixes weren't applied to Eval and Function Ctor before.

Edit after tests: Tolerant = true breaks other tests. Do you know what may be a better solution?

@lahma
Copy link
Collaborator

lahma commented Aug 23, 2021

I quickly checked just one test case and its contents:

function testcase()
{
  try 
  {
    eval('"use strict"; var x = "\\17";');
    return false;
  }
  catch (e) {
    return (e instanceof SyntaxError);
  }
}
runTestCase(testcase);

So my hunch is that it's a strict mode issue. At least PerformEval has strictCaller parameter which you could use to choose between two different option setups (tolerant true/false), so maybe another static field for choosing between. Didn't dig deep into the issue so not sure.

@KurtGokhan
Copy link
Contributor Author

KurtGokhan commented Aug 24, 2021

I think this should be solved in Esprima.NET.

Basically, this should work: ({ __proto__: [] })

This should fail: ({ __proto__: [], __proto__: [] })

I will try to fix it.

@KurtGokhan KurtGokhan marked this pull request as draft August 24, 2021 11:50
@KurtGokhan
Copy link
Contributor Author

Created sebastienros/esprima-dotnet#192

There is no need to change any code in Jint. Only tests.

But I noticed that by making Tolerant = true in the engine in this PR, some behaviors are not compliant with spec. Should we revert it? But it can be a breaking change.

@lahma
Copy link
Collaborator

lahma commented Aug 30, 2021

Main branch has been updated with latest Esprima. Is there still an issue that we need to fix?

@KurtGokhan
Copy link
Contributor Author

Let me test real quick.

@KurtGokhan KurtGokhan marked this pull request as ready for review August 30, 2021 18:32
@KurtGokhan
Copy link
Contributor Author

Good to go. It is only tests.

@lahma lahma merged commit d8282ef into sebastienros:main Aug 30, 2021
@lahma
Copy link
Collaborator

lahma commented Aug 30, 2021

Thanks for verifying!

@KurtGokhan KurtGokhan deleted the proto-prop-bug branch August 30, 2021 18:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants