Skip to:
Content

bbPress.org

Opened 2 years ago

Closed 7 months ago

Last modified 3 months ago

#3580 closed defect (bug) (fixed)

delete_orphaned_spam_meta() has a bug that causes it to loop forever whenever it runs

Reported by: terresquall's profile terresquall Owned by: johnjamesjacoby's profile johnjamesjacoby
Milestone: 2.6.14 Priority: high
Severity: major Version: 2.6.9
Component: Extend - Akismet Keywords: has-patch commit
Cc:

Description

The delete_orphaned_spam_meta() function inside bbpress/includes/extend/akismet.php will run forever when there are orphaned post meta keys, because the query keeps searching for orphaned post meta keys by batches of 1000 and deleting them. But there is a check to ignore meta keys that do not start with akismet_.

What happens then is that, if you have any orphaned post meta keys that do not start with 'akismet_', the method will run forever until the max_execution_time expires, because the query finds the rows, but is unable to delete it.

I propose to add the 'akismet_' check into the SQL query, like so:

AND m.meta_key LIKE 'akismet\\_%'

And removing the following check:

// Skip if not an Akismet key
if ( 'akismet_' !== substr( $spam_meta->meta_key, 0, 8 ) ) {
        continue;
}

This will prevent the function from looping forever.

Also, since this function finds orphaned bbp_ post meta too, we may want to add this so that it finds both orphaned Akismet and BBP post metas:

AND m.meta_key LIKE 'akismet\\_%' OR m.meta_key LIKE '\\_bbp\\_%')

Here is the final edited code:

public function delete_orphaned_spam_meta() {
        global $wpdb;

        // Get the deletion limit
        $delete_limit = $this->get_delete_limit( '_bbp_akismet_delete_spam_orphaned_limit' );

        // Default last meta ID
        $last_meta_id = 0;

        // Start time (float)
        $start_time = isset( $_SERVER['REQUEST_TIME_FLOAT'] )
                ? (float) $_SERVER['REQUEST_TIME_FLOAT']
                : microtime( true );

        // Maximum time
        $max_exec_time = (float) max( ini_get( 'max_execution_time' ) - 5, 3 );

        // Setup the query
        $sql = "SELECT m.meta_id, m.post_id, m.meta_key FROM {$wpdb->postmeta} as m LEFT JOIN {$wpdb->posts} as p ON m.post_id = p.ID WHERE p.ID IS NULL AND m.meta_id > %d AND (m.meta_key LIKE 'akismet\\_%' OR m.meta_key LIKE '\\_bbp\\_%' ORDER BY m.meta_id LIMIT %d";

        // Query loop of topic & reply IDs
        while ( $spam_meta_results = $wpdb->get_results( $wpdb->prepare( $sql, $last_meta_id, $delete_limit ) ) ) {

                // Exit loop if no spam IDs
                if ( empty( $spam_meta_results ) ) {
                        break;
                }

                // Reset queries
                $wpdb->queries = array();

                // Reset deleted meta count
                $spam_meta_deleted = array();

                // Loop through each of the metas
                foreach ( $spam_meta_results as $spam_meta ) {

                        // Skip if not an Akismet key
                        __if ( 'akismet_' !== substr( $spam_meta->meta_key, 0, 8 ) ) {
                                continue;
                        }__

                        // Delete the meta
                        delete_post_meta( $spam_meta->post_id, $spam_meta->meta_key );

                        /**
                         * Perform a single action on the single topic/reply ID for
                         * simpler batch processing.
                         *
                         * @param string The current function.
                         * @param int    The current topic/reply ID.
                         */
                        do_action( '_bbp_akismet_batch_delete', __FUNCTION__, $spam_meta );

                        // Stash the meta ID being deleted
                        $spam_meta_deleted[] = $last_meta_id = $spam_meta->meta_id;
                }

                /**
                 * Single action that encompasses all topic/reply IDs after the
                 * delete queries have been run.
                 *
                 * @param int   Count of spam meta IDs
                 * @param array Array of spam meta IDs
                 */
                do_action( '_bbp_akismet_delete_spam_meta_count', count( $spam_meta_deleted ), $spam_meta_deleted );

                // Break if getting close to max_execution_time.
                if ( ( microtime( true ) - $start_time ) > $max_exec_time ) {
                        break;
                }
        }

        // Maybe optimize
        $this->maybe_optimize_postmeta();
}

Change History (6)

This ticket was mentioned in PR #14 on bbpress/bbPress by terresquall.


2 years ago
#1

  • Keywords has-patch added; needs-patch removed

#2 @terresquall
2 years ago

I've added a pull request for this.

#4 @johnjamesjacoby
7 months ago

  • Keywords commit added
  • Milestone changed from Awaiting Review to 2.6.14
  • Owner set to johnjamesjacoby
  • Status changed from new to assigned

#5 @johnjamesjacoby
7 months ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 7324:

Extend - Akismet: avoid timeouts in the delete_orphaned_spam_meta() method.

This change modifies the SQL used to query for Akismet post-meta keys, such that it will only retrieve rows where meta_key is like akismet_.

This results in a less-optimized database query, but circumvents a bug where unnecessary rows were being looped through in a way that would never finish.

Props terresquall.

In branches/2.6, for 2.6.14.

Fixes #3580.

#6 @johnjamesjacoby
7 months ago

In 7325:

Extend - Akismet: avoid timeouts in the delete_orphaned_spam_meta() method.

This change modifies the SQL used to query for Akismet post-meta keys, such that it will only retrieve rows where meta_key is like akismet_.

This results in a less-optimized database query, but circumvents a bug where unnecessary rows were being looped through in a way that would never finish.

Props terresquall.

In trunk, for 2.7.

Fixes #3580.

Note: See TracTickets for help on using tickets.