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

Overridden RegExp "global" property potentially causes infinite loop #528

Closed
goyakin opened this Issue Apr 8, 2016 · 4 comments

Comments

Projects
None yet
4 participants
@goyakin
Member

goyakin commented Apr 8, 2016

#494 updated RegExpBuiltinExec to infer global from [[OriginalFlags]] instead of performing a Get for the property. If we have a RegExp instance created without the global flag, but we overridde the global property to return true, this potentially creates an infinite loop since RegExpBuiltinExec now resets lastIndex to 0. For example:

var re = /-/;
Object.defineProperty(re, "global", {value: true});
re[Symbol.match]('--'); // Infinite loop

This applies to RegExp.prototype[Symbol.match] and RegExp.prototype[Symbol.replace].

From the caller's perspective, I don't think this should happen since the property returns a consistent value.

@littledan

This comment has been minimized.

Show comment
Hide comment
@littledan

littledan Apr 8, 2016

Member

I think it's fair to say that it's possible to cause bad situations by overriding the flags getters. But Is this so surprising and bad, that when a protocol is violated, that weird results happen? I bet you could also mess up the Array methods pretty easily by returning a Proxy from an Array subclass constructor.

Because RegExp methods like RegExp.prototype[Symbol.replace] are not specialized to objects with a [[RegExpMatcher]] internal slot (a possibility that I proposed at the November 2015 TC39 meeting, but which was rejected), they cannot refer to the [[OriginalFlags]] value.

Member

littledan commented Apr 8, 2016

I think it's fair to say that it's possible to cause bad situations by overriding the flags getters. But Is this so surprising and bad, that when a protocol is violated, that weird results happen? I bet you could also mess up the Array methods pretty easily by returning a Proxy from an Array subclass constructor.

Because RegExp methods like RegExp.prototype[Symbol.replace] are not specialized to objects with a [[RegExpMatcher]] internal slot (a possibility that I proposed at the November 2015 TC39 meeting, but which was rejected), they cannot refer to the [[OriginalFlags]] value.

@ljharb

This comment has been minimized.

Show comment
Hide comment
@ljharb

ljharb Apr 8, 2016

Member

What's the use case where someone would do that via a defineProperty, when they can new RegExp(re, re.flags + 'g') if they want the same regex with the global flag set?

Member

ljharb commented Apr 8, 2016

What's the use case where someone would do that via a defineProperty, when they can new RegExp(re, re.flags + 'g') if they want the same regex with the global flag set?

@bterlson

This comment has been minimized.

Show comment
Hide comment
@bterlson

bterlson Apr 12, 2016

Member

I find this a bit gross but I doubt this is an issue that someone would come across in real code. Basically the guidance is: if you want to modify a regexp instance or implement a subclass, and you want the getters for global and sticky to differ from the internal flags, then you need to implement @@match/@@replace to handle this.

Member

bterlson commented Apr 12, 2016

I find this a bit gross but I doubt this is an issue that someone would come across in real code. Basically the guidance is: if you want to modify a regexp instance or implement a subclass, and you want the getters for global and sticky to differ from the internal flags, then you need to implement @@match/@@replace to handle this.

@goyakin

This comment has been minimized.

Show comment
Hide comment
@goyakin

goyakin Apr 15, 2016

Member

@ljharb, I don't personally see why one would want to do this, but the language shouldn't allow such a thing if possible.

@littledan, I was thinking of using [[OriginalFlags]] if it exists and fall back to the properties if not, but this doesn't work well with subclasses. It now also looks like the use of this internal slot was rejected by the committee.

@bterlson, 👍

I agree with the reasoning behind #494. Given that there is no good way of fixing this, I'm closing the issue.

Member

goyakin commented Apr 15, 2016

@ljharb, I don't personally see why one would want to do this, but the language shouldn't allow such a thing if possible.

@littledan, I was thinking of using [[OriginalFlags]] if it exists and fall back to the properties if not, but this doesn't work well with subclasses. It now also looks like the use of this internal slot was rejected by the committee.

@bterlson, 👍

I agree with the reasoning behind #494. Given that there is no good way of fixing this, I'm closing the issue.

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