Invalid match for "Using the e flag in preg_match"

Hmm, I think this is because the current check doesn't figure out which kind of delimiter is used. It only deals with / as this is the most commonly used delimiter.

I've extended the code to check if the pattern is a string enclosed by quotes and dynamically detect the delimiter. Commit: http://drupalcode.org/project/drupalcs.git/commit/353be94

Still gives false positives.

I where able to spot two issues with the current implementation:

1: The current test relies on word boundaries, and the pattern delimiteres will most likely qualify as such. Current pattern:

'/' . $delimiter . '[\w]{0,}e[\w]{0,}\b/'

eg: <0 or more wordchars>e<0 or more wordchars>

Which will match the following as "/" is a word boundary

/hello/

2: If the pattern we're testing is made up of concatenated strings, the current code only works on the first part of the concatenation. Eg if the code being tested contained something like:

<?php
preg_match('/hel' . 'lo/', $string); 
?>

The test would be made against '/hel'

One way to fix the first issue is to use $ instead of \b as it matched end of line, and then strip off the last char of the pattern to avoid the closing quote. (patch attached).

I don't have enough experience with phpcs to fix the other issue though.

drupalcs has been moved into coder.

Iv'e tested the patch, it works and rerolled if for the current coder HEAD.

This needs a test case for good.php. Can we have a better example than in #2 where coder_sniffer currently fails and what the patch fixes?

Now coder_sniffer fails "/yaCounter(\d+)/is" (http://ventral.org/pareview/httpgitdrupalorgsandboxholdmann1557584git).
Patch from #4 fixes it.

Ok, I added a test sample and checked that the existing examples in bad.php still throw errors.

Committed, thanks!

Please re-open if there are still cases where this throws false positives.

I found another false positive for this, which fails in the latest dev branch:

/node\/(\d+)/

Also I noticed that the good.php file is generating this error now, this has been introduced in commit a98019b749.

[pieter@archlinux coder (7.x-2.x=)]$ drush dcs coder_sniffer/Test/good.php 

FILE: .../sites/all/modules/coder/coder_sniffer/Test/good.php
--------------------------------------------------------------------------------
FOUND 2 ERROR(S) AFFECTING 2 LINE(S)
--------------------------------------------------------------------------------
 191 | ERROR | Blank lines are not allowed after CASE statements
 569 | ERROR | Using the e flag in preg_match is a possible security risk. For
     |       | details see http://drupal.org/node/750148
--------------------------------------------------------------------------------

This is on line 569:

preg_match("/test(\d+)/is", 'subject');

Works for me as expected, please use the latest version of PHP_CodeSniffer.

There is now a fail in good.php on line 582, but that has nothing to do with this issue ...

I forgot to copy the latest Drupal ruleset to the /usr/share/pear/PHP/CodeSniffer/Standards folder. My bad. After doing this I did not get any false positives any more.

I just applied this fix and it worked great since I wasn't using an e flag. Thanks so much guys for providing this module for helping someone to develop better module code.