Skip to content

Validate the UCP location and number preferences - #209

Merged
avandenberghe merged 1 commit into
develop33from
issue-198-validate-ucp-prefs
Oct 4, 2026
Merged

avandenberghe merged 1 commit into
develop33from
issue-198-validate-ucp-prefs

Conversation

@avandenberghe

Copy link
Copy Markdown
Collaborator

Closes #198.

Problem
ucp_prefs_get_data() copied rt_location, rt_viewforum_location and rt_number from the request unchecked, and ucp_prefs_set_data() saved them. Any string up to the column length and any integer could be stored. An unknown location just hid the user's own Recent Topics block.

Fix (event/ucp_listener.php)

  • Locations: each location must be one of the options the UCP offers: RT_TOP/RT_BOTTOM/RT_SIDE for the index, RT_TOP/RT_BOTTOM for the forum view (no side column there). Anything else falls back to the user's stored value, then the board default. This is done in a small valid_location() helper, with the option sets as class constants.
  • Number: rt_number is clamped to 1–999.
  • Form: the UCP form's min goes from 0 to 1, matching the server-side bound. 0 only produced an empty block.

The values are checked where they're read, so both the form display and the save get clean values.

Tests

  • New in tests/event/ucp_listener_test.php, documented in tests/tests.md:
    • a data-provider test: valid values pass through; an unknown location; RT_SIDE as a forum-view location; numbers 100000, 0 and -5. All five invalid cases failed before the fix.
    • an invalid stored location falls back to the board default.
  • Unit suite: 55 tests, 0 failures, run locally. The functional tests run in CI.

Checked on the local board (values posted through phpBB's real request object, admin user; this only computes the would-be sql_ary, nothing was saved):

Posted current develop33 saves this PR saves
RT_SIDE, RT_BOTTOM, 20 RT_SIDE, RT_BOTTOM, 20 RT_SIDE, RT_BOTTOM, 20
<script>, RT_SIDE, 100000 &lt;script&gt;, RT_SIDE, 100000 RT_SIDE, RT_TOP, 999
RT_EVIL, x, -5 RT_EVIL, x, -5 RT_SIDE, RT_TOP, 1

Not in this PR: the ACP settings for the same three values (acp/recenttopics_module.php) are saved unvalidated too. That's admin-only input and a separate change.

No changelog entry or version bump, per this repo's convention.

🤖 Generated with Claude Code

ucp_prefs_get_data() copied rt_location, rt_viewforum_location and
rt_number from the request unchecked, and ucp_prefs_set_data() saved
them. Any string up to the column length and any integer could be
stored; an unknown location just hid the user's own block.

Keep each location within the options the UCP offers (RT_TOP, RT_BOTTOM,
RT_SIDE; no side column in the forum view), falling back to the user's
stored value and then the board default. Clamp the number to 1-999, and
set the form's min to 1 to match.

Closes #198

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@avandenberghe
avandenberghe merged commit 7652676 into develop33 Oct 4, 2026
4 checks passed
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.

UCP location/number preferences have no server-side whitelist or bound

1 participant