plugin-check icon indicating copy to clipboard operation
plugin-check copied to clipboard

Localhost check does not show errors in multiple files

Open ernilambar opened this issue 1 year ago • 1 comments

I tested with keeping http://localhost:8888 in two PHP files. And in the check, error is shown for only file.

/includes/Checker/Checks/Localhost_Check.php

Line 48:

$file = self::file_preg_match( '#https?://(localhost|127.0.0.1)#', $php_files );

This seems to be happening because self::file_preg_match() only returns the first match.

ernilambar avatar Dec 28 '23 06:12 ernilambar

This is not only an issue with the localhost check, no? Every check using self::file_preg_match() or self::file_str_contains() currently only return the first matched file. It would make more sense to address this holistically instead of only the localhost check.

On one hand, it might be desirable to return all found results, on the other hand it's reasonable to only return the first found result as otherwise it can be very repetitive (for example if you use a code obfuscation tool, Code_Obfuscation_Check would flag all your files). The downside is that once you fix the file, the error message pops up again for the next file, and so on.

Let's think about how best to address this. Some possibilities:

  1. Do nothing, leave as is
  2. Make file_preg_match() and file_str_contains() always return all files and then leave it up to the consumer to process all results or just the first one. Downside: very expensive, so doesn't really make sense.
  3. Introduce new methods like in #399, but look at which checks would really need this.

swissspidy avatar Jan 23 '24 10:01 swissspidy

I'd be up to have both as those might be useful for different checks. The files_preg_match done on #399 looks great and would be amazing to have maybe a files_preg_match_all ? that returns file + line + column

frantorres avatar Jun 22 '24 14:06 frantorres