Skip to content

Commit 4a8e464

Browse files
GaryJonesGaryJones
committed
HTMLExecutingFunctions: Add more functions
There are different ways to insert variables into the DOM, and our sniffs only covered some of them. The sniff now has an expanded list, which covers functions that have a syntax where the content is in the function arg, and also where the function arg is the target and the content is in the preceding method's arg. Fixes #435.
1 parent fe60d9f commit 4a8e464

File tree

3 files changed

+116
-27
lines changed

3 files changed

+116
-27
lines changed

‎WordPressVIPMinimum/Sniffs/JS/HTMLExecutingFunctionsSniff.php‎

Lines changed: 57 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -22,13 +22,26 @@ class HTMLExecutingFunctionsSniff extends Sniff {
2222
/**
2323
* List of HTML executing functions.
2424
*
25+
* Name of function => content or target.
26+
* Value indicates whether the function's arg is the content to be inserted, or the target where the inserted
27+
* content is to be inserted before/after/replaced. For the latter, the content is in the preceding method's arg.
28+
*
2529
* @var array
2630
*/
2731
public $HTMLExecutingFunctions = [
28-
'html',
29-
'append',
30-
'write',
31-
'writeln',
32+
'after' => 'content', // jQuery.
33+
'append' => 'content', // jQuery.
34+
'appendTo' => 'target', // jQuery.
35+
'before' => 'content', // jQuery.
36+
'html' => 'content', // jQuery.
37+
'insertAfter' => 'target', // jQuery.
38+
'insertBefore' => 'target', // jQuery.
39+
'prepend' => 'content', // jQuery.
40+
'prependTo' => 'target', // jQuery.
41+
'replaceAll' => 'target', // jQuery.
42+
'replaceWith' => 'content', // jQuery.
43+
'write' => 'content',
44+
'writeln' => 'content',
3245
];
3346

3447
/**
@@ -58,29 +71,56 @@ public function register() {
5871
*/
5972
public function process_token( $stackPtr ) {
6073

61-
if ( false === in_array( $this->tokens[ $stackPtr ]['content'], $this->HTMLExecutingFunctions, true ) ) {
74+
if ( ! isset( $this->HTMLExecutingFunctions[ $this->tokens[ $stackPtr ]['content'] ] ) ) {
6275
// Looking for specific functions only.
6376
return;
6477
}
6578

66-
$nextToken = $this->phpcsFile->findNext( Tokens::$emptyTokens, $stackPtr + 1, null, true, null, true );
79+
if ( 'content' === $this->HTMLExecutingFunctions[ $this->tokens[ $stackPtr ]['content'] ] ) {
80+
$nextToken = $this->phpcsFile->findNext( Tokens::$emptyTokens, $stackPtr + 1, null, true, null, true );
6781

68-
if ( T_OPEN_PARENTHESIS !== $this->tokens[ $nextToken ]['code'] ) {
69-
// Not a function.
70-
return;
71-
}
82+
if ( T_OPEN_PARENTHESIS !== $this->tokens[ $nextToken ]['code'] ) {
83+
// Not a function.
84+
return;
85+
}
86+
87+
$parenthesis_closer = $this->tokens[ $nextToken ]['parenthesis_closer'];
7288

73-
$parenthesis_closer = $this->tokens[ $nextToken ]['parenthesis_closer'];
89+
while ( $nextToken < $parenthesis_closer ) {
90+
$nextToken = $this->phpcsFile->findNext( Tokens::$emptyTokens, $nextToken + 1, null, true, null, true );
91+
if ( T_STRING === $this->tokens[ $nextToken ]['code'] ) { // Contains a variable.
92+
$message = 'Any HTML passed to `%s` gets executed. Make sure it\'s properly escaped.';
93+
$data = [ $this->tokens[ $stackPtr ]['content'] ];
94+
$this->phpcsFile->addWarning( $message, $stackPtr, $this->tokens[ $stackPtr ]['content'], $data );
7495

75-
while ( $nextToken < $parenthesis_closer ) {
76-
$nextToken = $this->phpcsFile->findNext( Tokens::$emptyTokens, $nextToken + 1, null, true, null, true );
77-
if ( T_STRING === $this->tokens[ $nextToken ]['code'] ) {
78-
$message = 'Any HTML passed to `%s` gets executed. Make sure it\'s properly escaped.';
79-
$data = [ $this->tokens[ $stackPtr ]['content'] ];
80-
$this->phpcsFile->addWarning( $message, $stackPtr, $this->tokens[ $stackPtr ]['content'], $data );
96+
return;
97+
}
98+
}
99+
} elseif ( 'target' === $this->HTMLExecutingFunctions[ $this->tokens[ $stackPtr ]['content'] ] ) {
100+
$prevToken = $this->phpcsFile->findPrevious( Tokens::$emptyTokens, $stackPtr - 1, null, true, null, true );
81101

102+
if ( T_OBJECT_OPERATOR !== $this->tokens[ $prevToken ]['code'] ) {
82103
return;
83104
}
105+
106+
$prevPrevToken = $this->phpcsFile->findPrevious( Tokens::$emptyTokens, $prevToken - 1, null, true, null, true );
107+
108+
if ( T_CLOSE_PARENTHESIS !== $this->tokens[ $prevPrevToken ]['code'] ) {
109+
return;
110+
}
111+
112+
$parenthesis_opener = $this->tokens[ $prevPrevToken ]['parenthesis_opener'];
113+
114+
while ( $prevPrevToken > $parenthesis_opener ) {
115+
$prevPrevToken = $this->phpcsFile->findPrevious( Tokens::$emptyTokens, $prevPrevToken - 1, null, true, null, true );
116+
if ( T_STRING === $this->tokens[ $prevPrevToken ]['code'] ) { // Contains a variable.
117+
$message = 'Any HTML used with `%s` gets executed. Make sure it\'s properly escaped.';
118+
$data = [ $this->tokens[ $stackPtr ]['content'] ];
119+
$this->phpcsFile->addWarning( $message, $stackPtr, $this->tokens[ $stackPtr ]['content'], $data );
120+
121+
return;
122+
}
123+
}
84124
}
85125
}
86126

‎WordPressVIPMinimum/Tests/JS/HTMLExecutingFunctionsUnitTest.js‎

Lines changed: 40 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,44 @@
11
(function(){
22
var el = document.querySelector(".myclass");
3-
el.html(''); // OK.
4-
el.html('<b>Hand written HTML</b>'); // OK.
5-
el.html( '<b>' + variable + '</b>' ); // NOK.
6-
el.html( variable ); // NOK.
7-
el.append( variable ); // NOK.
3+
el.after(''); // OK.
4+
el.after('<b>Hand written HTML</b>'); // OK.
5+
el.after( '<b>' + variable + '</b>' ); // Warning.
6+
el.after( variable ); // Warning.
87
el.append( '<b>Hand written HTML</b>' ); // OK.
9-
el.append( '<b>' + variable + '</b>' ); // NOK.
8+
el.append( '<b>' + variable + '</b>' ); // Warning.
9+
el.append( variable ); // Warning.
10+
el.before('<b>Hand written HTML</b>'); // OK.
11+
el.before( '<b>' + variable + '</b>' ); // Warning.
12+
el.before( variable ); // Warning.
13+
el.html('<b>Hand written HTML</b>'); // OK.
14+
el.html( '<b>' + variable + '</b>' ); // Warning.
15+
el.html( variable ); // Warning.
16+
el.prepend('<b>Hand written HTML</b>'); // OK.
17+
el.prepend( '<b>' + variable + '</b>' ); // Warning.
18+
el.prepend( variable ); // Warning.
19+
el.replaceWith('<b>Hand written HTML</b>'); // OK.
20+
el.replaceWith( '<b>' + variable + '</b>' ); // Warning.
21+
el.replaceWith( variable ); // Warning.
1022
document.write( '<script>console.log("hello")</script>' ); // OK. No variable, conscious.
11-
document.write( hello ); // NOK.
12-
document.writeln( hey ); // NOK.
13-
})();
23+
document.write( hello ); // Warning.
24+
document
25+
. writeln( hey ); // Warning.
26+
27+
$('').appendTo(el); // OK.
28+
$('<b>Hand written HTML</b>').appendTo( el ); // OK.
29+
$( '<b>' + variable + '</b>' ).appendTo( el ); // Warning.
30+
$( variable ).appendTo( el ); // Warning.
31+
$('<b>Hand written HTML</b>').insertAfter( el ); // OK.
32+
$( '<b>' + variable + '</b>' ).insertAfter( el ); // Warning.
33+
$( variable ).insertAfter( el ); // Warning.
34+
$('<b>Hand written HTML</b>').insertBefore( el ); // OK.
35+
$( '<b>' + variable + '</b>' ).insertBefore( el ); // Warning.
36+
$( variable ).insertBefore( el ); // Warning.
37+
$('<b>Hand written HTML</b>').prependTo( el ); // OK.
38+
$( '<b>' + variable + '</b>' ).prependTo( el ); // Warning.
39+
$( variable ).prependTo( el ); // Warning.
40+
$('<b>Hand written HTML</b>').replaceAll( el ); // OK.
41+
$( '<b>' + variable + '</b>' ).replaceAll( el ); // Warning.
42+
$( variable )
43+
. replaceAll( el ); // Warning.
44+
})();

‎WordPressVIPMinimum/Tests/JS/HTMLExecutingFunctionsUnitTest.php‎

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,10 +34,28 @@ public function getWarningList() {
3434
return [
3535
5 => 1,
3636
6 => 1,
37-
7 => 1,
37+
8 => 1,
3838
9 => 1,
3939
11 => 1,
4040
12 => 1,
41+
14 => 1,
42+
15 => 1,
43+
17 => 1,
44+
18 => 1,
45+
20 => 1,
46+
21 => 1,
47+
23 => 1,
48+
25 => 1,
49+
29 => 1,
50+
30 => 1,
51+
32 => 1,
52+
33 => 1,
53+
35 => 1,
54+
36 => 1,
55+
38 => 1,
56+
39 => 1,
57+
41 => 1,
58+
43 => 1,
4159
];
4260
}
4361

0 commit comments

Comments
 (0)