From 198f9b8da58c1f4407787eebc59973295980aa69 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Tue, 17 Jan 2017 07:08:32 +0100 Subject: [PATCH 1/2] FileSystemWritesDisallow: Use the WPCS abstract function call sniff. This PR contains no significant functional changes. The sniff effectively still does the same. Notes: * The errorcode changes to the groupname instead of being fixed at `FileWriteDetected`. * Split up the functions array into logical groups and alphabetized the lists. * Removed some duplicates from the functions array. --- .../VIP/FileSystemWritesDisallowSniff.php | 124 ++++++++++-------- 1 file changed, 66 insertions(+), 58 deletions(-) diff --git a/WordPress/Sniffs/VIP/FileSystemWritesDisallowSniff.php b/WordPress/Sniffs/VIP/FileSystemWritesDisallowSniff.php index 674a077793..286034a913 100644 --- a/WordPress/Sniffs/VIP/FileSystemWritesDisallowSniff.php +++ b/WordPress/Sniffs/VIP/FileSystemWritesDisallowSniff.php @@ -7,10 +7,6 @@ * @license https://opensource.org/licenses/MIT MIT */ -if ( ! class_exists( 'Generic_Sniffs_PHP_ForbiddenFunctionsSniff', true ) ) { - throw new PHP_CodeSniffer_Exception( 'Class Generic_Sniffs_PHP_ForbiddenFunctionsSniff not found' ); -} - /** * Disallow Filesystem writes. * @@ -19,44 +15,10 @@ * @package WPCS\WordPressCodingStandards * * @since 0.3.0 + * @since 0.11.0 Extends the WordPress_AbstractFunctionRestrictionsSniff instead of the + * Generic_Sniffs_PHP_ForbiddenFunctionsSniff. */ -class WordPress_Sniffs_VIP_FileSystemWritesDisallowSniff extends Generic_Sniffs_PHP_ForbiddenFunctionsSniff { - - /** - * A list of forbidden functions with their alternatives. - * - * The value is NULL if no alternative exists. IE, the - * function should just not be used. - * - * @var array(string => string|null) - */ - public $forbiddenFunctions = array( - 'file_put_contents' => null, - 'fwrite' => null, - 'fputcsv' => null, - 'fputs' => null, - 'ftruncate' => null, - 'link' => null, - 'symlink' => null, - 'mkdir' => null, - 'rename' => null, - 'rmdir' => null, - 'tempnam' => null, - 'touch' => null, - 'unlink' => null, - 'is_writable' => null, - 'is_writeable' => null, - 'lchgrp' => null, - 'lchown' => null, - 'fputcsv' => null, - 'delete' => null, - 'chmod' => null, - 'chown' => null, - 'chgrp' => null, - 'chmod' => null, - 'chmod' => null, - 'flock' => null, - ); +class WordPress_Sniffs_VIP_FileSystemWritesDisallowSniff extends WordPress_AbstractFunctionRestrictionsSniff { /** * If true, an error will be thrown; otherwise a warning. @@ -66,28 +28,74 @@ class WordPress_Sniffs_VIP_FileSystemWritesDisallowSniff extends Generic_Sniffs_ public $error = true; /** - * Generates the error or warning for this sniff. + * Groups of functions to restrict. * - * Overloads parent addError method. + * Example: groups => array( + * 'lambda' => array( + * 'type' => 'error' | 'warning', + * 'message' => 'Use anonymous functions instead please!', + * 'functions' => array( 'eval', 'create_function' ), + * ) + * ) * - * @param PHP_CodeSniffer_File $phpcsFile The file being scanned. - * @param int $stackPtr The position of the forbidden function - * in the token array. - * @param string $function The name of the forbidden function. - * @param string $pattern The pattern used for the match. - * - * @return void + * @return array */ - protected function addError( $phpcsFile, $stackPtr, $function, $pattern = null ) { - $data = array( $function ); - $error = 'Filesystem writes are forbidden, you should not be using %s()'; + public function getGroups() { + $groups = array( + 'file_ops' => array( + 'type' => 'error', + 'message' => 'Filesystem writes are forbidden, you should not be using %s()', + 'functions' => array( + 'delete', + 'file_put_contents', + 'flock', + 'fputcsv', + 'fputs', + 'fwrite', + 'ftruncate', + 'is_writable', + 'is_writeable', + 'link', + 'rename', + 'symlink', + 'tempnam', + 'touch', + 'unlink', + ), + ), + 'directory' => array( + 'type' => 'error', + 'message' => 'Filesystem writes are forbidden, you should not be using %s()', + 'functions' => array( + 'mkdir', + 'rmdir', + ), + ), + 'chmod' => array( + 'type' => 'error', + 'message' => 'Filesystem writes are forbidden, you should not be using %s()', + 'functions' => array( + 'chgrp', + 'chown', + 'chmod', + 'lchgrp', + 'lchown', + ), + ), + ); - if ( true === $this->error ) { - $phpcsFile->addError( $error, $stackPtr, 'FileWriteDetected', $data ); - } else { - $phpcsFile->addWarning( $error, $stackPtr, 'FileWriteDetected', $data ); + /* + * 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; + + } // End getGroups(). } // End class. From cbfbd51bf757b356002c9958bb91f0bef7c3626f Mon Sep 17 00:00:00 2001 From: jrfnl Date: Tue, 17 Jan 2017 07:08:53 +0100 Subject: [PATCH 2/2] FileSystemWritesDisallow: Make sure all functions are unit tested. --- .../VIP/FileSystemWritesDisallowUnitTest.inc | 30 ++++++++++++++++--- .../VIP/FileSystemWritesDisallowUnitTest.php | 19 ++++++++++++ 2 files changed, 45 insertions(+), 4 deletions(-) diff --git a/WordPress/Tests/VIP/FileSystemWritesDisallowUnitTest.inc b/WordPress/Tests/VIP/FileSystemWritesDisallowUnitTest.inc index a04c755839..80fff24073 100644 --- a/WordPress/Tests/VIP/FileSystemWritesDisallowUnitTest.inc +++ b/WordPress/Tests/VIP/FileSystemWritesDisallowUnitTest.inc @@ -1,15 +1,37 @@ $loop ) { - if ( flock( $fp, LOCK_EX ) ) { - fwrite( $fp, $text ); + if ( flock( $fp, LOCK_EX ) ) { // Bad. + fwrite( $fp, $text ); // Bad. } - flock( $fp, LOCK_UN ); + 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(); diff --git a/WordPress/Tests/VIP/FileSystemWritesDisallowUnitTest.php b/WordPress/Tests/VIP/FileSystemWritesDisallowUnitTest.php index 0b13aa5da7..5686a7a27e 100644 --- a/WordPress/Tests/VIP/FileSystemWritesDisallowUnitTest.php +++ b/WordPress/Tests/VIP/FileSystemWritesDisallowUnitTest.php @@ -26,6 +26,25 @@ public function getErrorList() { 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, ); }