Changeset 38778

Timestamp:

10/11/2016 03:42:28 AM (8 years ago)

Author:

boonebgorges

Message:

Comments: Abstract die() calls from comment submission routine.

Since 4.4, comment submission has been mostly abstracted into a function,
rather than being processed inline in wp-comments-post.php. This change
made it easier to write automated tests against the bulk of the comment
submission process. wp_allow_comment() remained untestable, however:
when a comment failed one of its checks (flooding, duplicates, etc),
die() or wp_die() would be called directly. This shortcoming posed
problems for any application attempting to use WP's comment verification
functions in an abstract way - from PHPUnit to the REST API.

The current changeset introduces a new parameter, $avoid_die, to the
wp_new_comment() stack. When set to true, wp_new_comment() and
wp_allow_comment() will return WP_Error objects when a comment check
fails. When set to false - the default, for backward compatibility -
a failed check will result in a die() or wp_die(), as appropriate.

Prior to this changeset, default comment flood checks took place in the
function check_comment_flood_db(), which was hooked to the
'check_comment_flood' action. This design allowed the default comment
flood routine to be bypassed or replaced using remove_action().
In order to maintain backward compatibility with this usage, while
simultaneously converting the comment flood logic into something that
returns a value rather than calling die() directly,
check_comment_flood_db() has been changed into a wrapper function for
a call to add_filter(); this, in turn, adds the *actual* comment flood
check to a new filter, 'wp_is_comment_flood'. Note that direct calls
to check_comment_flood_db() will no longer do anything in isolation.

Props websupporter, rachelbaker.
Fixes #36901.

Location:

trunk

Files:

• src/wp-includes/comment.php (modified) (12 diffs)
• src/wp-includes/default-filters.php (modified) (1 diff)
• tests/phpunit/tests/comment-submission.php (modified) (1 diff)

trunk/src/wp-includes/comment.php

@@ -589 +589 @@
  *
  * @since 2.0.0
+ * @since 4.7.0 The `$avoid_die` parameter was added, allowing the function to
+ *              return a WP_Error object instead of dying.
  *
  * @global wpdb $wpdb WordPress database abstraction object.
  *
- * @param array $commentdata Contains information on the comment
- * @return int|string Signifies the approval status (0|1|'spam')
- */
-function wp_allow_comment( $commentdata ) {
+ * @param array $commentdata Contains information on the comment.
+ * @param bool  $avoid_die   When true, a disallowed comment will result in the function
+ *                           returning a WP_Error object, rather than executing wp_die().
+ *                           Default false.
+ * @return int|string|WP_Error Allowed comments return the approval status (0|1|'spam').
+ *                             If `$avoid_die` is true, disallowed comments return a WP_Error.
+ */
+function wp_allow_comment( $commentdata, $avoid_die = false ) {
     global $wpdb;
 
@@ -640 +646 @@
          */
         do_action( 'comment_duplicate_trigger', $commentdata );
-        if ( wp_doing_ajax() ) {
-            die( __('Duplicate comment detected; it looks as though you’ve already said that!') );
+        if ( true === $avoid_die ) {
+            return new WP_Error( 'comment_duplicate', __( 'Duplicate comment detected; it looks as though you’ve already said that!' ), $dupe_id );
+        } else {
+            if ( wp_doing_ajax() ) {
+                die( __('Duplicate comment detected; it looks as though you’ve already said that!') );
+            }
+
+            wp_die( __( 'Duplicate comment detected; it looks as though you’ve already said that!' ), 409 );
         }
-        wp_die( __( 'Duplicate comment detected; it looks as though you’ve already said that!' ), 409 );
     }
 
@@ -652 +663 @@
      *
      * @since 2.3.0
+     * @since 4.7.0 The `$avoid_die` parameter was added.
      *
      * @param string $comment_author_IP    Comment author's IP address.
      * @param string $comment_author_email Comment author's email.
      * @param string $comment_date_gmt     GMT date the comment was posted.
+     * @param bool   $avoid_die            Whether to prevent executing wp_die()
+     *                                     or die() if a comment flood is occurring.
      */
     do_action(
@@ -661 +675 @@
         $commentdata['comment_author_IP'],
         $commentdata['comment_author_email'],
-        $commentdata['comment_date_gmt']
+        $commentdata['comment_date_gmt'],
+        $avoid_die
     );
+
+    /**
+     * Filters whether a comment is part of a comment flood.
+     *
+     * The default check is wp_check_comment_flood(). See check_comment_flood_db().
+     *
+     * @since 4.7.0
+     *
+     * @param bool   $is_flood             Is a comment flooding occurring? Default false.
+     * @param string $comment_author_IP    Comment author's IP address.
+     * @param string $comment_author_email Comment author's email.
+     * @param string $comment_date_gmt     GMT date the comment was posted.
+     * @param bool   $avoid_die            Whether to prevent executing wp_die()
+     *                                     or die() if a comment flood is occurring.
+     */
+    $is_flood = apply_filters(
+        'wp_is_comment_flood',
+        false,
+        $commentdata['comment_author_IP'],
+        $commentdata['comment_author_email'],
+        $commentdata['comment_date_gmt'],
+        $avoid_die
+    );
+
+    if ( $is_flood ) {
+        return new WP_Error( 'comment_flood', __( 'You are posting comments too quickly. Slow down.' ) );
+    }
 
     if ( ! empty( $commentdata['user_id'] ) ) {
@@ -716 +758 @@
 
 /**
- * Check whether comment flooding is occurring.
+ * Hooks WP's native database-based comment-flood check.
+ *
+ * This wrapper maintains backward compatibility with plugins that expect to
+ * be able to unhook the legacy check_comment_flood_db() function from
+ * 'check_comment_flood' using remove_action().
+ *
+ * @since 2.3.0
+ * @since 4.7.0 Converted to be an add_filter() wrapper.
+ */
+function check_comment_flood_db() {
+    add_filter( 'wp_is_comment_flood', 'wp_check_comment_flood', 10, 5 );
+}
+
+/**
+ * Checks whether comment flooding is occurring.
  *
  * Won't run, if current user can manage options, so to not block
  * administrators.
  *
- * @since 2.3.0
+ * @since 4.7.0
  *
  * @global wpdb $wpdb WordPress database abstraction object.
  *
- * @param string $ip Comment IP.
- * @param string $email Comment author email address.
- * @param string $date MySQL time string.
- */
-function check_comment_flood_db( $ip, $email, $date ) {
+ * @param bool   $is_flood  Is a comment flooding occurring?
+ * @param string $ip        Comment IP.
+ * @param string $email     Comment author email address.
+ * @param string $date      MySQL time string.
+ * @param bool   $avoid_die When true, a disallowed comment will result in the function
+ *                          returning a WP_Error object, rather than executing wp_die().
+ *                          Default false.
+ * @return bool Whether comment flooding is occurring.
+ */
+function wp_check_comment_flood( $is_flood, $ip, $email, $date, $avoid_die = false ) {
+
     global $wpdb;
+
+    // Another callback has declared a flood. Trust it.
+    if ( true === $is_flood ) {
+        return $is_flood;
+    }
+
     // don't throttle admins or moderators
     if ( current_user_can( 'manage_options' ) || current_user_can( 'moderate_comments' ) ) {
-        return;
+        return false;
     }
     $hour_ago = gmdate( 'Y-m-d H:i:s', time() - HOUR_IN_SECONDS );
@@ -775 +843 @@
              */
             do_action( 'comment_flood_trigger', $time_lastcomment, $time_newcomment );
-
-            if ( wp_doing_ajax() )
-                die( __('You are posting comments too quickly. Slow down.') );
-
-            wp_die( __( 'You are posting comments too quickly. Slow down.' ), 429 );
+            if ( true === $avoid_die ) {
+                return true;
+            } else {
+                if ( wp_doing_ajax() ) {
+                    die( __('You are posting comments too quickly. Slow down.') );
+                }
+
+                wp_die( __( 'You are posting comments too quickly. Slow down.' ), 429 );
+            }
         }
     }
+
+    return false;
 }
 
@@ -1731 +1805 @@
  * @since 1.5.0
  * @since 4.3.0 'comment_agent' and 'comment_author_IP' can be set via `$commentdata`.
+ * @since 4.7.0 The `$avoid_die` parameter was added, allowing the function to
+ *              return a WP_Error object instead of dying.
  *
  * @see wp_insert_comment()
@@ -1754 +1830 @@
  *                                        'REMOTE_ADDR' in the `$_SERVER` superglobal sent in the original request.
  * }
- * @return int|false The ID of the comment on success, false on failure.
- */
-function wp_new_comment( $commentdata ) {
+ * @param bool $avoid_die Should errors be returned as WP_Error objects instead of
+ *                        executing wp_die()? Default false.
+ * @return int|false|WP_Error The ID of the comment on success, false or WP_Error on failure.
+ */
+function wp_new_comment( $commentdata, $avoid_die = false ) {
     global $wpdb;
 
@@ -1805 +1883 @@
     $commentdata = wp_filter_comment($commentdata);
 
-    $commentdata['comment_approved'] = wp_allow_comment($commentdata);
+    $commentdata['comment_approved'] = wp_allow_comment( $commentdata, $avoid_die );
+    if ( is_wp_error( $commentdata['comment_approved'] ) ) {
+        return $commentdata['comment_approved'];
+    }
 
     $comment_ID = wp_insert_comment($commentdata);
@@ -1819 +1900 @@
         $commentdata = wp_filter_comment( $commentdata );
 
-        $commentdata['comment_approved'] = wp_allow_comment( $commentdata );
+        $commentdata['comment_approved'] = wp_allow_comment( $commentdata, $avoid_die );
+        if ( is_wp_error( $commentdata['comment_approved'] ) ) {
+            return $commentdata['comment_approved'];
+        }
 
         $comment_ID = wp_insert_comment( $commentdata );
@@ -2941 +3025 @@
     );
 
-    $comment_id = wp_new_comment( wp_slash( $commentdata ) );
+    $comment_id = wp_new_comment( wp_slash( $commentdata ), true );
+    if ( is_wp_error( $comment_id ) ) {
+        return $comment_id;
+    }
+
     if ( ! $comment_id ) {
         return new WP_Error( 'comment_save_error', __( '<strong>ERROR</strong>: The comment could not be saved. Please try again later.' ), 500 );
@@ -2947 +3035 @@
 
     return get_comment( $comment_id );
-
-}
+}

trunk/src/wp-includes/default-filters.php

@@ -198 +198 @@
 add_filter( 'pre_kses',                 'wp_pre_kses_less_than'               );
 add_filter( 'sanitize_title',           'sanitize_title_with_dashes',   10, 3 );
-add_action( 'check_comment_flood',      'check_comment_flood_db',       10, 3 );
+add_action( 'check_comment_flood',      'check_comment_flood_db',       10, 4 );
 add_filter( 'comment_flood_filter',     'wp_throttle_comment_flood',    10, 3 );
 add_filter( 'pre_comment_content',      'wp_rel_nofollow',              15    );

trunk/tests/phpunit/tests/comment-submission.php

@@ -715 +715 @@
     }
 
+    /**
+     * @ticket 36901
+     */
+    public function test_submitting_duplicate_comments() {
+        $post = self::factory()->post->create_and_get( array(
+            'post_status' => 'publish',
+        ) );
+        $data = array(
+            'comment_post_ID' => $post->ID,
+            'comment'         => 'Did I say that?',
+            'author'          => 'Repeat myself',
+            'email'           => 'mail@example.com',
+        );
+        $first_comment = wp_handle_comment_submission( $data );
+        $second_comment = wp_handle_comment_submission( $data );
+        $this->assertWPError( $second_comment );
+        $this->assertSame( 'comment_duplicate', $second_comment->get_error_code() );
+    }
+
+    /**
+     * @ticket 36901
+     */
+    public function test_comments_flood() {
+        $post = self::factory()->post->create_and_get( array(
+            'post_status' => 'publish',
+        ) );
+        $data = array(
+            'comment_post_ID' => $post->ID,
+            'comment'         => 'Did I say that?',
+            'author'          => 'Repeat myself',
+            'email'           => 'mail@example.com',
+        );
+        $first_comment = wp_handle_comment_submission( $data );
+
+        $data['comment'] = 'Wow! I am quick!';
+        $second_comment = wp_handle_comment_submission( $data );
+
+        $this->assertWPError( $second_comment );
+        $this->assertSame( 'comment_flood', $second_comment->get_error_code() );
+    }
+
+    /**
+     * @ticket 36901
+     */
+    public function test_comments_flood_user_is_admin() {
+        $user = self::factory()->user->create_and_get( array(
+            'role' => 'administrator',
+        ) );
+        wp_set_current_user( $user->ID );
+
+        $post = self::factory()->post->create_and_get( array(
+            'post_status' => 'publish',
+        ) );
+        $data = array(
+            'comment_post_ID' => $post->ID,
+            'comment'         => 'Did I say that?',
+            'author'          => 'Repeat myself',
+            'email'           => 'mail@example.com',
+        );
+        $first_comment = wp_handle_comment_submission( $data );
+
+        $data['comment'] = 'Wow! I am quick!';
+        $second_comment = wp_handle_comment_submission( $data );
+
+        $this->assertNotWPError( $second_comment );
+        $this->assertEquals( $post->ID, $second_comment->comment_post_ID );
+    }
 }