IncludingFile: Add custom keywords to detect to reduce false positives#626
IncludingFile: Add custom keywords to detect to reduce false positives#626rebeccahum wants to merge 2 commits into
Conversation
jrfnl
left a comment
There was a problem hiding this comment.
Hi Rebecca,
I've been looking at this change and notice I find it difficult to review it as there is so much more which needs improving in this sniff, that it is hard to focus on this specific change.
This might actually be a good sniff to take the time to dissect in detail in a live coding call and talk through every code inefficiency and assumption in the code which should be fixed.
For now, specifically for this PR, let's start with some questions (left in-line)...
| return; | ||
| } | ||
|
|
||
| if ( $this->tokens[ $nextToken ]['code'] === T_CONSTANT_ENCAPSED_STRING && filter_var( str_replace( [ '"', "'" ], '', $this->tokens[ $nextToken ]['content'] ), FILTER_VALIDATE_URL ) ) { |
There was a problem hiding this comment.
Why did you move this check up ?
There was a problem hiding this comment.
I moved it up so that it doesn't bail before checking for an external URL with a keyword from $customPaths in it. e.g.:
include( "http://path.com/bad_file.php" );
4a22eca to
fc3d048
Compare
5a77ddc to
4b141dd
Compare
|
Closing for now as per our discussion on making this an atomic commit for the next release and then doing more re-factoring for 3.0. |
Fixes #407.
Takes into account for URLs and $restrictedConstants with keywords from $customPaths.