From b2356513df8b0090c21d55404b7a78e59c5ad4a7 Mon Sep 17 00:00:00 2001 From: Rebecca Hum Date: Thu, 27 Dec 2018 22:19:06 -0700 Subject: [PATCH 1/2] Move WPCS Deprecated VIP Sniffs to VIPCS Rather than the `WordPressVIPMinimum` and `WordPress-VIP-Go` rulesets relying directly on sniffs in `WordPress.VIP` category, the sniffs have been copied over and adapted for VIPCS. Why? WPCS had marked the `WordPress.VIP` sniffs as deprecated in WPCS 1.0, and removed them in WPCS 2.0. VIPCS still has some dependency on WPCS, via classes extended from WPCS abstract sniff classes, and through the referencing of other `WordPress` sniffs in the VIPCS rulesets. Fixes #187. --- WordPress-VIP-Go/ruleset.xml | 17 +- .../Sniffs/VIP/AdminBarRemovalSniff.php | 403 ++++++++++++++++++ .../Sniffs/VIP/CronIntervalSniff.php | 37 ++ .../Sniffs/VIP/ErrorControlSniff.php | 4 +- .../VIP/EscapingVoidReturnFunctionsSniff.php | 4 +- .../Sniffs/VIP/ExitAfterRedirectSniff.php | 2 +- .../Sniffs/VIP/FetchingRemoteDataSniff.php | 2 +- .../VIP/FileSystemWritesDisallowSniff.php | 102 +++++ .../Sniffs/VIP/MergeConflictSniff.php | 5 +- .../Sniffs/VIP/NoPagingSniff.php | 61 +++ .../Sniffs/VIP/OrderByRandSniff.php | 59 +++ .../VIP/ProperEscapingFunctionSniff.php | 2 +- .../Sniffs/VIP/RestrictedVariablesSniff.php | 65 +++ .../Sniffs/VIP/SessionFunctionsUsageSniff.php | 75 ++++ .../Sniffs/VIP/SessionVariableUsageSniff.php | 53 +++ .../Sniffs/VIP/StaticStrreplaceSniff.php | 2 +- .../Sniffs/VIP/TaxonomyMetaInOptionsSniff.php | 2 +- .../Sniffs/VIP/WPQueryParamsSniff.php | 4 +- .../Tests/VIP/AdminBarRemovalUnitTest.css | 55 +++ .../Tests/VIP/AdminBarRemovalUnitTest.inc | 108 +++++ .../Tests/VIP/AdminBarRemovalUnitTest.php | 100 +++++ .../Tests/VIP/CronIntervalUnitTest.inc | 6 + .../Tests/VIP/CronIntervalUnitTest.php | 41 ++ .../VIP/FileSystemWritesDisallowUnitTest.inc | 49 +++ .../VIP/FileSystemWritesDisallowUnitTest.php | 63 +++ .../Tests/VIP/NoPagingUnitTest.inc | 9 + .../Tests/VIP/NoPagingUnitTest.php | 42 ++ .../Tests/VIP/OrderByRandUnitTest.inc | 18 + .../Tests/VIP/OrderByRandUnitTest.php | 46 ++ .../Tests/VIP/RestrictedVariablesUnitTest.inc | 34 ++ .../Tests/VIP/RestrictedVariablesUnitTest.php | 51 +++ .../VIP/SessionFunctionsUsageUnitTest.inc | 34 ++ .../VIP/SessionFunctionsUsageUnitTest.php | 39 ++ .../VIP/SessionVariableUsageUnitTest.inc | 4 + .../VIP/SessionVariableUsageUnitTest.php | 42 ++ WordPressVIPMinimum/ruleset.xml | 60 +-- 36 files changed, 1623 insertions(+), 77 deletions(-) create mode 100644 WordPressVIPMinimum/Sniffs/VIP/AdminBarRemovalSniff.php create mode 100644 WordPressVIPMinimum/Sniffs/VIP/CronIntervalSniff.php create mode 100644 WordPressVIPMinimum/Sniffs/VIP/FileSystemWritesDisallowSniff.php create mode 100644 WordPressVIPMinimum/Sniffs/VIP/NoPagingSniff.php create mode 100644 WordPressVIPMinimum/Sniffs/VIP/OrderByRandSniff.php create mode 100644 WordPressVIPMinimum/Sniffs/VIP/RestrictedVariablesSniff.php create mode 100644 WordPressVIPMinimum/Sniffs/VIP/SessionFunctionsUsageSniff.php create mode 100644 WordPressVIPMinimum/Sniffs/VIP/SessionVariableUsageSniff.php create mode 100644 WordPressVIPMinimum/Tests/VIP/AdminBarRemovalUnitTest.css create mode 100644 WordPressVIPMinimum/Tests/VIP/AdminBarRemovalUnitTest.inc create mode 100644 WordPressVIPMinimum/Tests/VIP/AdminBarRemovalUnitTest.php create mode 100644 WordPressVIPMinimum/Tests/VIP/CronIntervalUnitTest.inc create mode 100644 WordPressVIPMinimum/Tests/VIP/CronIntervalUnitTest.php create mode 100644 WordPressVIPMinimum/Tests/VIP/FileSystemWritesDisallowUnitTest.inc create mode 100644 WordPressVIPMinimum/Tests/VIP/FileSystemWritesDisallowUnitTest.php create mode 100644 WordPressVIPMinimum/Tests/VIP/NoPagingUnitTest.inc create mode 100644 WordPressVIPMinimum/Tests/VIP/NoPagingUnitTest.php create mode 100644 WordPressVIPMinimum/Tests/VIP/OrderByRandUnitTest.inc create mode 100644 WordPressVIPMinimum/Tests/VIP/OrderByRandUnitTest.php create mode 100644 WordPressVIPMinimum/Tests/VIP/RestrictedVariablesUnitTest.inc create mode 100644 WordPressVIPMinimum/Tests/VIP/RestrictedVariablesUnitTest.php create mode 100644 WordPressVIPMinimum/Tests/VIP/SessionFunctionsUsageUnitTest.inc create mode 100644 WordPressVIPMinimum/Tests/VIP/SessionFunctionsUsageUnitTest.php create mode 100644 WordPressVIPMinimum/Tests/VIP/SessionVariableUsageUnitTest.inc create mode 100644 WordPressVIPMinimum/Tests/VIP/SessionVariableUsageUnitTest.php diff --git a/WordPress-VIP-Go/ruleset.xml b/WordPress-VIP-Go/ruleset.xml index 809d5ab7..fa423255 100644 --- a/WordPress-VIP-Go/ruleset.xml +++ b/WordPress-VIP-Go/ruleset.xml @@ -7,7 +7,7 @@ - + error 6 File system writes only work in /tmp/ and inside the /uploads/ folder on VIP Go. To do filesystem writes you must use the WP_Filesystem class, using functions such as %s() won't work or will return unexpected results. Read more here: https://vip.wordpress.com/documentation/using-wp_filesystem-instead-of-direct-file-access-functions/ @@ -22,12 +22,12 @@ 6 Due to server-side caching, server-side based client related logic might not work. We recommend implementing client side logic in JavaScript instead. - + error 6 Due to server-side caching, server-side based client related logic might not work. We recommend implementing client side logic in JavaScript instead. - + error 6 Due to server-side caching, server-side based client related logic might not work. We recommend implementing client side logic in JavaScript instead. @@ -135,11 +135,6 @@ warning 10 - - - 7 - warning - warning @@ -237,10 +232,10 @@ 1 - + 3 - + 3 @@ -320,7 +315,7 @@ - + 0 diff --git a/WordPressVIPMinimum/Sniffs/VIP/AdminBarRemovalSniff.php b/WordPressVIPMinimum/Sniffs/VIP/AdminBarRemovalSniff.php new file mode 100644 index 00000000..1b9fde22 --- /dev/null +++ b/WordPressVIPMinimum/Sniffs/VIP/AdminBarRemovalSniff.php @@ -0,0 +1,403 @@ + true, + 'add_filter' => true, + ]; + + /** + * CSS properties this sniff is looking for. + * + * @var array + */ + protected $target_css_properties = [ + 'visibility' => [ + 'type' => '!=', + 'value' => 'hidden', + ], + 'display' => [ + 'type' => '!=', + 'value' => 'none', + ], + 'opacity' => [ + 'type' => '>', + 'value' => 0.3, + ], + ]; + + /** + * CSS selectors this sniff is looking for. + * + * @var array + */ + protected $target_css_selectors = [ + '.show-admin-bar', + '#wpadminbar', + ]; + + /** + * String tokens within PHP files we want to deal with. + * + * Set from the register() method. + * + * @var array + */ + private $string_tokens = []; + + /** + * Regex template for use with the CSS selectors in combination with PHP text strings. + * + * @var string + */ + private $target_css_selectors_regex = '`(?:%s).*?\{(.*)$`'; + + /** + * Property to keep track of whether a ' ) ) { + // Make sure we check any content on this line before the closing style tag. + $this->in_style[ $file_name ] = false; + $content = trim( substr( $content, 0, strpos( $content, '' ) ) ); + } + } elseif ( true === $this->has_html_open_tag( 'style', $stackPtr, $content ) ) { + // Ok, found a ' ) ) { + // Make sure we check any content on this line after the opening style tag. + $this->in_style[ $file_name ] = true; + $content = trim( substr( $content, ( strpos( $content, '' ); + $content = trim( substr( $content, $start, ( $end - $start ) ) ); + unset( $start, $end ); + } + } else { + return; + } + + // Are we in one of the target selectors ? + if ( true === $this->in_target_selector[ $file_name ] ) { + if ( false !== strpos( $content, '}' ) ) { + // Make sure we check any content on this line before the selector closing brace. + $this->in_target_selector[ $file_name ] = false; + $content = trim( substr( $content, 0, strpos( $content, '}' ) ) ); + } + } elseif ( preg_match( $this->target_css_selectors_regex, $content, $matches ) > 0 ) { + // Ok, found a new target selector. + $content = ''; + + if ( isset( $matches[1] ) && '' !== $matches[1] ) { + if ( false === strpos( $matches[1], '}' ) ) { + // Make sure we check any content on this line before the closing brace. + $this->in_target_selector[ $file_name ] = true; + $content = trim( $matches[1] ); + } else { + // Ok, we have the selector open and close brace on the same line. + $content = trim( substr( $matches[1], 0, strpos( $matches[1], '}' ) ) ); + } + } else { + $this->in_target_selector[ $file_name ] = true; + } + } else { + return; + } + unset( $matches ); + + // Now let's do the check for the CSS properties. + if ( ! empty( $content ) ) { + foreach ( $this->target_css_properties as $property => $requirements ) { + if ( false !== strpos( $content, $property ) ) { + $error = true; + + if ( true === $this->remove_only ) { + // Check the value of the CSS property. + if ( preg_match( '`' . preg_quote( $property, '`' ) . '\s*:\s*(.+?)\s*(?:!important)?;`', $content, $matches ) > 0 ) { + $value = trim( $matches[1] ); + $valid = $this->validate_css_property_value( $value, $requirements['type'], $requirements['value'] ); + if ( true === $valid ) { + $error = false; + } + } + } + + if ( true === $error ) { + $this->phpcsFile->addError( 'Hiding of the admin bar is not allowed.', $stackPtr, 'HidingDetected' ); + } + } + } + } + } + + /** + * Processes this test for T_STYLE tokens in CSS files. + * + * @param int $stackPtr The position of the current token in the stack passed in $tokens. + * + * @return void + */ + protected function process_css_style( $stackPtr ) { + if ( ! isset( $this->target_css_properties[ $this->tokens[ $stackPtr ]['content'] ] ) ) { + // Not one of the CSS properties we're interested in. + return; + } + + $css_property = $this->target_css_properties[ $this->tokens[ $stackPtr ]['content'] ]; + + // Check if the CSS selector matches. + $opener = $this->phpcsFile->findPrevious( \T_OPEN_CURLY_BRACKET, $stackPtr ); + if ( false !== $opener ) { + for ( $i = ( $opener - 1 ); $i >= 0; $i-- ) { + if ( isset( Tokens::$commentTokens[ $this->tokens[ $i ]['code'] ] ) + || \T_CLOSE_CURLY_BRACKET === $this->tokens[ $i ]['code'] + ) { + break; + } + } + $start = ( $i + 1 ); + $selector = trim( $this->phpcsFile->getTokensAsString( $start, ( $opener - $start ) ) ); + unset( $i ); + + foreach ( $this->target_css_selectors as $target_selector ) { + if ( false !== strpos( $selector, $target_selector ) ) { + $error = true; + + if ( true === $this->remove_only ) { + // Check the value of the CSS property. + $valuePtr = $this->phpcsFile->findNext( [ \T_COLON, \T_WHITESPACE ], ( $stackPtr + 1 ), null, true ); + $value = $this->tokens[ $valuePtr ]['content']; + $valid = $this->validate_css_property_value( $value, $css_property['type'], $css_property['value'] ); + if ( true === $valid ) { + $error = false; + } + } + + if ( true === $error ) { + $this->phpcsFile->addError( 'Hiding of the admin bar is not allowed.', $stackPtr, 'HidingDetected' ); + } + } + } + } + } + + /** + * Verify if a CSS property value complies with an expected value. + * + * {@internal This is a method stub, doing only what is needed for this sniff. + * If at some point in the future other sniff would need similar functionality, + * this method should be moved to the WordPress_Sniff class and expanded to cover + * all types of comparisons.}} + * + * @param mixed $value The value of CSS property. + * @param string $compare_type The type of comparison to use for the validation. + * @param string $compare_value The value to compare against. + * + * @return bool True if the property value complies, false otherwise. + */ + protected function validate_css_property_value( $value, $compare_type, $compare_value ) { + switch ( $compare_type ) { + case '!=': + return $value !== $compare_value; + + case '>': + return $value > $compare_value; + + default: + return false; + } + } + +} diff --git a/WordPressVIPMinimum/Sniffs/VIP/CronIntervalSniff.php b/WordPressVIPMinimum/Sniffs/VIP/CronIntervalSniff.php new file mode 100644 index 00000000..0ecaee53 --- /dev/null +++ b/WordPressVIPMinimum/Sniffs/VIP/CronIntervalSniff.php @@ -0,0 +1,37 @@ + array( + * 'lambda' => array( + * 'type' => 'error' | 'warning', + * 'message' => 'Use anonymous functions instead please!', + * 'functions' => array( 'file_get_contents', 'create_function' ), + * ) + * ) + * + * @return array + */ + public function getGroups() { + $groups = [ + 'file_ops' => [ + 'type' => 'error', + 'message' => 'Filesystem writes are forbidden, you should not be using %s()', + 'functions' => [ + 'delete', + 'file_put_contents', + 'flock', + 'fputcsv', + 'fputs', + 'fwrite', + 'ftruncate', + 'is_writable', + 'is_writeable', + 'link', + 'rename', + 'symlink', + 'tempnam', + 'touch', + 'unlink', + ], + ], + 'directory' => [ + 'type' => 'error', + 'message' => 'Filesystem writes are forbidden, you should not be using %s()', + 'functions' => [ + 'mkdir', + 'rmdir', + ], + ], + 'chmod' => [ + 'type' => 'error', + 'message' => 'Filesystem writes are forbidden, you should not be using %s()', + 'functions' => [ + 'chgrp', + 'chown', + 'chmod', + 'lchgrp', + 'lchown', + ], + ], + ]; + + /* + * Maintain old behaviour - allow for changing the error type from the ruleset + * using the `error` property. + */ + if ( false === $this->error ) { + foreach ( $groups as $group_name => $details ) { + $groups[ $group_name ]['type'] = 'warning'; + } + } + + return $groups; + } + +} diff --git a/WordPressVIPMinimum/Sniffs/VIP/MergeConflictSniff.php b/WordPressVIPMinimum/Sniffs/VIP/MergeConflictSniff.php index bcd7da1f..4b6099cb 100644 --- a/WordPressVIPMinimum/Sniffs/VIP/MergeConflictSniff.php +++ b/WordPressVIPMinimum/Sniffs/VIP/MergeConflictSniff.php @@ -1,8 +1,9 @@ [ + 'type' => 'error', + 'keys' => [ + 'nopaging', + ], + ], + ]; + } + + /** + * Callback to process each confirmed key, to check value. + * + * @param string $key Array index / key. + * @param mixed $val Assigned value. + * @param int $line Token line. + * @param array $group Group definition. + * @return mixed FALSE if no match, TRUE if matches, STRING if matches + * with custom error message passed to ->process(). + */ + public function callback( $key, $val, $line, $group ) { + $key = strtolower( $key ); + + if ( 'nopaging' === $key && ( 'true' === $val || 1 === $val ) ) { + return 'Disabling pagination is prohibited in VIP context, do not set `%s` to `%s` ever.'; + } + + return false; + } + +} diff --git a/WordPressVIPMinimum/Sniffs/VIP/OrderByRandSniff.php b/WordPressVIPMinimum/Sniffs/VIP/OrderByRandSniff.php new file mode 100644 index 00000000..b94ac9fc --- /dev/null +++ b/WordPressVIPMinimum/Sniffs/VIP/OrderByRandSniff.php @@ -0,0 +1,59 @@ + rand. + * + * @link https://vip.wordpress.com/documentation/vip-go/code-review-blockers-warnings-notices/#order-by-rand + * + * @package VIPCS\WordPressVIPMinimum + * + * @since 0.5.0 + */ +class OrderByRandSniff extends AbstractArrayAssignmentRestrictionsSniff { + + /** + * Groups of variables to restrict. + * + * @return array + */ + public function getGroups() { + return [ + 'orderby' => [ + 'type' => 'error', + 'keys' => [ + 'orderby', + ], + ], + ]; + } + + /** + * Callback to process each confirmed key, to check value + * This must be extended to add the logic to check assignment value + * + * @param string $key Array index / key. + * @param mixed $val Assigned value. + * @param int $line Token line. + * @param array $group Group definition. + * @return mixed FALSE if no match, TRUE if matches, STRING if matches with custom error message passed to ->process(). + */ + public function callback( $key, $val, $line, $group ) { + if ( 'rand' === strtolower( $val ) ) { + return 'Detected forbidden query_var "%s" of "%s". Use vip_get_random_posts() instead.'; + } else { + return false; + } + } + +} diff --git a/WordPressVIPMinimum/Sniffs/VIP/ProperEscapingFunctionSniff.php b/WordPressVIPMinimum/Sniffs/VIP/ProperEscapingFunctionSniff.php index 4e324648..8df6b473 100644 --- a/WordPressVIPMinimum/Sniffs/VIP/ProperEscapingFunctionSniff.php +++ b/WordPressVIPMinimum/Sniffs/VIP/ProperEscapingFunctionSniff.php @@ -1,6 +1,6 @@ array( + * 'wpdb' => array( + * 'type' => 'error' | 'warning', + * 'message' => 'Dont use this one please!', + * 'variables' => array( '$val', '$var' ), + * 'object_vars' => array( '$foo->bar', .. ), + * 'array_members' => array( '$foo['bar']', .. ), + * ) + * ) + * + * @return array + */ + public function getGroups() { + return [ + // @link https://lobby.vip.wordpress.com/wordpress-com-documentation/code-review-what-we-look-for/#wp_users-and-user_meta + 'user_meta' => [ + 'type' => 'error', + 'message' => 'Usage of users/usermeta tables is highly discouraged in VIP context, For storing user additional user metadata, you should look at User Attributes.', + 'object_vars' => [ + '$wpdb->users', + '$wpdb->usermeta', + ], + ], + + // @link https://lobby.vip.wordpress.com/wordpress-com-documentation/code-review-what-we-look-for/#caching-constraints + 'cache_constraints' => [ + 'type' => 'warning', + 'message' => 'Due to using Batcache, server side based client related logic will not work, use JS instead.', + 'variables' => [ + '$_COOKIE', + ], + 'array_members' => [ + '$_SERVER[\'HTTP_USER_AGENT\']', + '$_SERVER[\'REMOTE_ADDR\']', + ], + ], + ]; + } + +} diff --git a/WordPressVIPMinimum/Sniffs/VIP/SessionFunctionsUsageSniff.php b/WordPressVIPMinimum/Sniffs/VIP/SessionFunctionsUsageSniff.php new file mode 100644 index 00000000..d5c14262 --- /dev/null +++ b/WordPressVIPMinimum/Sniffs/VIP/SessionFunctionsUsageSniff.php @@ -0,0 +1,75 @@ + array( + * 'lambda' => array( + * 'type' => 'error' | 'warning', + * 'message' => 'Use anonymous functions instead please!', + * 'functions' => array( 'file_get_contents', 'create_function' ), + * ) + * ) + * + * @return array + */ + public function getGroups() { + return [ + 'session' => [ + 'type' => 'error', + 'message' => 'The use of PHP session function %s() is prohibited.', + 'functions' => [ + 'session_abort', + 'session_cache_expire', + 'session_cache_limiter', + 'session_commit', + 'session_create_id', + 'session_decode', + 'session_destroy', + 'session_encode', + 'session_gc', + 'session_get_cookie_params', + 'session_id', + 'session_is_registered', + 'session_module_name', + 'session_name', + 'session_regenerate_id', + 'session_register_shutdown', + 'session_register', + 'session_reset', + 'session_save_path', + 'session_set_cookie_params', + 'session_set_save_handler', + 'session_start', + 'session_status', + 'session_unregister', + 'session_unset', + 'session_write_close', + ], + ], + ]; + } + +} diff --git a/WordPressVIPMinimum/Sniffs/VIP/SessionVariableUsageSniff.php b/WordPressVIPMinimum/Sniffs/VIP/SessionVariableUsageSniff.php new file mode 100644 index 00000000..f8c9b92e --- /dev/null +++ b/WordPressVIPMinimum/Sniffs/VIP/SessionVariableUsageSniff.php @@ -0,0 +1,53 @@ +tokens[ $stackPtr ]['content'] ) { + $this->phpcsFile->addError( + 'Usage of $_SESSION variable is prohibited.', + $stackPtr, + 'SessionVarsProhibited' + ); + } + } +} diff --git a/WordPressVIPMinimum/Sniffs/VIP/StaticStrreplaceSniff.php b/WordPressVIPMinimum/Sniffs/VIP/StaticStrreplaceSniff.php index de8cab40..484e3f8c 100644 --- a/WordPressVIPMinimum/Sniffs/VIP/StaticStrreplaceSniff.php +++ b/WordPressVIPMinimum/Sniffs/VIP/StaticStrreplaceSniff.php @@ -1,6 +1,6 @@ +#wpadminbar { + visibility: hidden; /* Bad. */ + display: none; /* Bad. */ + opacity: 0; /* Bad. */ +} +'; + +// Various text before/after combinations. +echo ''; // Bad. + +echo 'Some text about .show-admin-bar before a style tag '; // Ok. +echo 'Some text about .show-admin-bar after a style tag'; // Ok. + +echo ''; + +// Testing T_DOUBLE_QUOTED_STRING. +echo " +"; + +// Testing T_HEREDOC. +$style = << + .show-admin-bar { + visibility: $hidden; /* Ok, value not 'hidden'. */ + } + +EOT; + +// Testing T_NOWDOC. +$style = <<<'EOT' + +EOT; + +// Testing T_INLINE_HTML +?> + + + + + + + .show-admin-bar { + visibility: $hidden; /* Bad. */ + } + .my-admin-bar { + visibility: $hidden; /* OK. */ + } + +EOT; +?> + + +/* phpcs:set WordPressVIPMinimum.VIP.AdminBarRemoval remove_only true */ diff --git a/WordPressVIPMinimum/Tests/VIP/AdminBarRemovalUnitTest.php b/WordPressVIPMinimum/Tests/VIP/AdminBarRemovalUnitTest.php new file mode 100644 index 00000000..9b983299 --- /dev/null +++ b/WordPressVIPMinimum/Tests/VIP/AdminBarRemovalUnitTest.php @@ -0,0 +1,100 @@ + => + */ + public function getErrorList( $testFile = '' ) { + + switch ( $testFile ) { + case 'AdminBarRemovalUnitTest.inc': + return [ + 3 => 1, + 6 => 1, + 9 => 1, + 12 => 1, + 13 => 1, + 19 => 1, + 20 => 1, + 21 => 1, + 26 => 1, + 32 => 1, + 56 => 1, + 57 => 1, + 58 => 1, + 68 => 1, + 69 => 1, + 70 => 1, + 81 => 1, + 82 => 1, + 83 => 1, + 92 => 1, + 103 => 1, + 104 => 1, + 105 => 1, + ]; + + case 'AdminBarRemovalUnitTest.css': + return [ + 15 => 1, + 16 => 1, + 17 => 1, + 22 => 1, + 23 => 1, + 24 => 1, + 29 => 1, + 30 => 1, + 31 => 1, + 38 => 1, + 39 => 1, + 40 => 1, + 46 => 1, + 47 => 1, + 48 => 1, + ]; + + default: + return []; + } + } + + /** + * Returns the lines where warnings should occur. + * + * @param string $testFile The name of the file being tested. + * @return array => + */ + public function getWarningList( $testFile = '' ) { + switch ( $testFile ) { + case 'AdminBarRemovalUnitTest.css': + return []; + + case 'AdminBarRemovalUnitTest.inc': + return []; + + default: + return []; + } + } +} diff --git a/WordPressVIPMinimum/Tests/VIP/CronIntervalUnitTest.inc b/WordPressVIPMinimum/Tests/VIP/CronIntervalUnitTest.inc new file mode 100644 index 00000000..2bc21c70 --- /dev/null +++ b/WordPressVIPMinimum/Tests/VIP/CronIntervalUnitTest.inc @@ -0,0 +1,6 @@ + => + */ + public function getErrorList() { + return []; + } + + /** + * Returns the lines where warnings should occur. + * + * @return array => + */ + public function getWarningList() { + return [ + 4 => 1, + ]; + } + +} diff --git a/WordPressVIPMinimum/Tests/VIP/FileSystemWritesDisallowUnitTest.inc b/WordPressVIPMinimum/Tests/VIP/FileSystemWritesDisallowUnitTest.inc new file mode 100644 index 00000000..9082baac --- /dev/null +++ b/WordPressVIPMinimum/Tests/VIP/FileSystemWritesDisallowUnitTest.inc @@ -0,0 +1,49 @@ + $loop ) { + if ( flock( $fp, LOCK_EX ) ) { // Bad. + fwrite( $fp, $text ); // Bad. + } + flock( $fp, LOCK_UN ); // Bad. +} + +fclose( $fp ); + +delete(); +fputcsv(); +fputs(); +ftruncate(); +is_writable(); +is_writeable(); +link(); +rename(); +symlink(); +tempnam(); +touch(); +unlink(); + +mkdir(); +rmdir(); + +chgrp(); +chown(); +chmod(); +lchgrp(); +lchown(); + +// phpcs:set WordPressVIPMinimum.VIP.FileSystemWritesDisallow exclude directory +mkdir(); + +// phpcs:set WordPressVIPMinimum.VIP.FileSystemWritesDisallow exclude false + + +// Issue #956. +use Fieldmanager_Link as Link; + +namespace Fieldmanager\Link\Something; +namespace Fieldmanager\Link; diff --git a/WordPressVIPMinimum/Tests/VIP/FileSystemWritesDisallowUnitTest.php b/WordPressVIPMinimum/Tests/VIP/FileSystemWritesDisallowUnitTest.php new file mode 100644 index 00000000..a1f78022 --- /dev/null +++ b/WordPressVIPMinimum/Tests/VIP/FileSystemWritesDisallowUnitTest.php @@ -0,0 +1,63 @@ + => + */ + public function getErrorList() { + return [ + 3 => 1, + 9 => 1, + 10 => 1, + 12 => 1, + 17 => 1, + 18 => 1, + 19 => 1, + 20 => 1, + 21 => 1, + 22 => 1, + 23 => 1, + 24 => 1, + 25 => 1, + 26 => 1, + 27 => 1, + 28 => 1, + 30 => 1, + 31 => 1, + 33 => 1, + 34 => 1, + 35 => 1, + 36 => 1, + 37 => 1, + ]; + } + + /** + * Returns the lines where warnings should occur. + * + * @return array => + */ + public function getWarningList() { + return []; + } + +} diff --git a/WordPressVIPMinimum/Tests/VIP/NoPagingUnitTest.inc b/WordPressVIPMinimum/Tests/VIP/NoPagingUnitTest.inc new file mode 100644 index 00000000..87ec19c2 --- /dev/null +++ b/WordPressVIPMinimum/Tests/VIP/NoPagingUnitTest.inc @@ -0,0 +1,9 @@ + true, // Bad. +); + +_query_posts( 'nopaging=true' ); // Bad. + +$query_args['my_posts_per_page'] = -1; // OK. diff --git a/WordPressVIPMinimum/Tests/VIP/NoPagingUnitTest.php b/WordPressVIPMinimum/Tests/VIP/NoPagingUnitTest.php new file mode 100644 index 00000000..cd735a43 --- /dev/null +++ b/WordPressVIPMinimum/Tests/VIP/NoPagingUnitTest.php @@ -0,0 +1,42 @@ + => + */ + public function getErrorList() { + return [ + 4 => 1, + 7 => 1, + ]; + } + + /** + * Returns the lines where warnings should occur. + * + * @return array => + */ + public function getWarningList() { + return []; + } + +} diff --git a/WordPressVIPMinimum/Tests/VIP/OrderByRandUnitTest.inc b/WordPressVIPMinimum/Tests/VIP/OrderByRandUnitTest.inc new file mode 100644 index 00000000..5ff651ce --- /dev/null +++ b/WordPressVIPMinimum/Tests/VIP/OrderByRandUnitTest.inc @@ -0,0 +1,18 @@ + 'rand', // Bad. + "orderby" => "rand", // Bad. + "orderby" => "RAND", // Bad. +); + +_query_posts( 'orderby=rand' ); // Bad. + +$query_args['orderby'] = 'rand'; // Bad. + +$query_args['orderby'] = 'date'; // Ok. + +// phpcs:set WordPress.VIP.OrderByRand exclude something +$query_args['orderby'] = 'rand'; // Bad. + +// phpcs:set WordPress.VIP.OrderByRand exclude false diff --git a/WordPressVIPMinimum/Tests/VIP/OrderByRandUnitTest.php b/WordPressVIPMinimum/Tests/VIP/OrderByRandUnitTest.php new file mode 100644 index 00000000..7f051923 --- /dev/null +++ b/WordPressVIPMinimum/Tests/VIP/OrderByRandUnitTest.php @@ -0,0 +1,46 @@ + => + */ + public function getErrorList() { + return [ + 4 => 1, + 5 => 1, + 6 => 1, + 9 => 1, + 11 => 1, + 16 => 1, + ]; + } + + /** + * Returns the lines where warnings should occur. + * + * @return array => + */ + public function getWarningList() { + return []; + } + +} diff --git a/WordPressVIPMinimum/Tests/VIP/RestrictedVariablesUnitTest.inc b/WordPressVIPMinimum/Tests/VIP/RestrictedVariablesUnitTest.inc new file mode 100644 index 00000000..8193a4a1 --- /dev/null +++ b/WordPressVIPMinimum/Tests/VIP/RestrictedVariablesUnitTest.inc @@ -0,0 +1,34 @@ +users"; // Error. + +$wp_db->update( $wpdb->users, array( 'displayname' => 'Kanobe!' ), array( 'ID' => 1 ) ); // Error. + +$query = "SELECT * FROM $wpdb->usermeta"; // Error. + +$wp_db->update( $wpdb->usermeta, array( 'meta_value' => 'bar!' ), array( 'user_id' => 1, 'meta_key' => 'foo' ) ); // Error. + +$query = "SELECT * FROM $wpdb->posts"; // Ok. + +if ( isset( $_SERVER['REMOTE_ADDR'] ) ) { // Warning. + foo( $_SERVER['HTTP_USER_AGENT'] ); // Warning. +} + +$x = $_COOKIE['bar']; // Warning. + +$y = $_SERVER['REQUEST_URI']; // Ok. + +// Error. +$query = <<usermeta +EOD; + +// Warning +$phrase = << => + */ + public function getErrorList() { + return [ + 3 => 1, + 5 => 1, + 7 => 1, + 9 => 1, + 23 => 1, + ]; + } + + /** + * Returns the lines where warnings should occur. + * + * @return array => + */ + public function getWarningList() { + return [ + 13 => 1, + 14 => 1, + 17 => 1, + 28 => 1, + ]; + } + +} diff --git a/WordPressVIPMinimum/Tests/VIP/SessionFunctionsUsageUnitTest.inc b/WordPressVIPMinimum/Tests/VIP/SessionFunctionsUsageUnitTest.inc new file mode 100644 index 00000000..4a1f02d5 --- /dev/null +++ b/WordPressVIPMinimum/Tests/VIP/SessionFunctionsUsageUnitTest.inc @@ -0,0 +1,34 @@ + => + */ + public function getErrorList() { + return array_fill( 3, 26, 1 ); + } + + /** + * Returns the lines where warnings should occur. + * + * @return array => + */ + public function getWarningList() { + return []; + } + +} diff --git a/WordPressVIPMinimum/Tests/VIP/SessionVariableUsageUnitTest.inc b/WordPressVIPMinimum/Tests/VIP/SessionVariableUsageUnitTest.inc new file mode 100644 index 00000000..d635ed98 --- /dev/null +++ b/WordPressVIPMinimum/Tests/VIP/SessionVariableUsageUnitTest.inc @@ -0,0 +1,4 @@ + => + */ + public function getErrorList() { + return [ + 3 => 1, + 4 => 1, + ]; + } + + /** + * Returns the lines where warnings should occur. + * + * @return array => + */ + public function getWarningList() { + return []; + } + +} diff --git a/WordPressVIPMinimum/ruleset.xml b/WordPressVIPMinimum/ruleset.xml index c85f5bb4..4732a279 100644 --- a/WordPressVIPMinimum/ruleset.xml +++ b/WordPressVIPMinimum/ruleset.xml @@ -14,16 +14,18 @@ *.twig - - - - - + + + + - + + + + @@ -132,52 +134,6 @@ `%1$s()` performs a no-LIMIT query by default, make sure to set a reasonable `posts_per_page`. `%1$s()` will do a -1 query by default, a maximum of 100 should be used. - - - - - 0 - - - 0 - - - 0 - - - 0 - - - 0 - - - 0 - - - 0 - - - 0 - - - 0 - - - 0 - - - 0 - - - 0 - - - 0 - - - 0 - - From 20f48381db35fa37d6864eb3905ccae0bcc11660 Mon Sep 17 00:00:00 2001 From: Rebecca Hum Date: Wed, 2 Jan 2019 14:38:30 -0700 Subject: [PATCH 2/2] Integration test: Refresh for removal of sniffs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `SuperGlobalInputUsageSniff` was not carried over from `WordPress.VIP`, so this section removed. `FlushRewriteRulesSniff` was removed from `WordPressVIPMinimum`, so testable code now moved to RestrictedFunctions group. Also clarifies whether lines should cause Errors, Warnings or Messages, instead of just “Bad.”. See #309. See #187. --- ruleset_test.inc | 108 +++++++++++++++++++++++------------------------ ruleset_test.php | 93 ++++++++++++++++++++-------------------- 2 files changed, 99 insertions(+), 102 deletions(-) diff --git a/ruleset_test.inc b/ruleset_test.inc index 93098df7..8861dc38 100644 --- a/ruleset_test.inc +++ b/ruleset_test.inc @@ -1,31 +1,30 @@ - + query( "SELECT * FROM $wpdb->posts WHERE post_title LIKE '" . $_GET['title'] . "';" ); // Bad. Error + Warning. +$wpdb->query( "SELECT * FROM $wpdb->posts WHERE post_title LIKE '" . $_GET['title'] . "';" ); // Error + Warning. // WordPress.Variables.GlobalVariables function foo() { - global $page; // WordPressVIPMinimum.Variables.VariableAnalysis.UnusedVariable // Bad. Warning. - $page = get_post( $post_id ); // Bad. // WordPressVIPMinimum.Variables.VariableAnalysis.UndefinedVariable Bad. Warning. + global $page; // WordPressVIPMinimum.Variables.VariableAnalysis.UnusedVariable // Warning. + $page = get_post( $post_id ); // WordPressVIPMinimum.Variables.VariableAnalysis.UndefinedVariable // Error + Warning. } // WordPress.PHP.StrictComparisons -if ( true == $true ) { // Bad. Warning. +if ( true == $true ) { // Warning. } // WordPress.PHP.YodaConditions @@ -33,87 +32,82 @@ if ( $true === true ) { // Ok. } // WordPress.PHP.StrictInArray -if ( true === in_array( $foo, $bar ) ) { // Bad. Warning. +if ( true === in_array( $foo, $bar ) ) { // Warning. } // WordPress.Functions.DontExtract -extract( $foobar ); // Bad. +extract( $foobar ); // Error. // WordPress.PHP.DevelopmentFunctions -error_log( 'Hey there!' ); // Bad. Error. -var_export( $foo ); // Bad. Warning. -var_dump( $bar ); // Bad. +error_log( 'Hey there!' ); // Error. +var_export( $foo ); // Warning. +var_dump( $bar ); // Warning. // WordPress.PHP.DiscouragedPHPFunctions -error_reporting(); // Bad. -ini_set(); // Bad. +error_reporting(); // Error. +ini_set(); // Error. // Generic.NamingConventions.ConstructorName class TestClass extends MyClass { - function TestClass() { // Bad. - parent::MyClass(); // Bad. + function TestClass() { // Error. + parent::MyClass(); // Error. parent::__construct(); } } ?> - + is_search() ) { - $wp_query->set( 'cat', '-5' ); // Bad. Warning. + $wp_query->set( 'cat', '-5' ); // Warning. } } ); // WordPressVIPMinimum.Cache.CacheValueOverride $bad_wp_users = wp_cache_get( md5( self::CACHE_KEY . '_wp_users'), self::CACHE_GROUP ); -$bad_wp_users = false; // Bad. +$bad_wp_users = false; // Error. // WordPressVIPMinimum.Classes.DeclarationCompatibility class MyWidget extends WP_Widget { function widget() { - } // Bad, missing $args and $instance params. + } // Error (line above), missing $args and $instance params. } // WordPressVIPMinimum.Constants.ConstantRestictions -if ( A8C_PROXIED_REQUEST === true ) { // Bad. Warning. +if ( A8C_PROXIED_REQUEST === true ) { // Warning. } // WordPressVIPMinimum.Files.IncludingFile -require_once "my_file.php"; // NOK. +require_once "my_file.php"; // Error. // WordPressVIPMinimum.Functions.CheckReturnValue $my_theme_options = get_option( 'my_theme', false ); -if ( array_key_exists( 'key', $my_theme_options ) ) { // Bad. +if ( array_key_exists( 'key', $my_theme_options ) ) { // Error. } // WordPressVIPMinimum.VIP.ErrorControl + WordPressVIPMinimum.VIP.RestrictedFunctions.file_get_contents_file_get_contents @file_get_contents( $foo ); // Error + Warning. -// WordPressVIPMinimum.VIP.FlushRewriteRules -global $wp_rewrite; -$wp_rewrite->flush_rules(); // Bad. -flush_rewrite_rules(); // Bad. - // WordPressVIPMinimum.VIP.RegexpCompare + WordPress.VIP.SlowDBQuery $query_args = array( 'posts_per_page' => 1, 'post_status' => 'draft', - 'meta_key' => 'my_awesome_meta_key', // Bad. WordPress.VIP.SlowDBQuery. Warning. - 'meta_value' => "(^|\n|\r\n)99999($|\n|\r\n)", // Bad. WordPress.VIP.SlowDBQuery. Warning. - 'meta_compare' => 'REGEXP', // Bad. + 'meta_key' => 'my_awesome_meta_key', // WordPress.VIP.SlowDBQuery. // Warning. + 'meta_value' => "(^|\n|\r\n)99999($|\n|\r\n)", // WordPress.VIP.SlowDBQuery. // Warning. + 'meta_compare' => 'REGEXP', // Error. ); // WordPressVIPMinimum.VIP.RemoteRequestTimeout wp_remote_post( $this->endpoint, array( 'method' => 'POST', - 'timeout' => 45, // Bad. + 'timeout' => 45, // Error. 'httpversion' => '1.1', 'blocking' => false, 'body' => wp_json_encode( $this->logs, JSON_UNESCAPED_SLASHES ), @@ -121,22 +115,22 @@ wp_remote_post( $this->endpoint, array( ); // Squiz.PHP.Eval -eval( ';' ); // Bad. Error. +eval( ';' ); // Error. // WordPressVIPMinimum.VIP.RestrictedFunctions -wpcom_vip_irc(); // Bad. Error. +wpcom_vip_irc(); // Error. -get_children(); // Bad. Warning. Message. +get_children(); // Error + Message. -attachment_url_to_postid(); // Bad. Error. +attachment_url_to_postid(); // Error. -str_replace( 'foo', 'bar', 'foobar' ); // Bad. Error. +str_replace( 'foo', 'bar', 'foobar' ); // Error. -wpcom_vip_get_term_link(); // Bad. Warning. +wpcom_vip_get_term_link(); // Warning. -wpcom_vip_get_term_by(); // Bad. Warning. +wpcom_vip_get_term_by(); // Warning. -wpcom_vip_get_category_by_slug(); // Bad. Warning. +wpcom_vip_get_category_by_slug(); // Warning. get_cat_ID(); // Ok. @@ -150,31 +144,35 @@ get_term_by(); // Ok. get_term_link(); // Ok. -wp_mail(); // Bad. Warning. +wp_mail(); // Warning. -mail(); // Bad. Warning. +mail(); // Warning. -dbDelta(); // Bad. Error. +dbDelta(); // Error. + +flush_rewrite_rules(); // Error. +global $wp_rewrite; +$wp_rewrite->flush_rules(); // Error. // WordPress.CodeAnalysis.AssignmentInCondition. -if ( $a = 1 ) {} // Bad. Warning. +if ( $a = 1 ) {} // Warning. -add_option( 'taxonomy_rating_' . $obj->term_id ); // Bad. Warning. +add_option( 'taxonomy_rating_' . $obj->term_id ); // Warning. -//wpcom_vip_load_plugin( 'disqus' ); // Bad. Warning. +//wpcom_vip_load_plugin( 'disqus' ); // Warning. -echo ""; // NOK. Error. +echo ""; // Error. -$hello = true === isset( $_GET['utm_medium'] ) ? true : false; // NOK. Warning 3 times. +$hello = true === isset( $_GET['utm_medium'] ) ? true : false; // Warning. -wp_safe_redirect( 'https.//vip.wordpress.com' ); // NOK. Error. +wp_safe_redirect( 'https.//vip.wordpress.com' ); // Error. -is_multi_author(); // NOK. Warning. +is_multi_author(); // Warning. -include( 'non-php-file.svg' ); // NOK. Error. Including non-php file. +include( 'non-php-file.svg' ); // Error. Including non-php file. -echo file_get_contents( 'non-php-file.svg' ); // XSS OK. Preferred way of including SVG/CSS file. WP_Filesystem Warning. +echo file_get_contents( 'non-php-file.svg' ); // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped -- Preferred way of including SVG/CSS file. -thisisasyntaxerror! // Bad. Error. +thisisasyntaxerror! // Error. ?> diff --git a/ruleset_test.php b/ruleset_test.php index 693aa7bb..e0fae398 100644 --- a/ruleset_test.php +++ b/ruleset_test.php @@ -21,65 +21,64 @@ $expected = [ 'errors' => [ 4 => 1, - 9 => 1, - 16 => 1, - 19 => 1, - 24 => 1, - 40 => 1, - 43 => 1, + 8 => 1, + 15 => 1, + 18 => 1, + 23 => 1, + 39 => 1, + 42 => 1, + 47 => 1, 48 => 1, - 49 => 1, + 52 => 1, 53 => 1, - 54 => 1, - 61 => 1, - 76 => 1, - 80 => 1, - 89 => 1, - 93 => 1, - 97 => 1, - 101 => 1, - 102 => 1, + 60 => 1, + 75 => 1, + 79 => 1, + 88 => 1, + 92 => 1, + 96 => 1, + 104 => 1, 110 => 1, - 116 => 1, - 124 => 1, + 118 => 1, + 121 => 1, + 123 => 1, + 125 => 1, 127 => 1, - 129 => 1, - 131 => 1, - 133 => 1, - 157 => 1, - 166 => 1, - 170 => 1, - 174 => 1, - 178 => 1, + 151 => 1, + 153 => 1, + 155 => 1, + 164 => 1, + 168 => 1, + 172 => 1, + 176 => 1, ], 'warnings' => [ - 9 => 1, - 19 => 1, + 18 => 1, + 22 => 1, 23 => 1, - 24 => 1, - 28 => 1, - 36 => 1, + 27 => 1, + 35 => 1, + 43 => 1, 44 => 1, - 45 => 1, - 65 => 1, - 70 => 1, - 85 => 1, - 97 => 1, - 108 => 1, - 109 => 1, - 135 => 1, - 137 => 1, - 139 => 1, - 153 => 1, - 155 => 1, + 64 => 1, + 69 => 1, + 84 => 1, + 96 => 1, + 102 => 1, + 103 => 1, + 129 => 1, + 131 => 1, + 133 => 1, + 147 => 1, + 149 => 1, + 158 => 1, 160 => 1, 162 => 1, - 164 => 1, - 168 => 1, - 172 => 1, + 166 => 1, + 170 => 1, ], 'messages' => [ - 129 => [ + 123 => [ '`get_children()` performs a no-LIMIT query by default, make sure to set a reasonable `posts_per_page`. `get_children()` will do a -1 query by default, a maximum of 100 should be used.', ], ],