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

isRegex changes lastIndex since node v6 #3

Closed
Xotic750 opened this issue Feb 17, 2017 · 3 comments
Closed

isRegex changes lastIndex since node v6 #3

Xotic750 opened this issue Feb 17, 2017 · 3 comments
Labels

Comments

@Xotic750
Copy link

Xotic750 commented Feb 17, 2017

Since node v6 (I believe), testing if an object is a regexp causes lastIndex to be reset to 0. I haven't had a chance to track it down, but my guess is executing call on it?

regexExec.call(value);

test('lastIndex', function (t) {
	var re1 = /a/;
	re1.lastIndex = 3;
	t.ok(re1.lastIndex === 3, 'lastIndex is 3 before isRegex');
	t.ok(isRegex(re1), 'is regex');
	t.ok(re1.lastIndex === 3, 'lastIndex is 3 after isRegex');
	t.end();
});

Running the following in node v6 and above confirms the problem for me.

var regexExec = RegExp.prototype.exec;
var re1 = /a/;
re1.lastIndex = 3;
console.log(re1.lastIndex);
regexExec.call(re1);
console.log(re1.lastIndex);

I tried with RegExp.prototype.test and from v6 onward I get the same result.

The following change appears to fix the problem with the tests that I have performed.

var tryRegexExecCall = function tryRegexExec(value) {
	try {
		var lastIndex = value.lastIndex;
		regexExec.call(value);
		value.lastIndex = lastIndex;
		return true;
	} catch (e) {
		return false;
	}
};

Although I've opened this issue here, I'm not sure that this wouldn't be more appropriate logged against an es-shim?

@ljharb
Copy link
Member

ljharb commented Feb 18, 2017

It's certainly appropriate here; checking the type of it shouldn't cause it to be mutated.

Whether it's an es shim bug depends entirely on whether v8 made a change, and whether it was part of the spec to do so.

However, regardless, one should never rely on lastIndex of a regex you didn't create - my expectation after verifying that you had a regex would be that you'd do var copy = new RegExp(re.source, re.flags) so as to ensure not mutating re.

@ljharb
Copy link
Member

ljharb commented Feb 18, 2017

It appears that this is not a problem in node 0.10 or 0.12, but it is a problem starting with io.js v1.

@ljharb ljharb closed this as completed in d4a0a6b Feb 18, 2017
@ljharb
Copy link
Member

ljharb commented Feb 18, 2017

Thanks for the report!

@ljharb ljharb added the bug label Feb 18, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants