Merge pull request #128 from xwp/bugfix/phpcs

Fix phpcs issues in WordPress-VIP ruleset
xwp · Mar 3, 2017 · 973480f · 973480f
2 parents 5e15e85 + 5328a8d
commit 973480f
Show file tree

Hide file tree

Showing 9 changed files with 110 additions and 65 deletions.
diff --git a/php/class-customize-snapshot-back-compat.php b/php/class-customize-snapshot-back-compat.php
@@ -226,15 +226,19 @@ function( $sanitized ) {
 				'unauthorized_settings',
 				/* translators: %s is the list of unauthorized setting ids */
 				sprintf( __( 'Unauthorized settings: %s', 'customize-snapshots' ), join( ',', $unauthorized_setting_ids ) ),
-				array( 'setting_ids' => $unauthorized_setting_ids )
+				array(
+					'setting_ids' => $unauthorized_setting_ids,
+				)
 			);
 		}
 		if ( ! empty( $unrecognized_setting_ids ) ) {
 			$error->add(
 				'unrecognized_settings',
 				/* translators: %s is the list of unrecognized setting ids */
 				sprintf( __( 'Unrecognized settings: %s', 'customize-snapshots' ), join( ',', $unrecognized_setting_ids ) ),
-				array( 'setting_ids' => $unrecognized_setting_ids )
+				array(
+					'setting_ids' => $unrecognized_setting_ids,
+				)
 			);
 		}
 		if ( 0 !== count( $invalid_setting_ids ) ) {

diff --git a/php/class-customize-snapshot-manager-back-compat.php b/php/class-customize-snapshot-manager-back-compat.php
@@ -33,7 +33,7 @@ function init() {
 		$this->hooks();
 		if ( $this->read_current_snapshot_uuid() ) {
 			$this->load_snapshot();
-		} elseif ( is_customize_preview() && isset( $_REQUEST['wp_customize_preview_ajax'] ) && 'true' === $_REQUEST['wp_customize_preview_ajax'] ) {
+		} elseif ( is_customize_preview() && isset( $_REQUEST['wp_customize_preview_ajax'] ) && 'true' === $_REQUEST['wp_customize_preview_ajax'] ) { // WPCS: input var ok; CSRF ok.
 			add_action( 'wp_loaded', array( $this, 'setup_preview_ajax_requests' ), 12 );
 		}
 	}
@@ -491,7 +491,7 @@ public function override_request_method() {
 		global $wp;
 
 		// Skip of X-HTTP-Method-Override request header is not present.
-		if ( ! isset( $_SERVER['HTTP_X_HTTP_METHOD_OVERRIDE'] ) ) {
+		if ( ! isset( $_SERVER['HTTP_X_HTTP_METHOD_OVERRIDE'] ) || ! isset( $_SERVER['REQUEST_METHOD'] ) ) { // WPCS: input var ok.
 			return false;
 		}
 
@@ -501,17 +501,17 @@ public function override_request_method() {
 		}
 
 		// Skip if the request method is not GET or POST, or the override is the same as the original.
-		$original_request_method = $_SERVER['REQUEST_METHOD'];
-		$override_request_method = strtoupper( $_SERVER['HTTP_X_HTTP_METHOD_OVERRIDE'] );
+		$original_request_method = strtoupper( sanitize_key( $_SERVER['REQUEST_METHOD'] ) ); // WPCS: input var ok.
+		$override_request_method = strtoupper( sanitize_key( $_SERVER['HTTP_X_HTTP_METHOD_OVERRIDE'] ) ); // WPCS: input var ok.
 		if ( ! in_array( $override_request_method, array( 'GET', 'POST' ), true ) || $original_request_method === $override_request_method ) {
 			return false;
 		}
 
 		// Convert a POST request into a GET request.
 		if ( 'GET' === $override_request_method && 'POST' === $original_request_method ) {
 			$_SERVER['REQUEST_METHOD'] = $override_request_method;
-			$_GET = array_merge( $_GET, $_POST );
-			$_SERVER['QUERY_STRING'] = build_query( array_map( 'rawurlencode', wp_unslash( $_GET ) ) );
+			$_GET = array_merge( $_GET, $_POST ); // WPCS: input var ok; CSRF ok.
+			$_SERVER['QUERY_STRING'] = build_query( array_map( 'rawurlencode', wp_unslash( $_GET ) ) ); // WPCS: input var ok. CSRF ok.
 			return true;
 		}
 
@@ -570,7 +570,9 @@ function( $value ) {
 			},
 			$this->customize_manager->unsanitized_post_values()
 		);
-		$result = $this->snapshot->set( $settings_data, array( 'skip_validation' => true ) );
+		$result = $this->snapshot->set( $settings_data, array(
+			'skip_validation' => true,
+		) );
 		if ( ! empty( $result['errors'] ) ) {
 			add_filter( 'customize_save_response', function( $response ) use ( $result, $that ) {
 				$response['snapshot_errors'] = $that->prepare_errors_for_response( $result['errors'] );
@@ -590,8 +592,11 @@ function( $value ) {
 				$args['date_gmt'] = current_time( 'mysql', true );
 			}
 
-			if ( isset( $_POST['title'] ) && '' !== trim( $_POST['title'] ) ) {
-				$args['post_title'] = sanitize_text_field( wp_unslash( $_POST['title'] ) );
+			if ( isset( $_POST['title'] ) ) { // WPCS: input var ok. CSRF ok because customize_save_after happens after nonce check.
+				$title = sanitize_text_field( wp_unslash( $_POST['title'] ) ); // WPCS: Input var ok. CSRF ok because customize_save_after happens after nonce check.
+				if ( ! empty( $title ) ) {
+					$args['post_title'] = $title;
+				}
 			}
 
 			$r = $this->snapshot->save( $args );
@@ -682,7 +687,7 @@ public function save_settings_with_publish_snapshot( $new_status, $old_status, $
 			do_action( 'customize_register', $this->customize_manager );
 
 			// undefine( 'DOING_AJAX' )... just kidding. This is the end of the unfortunate hack and it should be fixed in Core.
-			unset( $_REQUEST['action'] );
+			unset( $_REQUEST['action'] ); // WPCS: Input var ok.
 		}
 
 		if ( method_exists( $this->customize_manager, 'validate_setting_values' ) ) {
@@ -822,7 +827,7 @@ public function handle_update_snapshot_request() {
 		}
 
 		if ( isset( $_POST['status'] ) ) { // WPCS: input var ok.
-			$status = sanitize_key( $_POST['status'] );
+			$status = sanitize_key( $_POST['status'] ); // WPCS: input var ok.
 		} else {
 			$status = 'draft';
 		}
@@ -834,7 +839,7 @@ public function handle_update_snapshot_request() {
 			status_header( 400 );
 			wp_send_json_error( 'customize_not_allowed' );
 		}
-		$publish_date = isset( $_POST['date'] ) ? $_POST['date'] : '';
+		$publish_date = isset( $_POST['date'] ) ? sanitize_text_field( wp_unslash( $_POST['date'] ) ) : ''; // WPCS: input var ok.
 		if ( 'future' === $status ) {
 			$publish_date_obj = new \DateTime( $publish_date );
 			$current_date = new \DateTime( current_time( 'mysql' ) );
@@ -899,8 +904,11 @@ function( $value ) {
 		$args = array(
 			'status' => $status,
 		);
-		if ( isset( $_POST['title'] ) && '' !== trim( $_POST['title'] ) ) {
-			$args['post_title'] = sanitize_text_field( wp_unslash( $_POST['title'] ) );
+		if ( isset( $_POST['title'] ) ) { // WPCS: input var ok.
+			$title = sanitize_text_field( wp_unslash( $_POST['title'] ) ); // WPCS: input var ok.
+			if ( '' !== $title ) {
+				$args['post_title'] = $title;
+			}
 		}
 
 		if ( isset( $publish_date_obj ) && 'future' === $status ) {

diff --git a/php/class-customize-snapshot-manager.php b/php/class-customize-snapshot-manager.php
@@ -152,18 +152,16 @@ public function add_snapshot_var_to_customize_save( $response, $customize_manage
 	public function read_current_snapshot_uuid() {
 		$customize_arg = $this->get_customize_uuid_param();
 		$frontend_arg = $this->get_front_uuid_param();
-		if ( isset( $_REQUEST[ $customize_arg ] ) ) {
-			$uuid = $_REQUEST[ $customize_arg ]; // WPCS: input var ok.
-		} elseif ( isset( $_REQUEST[ $frontend_arg ] ) ) {
-			$uuid = $_REQUEST[ $frontend_arg ]; // WPCS: input var ok.
+		$uuid = null;
+		if ( isset( $_REQUEST[ $customize_arg ] ) ) { // WPCS: input var ok. CSRF ok.
+			$uuid = sanitize_key( wp_unslash( $_REQUEST[ $customize_arg ] ) ); // WPCS: input var ok. CSRF ok.
+		} elseif ( isset( $_REQUEST[ $frontend_arg ] ) ) { // WPCS: input var ok. CSRF ok.
+			$uuid = sanitize_key( wp_unslash( $_REQUEST[ $frontend_arg ] ) ); // WPCS: input var ok. CSRF ok.
 		}
 
-		if ( isset( $uuid ) ) {
-			$uuid = sanitize_key( wp_unslash( $uuid ) );
-			if ( static::is_valid_uuid( $uuid ) ) {
-				$this->current_snapshot_uuid = $uuid;
-				return true;
-			}
+		if ( $uuid && static::is_valid_uuid( $uuid ) ) {
+			$this->current_snapshot_uuid = $uuid;
+			return true;
 		}
 		$this->current_snapshot_uuid = null;
 		return false;
@@ -175,7 +173,7 @@ public function read_current_snapshot_uuid() {
 	 * @return bool True if it's an Ajax request, false otherwise.
 	 */
 	public function doing_customize_save_ajax() {
-		return isset( $_REQUEST['action'] ) && wp_unslash( $_REQUEST['action'] ) === 'customize_save';
+		return isset( $_REQUEST['action'] ) && sanitize_key( wp_unslash( $_REQUEST['action'] ) ) === 'customize_save'; // WPCS: input var ok. CSRF ok.
 	}
 
 	/**
@@ -188,7 +186,9 @@ public function ensure_customize_manager() {
 		if ( empty( $wp_customize ) || ! ( $wp_customize instanceof \WP_Customize_Manager ) ) {
 			require_once( ABSPATH . WPINC . '/class-wp-customize-manager.php' );
 			if ( null !== $this->current_snapshot_uuid ) {
-				$wp_customize = new \WP_Customize_Manager( array( 'changeset_uuid' => $this->current_snapshot_uuid ) ); // WPCS: override ok.
+				$wp_customize = new \WP_Customize_Manager( array(
+					'changeset_uuid' => $this->current_snapshot_uuid,
+				) ); // WPCS: override ok.
 			} else {
 				$wp_customize = new \WP_Customize_Manager(); // WPCS: override ok.
 			}
@@ -473,6 +473,7 @@ public function customize_menu( $wp_admin_bar ) {
 	 * Print admin bar styles.
 	 */
 	public function print_admin_bar_styles() {
+		// @codingStandardsIgnoreStart A WordPress-VIP sniff has false positive on admin bar being hidden.
 		?>
 		<style type="text/css">
 			#wpadminbar #wp-admin-bar-resume-customize-snapshot {
@@ -492,6 +493,7 @@ public function print_admin_bar_styles() {
 			}
 		</style>
 		<?php
+		// @codingStandardsIgnoreEnd
 	}
 
 	/**
@@ -524,7 +526,9 @@ public function replace_customize_link( $wp_admin_bar ) {
 
 		// Add customize_snapshot_uuid param as param to customize.php itself.
 		$customize_node->href = add_query_arg(
-			array( $this->get_customize_uuid_param() => $this->current_snapshot_uuid ),
+			array(
+				$this->get_customize_uuid_param() => $this->current_snapshot_uuid,
+			),
 			$customize_node->href
 		);
 
@@ -873,7 +877,9 @@ public function get_month_choices() {
 			$months[ $i ]['text'] = sprintf( __( '%1$s-%2$s', 'customize-snapshots' ), $month_number, $month_text );
 			$months[ $i ]['value'] = $month_number;
 		}
-		return array( 'month_choices' => $months );
+		return array(
+			'month_choices' => $months,
+		);
 	}
 
 	/**

diff --git a/php/class-migrate.php b/php/class-migrate.php
@@ -66,7 +66,7 @@ public function maybe_migrate() {
 	 */
 	public function handle_migrate_changeset_request() {
 		check_ajax_referer( 'customize-snapshot-migration', 'nonce' );
-		$limit = isset( $_REQUEST['limit'] ) ? absint( $_REQUEST['limit'] ) : 20;
+		$limit = isset( $_REQUEST['limit'] ) ? absint( $_REQUEST['limit'] ) : 20; // WPCS: input var ok.
 		$found_posts = $this->changeset_migrate( $limit );
 		$remaining_post = ( $found_posts < $limit ) ? 0 : $found_posts - $limit;
 		$data = array(
@@ -223,14 +223,15 @@ public function migrate_post( $id ) {
 				$post_data[ $prefixed_setting_id ]['type'] = $setting->type;
 			}
 		}
-		$maybe_updated = $wpdb->update( $wpdb->posts, array(
+		$maybe_updated = $wpdb->update( $wpdb->posts,
+			array(
 				'post_type'    => 'customize_changeset',
 				'post_content' => Customize_Snapshot_Manager::encode_json( $post_data ),
 			),
 			array(
 				'ID' => $post->ID,
 			)
-		);
+		); // WPCS: DB call ok and cache ok, because doing update query, and using direct DB call to bypass weight of triggered hooks.
 		clean_post_cache( $post );
 
 		$wp_customize = $original_manager; // Restore previous manager. WPCS: override ok.

diff --git a/php/class-plugin-base.php b/php/class-plugin-base.php
@@ -201,7 +201,7 @@ public function is_wpcom_vip_prod() {
 	 */
 	public function trigger_warning( $message, $code = \E_USER_WARNING ) {
 		if ( ! $this->is_wpcom_vip_prod() ) {
-			trigger_error( esc_html( get_class( $this ) . ': ' . $message ), $code );
+			trigger_error( esc_html( get_class( $this ) . ': ' . $message ), $code ); // @codingStandardsIgnoreLine because this line will not get run on WordPress.com per the condition.
 		}
 	}
 
@@ -214,7 +214,10 @@ public function trigger_warning( $message, $code = \E_USER_WARNING ) {
 	 *
 	 * @return mixed
 	 */
-	public function add_filter( $name, $callback, $args = array( 'priority' => 10, 'arg_count' => PHP_INT_MAX ) ) {
+	public function add_filter( $name, $callback, $args = array(
+		'priority' => 10,
+		'arg_count' => PHP_INT_MAX,
+	) ) {
 		return $this->_add_hook( 'filter', $name, $callback, $args );
 	}
 
@@ -227,7 +230,10 @@ public function add_filter( $name, $callback, $args = array( 'priority' => 10, '
 	 *
 	 * @return mixed
 	 */
-	public function add_action( $name, $callback, $args = array( 'priority' => 10, 'arg_count' => PHP_INT_MAX ) ) {
+	public function add_action( $name, $callback, $args = array(
+		'priority' => 10,
+		'arg_count' => PHP_INT_MAX,
+	) ) {
 		return $this->_add_hook( 'action', $name, $callback, $args );
 	}
 
@@ -262,7 +268,7 @@ public function add_doc_hooks( $object = null ) {
 		if ( isset( $this->_called_doc_hooks[ $class_name ] ) ) {
 			$notice = sprintf( 'The add_doc_hooks method was already called on %s. Note that the Plugin_Base constructor automatically calls this method.', $class_name );
 			if ( ! $this->is_wpcom_vip_prod() ) {
-				trigger_error( esc_html( $notice ), \E_USER_NOTICE );
+				trigger_error( esc_html( $notice ), \E_USER_NOTICE ); // @codingStandardsIgnoreLine because this line will not get run on WordPress.com per the condition.
 			}
 			return;
 		}

diff --git a/php/class-plugin.php b/php/class-plugin.php
@@ -47,7 +47,7 @@ class Plugin extends Plugin_Base {
 	 */
 	public function __construct() {
 		// Parse plugin version.
-		if ( preg_match( '/Version:\s*(\S+)/', file_get_contents( __DIR__ . '/../customize-snapshots.php' ), $matches ) ) {
+		if ( preg_match( '/Version:\s*(\S+)/', file_get_contents( __DIR__ . '/../customize-snapshots.php' ), $matches ) ) { // @codingStandardsIgnoreLine because file_get_contents() is not requesting a URL.
 			$this->version = $matches[1];
 		}
 		$this->compat = is_back_compat();
@@ -161,10 +161,10 @@ public function register_styles( \WP_Styles $wp_styles ) {
 	 * Continue allowing support of param customize_snapshot_uuid in 4.7+.
 	 */
 	public function param_back_compat() {
-		if ( isset( $_REQUEST['customize_snapshot_uuid'] ) && ! $this->compat ) {
-			$_REQUEST['customize_changeset_uuid'] = $_REQUEST['customize_snapshot_uuid'];
-			$_GET['customize_changeset_uuid'] = $_REQUEST['customize_snapshot_uuid'];
-			$_POST['customize_changeset_uuid'] = $_REQUEST['customize_snapshot_uuid'];
+		if ( isset( $_REQUEST['customize_snapshot_uuid'] ) && ! $this->compat ) { // WPCS: input var ok. CSRF ok.
+			$_REQUEST['customize_changeset_uuid'] = $_REQUEST['customize_snapshot_uuid']; // WPCS: input var ok. CSRF ok. Sanitization ok.
+			$_GET['customize_changeset_uuid'] = $_REQUEST['customize_snapshot_uuid']; // WPCS: input var ok. CSRF ok. Sanitization ok.
+			$_POST['customize_changeset_uuid'] = $_REQUEST['customize_snapshot_uuid']; // WPCS: input var ok. CSRF ok. Sanitization ok.
 		}
 	}
 }
diff --git a/php/class-post-type-back-compat.php b/php/class-post-type-back-compat.php
@@ -133,11 +133,14 @@ public function snapshot_merge_print_script() {
 	public function handle_snapshot_merge_workaround() {
 		$wp_list_table = _get_list_table( 'WP_Posts_List_Table' );
 		$action = $wp_list_table->current_action();
-		if ( 'merge_snapshot' !== $action || ( isset( $_REQUEST['post_type'] ) && static::SLUG !== wp_unslash( $_REQUEST['post_type'] ) ) ) {
+		if ( 'merge_snapshot' !== $action || ( isset( $_REQUEST['post_type'] ) && static::SLUG !== sanitize_key( wp_unslash( $_REQUEST['post_type'] ) ) ) ) { // WPCS: input var ok. CSRF ok.
+			return;
+		}
+		if ( ! isset( $_REQUEST['post'] ) || ! is_array( $_REQUEST['post'] ) ) { // WPCS: input var ok. CSRF ok.
 			return;
 		}
 		check_admin_referer( 'bulk-posts' );
-		$post_ids = array_map( 'intval', $_REQUEST['post'] );
+		$post_ids = array_map( 'intval', $_REQUEST['post'] ); // WPCS: input var ok. CSRF ok.
 		if ( empty( $post_ids ) ) {
 			return;
 		}
@@ -226,7 +229,7 @@ public function show_publish_error_admin_notice() {
 		if ( ! $current_screen || static::SLUG !== $current_screen->id || 'post' !== $current_screen->base ) {
 			return;
 		}
-		if ( ! isset( $_REQUEST['snapshot_error_on_publish'] ) ) {
+		if ( ! isset( $_REQUEST['snapshot_error_on_publish'] ) ) { // WPCS: input var ok. CSRF ok.
 			return;
 		}
 		?>