From 8c26572c8a092137ef02d03cbb03783cfa963e8f Mon Sep 17 00:00:00 2001 From: Rachel Baker Date: Fri, 2 Dec 2016 22:43:03 +0000 Subject: [PATCH] REST API: Fix bug where comment author and author email could be an empty string when creating a comment. If the `require_name_email` option is true, creating a comment with an empty string for the author name or email should not be accepted. Both values can be an empty string on update. Props flixos90, hnle, dd32, rachelbaker, jnylen0, ChopinBach, joehoyle, pento. Fixes #38971. git-svn-id: https://develop.svn.wordpress.org/trunk@39444 602fd350-edb4-49c9-b593-d223f7449a82 --- .../class-wp-rest-comments-controller.php | 43 ++++- .../rest-api/rest-comments-controller.php | 166 ++++++++++++++---- 2 files changed, 170 insertions(+), 39 deletions(-) diff --git a/src/wp-includes/rest-api/endpoints/class-wp-rest-comments-controller.php b/src/wp-includes/rest-api/endpoints/class-wp-rest-comments-controller.php index b07ced4458..9fe4f90e52 100644 --- a/src/wp-includes/rest-api/endpoints/class-wp-rest-comments-controller.php +++ b/src/wp-includes/rest-api/endpoints/class-wp-rest-comments-controller.php @@ -508,17 +508,9 @@ class WP_REST_Comments_Controller extends WP_REST_Controller { // Honor the discussion setting that requires a name and email address of the comment author. if ( get_option( 'require_name_email' ) ) { - if ( ! isset( $prepared_comment['comment_author'] ) && ! isset( $prepared_comment['comment_author_email'] ) ) { + if ( empty( $prepared_comment['comment_author'] ) || empty( $prepared_comment['comment_author_email'] ) ) { return new WP_Error( 'rest_comment_author_data_required', __( 'Creating a comment requires valid author name and email values.' ), array( 'status' => 400 ) ); } - - if ( ! isset( $prepared_comment['comment_author'] ) ) { - return new WP_Error( 'rest_comment_author_required', __( 'Creating a comment requires a valid author name.' ), array( 'status' => 400 ) ); - } - - if ( ! isset( $prepared_comment['comment_author_email'] ) ) { - return new WP_Error( 'rest_comment_author_email_required', __( 'Creating a comment requires a valid author email.' ), array( 'status' => 400 ) ); - } } if ( ! isset( $prepared_comment['comment_author_email'] ) ) { @@ -1155,6 +1147,10 @@ class WP_REST_Comments_Controller extends WP_REST_Controller { 'type' => 'string', 'format' => 'email', 'context' => array( 'edit' ), + 'arg_options' => array( + 'sanitize_callback' => array( $this, 'check_comment_author_email' ), + 'validate_callback' => null, // skip built-in validation of 'email'. + ), ), 'author_ip' => array( 'description' => __( 'IP address for the object author.' ), @@ -1581,4 +1577,33 @@ class WP_REST_Comments_Controller extends WP_REST_Controller { return current_user_can( 'edit_comment', $comment->comment_ID ); } + + /** + * Checks a comment author email for validity. + * + * Accepts either a valid email address or empty string as a valid comment + * author email address. Setting the comment author email to an empty + * string is allowed when a comment is being updated. + * + * @since 4.7.0 + * + * @param string $value Author email value submitted. + * @param WP_REST_Request $request Full details about the request. + * @param string $param The parameter name. + * @return WP_Error|string The sanitized email address, if valid, + * otherwise an error. + */ + public function check_comment_author_email( $value, $request, $param ) { + $email = (string) $value; + if ( empty( $email ) ) { + return $email; + } + + $check_email = rest_validate_request_arg( $email, $request, $param ); + if ( is_wp_error( $check_email ) ) { + return $check_email; + } + + return $email; + } } diff --git a/tests/phpunit/tests/rest-api/rest-comments-controller.php b/tests/phpunit/tests/rest-api/rest-comments-controller.php index 1daa35ea38..a103d21c5c 100644 --- a/tests/phpunit/tests/rest-api/rest-comments-controller.php +++ b/tests/phpunit/tests/rest-api/rest-comments-controller.php @@ -97,10 +97,6 @@ class WP_Test_REST_Comments_Controller extends WP_Test_REST_Controller_Testcase } } - public function tearDown() { - parent::tearDown(); - } - public function test_register_routes() { $routes = $this->server->get_routes(); @@ -987,30 +983,10 @@ class WP_Test_REST_Comments_Controller extends WP_Test_REST_Controller_Testcase $this->assertEquals( $params['content']['raw'], $new_comment->comment_content ); } - public function test_create_comment_missing_required_author_name_and_email_per_option_value() { + public function test_create_comment_missing_required_author_name() { add_filter( 'rest_allow_anonymous_comments', '__return_true' ); update_option( 'require_name_email', 1 ); - $params = array( - 'post' => self::$post_id, - 'content' => 'Now, I don\'t want you to worry class. These tests will have no affect on your grades. They merely determine your future social status and financial success. If any.', - ); - - $request = new WP_REST_Request( 'POST', '/wp/v2/comments' ); - $request->add_header( 'content-type', 'application/json' ); - $request->set_body( wp_json_encode( $params ) ); - - $response = $this->server->dispatch( $request ); - - $this->assertErrorResponse( 'rest_comment_author_data_required', $response, 400 ); - - update_option( 'require_name_email', 0 ); - } - - public function test_create_comment_missing_required_author_name_per_option_value() { - wp_set_current_user( self::$admin_id ); - update_option( 'require_name_email', 1 ); - $params = array( 'post' => self::$post_id, 'author_email' => 'ekrabappel@springfield-elementary.edu', @@ -1022,12 +998,31 @@ class WP_Test_REST_Comments_Controller extends WP_Test_REST_Controller_Testcase $request->set_body( wp_json_encode( $params ) ); $response = $this->server->dispatch( $request ); - $this->assertErrorResponse( 'rest_comment_author_required', $response, 400 ); - update_option( 'require_name_email', 0 ); + $this->assertErrorResponse( 'rest_comment_author_data_required', $response, 400 ); } - public function test_create_comment_missing_required_author_email_per_option_value() { + public function test_create_comment_empty_required_author_name() { + add_filter( 'rest_allow_anonymous_comments', '__return_true' ); + update_option( 'require_name_email', 1 ); + + $params = array( + 'author_name' => '', + 'author_email' => 'ekrabappel@springfield-elementary.edu', + 'post' => self::$post_id, + 'content' => 'Now, I don\'t want you to worry class. These tests will have no affect on your grades. They merely determine your future social status and financial success. If any.', + ); + + $request = new WP_REST_Request( 'POST', '/wp/v2/comments' ); + $request->add_header( 'content-type', 'application/json' ); + $request->set_body( wp_json_encode( $params ) ); + + $response = $this->server->dispatch( $request ); + + $this->assertErrorResponse( 'rest_comment_author_data_required', $response, 400 ); + } + + public function test_create_comment_missing_required_author_email() { wp_set_current_user( self::$admin_id ); update_option( 'require_name_email', 1 ); @@ -1042,9 +1037,26 @@ class WP_Test_REST_Comments_Controller extends WP_Test_REST_Controller_Testcase $request->set_body( wp_json_encode( $params ) ); $response = $this->server->dispatch( $request ); - $this->assertErrorResponse( 'rest_comment_author_email_required', $response, 400 ); + $this->assertErrorResponse( 'rest_comment_author_data_required', $response, 400 ); + } - update_option( 'require_name_email', 0 ); + public function test_create_comment_empty_required_author_email() { + wp_set_current_user( self::$admin_id ); + update_option( 'require_name_email', 1 ); + + $params = array( + 'post' => self::$post_id, + 'author_name' => 'Edna Krabappel', + 'author_email' => '', + 'content' => 'Now, I don\'t want you to worry class. These tests will have no affect on your grades. They merely determine your future social status and financial success. If any.', + ); + + $request = new WP_REST_Request( 'POST', '/wp/v2/comments' ); + $request->add_header( 'content-type', 'application/json' ); + $request->set_body( wp_json_encode( $params ) ); + + $response = $this->server->dispatch( $request ); + $this->assertErrorResponse( 'rest_comment_author_data_required', $response, 400 ); } public function test_create_comment_author_email_too_short() { @@ -1992,6 +2004,100 @@ class WP_Test_REST_Comments_Controller extends WP_Test_REST_Controller_Testcase $this->assertEquals( $params['date_gmt'], mysql_to_rfc3339( $updated->comment_date_gmt ) ); } + public function test_update_comment_author_email_only() { + wp_set_current_user( self::$editor_id ); + update_option( 'require_name_email', 1 ); + + $params = array( + 'post' => self::$post_id, + 'author_email' => 'ekrabappel@springfield-elementary.edu', + 'content' => 'Now, I don\'t want you to worry class. These tests will have no affect on your grades. They merely determine your future social status and financial success. If any.', + ); + + $request = new WP_REST_Request( 'PUT', sprintf( '/wp/v2/comments/%d', self::$approved_id ) ); + $request->add_header( 'content-type', 'application/json' ); + $request->set_body( wp_json_encode( $params ) ); + + $response = $this->server->dispatch( $request ); + $this->assertEquals( 200, $response->get_status() ); + } + + public function test_update_comment_empty_author_name() { + wp_set_current_user( self::$editor_id ); + update_option( 'require_name_email', 1 ); + + $params = array( + 'author_name' => '', + 'author_email' => 'ekrabappel@springfield-elementary.edu', + 'post' => self::$post_id, + 'content' => 'Now, I don\'t want you to worry class. These tests will have no affect on your grades. They merely determine your future social status and financial success. If any.', + ); + + $request = new WP_REST_Request( 'PUT', sprintf( '/wp/v2/comments/%d', self::$approved_id ) ); + $request->add_header( 'content-type', 'application/json' ); + $request->set_body( wp_json_encode( $params ) ); + + $response = $this->server->dispatch( $request ); + $this->assertEquals( 200, $response->get_status() ); + } + + public function test_update_comment_author_name_only() { + wp_set_current_user( self::$admin_id ); + update_option( 'require_name_email', 1 ); + + $params = array( + 'post' => self::$post_id, + 'author_name' => 'Edna Krabappel', + 'content' => 'Now, I don\'t want you to worry class. These tests will have no affect on your grades. They merely determine your future social status and financial success. If any.', + ); + + $request = new WP_REST_Request( 'PUT', sprintf( '/wp/v2/comments/%d', self::$approved_id ) ); + $request->add_header( 'content-type', 'application/json' ); + $request->set_body( wp_json_encode( $params ) ); + + $response = $this->server->dispatch( $request ); + $this->assertEquals( 200, $response->get_status() ); + } + + public function test_update_comment_empty_author_email() { + wp_set_current_user( self::$admin_id ); + update_option( 'require_name_email', 1 ); + + $params = array( + 'post' => self::$post_id, + 'author_name' => 'Edna Krabappel', + 'author_email' => '', + 'content' => 'Now, I don\'t want you to worry class. These tests will have no affect on your grades. They merely determine your future social status and financial success. If any.', + ); + + $request = new WP_REST_Request( 'PUT', sprintf( '/wp/v2/comments/%d', self::$approved_id ) ); + $request->add_header( 'content-type', 'application/json' ); + $request->set_body( wp_json_encode( $params ) ); + + $response = $this->server->dispatch( $request ); + $this->assertEquals( 200, $response->get_status() ); + } + + public function test_update_comment_author_email_too_short() { + wp_set_current_user( self::$admin_id ); + + $params = array( + 'post' => self::$post_id, + 'author_name' => 'Homer J. Simpson', + 'author_email' => 'a@b', + 'content' => 'in this house, we obey the laws of thermodynamics!', + ); + + $request = new WP_REST_Request( 'PUT', sprintf( '/wp/v2/comments/%d', self::$approved_id ) ); + $request->add_header( 'content-type', 'application/json' ); + $request->set_body( wp_json_encode( $params ) ); + $response = $this->server->dispatch( $request ); + + $this->assertErrorResponse( 'rest_invalid_param', $response, 400 ); + $data = $response->get_data(); + $this->assertArrayHasKey( 'author_email', $data['data']['params'] ); + } + public function test_update_comment_invalid_type() { wp_set_current_user( self::$admin_id );