Uploaded image for project: 'phpBB'
  1. phpBB
  2. PHPBB-17743

sync('topic') undercounts topic_posts_unapproved with mixed unapproved/re-approval posts, causing SQL error 1690 on approval

XMLWordPrintable

    • Icon: Bug Bug
    • Resolution: Unresolved
    • Icon: Major Major
    • None
    • 3.3.16, 3.3.19
    • None

      When a topic contains both a never-approved post (post_visibility = ITEM_UNAPPROVED) and an edited post awaiting re-approval (ITEM_REAPPROVE), sync('topic') stores only one of the two counts in topic_posts_unapproved instead of their sum.

      Subsequent approvals can attempt to decrement an already-zero unsigned counter. This produced SQL error 1690 on MariaDB in a production incident and in browser tests, leaving the approval partly completed.

      Fresh vanilla reproduction

      Reproduced through the browser on a fresh phpBB 3.3.19 installation using the official release package and installer, default prosilver, no enabled extensions, and a new database with correct starting counters. Runtime: PHP 8.4.25 and MariaDB 10.11.19.

      A no-edit control was completed successfully before performing the edit scenario. All topic creation, replies, edits and approvals were performed through the browser UI.

      The defect was also observed on production phpBB 3.3.19 and an earlier 3.3.16 clone. The fresh vanilla installation supplies the reproduction described below.

      Line numbers refer to phpBB 3.3.19.

      Cause

      In includes/functions_admin.php, sync(), 'topic' mode, line 2009:

      else if ($row['post_visibility'] == ITEM_UNAPPROVED || $row['post_visibility'] == ITEM_REAPPROVE)
      {
          $topic_data[$topic_id]['posts_unapproved'] = $row['total_posts'];
      }
      

      The query groups by (topic_id, post_visibility), so a topic containing both statuses returns two rows for this branch. The second assignment overwrites the first instead of adding the counts. Which count survives depends on row order.

      This is reached through ordinary posting/editing:

      • A user without f_noapprove edits their own post while it is still awaiting approval.
      • submit_post() changes its visibility from ITEM_UNAPPROVED to ITEM_REAPPROVE (functions_posting.php line 1727).
      • Because the old visibility is not ITEM_APPROVED, $is_starter is true (line 2261).
      • set_post_visibility() calls sync('topic', 'topic_id', $topic_id, true) (content_visibility.php line 570).

      The recursive forum recount derives forum totals from the topic totals, so forum_posts_unapproved can also become too low.

      Other topic recounts, including the ACP forum resync, use the same faulty counting code.

      The faulty assignment was also present on upstream 3.3.x and master when checked on 2026-10-15.

      Steps to reproduce

      1. In a test forum, use a normal user who can reply and edit their own posts. Set that user's forum permission "Can post without approval" to Never. Perform the edit within any configured editing time limit.
      2. As an admin, create an approved topic there.
      3. As the user, post two replies. Both enter the moderation queue; topic_posts_unapproved = 2.
      4. As the user, edit the second reply while it is still awaiting approval.
      5. topic_posts_unapproved is now 1, although two replies are awaiting approval.
      6. As a moderator, approve the edited reply in the MCP queue. It succeeds; topic_posts_unapproved = 0.
      7. Approve the other reply.

      Expected result

      After step 4, topic_posts_unapproved remains 2.

      Both approvals succeed, and the topic/forum counters remain consistent with the posts.

      Actual result

      Step 7 fails with "General Error".

      On the fresh vanilla board, the failing query was the forum-counter update:

      SQL ERROR [ mysqli ]
       
      BIGINT UNSIGNED value is out of range in '`<db>`.`phpbb_forums`.`forum_posts_unapproved` - 1' [1690]
       
      UPDATE phpbb_forums SET forum_posts_unapproved = forum_posts_unapproved - 1, forum_posts_approved = forum_posts_approved + 1 WHERE forum_id = 2
      

      In the production incident, the equivalent topic_posts_unapproved decrement failed instead. The production forum had other queued posts, so its forum counter had not yet reached zero.

      For these reply approvals, set_post_visibility() does not wrap the post and counter updates in a transaction. mcp_queue::approve_posts() performs logging and notification handling only after that function returns.

      In the fresh vanilla failure:

      • The post had already become approved and was verified publicly visible in a logged-out browser.
      • The topic stored 2 approved posts but actually contained 3.
      • The forum's approved count was also one lower than the actual number of approved posts.
      • There was no LOG_POST_APPROVED entry for the failed approval.
      • Its post_in_queue notification remained.

      The fresh vanilla failure occurred on MariaDB 10.11.19; the earlier failures occurred on MariaDB 10.11.18. Exact error wording and number can depend on the database and session sql_mode. MySQL and other DBMSs were not tested.

      Control and recorded counters

      Before the edit scenario, a separate control topic in the same forum was tested with the same user and two queued replies, neither edited.

      Both control replies were approved individually through the MCP. The topic ended with 3 approved / 0 pending posts, matching the actual posts. The forum counters also matched the actual posts before starting the edit scenario.

      For the reproduction topic:

      • Before the edit: stored 2 pending / actual 2 pending.
      • After editing reply 2: stored 1 pending / actual 2 pending; the replies had visibility values ITEM_UNAPPROVED and ITEM_REAPPROVE.
      • After approving the edited reply: stored 0 pending / actual 1 pending.
      • After attempting to approve the other reply: SQL error 1690, with the partial state described above.

      The four relevant PHP files on the fresh test board matched the official 3.3.19 package byte for byte: functions_admin.php, content_visibility.php, functions_posting.php and mcp_queue.php. No candidate patch was applied to this browser test.

      Suggested fix and isolated test

      posts_unapproved is initialised to zero for every topic earlier in sync() (line 1960), so accumulate the two status groups:

      $topic_data[$topic_id]['posts_unapproved'] += $row['total_posts'];
      

      An isolated test called the actual unmodified 3.3.19 sync() function using simulated grouped query results, with a no-op event dispatcher and no database connection.

      Mixed-status cases tested:

      • 1 unapproved + 1 re-approval
      • 5 unapproved + 1 re-approval
      • 1 unapproved + 5 re-approval

      Each was tested in both grouped-row orders.

      The stock code undercounted all six mixed-status cases. Four single-status controls counted correctly.

      Changing only the assignment from = to += made all ten cases count correctly.

      This validates the proposed counting correction in isolation. The patched code has not yet been tested through the full posting/approval workflow against a real database.

      Affected topic/forum counters will also need reconciliation after the counting fix is applied. Resyncing with the current code can recreate the undercount.

      Related, but a different issue on the editing path: PHPBB-13264.

            Unassigned Unassigned
            Macko Mac Ko
            Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

              Created:
              Updated: