[3.0] Build a Time before the user is loaded

[albertlast]

Description

A direct link to a post (?msg=6000, or msgs/6000) is a 500 when queryless URLs are turned on:

PHP Fatal error:  Uncaught Error: Typed static property SMF\User::$me must not be accessed before initialization in Sources/Time.php:191
#0 Sources/Time.php(773): SMF\Time->__construct()
#1 Sources/Topic.php(1982): SMF\Time::create()
#2 Sources/Topic.php(827): SMF\Topic->loadTopicInfo()
#3 Sources/Topic.php(1831): SMF\Topic::load()
#4 Sources/QueryString.php(502): SMF\Topic::buildRoute()
...
#10 Sources/QueryString.php(167): SMF\QueryString::redirectFromMsg()
#11 Sources/Forum.php(413): SMF\QueryString::cleanRequest()

The chain is: cleanRequest() calls redirectFromMsg(), which looks up the topic the message is in and redirects to it. Utils::redirectexit() then runs the target through QueryString::rewriteAsQueryless(), which asks Topic::buildRoute() for the route, and on a slug cache miss that loads the topic. All of this happens in Forum::__construct(), well before User::loadMe().

Topic::loadTopicInfo() is already written to survive being called without a user — it guards User::$me with isset() in four places. The one thing it does unguarded is build a Time for the topic's start date, and Time::__construct() reads User::$me->timezone. User::$me is a typed static with no default, so reading it before it is assigned is a fatal error rather than an empty value.

The read now falls through to the forum's default time zone, and then to PHP's, rather than throwing.

Fixing it in Time rather than in Topic::loadTopicInfo() covers the general case, and there is at least one other case: cron.php never loads a user at all — TaskRunner sets User::$sc and nothing else — so Tasks\PaidSubs.php:104, which builds a Time for the reminder email's END_DATE, was reaching the same fatal by a different road.

Verified on the Docker environment with queryless_urls enabled: ?msg=6000 and msgs/6000 both 302 to topics/<slug>-1187/msg6000#msg6000 and render, and smf_log_errors stays empty.

tests/Unit/TimeTest.php is new. Its regression test errors with the reported fatal before the change and passes after it. Note that Time caches the zone it works out in Time::$user_tz, and a typed static cannot be put back into its uninitialised state, so only the first Time built in a process exercises this path — the test asserts that precondition explicitly, so that if something later builds a Time earlier it fails rather than quietly passing while testing nothing.

Issues References (Fixes|Related|Closes)

1. Fixes [3.0]: Direct link to post doesn't work #9634

🤖 Generated with Claude Code

[Sesquipedalian]

Instead of all these changes, it would be better and cleaner to simply replace this:

		if (!isset(self::$user_tz)) {
			self::$user_tz = TimeZone::create(User::$me->timezone);
		}

... with this:

		if (!isset(self::$user_tz)) {
			self::$user_tz = TimeZone::create(User::$me?->timezone ?? Config::$modSettings['default_timezone'] ?? date_default_timezone_get());
		}

The secret sauce here is the introduction of the nullsafe operator (i.e. User::$me?->timezone instead of User::$me->timezone). When that is used, User::$me?->timezone simply evaluates to null rather than throwing an exception when User::$me is not set. That allows the null coalescing operator chain to proceed on its merry way.

[albertlast]

Taken — that's much better, thanks. Pushed as a one-line change; the helper method is gone.

Two notes from checking it, neither of them an argument against the suggestion.

?-> isn't the part doing the work here. Plain ?? already survives an uninitialised typed static, because the whole chain compiles down the isset path:

class U { public string $timezone = 'Asia/Tokyo'; }
class A { public static U $me; }

var_dump(A::$me?->timezone ?? 'fallback');  // string(8) "fallback"
var_dump(A::$me->timezone ?? 'fallback');   // string(8) "fallback"

Only a bare read throws — which is why User::$me->id ?? 0 on Topic.php:1936 ran fine in the reported trace and the fatal landed later on User::$me->timezone. I've kept the ?-> since it says the intent out loud, but noting it in case the "it throws without nullsafe" belief shapes a fix somewhere else.

cron.php was hitting this too. TaskRunner never sets User::$me at all, and Tasks\PaidSubs.php:104 builds a Time for the reminder email's END_DATE. So the same fatal was reachable without going near a URL. Your version fixes that as well.

The one behavioural difference from what I first pushed is that the fallback is now cached in Time::$user_tz for the rest of the process. In core that costs nothing — the only pre-user Time in a request that keeps going would have to come from a mod on integrate_actions / integrate_pre_log_stats / integrate_guest_actions (or integrate_SSI), since redirectFromMsg() ends in a redirect and cron has no user to get wrong. It does constrain the test, though: a typed static can't be put back into its uninitialised state, so only the first Time built in a process exercises this path. The regression test therefore asserts that precondition explicitly, so that if something later builds a Time earlier it fails rather than quietly passing while testing nothing, and tearDown() hands the zone back to the suite.