Skip to content

[3.0] Clear the topic and board statics instead of unsetting them - #9311

Open
albertlast wants to merge 1 commit into
SimpleMachines:release-3.0from
albertlast:fix/enforce-bans-unset-static-props
Open

[3.0] Clear the topic and board statics instead of unsetting them#9311
albertlast wants to merge 1 commit into
SimpleMachines:release-3.0from
albertlast:fix/enforce-bans-unset-static-props

Conversation

@albertlast

Copy link
Copy Markdown
Collaborator

Note

This change was produced by an LLM. The code, the commit message and this
description were all written by Claude (Anthropic), driven by @albertlast. It has
not yet had human code review.

Everything stated below was verified by actually running it against a fresh
PostgreSQL install of this branch, rather than only reasoned about. Even so,
please review it as untrusted work: the diagnosis may be right while the fix is
not what SMF would prefer stylistically or architecturally.

Description

User::enforceBans() calls unset() on four static properties when showing a
banned member as sitting on the board index:

unset(Topic::$topic_id, Topic::$info, Board::$board_id, Board::$info);

PHP does not allow that:

Attempt to unset static property SMF\Topic::$topic_id

The Error is thrown before the ban message is built, so a banned visitor gets the
generic "An error has occurred" page instead of being told why they are banned and
when it expires.

This assigns null instead, which is what SMF\ServerSideIncludes already does for
the same properties. All four are nullable (Topic::$topic_id and Topic::$info are
untyped, Board::$board_id is ?int, Board::$info is ?self).

How this was verified

Inserted a real cannot_access ban on the requesting IP, on PostgreSQL 17 / PHP 8.4:

before after
HTTP status 500 403
Ban reason shown no yes
Expiry shown no yes
Errors logged 1 per request 0

One caveat while reviewing: the ban page title is still "An error has occurred",
because ban messages go through the generic fatal error template. That is existing
behaviour and is not changed here. The reliable check that the real ban page is
rendering is whether the ban reason appears — the string "you are banned from
using this forum" is also present as a JavaScript variable on every page, so grepping
for it gives a false positive.

Relationship to other PRs

This is only reachable once bans work at all, so it should be reviewed together with
(or after) #9310 — without that fix every visitor is treated as banned and this code
path fires constantly. Merges cleanly alongside it.

Issues References (Fixes|Related|Closes)

  1. Related: [3.0] Only enforce bans that actually exist #9310

enforceBans() called unset() on four static properties when showing a
banned member as sitting on the board index. PHP does not allow that:

  Attempt to unset static property SMF\Topic::$topic_id

The Error was thrown before the ban message could be built, so a banned
visitor got the generic error page rather than being told why they were
banned and when it expires.

Assigns null instead, which is what SMF\ServerSideIncludes already does
for the same properties. All four are nullable.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: albertlast <mathiaspapealbert@hotmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant