eyre: GETting non-existent channels creates them

Previously, a channel could only be created by sending a PUT request,
and a GET request to receive the channel's stream would only succeed
after channel creation had happened that way. This forces client
libraries, that generally have an explicit "set up" step before allowing
normal operation, to do strange things, like sending faux pokes
(commonly hi-ing oneself) before connecting to the channel's stream as
normal.

Here, we update the GET request handling for channels to allow requests
for non-existent channels. When this happens, the channel will be
created, and eyre tracks the request as normal.

We do some... gentle restructuring... of +on-get-request:by-channel to
let the new creation case share code with the "already exists" codepath.
In the process, we find that duct-to-key was never getting updated in
the case where we replace the original channel request/connection with
the new incoming one. We fix this, it's trivial. We also identify two
other areas with vaguely-incorrect behavior, but consider them less
important and out of scope.

We also add a test case for "create channel through GET".
This commit is contained in:
fang 2023-09-13 13:37:39 +02:00 committed by Pyry Kovanen
parent e355b5090e
commit 4affae8181
2 changed files with 92 additions and 54 deletions

View File

@ -2122,7 +2122,7 @@
duct-to-key.channel-state duct-to-key.channel-state
(~(del by duct-to-key.channel-state.state) duct) (~(del by duct-to-key.channel-state.state) duct)
== ==
:: +set-timeout-timer-for: sets a timeout timer on a channel :: +update-timeout-timer-for: sets a timeout timer on a channel
:: ::
:: This creates a channel if it doesn't exist, cancels existing timers :: This creates a channel if it doesn't exist, cancels existing timers
:: if they're already set (we cannot have duplicate timers), and (if :: if they're already set (we cannot have duplicate timers), and (if
@ -2196,53 +2196,76 @@
++ on-get-request ++ on-get-request
|= [channel-id=@t [session-id=@uv =identity] =request:http] |= [channel-id=@t [session-id=@uv =identity] =request:http]
^- [(list move) server-state] ^- [(list move) server-state]
:: if there's no channel-id, we must 404
::TODO but arm description says otherwise?
::
?~ maybe-channel=(~(get by session.channel-state.state) channel-id)
%^ return-static-data-on-duct 404 'text/html'
(error-page 404 | url.request ~)
:: find the channel creator's identity, make sure it matches
::
?. =(identity identity.u.maybe-channel)
%^ return-static-data-on-duct 403 'text/html'
(error-page 403 | url.request ~)
:: find the requested "mode" and make sure it doesn't conflict
::
=/ mode=?(%json %jam) =/ mode=?(%json %jam)
(find-channel-mode %'GET' header-list.request) (find-channel-mode %'GET' header-list.request)
?. =(mode mode.u.maybe-channel) =^ [exit=? =wall moves=(list move)] state
%^ return-static-data-on-duct 406 'text/html' :: the request may include a 'Last-Event-Id' header
=; msg=tape (error-page 406 %.y url.request msg) ::
"channel already established in {(trip mode.u.maybe-channel)} mode" =/ maybe-last-event-id=(unit @ud)
:: when opening an event-stream, we must cancel our timeout timer ?~ maybe-raw-header=(get-header:http 'last-event-id' header-list.request)
:: if there's no duct already bound. Else, kill the old request ~
:: and replace it (rush u.maybe-raw-header dum:ag)
:: :: if the channel doesn't exist yet, simply instantiate it here
=^ cancel-moves state ::
?. ?=([%| *] state.u.maybe-channel) ?~ maybe-channel=(~(get by session.channel-state.state) channel-id)
:_ state =- [[| ~ ~] state(session.channel-state -)]
(cancel-timeout-move channel-id p.state.u.maybe-channel)^~ %+ ~(put by session.channel-state.state) channel-id
=/ cancel-heartbeat ::NOTE some other fields initialized at the end of this arm
?~ heartbeat.u.maybe-channel ~ %* . *channel
:_ ~ identity identity
%+ cancel-heartbeat-move channel-id next-id (fall maybe-last-event-id 0)
[date duct]:u.heartbeat.u.maybe-channel last-ack now
=- [(weld cancel-heartbeat -<) ->] ==
(handle-response(duct p.state.u.maybe-channel) [%cancel ~]) :: if the channel does exist, we put some demands on the get request,
:: the request may include a 'Last-Event-Id' header :: and may need to do some cleanup for prior requests.
:: ::
=/ maybe-last-event-id=(unit @ud) :: find the channel creator's identity, make sure it matches
?~ maybe-raw-header=(get-header:http 'last-event-id' header-list.request) ::
~ ?. =(identity identity.u.maybe-channel)
(rush u.maybe-raw-header dum:ag) =^ mos state
:: flush events older than the passed in 'Last-Event-ID' %^ return-static-data-on-duct 403 'text/html'
:: (error-page 403 | url.request ~)
=? state ?=(^ maybe-last-event-id) [[& ~ mos] state]
(acknowledge-events channel-id u.maybe-last-event-id) :: make sure the request "mode" doesn't conflict with a prior request
:: combine the remaining queued events to send to the client ::
:: ::TODO or could we change that on the spot, given that only a single
=/ event-replay=wall :: request will ever be listening to this channel?
?. =(mode mode.u.maybe-channel)
=^ mos state
%^ return-static-data-on-duct 406 'text/html'
=; msg=tape (error-page 406 %.y url.request msg)
"channel already established in {(trip mode.u.maybe-channel)} mode"
[[& ~ mos] state]
:: when opening an event-stream, we must cancel our timeout timer
:: if there's no duct already bound. else, kill the old request,
:: we will replace its duct at the end of this arm
::
=^ cancel-moves state
?: ?=([%& *] state.u.maybe-channel)
:_ state
(cancel-timeout-move channel-id p.state.u.maybe-channel)^~
=. duct-to-key.channel-state.state
(~(del by duct-to-key.channel-state.state) p.state.u.maybe-channel)
=/ cancel-heartbeat
?~ heartbeat.u.maybe-channel ~
:_ ~
%+ cancel-heartbeat-move channel-id
[date duct]:u.heartbeat.u.maybe-channel
=- [(weld cancel-heartbeat -<) ->]
(handle-response(duct p.state.u.maybe-channel) [%cancel ~])
:: flush events older than the passed in 'Last-Event-ID'
::
=? state ?=(^ maybe-last-event-id)
(acknowledge-events channel-id u.maybe-last-event-id)
::TODO that did not remove them from the u.maybe-channel queue though!
:: we may want to account for maybe-last-event-id, for efficiency.
:: (the client _should_ ignore events it heard previously if we do
:: end up re-sending them, but _requiring_ that feels kinda risky)
::
:: combine the remaining queued events to send to the client
::
=; event-replay=wall
[[| - cancel-moves] state]
%- zing %- zing
%- flop %- flop
=/ queue events.u.maybe-channel =/ queue events.u.maybe-channel
@ -2260,6 +2283,7 @@
(channel-event-to-tape u.maybe-channel request-id channel-event) (channel-event-to-tape u.maybe-channel request-id channel-event)
?~ said $ ?~ said $
$(events [(event-tape-to-wall id +.u.said) events]) $(events [(event-tape-to-wall id +.u.said) events])
?: exit [moves state]
:: send the start event to the client :: send the start event to the client
:: ::
=^ http-moves state =^ http-moves state
@ -2270,7 +2294,7 @@
['cache-control' 'no-cache'] ['cache-control' 'no-cache']
['connection' 'keep-alive'] ['connection' 'keep-alive']
== ==
(wall-to-octs event-replay) (wall-to-octs wall)
complete=%.n complete=%.n
== ==
:: associate this duct with this session key :: associate this duct with this session key
@ -2300,7 +2324,7 @@
heartbeat (some [heartbeat-time duct]) heartbeat (some [heartbeat-time duct])
== ==
:: ::
[[heartbeat :(weld http-moves cancel-moves moves)] state] [[heartbeat :(weld http-moves moves)] state]
:: +acknowledge-events: removes events before :last-event-id on :channel-id :: +acknowledge-events: removes events before :last-event-id on :channel-id
:: ::
++ acknowledge-events ++ acknowledge-events

View File

@ -262,7 +262,7 @@
== ==
:: ::
++ ex-channel-response ++ ex-channel-response
|= body=@t |= body=(unit @t)
|= mov=move |= mov=move
^- tang ^- tang
?. ?=([[[%http-blah ~] ~] %give %response %start * * %.n] mov) ?. ?=([[[%http-blah ~] ~] %give %response %start * * %.n] mov)
@ -273,7 +273,7 @@
['connection' 'keep-alive'] ['connection' 'keep-alive']
['set-cookie' cookie-string] ['set-cookie' cookie-string]
== ==
=/ body `(as-octs:mimes:html body) =/ body (bind body as-octs:mimes:html)
;: weld ;: weld
(expect-eq !>(200) !>(status-code.response-header.http-event.p.card.mov)) (expect-eq !>(200) !>(status-code.response-header.http-event.p.card.mov))
(expect-eq !>(body) !>(data.http-event.p.card.mov)) (expect-eq !>(body) !>(data.http-event.p.card.mov))
@ -743,6 +743,20 @@
=/ wire /channel/subscription/'0123456789abcdef'/1/~nul/two/~nul =/ wire /channel/subscription/'0123456789abcdef'/1/~nul/two/~nul
(expect-moves mos (ex-gall-deal wire ~nul %two %leave ~) ~) (expect-moves mos (ex-gall-deal wire ~nul %two %leave ~) ~)
:: ::
++ test-channel-open-with-get
%- eval-mare
=/ m (mare ,~)
;< ~ bind:m perform-init-wo-timer
;< ~ bind:m perform-born
;< ~ bind:m (wait ~d1)
;< ~ bind:m perform-authentication-2
;< mos=(list move) bind:m
(get '/~/channel/0123456789abcdef' cookie)
;< now=@da bind:m get-now
=/ mov-1 (ex-wait /channel/heartbeat/'0123456789abcdef' (add now ~s20))
=/ mov-2 (ex-channel-response ~)
(expect-moves mos mov-1 mov-2 ~)
::
++ test-channel-results-before-open ++ test-channel-results-before-open
%- eval-mare %- eval-mare
=/ m (mare ,~) =/ m (mare ,~)
@ -776,7 +790,7 @@
;< now=@da bind:m get-now ;< now=@da bind:m get-now
=/ mov-1 (ex-wait /channel/heartbeat/'0123456789abcdef' (add now ~s20)) =/ mov-1 (ex-wait /channel/heartbeat/'0123456789abcdef' (add now ~s20))
=/ mov-2 =/ mov-2
%- ex-channel-response %+ ex-channel-response ~
''' '''
id: 0 id: 0
data: {"ok":"ok","id":0,"response":"poke"} data: {"ok":"ok","id":0,"response":"poke"}
@ -920,7 +934,7 @@
;< now=@da bind:m get-now ;< now=@da bind:m get-now
=/ mov-1 (ex-wait /channel/heartbeat/'0123456789abcdef' (add now ~s20)) =/ mov-1 (ex-wait /channel/heartbeat/'0123456789abcdef' (add now ~s20))
=/ mov-2 =/ mov-2
%- ex-channel-response %+ ex-channel-response ~
''' '''
id: 0 id: 0
data: {"ok":"ok","id":0,"response":"poke"} data: {"ok":"ok","id":0,"response":"poke"}
@ -1022,7 +1036,7 @@
=/ heartbeat (add now ~s20) =/ heartbeat (add now ~s20)
=/ mov-1 (ex-wait /channel/heartbeat/'0123456789abcdef' heartbeat) =/ mov-1 (ex-wait /channel/heartbeat/'0123456789abcdef' heartbeat)
=/ mov-2 =/ mov-2
%- ex-channel-response %+ ex-channel-response ~
''' '''
id: 0 id: 0
data: {"ok":"ok","id":0,"response":"poke"} data: {"ok":"ok","id":0,"response":"poke"}
@ -1092,7 +1106,7 @@
=/ heartbeat (add now ~s20) =/ heartbeat (add now ~s20)
=/ mov-1 (ex-wait /channel/heartbeat/'0123456789abcdef' heartbeat) =/ mov-1 (ex-wait /channel/heartbeat/'0123456789abcdef' heartbeat)
=/ mov-2 =/ mov-2
%- ex-channel-response %+ ex-channel-response ~
''' '''
id: 2 id: 2
data: {"json":[1],"id":1,"response":"diff"} data: {"json":[1],"id":1,"response":"diff"}