Contrary to the argument made by 4affae8, this is the _actually correct_
behavior. Not creating server-side resources in response to GETs
respects the expected method semantics, and more importantly, serving a
404 is an important signal for clients trying to connect to a channel
they were using previously. Without that, they have no way of telling
whether, when reconnecting, if their channel was reaped in the mean time
or not.
The "empty PUT" affordance provided by 34148f9 makes requiring a PUT
request for channel creation more reasonable.
We leave the general refactoring done by #6789 in place, but do
emphasize the reasoning given here with a few additional comments.
Previously, we would reject this with a 400 error. Considering the
request body is expected to contain "array of requests" and that arrays
may be empty, we really should not be rejecting the requests.
Prior to 156ca21472, sending the empty array would have been convenient
for channel creation. Empty arrays getting rejected forced clients to
inject a faux poke (commonly hi-ing oneself). With that recent change,
the most common case for wanting to PUT the empty list of requests is
largely obsolete, but one can still imagine it being useful for clients
that want to keep their channel alive without necessarily being
connected to it. This also implements sloppier clients from running into
400 responses when they submit an empty "command queue" for whatever.
Regardless, there seems to be no clear reason why the empty request list
_shouldn't_ be accepted and processed as normal.
We add a small test to ensure eyre accepts this.
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".
Previously, for endpoints bound to agents, we would pass the request
onto the agent even if the agents wasn't currently running.
Here, we make eyre check to see if the agent is actually running, before
passing the request on. If the bound agent is not running, eyre serves a
503 synchronously instead.
This way, we avoid cluttering up the gall queue for the bound agent.
For the server case, we add tests for http-first finalization, incorrect
eauth tokens, attempt expiration, client abort signalling, and client
deletion requests.
For the client case, we add a test for approving eauth attempts without
being locally authenticated.
To this end, we refactor the layout of the eauth tests. The +eauth core
now contains the tasks/gifts to be handled by the client/server, but no
longer does move checking. This happens within the test arms only. This
way, we may trivially re-run parts of the eauth flow under different
conditions.
Concatenating before we truncate, instead of truncating the entropy by
itself, is slightly simpler.
Because this slightly changes the naming algorithm, we must update the
eyre tests to match.
This shaves off 1000 lines of testing code while maintaining the same
tests. It reduces boilerplate by introducing "mare", a monad for
testing Eyre. It's very simple (just maintains the current state of
Eyre and the current time), but it's easier to build helper functions in
this form, and that reduces the immense quantities of copy-and-paste
that were in the old tests. What's there now could surely be improved
further, but I think this is a good start.
The underlying mare machinery is not really specific to Eyre, so it
would be straightforward to apply this strategy to other vanes. The
work is in creating appropriate helper functions for each vane. Eyre is
undergoing work, so that's the only one I've changed here. Further,
it's not clear that this is the ultimate solution to unit testing vanes.
The resulting code is IMO clearer than before, but I wouldn't say it's
*clear*.
Unauthenticated requests now also create sessions. This affects most
HTTP request handling tests.
The situation here is not ideal, and worsening over time. Worth spending
some time to think about how to best refactor the Eyre tests to make
them more manageable and easier to maintain.
|pump and |sink call into each other in three places
related to nacks and naxplanations (sending a nack,
notifying the |pump of a naxplanation, or dropping a
nack from the |sink). This intra calls are making implicit
updates to more parts of the state than the core should
manage. To avoid that we emit a move to %arvo, encoded
as an %ames plea, to handle that in the next event.
