Skip to content

[3.0] Keep Stringable arguments instead of dropping them - #9409

Merged
Sesquipedalian merged 2 commits into
SimpleMachines:release-3.0from
albertlast:3.0/formatter-stringable-args
Aug 14, 2026
Merged

[3.0] Keep Stringable arguments instead of dropping them#9409
Sesquipedalian merged 2 commits into
SimpleMachines:release-3.0from
albertlast:3.0/formatter-stringable-args

Conversation

@albertlast

Copy link
Copy Markdown
Collaborator

Description

Admin → Members prints the literal text {member_ip} in the IP column, inside a link to ?action=trackip;searchip={member_ip}.

Localization\MessageFormatter::formatMessage() hands its arguments to intl like this:

$fmt = self::$message_formatters[…][$message]->format(array_filter($args, 'is_scalar'));

array_filter($args, 'is_scalar') throws away every object. That is the right call for an array or a plain object — intl fatals on stdClass with "could not be converted to string" — but a Stringable converts perfectly well. Dropping it means the argument never arrives, and ICU leaves the placeholder in the output rather than substituting anything.

Actions/Admin/Members.php::list_getMembers() puts an SMF\IP object in the row:

$row['member_ip'] = new IP($row['member_ip']);

and SMF\IP implements \Stringable. So the column's format_text never got its value.

Stringables are now converted to strings just before the MessageFormat escaping that already runs over string arguments, so one containing {, } or ' is protected the same as any other string, and the non-intl fallback path further down gets the same value.

Checked

On the running forum, Admin → Members:

before   <a href="…?action=trackip;searchip={member_ip}">{member_ip}</a>
after    <a href="…?action=trackip;searchip=172.19.0.1">172.19.0.1</a>

Ten pages re-rendered afterwards — board index, message index, stats, member list, recent, admin home, member list, error log, moderation centre, profile — no fatals and no other change.

The alternative

Members.php could instead declare the column as 'member_ip' => true, which makes ItemList do htmlspecialchars((string) $value) before formatting. That fixes this one column and leaves the trap in place for the next Stringable someone passes, so I fixed the formatter. Happy to do it the other way if you would rather keep formatMessage() strict.

Found by sweeping every rendered page of a stock forum for unsubstituted {placeholders}, alongside #9407 and #9408.

Issues References (Fixes|Related|Closes)

Related to #7933

formatMessage() hands its arguments to intl as

	->format(array_filter($args, 'is_scalar'))

which throws away objects. That is right for an array or a plain object,
which intl cannot use, but a Stringable converts perfectly well, and dropping
it means the argument never arrives and ICU leaves the placeholder sitting in
the output.

The member list is where this shows: Admin -> Members lists an SMF\IP object
for member_ip, so the IP column rendered the literal text {member_ip}, inside
a link to ?action=trackip;searchip={member_ip}. It now reads 172.19.0.1 and
links to that address.

Stringables are converted before the MessageFormat escaping just above, so
one containing a brace or an apostrophe is protected the same as any other
string, and the non-intl fallback path gets the same value.

Signed-off-by: Mathias Papenbrock <mathiaspapealbert@hotmail.com>
Signed-off-by: albertlast <mathiaspapealbert@hotmail.com>
@albertlast albertlast mentioned this pull request Aug 8, 2026
@jdarwood007 jdarwood007 added the Localization Language & internationalization label Aug 8, 2026
@jdarwood007 jdarwood007 added this to the 3.0 Alpha 5 milestone Aug 8, 2026
Comment thread Sources/Localization/MessageFormatter.php Outdated
@Sesquipedalian
Sesquipedalian merged commit 63f64fd into SimpleMachines:release-3.0 Aug 14, 2026
4 checks passed
live627 pushed a commit that referenced this pull request Aug 31, 2026
A second sweep of the bug fixes now on release-3.0, in the same spirit as
#9511: ask of each one whether the suite can reach it, and write a test where
it can. Everything merged since that sweep was looked at, along with the
backlog that landed in one batch on the 29th and 30th. Most of it is templates,
JavaScript, or PHP that wants Db::$db or User::$me. Four fixes do not.

#9484 made SMF\Unicode\SpoofDetector::checkReservedName() split the admin's
list on the two characters backslash and n as well as on a real newline. The
installer writes the default list with the separators spelled out that way, so
splitting on newlines alone gave one long name nobody would type and every
reserved name was free to register.

#9409 made SMF\Localization\MessageFormatter::formatMessage() flatten a
\Stringable argument to its string value. The class skips any argument that is
not already a string and hands the intl formatter only the scalar ones, so an
object argument reached neither and the member was shown the placeholder.

#9453 made SMF\PageIndex remember, across __toString(), that the start value it
was handed was out of bounds. fixStart() records that as a side effect of
clamping, and __toString() called it again on a value already clamped, so the
verdict was always thrown away: page 1 came out as plain text rather than a
link, with a "next page" link beside it.

#9440 and #9442 both concern a gallery avatar, which is stored as a path under
the avatars directory rather than as a URL. Read as a URL, it was worked back
to a file from the URL's path, which lands outside the avatar directories; and
on a forum at the root of its domain that path is null, so stripping the board
URL off it threw a TypeError on every page the member appeared on.

Each set was run against the code as it was before its fix, by checking out the
single source file at the commit before the merge:

- SpoofDetector.php before #9484: one failure, the installer's list.
- MessageFormatter.php before #9409: three failures, all the \Stringable cases.
  The plain string, the number and the no-placeholder message pass either side.
- PageIndex.php before #9453: two failures. The four tests covering an ordinary
  start pass either side, which is what makes them the control.
- Avatar.php before #9440: five of six fail, the root-of-domain cases with the
  TypeError and the subdirectory ones by falling through to default.png. With
  #9440 but not #9442, four still fail: every gallery avatar becomes the
  default image.

202 tests, 295 assertions, still under a second.

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

Localization Language & internationalization

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants