invalid session over websocket: the client never got to know

Whoever connected with an expired session received an ordinary http response
with an html page inside. To the browser WebSocket API that is just a failed
upgrade, whose content it cannot read: the client never learned it had to go
back to the login, and retried forever with a red indicator. On xhr, instead,
the same case had always worked.

The root is in Transport::gettype(), which did not know "websocketsec": the
encrypted variant uses the same Transport_websocket class, told apart only in
the constructor (create() does treat them together). Missing from the list, it
fell back on Transport_iframe, that is on an html page.

Three places that contributed to the same symptom were fixed:

 - Transport::gettype() recognises websocketsec;
 - stream_fini() completes the handshake when the transport is websocket, so
   that the exit command arrives as a real message over an established
   connection, instead of as the body of a response nobody will read;
 - Transport_websocket::fini() frames the message, which used to go out raw.

On top of that, in the table path the farewell was built with $transp_type,
which is the transport guessed from the User-Agent and not the requested one:
for a websocket client it was "xhr".

Checked: with a session that does not exist the answer is now 101 Switching
Protocols with a text frame (0x81) carrying the command to go back to the
login and the close frame right after, both for websocket and for
websocketsec. With a valid session the stream stays open and receives its
frames as before.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014M1jiEq9cHdE5SE5j6vFuE

diff --git a/web/Obj/brisk.phh b/web/Obj/brisk.phh
index 08c58088c8c393a2f94cf4b68ad4aff06cdb46b5..b556210f376fe4043aa6f5af35231c3b6343825d 100644 (file)
--- a/web/Obj/brisk.phh
+++ b/web/Obj/brisk.phh
@@ -2814,7 +2814,8 @@ class Brisk
               if (!isset($cookie['sess'])
                   || (($user = $this->get_user($cookie['sess'], $idx)) == FALSE)) {
 
-                  $content = User::stream_fini($transp, $s_a_p->rndstr, TRUE);
+                  $content = User::stream_fini($transp, $s_a_p->rndstr, TRUE,
+                                               $header, $header_out, $enc);
 
                   $s_a_p->pendpage_try_addflush($new_socket, 20, $enc, $header_out, $content);
                   return TRUE;

diff --git a/web/Obj/transports.phh b/web/Obj/transports.phh
index 3e028d6d128c1efb6e6d8d6ca35552e7a5b29720..03b15cb839ff5d00b78a546b936613eec0333b80 100644 (file)
--- a/web/Obj/transports.phh
+++ b/web/Obj/transports.phh
@@ -540,7 +540,13 @@ class Transport_websocket {
 
     static function fini($init_string, $base, $blockerr)
     {
-        return (sprintf('@BEGIN@ %s window.onbeforeunload = null; window.onunload = null; document.location.assign("%sindex.php"); @END@',  ($blockerr ? 'xstm.stop(); ' : ''), $base).self::close());
+        /* The message has to be framed as a websocket frame, exactly like
+           every other one: before it went out raw, and a client receiving it
+           would not have known what to do with it. */
+        $tr = new Transport_websocket();
+        return ($tr->chunk(0, sprintf(' %s window.onbeforeunload = null; window.onunload = null; document.location.assign("%sindex.php"); ',
+                                      ($blockerr ? 'xstm.stop(); ' : ''), $base))
+                . self::close());
     }
 
     function is_chunked()
@@ -719,6 +725,17 @@ class Transport {
     }
     static function gettype($transp)
     {
+        /* websocketsec is the encrypted variant of the same transport and
+           the class is Transport_websocket, which tells the two apart in its
+           constructor (see create() above, which indeed treats them
+           together). Missing from this list, every "websocketsec" fell back
+           on Transport_iframe, that is on an html page: the client that had
+           asked for a websocket upgrade got an html page in reply, which its
+           API cannot even read, and never learned it had to go back to the
+           login. */
+        if ($transp == 'websocketsec') {
+            return "Transport_websocket";
+        }
         if ($transp == 'websocket' || $transp == 'xhr' || $transp == 'htmlfile') {
             return "Transport_".$transp;
         }

diff --git a/web/Obj/user.phh b/web/Obj/user.phh
index 495e61a3ed1c0ec2abd9d2c5a35878e4fd78c12a..4c13325f9a1379b8e5f8711b7bdcb01df4256731 100644 (file)
--- a/web/Obj/user.phh
+++ b/web/Obj/user.phh
@@ -801,12 +801,28 @@ class User {
       return ($ret);
   }  //   function maincheck(...
 
-  public static function stream_fini($transp, $init_string, $is_unrecoverable)
+  /* $header and $header_out are only needed by the websocket transports: see
+     below. Whoever does not pass them gets the previous behaviour. */
+  public static function stream_fini($transp, $init_string, $is_unrecoverable,
+                                     $header = NULL, &$header_out = NULL, $enc = 'plain')
 {
     // printf("xXx user::stream_fini\n");
 
     // FIXME: dynamic "Transport_" type
     $trans_class = Transport::gettype($transp);
+
+    /* The client speaking websocket asked for an upgrade. Answering with an
+       ordinary http response, its API only sees a failure and cannot read the
+       content: it never learns that the session is not valid, so it does not
+       go back to the login and retries forever (red indicator and a loop of
+       reopenings). The handshake is therefore completed, so that the exit
+       command reaches it as a real message, and right after that the
+       connection is closed. */
+    if (($transp == 'websocket' || $transp == 'websocketsec') && $header !== NULL) {
+        $tr = Transport::create($transp);
+        $tr->init($enc, $header, $header_out, $init_string, self::base_get(), 0);
+    }
+
     $body = $trans_class::fini($init_string, self::base_get(), static::blocking_error($is_unrecoverable));
 
     // ELSE IF XHR THEN:

diff --git a/web/briskin5/Obj/briskin5.phh b/web/briskin5/Obj/briskin5.phh
index c8bbd6f6276ddba47ed53ce658fac3bcb8a2ba0b..86bd77dfd024f0afee9a2003ffc7d8c9589d5960 100644 (file)
--- a/web/briskin5/Obj/briskin5.phh
+++ b/web/briskin5/Obj/briskin5.phh
@@ -1532,7 +1532,13 @@ class Bin5 {
                     || ($bri = $s_a_p->app->match_get($table_idx, $table_token)) == NULL
                     || (($user = $bri->get_user($cookie['sess'], $idx)) == FALSE)) {
 
-                    $content = Bin5_user::stream_fini($transp_type, $s_a_p->rndstr, TRUE);
+                    /* it used to be $transp_type, which here is not the
+                       requested transport: the right value is $transp, read
+                       from the query a few lines above. With the wrong one the
+                       client got its farewell in the format of another
+                       transport. */
+                    $content = Bin5_user::stream_fini($transp, $s_a_p->rndstr, TRUE,
+                                                      $header, $header_out, $enc);
                     $s_a_p->pendpage_try_addflush($new_socket, 20, $enc, $header_out, $content);
 
                     return TRUE;