Skip to:
Content

bbPress.org

Opened 5 days ago

Closed 4 days ago

Last modified 4 days ago

#3672 closed defect (bug) (fixed)

bbp_get_reply_position_raw() triggers PHP 8.3 deprecation: increment on bool

Reported by: dd32's profile dd32 Owned by: johnjamesjacoby's profile johnjamesjacoby
Milestone: 2.7 Priority: normal
Severity: normal Version: 2.1
Component: Component - Replies Keywords: has-patch php8 php83 deprecation
Cc:

Description

This ticket was AI generated

On PHP 8.3+, bbp_get_reply_position_raw() emits:

PHP Warning:  Increment on type bool has no effect, this will change in the next major version of PHP in
.../bbpress/includes/replies/functions.php on line 2374

(In trunk the warning shifts up one line because the operator is now pre-increment, but the bug is identical.)

The deprecation is exposing a latent silent-wrong-answer bug, not just a syntactic issue — see below.

Affected code

<?php
$topic_replies  = array_reverse( $topic_replies );
$reply_position = array_search( $reply_id, $topic_replies, true );

// Bump the position to compensate for the lead topic post
++$reply_position;

array_search() returns false when the reply isn't in the children
list returned by bbp_get_all_child_ids(). That list is built by
(see includes/common/functions.php:2067):

SELECT ID FROM wp_posts
WHERE post_parent = %d
  AND post_status NOT IN ('draft', 'future')
  AND post_type = %s
ORDER BY ID DESC

So array_search returns false whenever any of these are true:

  • The reply has post_status of draft or future — both excluded by the SQL.
  • The reply's post_parent doesn't match the $topic_id passed in (callers pass an explicit $topic_id that's stale, or the reply was moved).
  • A plugin filters the reply out via the bbp_get_all_child_ids filter (moderation plugins hiding pending replies, etc.).
  • Object-cache staleness for the bbpress_posts cache group.
  • $reply_id resolves to 0 or to a reply outside this topic.

In all of those, $reply_position is false, ++false is a deprecated no-op (fatal in PHP 9), and the function returns (int) false === 0 — silently claiming the reply is in lead-post position.

Steps to reproduce

  1. PHP 8.3+.
  2. Create a topic; create a reply; set the reply's post_status to draft (or use a plugin that filters bbp_get_all_child_ids to remove the reply).
  3. Call bbp_get_reply_position_raw( $reply_id, $topic_id ) (or hit any page that does — reply permalinks, pagination, anchor links).
  4. Observe deprecation in the error log.

Proposed patch

--- a/src/includes/replies/functions.php
+++ b/src/includes/replies/functions.php
@@
                      $topic_replies = bbp_get_all_child_ids( $topic_id, bbp_get_reply_post_type() );
                      if ( ! empty( $topic_replies ) ) {

                              // Reverse replies array and search for current reply position
                              $topic_replies  = array_reverse( $topic_replies );
                              $reply_position = array_search( $reply_id, $topic_replies, true );

-                             // Bump the position to compensate for the lead topic post
-                             ++$reply_position;
+                             // Bump the position to compensate for the lead topic post
+                             if ( false !== $reply_position ) {
+                                     ++$reply_position;
+                             } else {
+                                     $reply_position = 0;
+                             }
                      }

Preserves the current return-0 behaviour when the reply can't be located (no behavioural change for downstream consumers), while removing the deprecated bool-increment.

Environment

PHP 8.4.21, bbPress 2.7.0-alpha-2.

Change History (4)

This ticket was mentioned in PR #42 on bbpress/bbPress by @dd32.


5 days ago
#1

Fixes the PHP 8.1+ Implicit conversion from float to int loses precision deprecation emitted every time the Akismet history metabox renders for a topic or reply:

PHP Deprecated:  Implicit conversion from float 1779210785.780218 to int
loses precision in .../bbpress/includes/extend/akismet.php on line 994

## Root cause

BBP_Akismet::update_post_history() stores a fractional microtime() float in each _bbp_akismet_history event:

// This used to be akismet_microtime() but it was removed in 3.0
$mtime        = explode( ' ', microtime() );
$message_time = $mtime[1] + $mtime[0];

$event = array(
    'time' => $message_time,   // float, e.g. 1779210785.780218
    ...
);

The two readers — gmdate() and bbp_time_since() — both expect int and operate on whole seconds, so the sub-second precision is recorded, persisted to post meta forever, then thrown away on every read. PHP 8.1+ deprecates the implicit lossy float-to-int conversion at the read site.

The inlined explode(' ', microtime()) + add pattern is a relic from the pre-PHP-5.0 string-form of microtime() (the inline comment says "This used to be akismet_microtime() but it was removed in 3.0"). It was inlined into bbPress verbatim when Akismet 3.0 dropped the helper, but the rationale for sub-second precision didn't carry over to bbPress.

## Fix

Two changes that need to ship together:

  1. Write site — store time() (int) so new history rows don't have unused sub-second precision.
  2. Read sites — cast (int) on the two consumers (gmdate() and bbp_time_since()), so the deprecation also goes away for the millions of legacy float rows already in _bbp_akismet_history post meta.

Read-site-only would still create new float rows; write-site-only would still warn forever on existing data.

## Related

## Test environment

PHP 8.4.21, bbPress 2.7.0-alpha-2 / current trunk.

#3 @johnjamesjacoby
4 days ago

  • Owner set to johnjamesjacoby
  • Resolution set to fixed
  • Status changed from new to closed

In 7409:

Replies: avoid bool-increment deprecation in bbp_get_reply_position_raw()

Fixes the PHP 8.3+ Increment on type bool has no effect warning, guarding the increment behind a false !== $reply_position check, and explicitly resetting to 0 if not-found.

In trunk, for 2.7.

Props dd32.

Fixes: #3672.

From: https://github.com/bbpress/bbPress/pull/41

#4 @johnjamesjacoby
4 days ago

  • Milestone changed from Awaiting Review to 2.7
  • Version set to 2.1
Note: See TracTickets for help on using tickets.