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 global function get polluted close #52 #63

Closed
wants to merge 2 commits into from
Closed

fix global function get polluted close #52 #63

wants to merge 2 commits into from

Conversation

Jack-Works
Copy link
Contributor

@Jack-Works Jack-Works commented Sep 28, 2019

this fix is split from #17

close #52

Copy link
Member

@erights erights left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Switching anything semantically significant on the caller is hugely hazardous, and likely is a form of dynamic scoping, which we have taken pains to avoid in the rest of the language. Also, the stack property of Errors is not yet standard, and is likely to change.

Please explain what concrete compatibility problems you are running into, and what you need them to do. Let's see how we can accommodate those cases, or how you can on top of the realms-shim, with minimal damage to the realms-shim itself.

Probably best would be to start an issue discussing the actual problem and constraints, and how we can bound these as narrowly as possible. Thanks. Closing this PR.

// Prevents the evaluation of source when calling constructor on the
// prototype of functions.
const TamedFunction = function() {
throw new TypeError('Not available');
if (isRunningInRealms()) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Switching anything semantically significant on the caller is hugely hazardous, and likely is a form of dynamic scoping, which we have taken pains to avoid in the rest of the language. Also, the stack property of Errors is not yet standard, and is likely to change.

Please explain what concrete compatibility problems you are running into, and what you need them to do. Let's see how we can accommodate those cases, or how you can on top of the realms-shim, with minimal damage to the realms-shim itself. Thanks.

@erights
Copy link
Member

erights commented Sep 28, 2019

Switching anything semantically significant on the caller is hugely hazardous, and likely is a form of dynamic scoping, which we have taken pains to avoid in the rest of the language. Also, the stack property of Errors is not yet standard, and is likely to change.

Please explain what concrete compatibility problems you are running into, and what you need them to do. Let's see how we can accommodate those cases, or how you can on top of the realms-shim, with minimal damage to the realms-shim itself. Thanks.

@erights erights closed this Sep 28, 2019
@Jack-Works
Copy link
Contributor Author

The issue is drafted in #52.

Realms shim will break the global function constructor.

My fix may have false positive (detect outside code as running in the realm then reject it) but no false negative (every code run in the realms must be have a eval string in it's stack. If there is no stack available, we assume it is running in the realms)

@erights
Copy link
Member

erights commented Sep 29, 2019

The issue is drafted in #52.

Ah. I think I understand now. I closed this because I thought you were trying to make the Function constructor in the new realm more compat with the normal unsafe Function constructor. Looking at #52 I see that you're trying to fix the bug that we're altering the Function constructor of the realm that would create a new realm. Yes, this is indeed still a bug that must be fixed. But the answer is to untangle the two Function constructors. Not to create one that tries to serve both purposes.

Attn @jfparadis

@Jack-Works
Copy link
Contributor Author

I just thought it is impossible to not pollute the outside function in some techincal reason so I made this pr

@Jack-Works
Copy link
Contributor Author

And the code of make the Function constructor more compatible with nonstrict mode is in #17, and it's ready for review

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.

Function.prototype.constructor is tamed in the host
2 participants