diff --git a/server/base/error.rs b/server/base/error.rs index bb06813..7534896 100644 --- a/server/base/error.rs +++ b/server/base/error.rs @@ -158,10 +158,10 @@ where #[macro_export] macro_rules! bail_t { ($t:ident, $e:expr) => { - return Err(failure::err_msg($e).context($crate::ErrorKind::$t).into()); + return Err($crate::Error::from(failure::err_msg($e).context($crate::ErrorKind::$t)).into()); }; ($t:ident, $fmt:expr, $($arg:tt)+) => { - return Err(failure::err_msg(format!($fmt, $($arg)+)).context($crate::ErrorKind::$t).into()); + return Err($crate::Error::from(failure::err_msg(format!($fmt, $($arg)+)).context($crate::ErrorKind::$t)).into()); }; } diff --git a/server/src/web.rs b/server/src/web.rs index 7f4c373..d453360 100644 --- a/server/src/web.rs +++ b/server/src/web.rs @@ -5,8 +5,8 @@ use crate::body::Body; use crate::json; use crate::mp4; -use base::clock::Clocks; use base::{bail_t, ErrorKind}; +use base::{clock::Clocks, format_err_t}; use bytes::{BufMut, BytesMut}; use core::borrow::Borrow; use core::str::FromStr; @@ -35,6 +35,26 @@ use tokio_tungstenite::tungstenite; use url::form_urlencoded; use uuid::Uuid; +/// An HTTP error response. +/// This is a thin wrapper over the hyper response type; it doesn't even verify +/// that the response actually uses a non-2xx status code. Its purpose is to +/// allow automatic conversion from `base::Error`. Rust's orphan rule prevents +/// this crate from defining a direct conversion from `base::Error` to +/// `hyper::Response`. +struct HttpError(Response); + +impl From> for HttpError { + fn from(response: Response) -> Self { + HttpError(response) + } +} + +impl From for HttpError { + fn from(err: base::Error) -> Self { + HttpError(from_base_error(err)) + } +} + #[derive(Debug, Eq, PartialEq)] enum Path { TopLevel, // "/api/" @@ -141,23 +161,28 @@ fn plain_response>(status: http::StatusCode, body: B) -> Response< .expect("hardcoded head should be valid") } -fn not_found>(body: B) -> Response { - plain_response(StatusCode::NOT_FOUND, body) +fn not_found>(body: B) -> HttpError { + HttpError(plain_response(StatusCode::NOT_FOUND, body)) } -fn bad_req>(body: B) -> Response { - plain_response(StatusCode::BAD_REQUEST, body) +fn bad_req>(body: B) -> HttpError { + HttpError(plain_response(StatusCode::BAD_REQUEST, body)) } -fn internal_server_err>(err: E) -> Response { - plain_response(StatusCode::INTERNAL_SERVER_ERROR, err.into().to_string()) +fn internal_server_err>(err: E) -> HttpError { + HttpError(plain_response( + StatusCode::INTERNAL_SERVER_ERROR, + err.into().to_string(), + )) } fn from_base_error(err: base::Error) -> Response { + use ErrorKind::*; let status_code = match err.kind() { - ErrorKind::PermissionDenied | ErrorKind::Unauthenticated => StatusCode::UNAUTHORIZED, - ErrorKind::InvalidArgument => StatusCode::BAD_REQUEST, - ErrorKind::NotFound => StatusCode::NOT_FOUND, + Unauthenticated => StatusCode::UNAUTHORIZED, + PermissionDenied => StatusCode::FORBIDDEN, + InvalidArgument | FailedPrecondition => StatusCode::BAD_REQUEST, + NotFound => StatusCode::NOT_FOUND, _ => StatusCode::INTERNAL_SERVER_ERROR, }; plain_response(status_code, err.to_string()) @@ -232,7 +257,7 @@ struct Caller { session: Option, } -type ResponseResult = Result, Response>; +type ResponseResult = Result, HttpError>; fn serve_json(req: &Request, out: &T) -> ResponseResult { let (mut resp, writer) = http_serve::streaming_body(&req).build(); @@ -277,12 +302,9 @@ fn extract_sid(req: &Request) -> Option { /// This returns the request body as bytes rather than performing /// deserialization. Keeping the bytes allows the caller to use a `Deserialize` /// that borrows from the bytes. -async fn extract_json_body(req: &mut Request) -> Result> { +async fn extract_json_body(req: &mut Request) -> Result { if *req.method() != http::method::Method::POST { - return Err(plain_response( - StatusCode::METHOD_NOT_ALLOWED, - "POST expected", - )); + return Err(plain_response(StatusCode::METHOD_NOT_ALLOWED, "POST expected").into()); } let correct_mime_type = match req.headers().get(header::CONTENT_TYPE) { Some(t) if t == "application/json" => true, @@ -376,10 +398,7 @@ impl Service { stream_type: db::StreamType, ) -> ResponseResult { if !caller.permissions.view_video { - return Err(plain_response( - StatusCode::UNAUTHORIZED, - "view_video required", - )); + bail_t!(PermissionDenied, "view_video required"); } let stream_id; @@ -389,10 +408,10 @@ impl Service { let mut db = self.db.lock(); open_id = match db.open { None => { - return Err(plain_response( - StatusCode::PRECONDITION_FAILED, - "database is read-only; there are no live streams", - )) + bail_t!( + FailedPrecondition, + "database is read-only; there are no live streams" + ); } Some(o) => o.id, }; @@ -400,10 +419,7 @@ impl Service { plain_response(StatusCode::NOT_FOUND, format!("no such camera {}", uuid)) })?; stream_id = camera.streams[stream_type.index()].ok_or_else(|| { - plain_response( - StatusCode::NOT_FOUND, - format!("no such stream {}/{}", uuid, stream_type), - ) + format_err_t!(NotFound, "no such stream {}/{}", uuid, stream_type) })?; db.watch_live( stream_id, @@ -525,10 +541,15 @@ impl Service { _ => Err(plain_response( StatusCode::METHOD_NOT_ALLOWED, "POST, GET, or HEAD expected", - )), + ) + .into()), } } + /// Serves an HTTP request. + /// Note that the `serve` wrapper handles responses the same whether they + /// are `Ok` or `Err`. But returning `Err` here with the `?` operator is + /// convenient for error paths. async fn serve_inner( self: Arc, req: Request<::hyper::Body>, @@ -586,6 +607,12 @@ impl Service { Ok(response) } + /// Serves an HTTP request. + /// An error return from this method causes hyper to abruptly drop the + /// HTTP connection rather than respond. That's not terribly useful, so this + /// method always returns `Ok`. It delegates to a `serve_inner` which is + /// allowed to generate `Err` results with the `?` operator, but returns + /// them to hyper as `Ok` results. pub async fn serve( self: Arc, req: Request<::hyper::Body>, @@ -600,7 +627,10 @@ impl Service { Ok(c) => c, Err(e) => return Ok(from_base_error(e)), }; - Ok(self.serve_inner(req, p, caller).await.unwrap_or_else(|e| e)) + Ok(self + .serve_inner(req, p, caller) + .await + .unwrap_or_else(|e| e.0)) } fn top_level(&self, req: &Request<::hyper::Body>, caller: Caller) -> ResponseResult { @@ -619,10 +649,7 @@ impl Service { if camera_configs { if !caller.permissions.read_camera_configs { - return Err(plain_response( - StatusCode::UNAUTHORIZED, - "read_camera_configs required", - )); + bail_t!(PermissionDenied, "read_camera_configs required"); } } @@ -756,10 +783,7 @@ impl Service { debug: bool, ) -> ResponseResult { if !caller.permissions.view_video { - return Err(plain_response( - StatusCode::UNAUTHORIZED, - "view_video required", - )); + bail_t!(PermissionDenied, "view_video required"); } let (stream_id, camera_name); { @@ -884,10 +908,11 @@ impl Service { }; if let Some(end) = s.end_time { if end > cur_off { - return Err(plain_response( - StatusCode::BAD_REQUEST, - format!("end time {} is beyond specified recordings", end), - )); + bail_t!( + InvalidArgument, + "end time {} is beyond specified recordings", + end + ); } } } @@ -1019,6 +1044,10 @@ impl Service { )) } + /// Returns true iff the client is connected over `https`. + /// Moonfire NVR currently doesn't directly serve `https`, but it supports + /// proxies which set the `X-Forwarded-Proto` header. See `guide/secure.md` + /// for more information. fn is_secure(&self, req: &Request<::hyper::Body>) -> bool { self.trust_forward_hdrs && req @@ -1124,10 +1153,7 @@ impl Service { async fn post_signals(&self, mut req: Request, caller: Caller) -> ResponseResult { if !caller.permissions.update_signals { - return Err(plain_response( - StatusCode::UNAUTHORIZED, - "update_signals required", - )); + bail_t!(PermissionDenied, "update_signals required"); } let r = extract_json_body(&mut req).await?; let r: json::PostSignalsRequest =