Skip to content

[3.0] Stop logging an error for every personal message search - #9465

Open
albertlast wants to merge 1 commit into
SimpleMachines:release-3.0from
albertlast:3.0/pm-search-start-key
Open

[3.0] Stop logging an error for every personal message search#9465
albertlast wants to merge 1 commit into
SimpleMachines:release-3.0from
albertlast:3.0/pm-search-start-key

Conversation

@albertlast

Copy link
Copy Markdown
Collaborator

Description

Searching your personal messages puts a row in the error log every time:

2: Undefined array key "start"
url: /index.php?action=pm;sa=search2

Search::performSearch() reads the start twice, and only one of the two guards itself:

$start = (int) ($_GET['start'] ?? 0);
Utils::$context['page_index'] = new PageIndex(..., $start, ...);

// If the supplied start value was invalid, redirect to the correct one.
if ($_GET['start'] != $start) {

A search arrives without a start — only the page links a member clicks afterwards carry one — so the second read fires on the first search anyone runs.

The value is already on hand as $this->start, read once and guarded when the search was set up, so this uses that instead of reading $_GET a third time. PageIndex takes its start by reference and clamps it, which is what makes the comparison below meaningful, so it gets a copy and the original stays to compare against. That is the same shape ItemList::buildPageIndex() uses.

Checked that the redirect still does its job:

request lands on log
sa=search2 (POST, no start) sa=search2 (empty)
sa=search2;start=0 sa=search2;start=0 (empty)
sa=search2;start=999 …;start=0 (empty)
sa=search2;start=-5 …;start=0 (empty)

Same one result found in every case.

Issues References (Fixes|Related|Closes)

n/a

Searching your personal messages put a row in the error log every time:

    2: Undefined array key "start"
    /index.php?action=pm;sa=search2

Search::performSearch() reads the start twice. The first read guards
itself, the second does not, and a search arrives without one - only the
page links a member clicks afterwards carry it.

The value is already on hand as $this->start, read once and guarded when
the search was set up, so use that. PageIndex takes its start by reference
and clamps it, which is the whole point of the comparison below, so it
gets a copy and the original stays around to compare against.

Out of range starts still redirect: start=999 and start=-5 both come back
as ;start=0. A plain search adds nothing to the log now.

Signed-off-by: Mathias Albert <mathiaspapealbert@hotmail.com>
Signed-off-by: albertlast <mathiaspapealbert@hotmail.com>
@jdarwood007 jdarwood007 added the PM label Aug 9, 2026
@jdarwood007 jdarwood007 added this to the 3.0 Alpha 6 milestone Aug 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants