diff --git a/devel/manual/sessions.md b/devel/manual/sessions.md index 7bb4ade261..6e0df56324 100644 --- a/devel/manual/sessions.md +++ b/devel/manual/sessions.md @@ -38,7 +38,9 @@ encoding](https://www.php.net/manual/en/function.session-encode.php). the email address of the first account to log in, `us[1]` the second, and so forth. A URL like `https://conf.hotcrp.com/u/N/` accesses the given conference using account `N`. If `us` is not set but `u` is, then `us` - defaults to the one-element list `[u]`. + defaults to the one-element list `[u]`. If an account is removed from an + active session, but the session remains active, then that entry in `us` + is set to an empty string. * `uchoice` (associative array): Account choice @@ -46,7 +48,7 @@ encoding](https://www.php.net/manual/en/function.session-encode.php). the most relevant account for each conference. On receiving a request for a plain conference URL (e.g., `https://conf.hotcrp.com/` rather than `https://conf.hotcrp.com/u/1/`), HotCRP looks up the best user for that - conference using `uchoice` and redirect appropriately. + conference using `uchoice` and redirects appropriately. `uchoice` is keyed by conference session key. The value for a conference is a two-element list `[USERINDEX, USETIME]`. `USERINDEX` is the most diff --git a/lib/qsession.php b/lib/qsession.php index c9477bed37..12ca5e2b06 100644 --- a/lib/qsession.php +++ b/lib/qsession.php @@ -9,7 +9,7 @@ class Qsession { /** @var bool */ protected $sopen = false; /** @var 0|1|2 */ - protected $opentype = 0; + private $opentype = 0; function maybe_open() { if (!$this->sopen && isset($_COOKIE[session_name()])) { @@ -34,10 +34,10 @@ function reopen() { } function handle_open() { - if ($this->sopen && $this->opentype !== 1) { + if ($this->opentype !== 1 && $this->sopen) { return; } - if ($this->sid !== null && $this->opentype === 2) { + if ($this->opentype === 2 && $this->sid !== null) { $sid = $this->start($this->sid); assert($sid === $this->sid); return; @@ -58,16 +58,55 @@ function handle_open() { $this->sopen = true; } - // if reopened, empty [session fixation], or old, reset session - $curv = $this->all(); - if (empty($curv) - || ($curv["deletedat"] ?? Conf::$now) < Conf::$now - 30) { - $this->opentype = 1; + // reset session while reopened, empty [session fixation], or deleted + if ($this->sid === $cookie_sid) { + $this->check_reopen(); + if ($this->sid === null) { + return; + } } - if ($this->sid === $cookie_sid && $this->opentype === 1) { - $nsid = $this->new_sid(); - if (!isset($curv["deletedat"])) { + + // maybe update session format + if (($this->get("v") ?? 0) < 2) { + if (empty($this->all())) { + $this->set("v", 2); + } else { + UpdateSession::run($this); + } + } + if ($this->get("u") || $this->sid !== $cookie_sid) { + $this->refresh(); + } + } + + private function check_reopen() { + $tries = 0; + while (true) { + $curv = $this->all(); + if (($this->opentype !== 1 || $tries > 0) + && (!empty($curv) || $tries > 0) + && !isset($curv["deletedat"])) { + return; + } + ++$tries; + + $nsid = null; + if (isset($curv["deletedat"])) { + $transfer = false; + if ($curv["deletedat"] >= Conf::$now - 30 + && isset($curv["new_sid"]) + && is_string($curv["new_sid"]) + && $tries < 10) { + $nsid = $curv["new_sid"]; + } + } else { + $transfer = !empty($curv); + } + + $nsid = $nsid ?? $this->new_sid(); + if ($transfer) { $this->set("deletedat", Conf::$now); + $this->set("new_sid", $nsid); } $this->commit(); @@ -77,23 +116,16 @@ function handle_open() { return; } $this->sopen = true; - unset($curv["deletedat"]); - foreach ($curv as $k => $v) { - $this->set($k, $v); - } - } - // maybe update session format - if (empty($this->all())) { - $this->set("v", 2); - } else { - if (($this->get("v") ?? 0) < 2) { - UpdateSession::run($this); + if ($transfer) { + // `unset` should be a no-op, because we never transfer data + // from a deleted session: + unset($curv["deletedat"], $curv["new_sid"]); + foreach ($curv as $k => $v) { + $this->set($k, $v); + } } } - if ($this->get("u") || $cookie_sid !== $this->sid) { - $this->refresh(); - } } diff --git a/src/updatesession.php b/src/updatesession.php index 09ffef703f..c744efd786 100644 --- a/src/updatesession.php +++ b/src/updatesession.php @@ -146,6 +146,7 @@ static function usec_query(Qrequest $qreq, $email, $type, $reason, $bound = 0) { * @param 0|1 $reason * @param bool $success */ static function usec_add(Qrequest $qreq, $email, $type, $reason, $success) { + // See `etc/devel/sessions.md` for format information $uindex = Contact::session_index_by_email($qreq, $email); $old_usec = $qreq->gsession("usec") ?? []; $nold_usec = count($old_usec);