Skip to content

Commit

Permalink
[fix] [#260] NB Prevent unnecessary participation of Ajax channels in…
Browse files Browse the repository at this point in the history
… `conns_` during handshake

Before this commit:

  Server channels (schs) for Ajax GET requests destined for handshaking are
  unnecessarily added to `conns_` before being closed to issue handshake.

  Sente presumed that once used, such schs would be permanently closed and
  all future send attempts against the sch would just fail.

  But as it turns out, @osbert identified [1][2] that at least http-kit may
  re-use (and re-open) schs for unrelated future requests.

  This could lead to cases like the following:

    - Handshake `req1` comes in, `sch1` gets added to `conns_` then closed.
    - `sch1` gets re-used (and re-opened) by http-kit for unrelated `req2`.
    - Before `req2` can respond, a broadcast call is made against `sch1`,
      which is still in `conns_`.

  The effect: random broadcast data incorrectly being sent to an unrelated
  request.

After this commit:

  Server channels (schs) for Ajax GET requests destined for handshaking are
  never added to `conns_`, preventing the possibility of handshake schs from
  being held for broadcast after closing.

  Instead, `conns_` contains only schs intended for long-polling (broadcast),
  and atomically removed from `conns_` on first use+close.

  This should prevent the issue described above.

  Separately, a change [2] is also being introduced upstream to http-kit for
  server-side prevention of this kind of unintentional re-use of channels.

[1] #260
[2] http-kit/http-kit#375
  • Loading branch information
ptaoussanis committed Mar 7, 2023
1 parent 74f468d commit 065c763
Showing 1 changed file with 16 additions and 13 deletions.
29 changes: 16 additions & 13 deletions src/taoensso/sente.cljc
Original file line number Diff line number Diff line change
Expand Up @@ -825,29 +825,32 @@
(recur udt-t1))))))

;; Ajax handshake/poll
(let [updated-conn (upd-conn! :ajax uid client-id :any server-ch)
udt-open (:udt updated-conn)
send-handshake? (or (:init? updated-conn) (:handshake? params))]
(let [send-handshake?
(or
(:handshake? params)
(nil? (get-in @conns_ [:ajax uid client-id])))]

(timbre/logf (if send-handshake? :info :trace)
"[ajax/on-open] New server Ajax sch (poll/handshake) for %s: %s"
(lid uid client-id)
{:send-handshake? send-handshake?})

(when (connect-uid! :ajax uid)
(receive-event-msg! [:chsk/uidport-open uid]))

(if send-handshake?
;; Client will immediately repoll
(send-handshake! server-ch websocket?)

(when-let [ms lp-timeout-ms]
(go
(<! (async/timeout ms))
(when-let [[sch udt-t1] (get-in @conns_ [:ajax uid client-id])]
(when (and (= identical? server-ch) (= udt-t1 udt-open))
(interfaces/sch-send! server-ch websocket?
(pack packer :chsk/timeout))))))))))
(let [updated-conn (upd-conn! :ajax uid client-id :any server-ch)
udt-open (:udt updated-conn)]
(when-let [ms lp-timeout-ms]
(go
(<! (async/timeout ms))
(when-let [[sch udt-t1] (get-in @conns_ [:ajax uid client-id])]
(when (and (= identical? server-ch) (= udt-t1 udt-open))
(interfaces/sch-send! server-ch websocket?
(pack packer :chsk/timeout))))))))

(when (connect-uid! :ajax uid)
(receive-event-msg! [:chsk/uidport-open uid])))))

:on-msg
(fn [server-ch websocket? req-ppstr]
Expand Down

0 comments on commit 065c763

Please sign in to comment.