DEV Community

Can you find the bug in this piece of code? - RegExp edition 🌍

Keff on November 03, 2021

Hey there! 👋 I'm back with another installment of Find the bug, this time with Typescript/Javascript. Regular expressions are useful, but can beha...

Read full post

Martin • Nov 3 '21

The regex instance keeps track of the lastIndex and therefore does not for all items start from zero. You can remove the g flag and instead use string start and end anchors /^[a-z0-9]+_[a-z0-9]+$/i so the regex won't keep the lastIndex. Or you reset the lastIndex before test isValidName = (value) => { TEST_REGEXP.lastIndex = 0; return TEST_REGEXP.test(value); }

𒎏Wii 🏳️‍⚧️ • Nov 3 '21 • Edited

Here I was, looking for an error in the actual regex xD

I was not even aware of this weird behaviour, and would probably have written it as

/* line 6 */ return value.match(TEST_REGEXP)

instead, and anchored the regex to beginning and end, of course :D

And for the actual answer:

--- before.js   2021-11-03 16:31:00.256761809 +0100
+++ after.js    2021-11-03 16:30:46.961086329 +0100
@@ -1,4 +1,4 @@
-const TEST_REGEXP = /[a-z0-9]+_[a-z0-9]+/gi;
+const TEST_REGEXP = /^[a-z0-9]+_[a-z0-9]+$/i;

 function isValidName(value) {
     if (typeof value !== 'string') return false;

Keff • Nov 3 '21

Fantastic, thanks for the solutions! I'd go with removing the global flag . Though the other solution is interesting, did not think of reseting lastIndex manually!

Siddharth • Nov 3 '21 • Edited

Got it! Because .test() tests the rest of the string when the g flag is enabled!

Siddharth • Nov 3 '21

Just remove the g flag to fix

Keff • Nov 3 '21

There you go, you found it!! Kinda weird if you don't expect that behaviour right?

Siddharth • Nov 3 '21

It is! I never knew that till today!

Keff • Nov 3 '21

Nice that's wonderful! That's what I aim this series to be, so I'm glad you learned something new!

𒎏Wii 🏳️‍⚧️ • Nov 3 '21

I also didn't know this. It's probably very useful when building a parser, but when you don't expect it, it's a super confusing error 😖

Keff • Nov 3 '21

Yup, I was super confused when I first encountered this error. I found it making some unit tests for work, and lost a bit of time trying to figure it out... just for the issue to be a silly little flag xD

Mrinmay Mukherjee • Nov 3 '21

Using the g(global) flag with .exec stores the index up to which the pattern matches. If the pattern does not match, the index is reset to 0.

If the previous search was successful, the next search starts after the stored index, even though the string is different. This is because the regular expression stores the matched index. I encountered this bug while writing an input validator. Took me half a day to figure this out. 😅😅

Here is the explanation from the docs

So for the above code:

'test_1' matches. So the stored index is 5.
'test_1' does not match, since the pattern matching starts at index 5, which is just an empty string. So the stored index becomes 0.
'test_2' matches. So the stored index becomes 5.
'other_test' does not match. So the stored index becomes 0.
'some_file' matches.

Thus the output is:

true
false
true
false
true

Keff • Nov 3 '21

Great breakdown, thanks for taking the time to explain it in such detail!!

Eljay-Adobe • Nov 4 '21

Tricky. Just needed to remove the g. Now it's a ReExp.

lionel-rowe • Nov 4 '21

2nd bug no-one has mentioned yet: the regex matches anywhere in the string, so for example, isValidName('叽叽喳喳 o_O 咔嚓咔嚓咔嚓') is true, because the o_O in the middle matches.

Keff • Nov 4 '21

Good catch, that's my mistake. When simplifying the example for this post I must've missed that.

Lars Grammel • Nov 3 '21

Great riddle!

Adam Crockett 🌀 • Nov 3 '21

A little note on the typescript in this file, it might as well not be here because it's better to state what is expected then guard against it rather than fall through to runtime

Keff • Nov 3 '21 • Edited

Yup, good point, my bad. I found this bug on a typescript project so I though to include it as TS, but I simplified the example so much that it does not make sense to have it now!

I will fix the type to make it more relevant (I've removed the any type actually).