diff --git a/src/web/api.rs b/src/web/api.rs index 38163bb9..c132e9e8 100644 --- a/src/web/api.rs +++ b/src/web/api.rs @@ -4,7 +4,10 @@ use std::{collections::HashMap, fs, path::PathBuf, str::FromStr, sync::Arc}; use futures::{sink::SinkExt, stream::StreamExt}; -use hyper::{body, Body, Method, Request, Response, StatusCode}; +use hyper::{ + body::{self, Bytes}, + Body, Method, Request, Response, StatusCode, +}; use hyper_tungstenite::{is_upgrade_request, tungstenite::Message, upgrade, HyperWebsocket}; use opener::OpenError; use rbx_dom_weak::{ @@ -132,7 +135,10 @@ impl ApiService { let session_id = self.serve_session.session_id(); let tree_mutation_sender = self.serve_session.tree_mutation_sender(); - let body = body::to_bytes(request.into_body()).await.unwrap(); + let body = match read_request_body(request.into_body()).await { + Ok(body) => body, + Err(response) => return response, + }; let request: WriteRequest = match deserialize_msgpack(&body) { Ok(request) => request, @@ -221,7 +227,10 @@ impl ApiService { /// `Value` property set to point to the requested Instance. async fn handle_api_serialize(&self, request: Request) -> Response { let session_id = self.serve_session.session_id(); - let body = body::to_bytes(request.into_body()).await.unwrap(); + let body = match read_request_body(request.into_body()).await { + Ok(body) => body, + Err(response) => return response, + }; let request: SerializeRequest = match deserialize_msgpack(&body) { Ok(request) => request, @@ -290,7 +299,10 @@ impl ApiService { /// endpoint is used. async fn handle_api_ref_patch(self, request: Request) -> Response { let session_id = self.serve_session.session_id(); - let body = body::to_bytes(request.into_body()).await.unwrap(); + let body = match read_request_body(request.into_body()).await { + Ok(body) => body, + Err(response) => return response, + }; let request: RefPatchRequest = match deserialize_msgpack(&body) { Ok(request) => request, @@ -423,6 +435,15 @@ impl ApiService { } } +async fn read_request_body(body: Body) -> Result> { + body::to_bytes(body).await.map_err(|err| { + msgpack( + ErrorResponse::bad_request(format!("Failed to read request body: {}", err)), + StatusCode::BAD_REQUEST, + ) + }) +} + /// If this instance is represented by a script, try to find the correct .lua or .luau /// file to open to edit it. fn pick_script_path(instance: InstanceWithMeta<'_>) -> Option { @@ -572,3 +593,45 @@ fn parent_requirements(class: &str) -> Option<&str> { _ => return None, }) } + +#[cfg(test)] +mod tests { + use super::*; + + use serde::Deserialize; + + #[derive(Debug, Deserialize, PartialEq)] + enum ApiErrorKind { + BadRequest, + } + + #[derive(Debug, Deserialize)] + #[serde(rename_all = "camelCase")] + struct ApiError { + kind: ApiErrorKind, + details: String, + } + + #[tokio::test] + async fn read_request_body_returns_body_bytes() { + let body = read_request_body(Body::from("hello")).await.unwrap(); + + assert_eq!(body, Bytes::from_static(b"hello")); + } + + #[tokio::test] + async fn read_request_body_returns_structured_bad_request_for_body_errors() { + let (sender, body) = Body::channel(); + sender.abort(); + + let response = read_request_body(body).await.unwrap_err(); + + assert_eq!(response.status(), StatusCode::BAD_REQUEST); + + let body = body::to_bytes(response.into_body()).await.unwrap(); + let error: ApiError = deserialize_msgpack(&body).unwrap(); + + assert_eq!(error.kind, ApiErrorKind::BadRequest); + assert!(error.details.contains("Failed to read request body")); + } +}