Changeset 39446 for branches/4.7

Timestamp:

12/02/2016 10:45:06 PM (8 years ago)

Author:

rachelbaker

Message:

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.

Merges [39444] into the 4.7 branch.
Props flixos90, hnle, dd32, rachelbaker, jnylen0, ChopinBach, joehoyle, pento.

Fixes #38971 for 4.7.

Location:

branches/4.7

Files:

• . (modified) (1 prop)
• src/wp-includes/rest-api/endpoints/class-wp-rest-comments-controller.php (modified) (3 diffs)
• tests/phpunit/tests/rest-api/rest-comments-controller.php (modified) (5 diffs)

branches/4.7/src/wp-includes/rest-api/endpoints/class-wp-rest-comments-controller.php

@@ -509 +509 @@
         // 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 ) );
             }
         }
@@ -1156 +1148 @@
                     '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(
@@ -1582 +1578 @@
         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;
+    }
 }

branches/4.7/tests/phpunit/tests/rest-api/rest-comments-controller.php

@@ -96 +96 @@
             update_site_option( 'site_admins', array( 'superadmin' ) );
         }
-    }
-
-    public function tearDown() {
-        parent::tearDown();
     }
 
@@ -988 +984 @@
     }
 
-    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 );
 
@@ -1023 +999 @@
 
         $response = $this->server->dispatch( $request );
-        $this->assertErrorResponse( 'rest_comment_author_required', $response, 400 );
-
-        update_option( 'require_name_email', 0 );
-    }
-
-    public function test_create_comment_missing_required_author_email_per_option_value() {
+
+        $this->assertErrorResponse( 'rest_comment_author_data_required', $response, 400 );
+    }
+
+    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 );
@@ -1043 +1038 @@
 
         $response = $this->server->dispatch( $request );
-        $this->assertErrorResponse( 'rest_comment_author_email_required', $response, 400 );
-
-        update_option( 'require_name_email', 0 );
+        $this->assertErrorResponse( 'rest_comment_author_data_required', $response, 400 );
+    }
+
+    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 );
     }
 
@@ -1991 +2003 @@
         $this->assertEquals( $params['date_gmt'], $comment['date_gmt'] );
         $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'] );
     }