Fixes#206. 307a388 switched to creating a single-threaded runtime for
each stream, then destroying prior to waiting for TEARDOWN on shutdown.
This meant that the shutdown process could panic with this error:
```
panic at '/home/slamb/git/retina/src/client/mod.rs:219:22': teardown Sender shouldn't be dropped: RecvError(())
```
Let's switch back to expecting a multithreaded runtime context.
Create one for the config subcommand, too.
Don't go all the way back to the old code with its channels, though.
That had the downside that the underlying retina::Session might outlive
the caller, so there could still be an active session when we start
the next one. I haven't seen this cause problems in practice but it
still doesn't seem right.
This alone improves interop and diagnostics, as noted in Retina's
release notes. We also now give the camera name to the session group
(for improved logging of TEARDOWN operations) and expose the RTSP
server's "tool" attribute in debug logs and the config UI's "Test"
button.
Fixes#209Fixes#213
This fixes a real cross-site WebSocket hijacking (CSWSH) vulnerability.
If the attacker knows the URL of an NVR installation this user is
authenticated to and the UUID of a camera, and can trick the user into
visiting their webpage, they can grab the live stream. At least there's
some entropy in the camera UUID, but it was never intended to be a
secret.
* switch the config interface over to use Retina and make the test
button honor rtsp_transport = udp.
* adjust the threading model of the Retina streaming code.
Before, it spawned a background future that read from the runtime and
wrote to a channel. Other calls read from this channel.
After, it does work directly from within the block_on calls (no
channels).
The immediate motivation was that the config interface didn't have
another runtime handy. And passing in a current thread runtime
deadlocked. I later learned this is a difference between
Runtime::block_on and Handle::block_on. The former will drive IO and
timers; the latter will not.
But this is also more efficient to avoid so many thread hand-offs.
Both the context switches and the extra spinning that
tokio appears to do as mentioned here:
https://github.com/scottlamb/retina/issues/5#issuecomment-871971550
This may not be the final word on the threading model. Eventually
I may not have per-stream writing threads at all. But I think it will
be easier to look at this after getting rid of the separate
`moonfire-nvr config` subcommand in favor of a web interface.
* in tests, read `.mp4` files via the `mp4` crate rather than ffmpeg.
The annoying part is that this doesn't parse edit lists; oh well.
* simplify the `Opener` interface. Formerly, it'd take either a RTSP
URL or a path to a `.mp4` file, and they'd share some code because
they both sometimes used ffmpeg. Now, they're totally different
libraries (`retina` vs `mp4`). Pull the latter out to a `testutil`
module with a different interface that exposes more of the `mp4`
stuff. Now `Opener` is just for RTSP.
* simplify the h264 module. It had a lot of logic to deal with Annex B.
Retina doesn't use this encoding.
Fixes#36Fixes#126
* switch from json to toml.
I think this will be more user-friendly. It allows comments and has
less punctuation. Fewer surprises than yaml (which has e.g. the
"Norway problem"). I might have stayed with JSON if I could see a
good serde json library that allows comments, but hson is unmaintained
and serde-json strictly follows the spec.
* switch from camelCase to snake_case. Seems more idiomatic for TOML
and matches the Rust source.
* forbid unknown keys. Better to spot errors sooner.
* rename "trust_forward_hdrs" to "trust_forward_headers". Nothing else
is abbreviated.
I did a full `cargo upgrade` and fixed what it broke:
* a couple things for the latest protobuf 3.0 alphas
(note alphas don't promise API stability)
* new minimum supported Rust version
This should have some other nice effects: parking_lot now uses inline
assembler, tokio has gotten faster, etc.
This is better particularly when the user is following the docker
instructions and doesn't have a local checkout at all. It also is a
rendered HTML view rather than raw markdown.
It'd be nice to link to the exact release we're using, not tip of
master. I didn't do this now because it'll likely take some work with
build.rs to check if the user is on a tagged release or not.
Fixes#180
Since 0.7.0, the one in the db package is used both for actual storage
within the database and for API use. I left the API-specific version
around by accident.
SelectView::set_selection doesn't seem to be working properly. The
symptom is editing an existing camera will clear the sample file dir,
and thus hitting edit without making any changes will fail.
This drops several older dependencies and reduces final binary size
(text section by ~200KiB, unstripped binary by ~12MiB)
I'll have to manually add new hash formats, and I won't ever be able
to take advantage of libpasta's (currently unused) facility to wrap
hashes, but I think it's worth it. libpasta isn't well-maintained.
- after 3->4 upgrade, it left the foreign key referring to the
nonexistent old_camera table. Likely no one who did the upgrade
has ever inserted anything into this table, so no one's noticed.
- 6->7 upgrade dropped tables in the wrong order, so if there was
anything in the signal_camera table, the upgrade would fail.
Now there's room to add arbitrary configuration to signals and types.
Several things are no longer fixed columns/tables but instead within
the configuration types.
This fixes#178. Before, everything got translated to 5xx status;
now it produces the correct type in several cases.
Ideally I'd get rid of the untyped errors in all of web.rs; this is
a small step.
Before it would produce this incorrect message that told you to run
the command you just ran:
```
$ nvr init --db-dir=/nonexistent/db
E20211021 09:08:23.798 main moonfire_nvr] Exiting due to error: db dir /nonexistent/db not found; try running moonfire-nvr init
caused by: ENOENT: No such file or directory
```
Now the same command produces the following:
```
$ nvr init --db-dir=/nonexistent/db
E20211021 09:09:11.056 main moonfire_nvr] Exiting due to error: unable to create db dir /nonexistent/db
caused by: ENOENT: No such file or directory
```
Add tests just for good measure.
After a frustrating search for a suitable channel to use for shutdown
(tokio::sync:⌚:Receiver and
futures::future::Shared<tokio::sync::oneshot::Receiver> didn't look
quite right) in which I rethought my life decisions, I finally just made
my own (server/base/shutdown.rs). We can easily poll it or wait for it
in async or sync contexts. Most importantly, it's convenient; not that
it really matters here, but it's also efficient.
We now do a slightly better job of propagating a "graceful" shutdown
signal, and this channel will give us tools to improve it over time.
* Shut down even when writer or syncer operations are stuck. Fixes#117
* Not done yet: streamers should instantly shut down without waiting for
a connection attempt or frame or something. I'll probably
implement that when removing --rtsp-library=ffmpeg. The code should be
cleaner then.
* Not done yet: fix a couple places that sleep for up to a second when
they could shut down immediately. I just need to do the plumbing for
mock clocks to work.
I also implemented an immediate shutdown mode, activated by a second
signal. I think this will mitigate the streamer wait situation.
I saw this recently while working on new-schema. It was probably due
to some manual upgrade or downgrade I did rather than an actual bug.
Improve debuggability a little nonetheless.
* upgrade to Retina 0.3.1 which automatically tears down sessions
* wait out stale sessions before reconnecting
* wait for teardown to complete before shutting down
This adds some pressure on #117: it will keep waiting for the stale
session to expire even if the user has requested shutdown. I'll try
to address that next.
I copied the example of the password field by introducing a setter.
But I forgot: it was only that way because the password field has
the complexity of hashing/salting. For fields where setting is
idempotent, it can be directly exposed.
This mostly affects .mp4 init segments (#146), which currently set the
Last-Modified: date to the epoch. It could also affect other resources
(static files, other .mp4 changes) but only if the clock has gone
backwards.
I forgot to do this in 27395ec resulting in #146. Bump the version now,
and update the digest used in the tests so they will remind me to
bump the version any time the headers change.
