Skip to content

Commit a2ea440

Browse files
committed
HTMLExecutingFunctions: Add further test cases
The extra unit test cases: - consider if the contents of a function was not a string but a function call (and update the inline comments to reflect that); this case was already correctly handled. - consider the case when the object that `appendTo()` (and similar) is being called in is a variable, and not a `$()` call (which we already check to see if it contains a simple string to avoid unnecessary violations) or other function call.
1 parent 4a8e464 commit a2ea440

File tree

3 files changed

+15
-2
lines changed

3 files changed

+15
-2
lines changed

WordPressVIPMinimum/Sniffs/JS/HTMLExecutingFunctionsSniff.php

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ public function process_token( $stackPtr ) {
8888

8989
while ( $nextToken < $parenthesis_closer ) {
9090
$nextToken = $this->phpcsFile->findNext( Tokens::$emptyTokens, $nextToken + 1, null, true, null, true );
91-
if ( T_STRING === $this->tokens[ $nextToken ]['code'] ) { // Contains a variable.
91+
if ( T_STRING === $this->tokens[ $nextToken ]['code'] ) { // Contains a variable, function call or something else dynamic.
9292
$message = 'Any HTML passed to `%s` gets executed. Make sure it\'s properly escaped.';
9393
$data = [ $this->tokens[ $stackPtr ]['content'] ];
9494
$this->phpcsFile->addWarning( $message, $stackPtr, $this->tokens[ $stackPtr ]['content'], $data );
@@ -106,14 +106,21 @@ public function process_token( $stackPtr ) {
106106
$prevPrevToken = $this->phpcsFile->findPrevious( Tokens::$emptyTokens, $prevToken - 1, null, true, null, true );
107107

108108
if ( T_CLOSE_PARENTHESIS !== $this->tokens[ $prevPrevToken ]['code'] ) {
109+
// Not a function call, but may be a variable containing an element reference, so just
110+
// flag all remaining instances of these target HTML executing functions.
111+
$message = 'Any HTML used with `%s` gets executed. Make sure it\'s properly escaped.';
112+
$data = [ $this->tokens[ $stackPtr ]['content'] ];
113+
$this->phpcsFile->addWarning( $message, $stackPtr, $this->tokens[ $stackPtr ]['content'], $data );
114+
109115
return;
110116
}
111117

118+
// Check if it's a function call (typically $() ) that contains a dynamic part.
112119
$parenthesis_opener = $this->tokens[ $prevPrevToken ]['parenthesis_opener'];
113120

114121
while ( $prevPrevToken > $parenthesis_opener ) {
115122
$prevPrevToken = $this->phpcsFile->findPrevious( Tokens::$emptyTokens, $prevPrevToken - 1, null, true, null, true );
116-
if ( T_STRING === $this->tokens[ $prevPrevToken ]['code'] ) { // Contains a variable.
123+
if ( T_STRING === $this->tokens[ $prevPrevToken ]['code'] ) { // Contains a variable, function call or something else dynamic.
117124
$message = 'Any HTML used with `%s` gets executed. Make sure it\'s properly escaped.';
118125
$data = [ $this->tokens[ $stackPtr ]['content'] ];
119126
$this->phpcsFile->addWarning( $message, $stackPtr, $this->tokens[ $stackPtr ]['content'], $data );

WordPressVIPMinimum/Tests/JS/HTMLExecutingFunctionsUnitTest.js

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,3 +42,7 @@
4242
$( variable )
4343
. replaceAll( el ); // Warning.
4444
})();
45+
46+
$( foo_that_contains_script_element() ).appendTo( el ); // Warning.
47+
var $foo = $( '.my-selector' );
48+
$foo.appendTo( el ); // Warning.

WordPressVIPMinimum/Tests/JS/HTMLExecutingFunctionsUnitTest.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,8 @@ public function getWarningList() {
5656
39 => 1,
5757
41 => 1,
5858
43 => 1,
59+
46 => 1,
60+
48 => 1,
5961
];
6062
}
6163

0 commit comments

Comments
 (0)