krate/publish: Avoid panic for truncated payloads (#6393)

`cargo` should not send such requests, but apparently some clever user with their Postman app has figured out that this is a great way to have the request handler panic... 😄

on the bright side, tokio/hyper/axum/something is catching it correctly and transforming it into an internal server error without actually crashing our server. 🎉

Author: Turbo87

src/controllers/krate/publish.rs

@@ -327,6 +327,11 @@ fn split_body<R: RequestPartsExt>(mut bytes: Bytes, req: &R) -> AppResult<(Bytes
     // .crate tarball length
     // .crate tarball file
 
+    if bytes.len() < 4 {
+        // Avoid panic in `get_u32_le()` if there is not enough remaining data
+        return Err(cargo_err("invalid metadata length"));
+    }
+
     let json_len = bytes.get_u32_le() as usize;
     req.request_log().add("metadata_length", json_len);
 
@@ -338,6 +343,11 @@ fn split_body<R: RequestPartsExt>(mut bytes: Bytes, req: &R) -> AppResult<(Bytes
 
     let json_bytes = bytes.split_to(json_len);
 
+    if bytes.len() < 4 {
+        // Avoid panic in `get_u32_le()` if there is not enough remaining data
+        return Err(cargo_err("invalid metadata length"));
+    }
+
     let tarball_len = bytes.get_u32_le() as usize;
     if tarball_len > bytes.len() {
         return Err(cargo_err(&format!(

src/tests/krate/publish.rs

@@ -1023,3 +1023,15 @@ fn new_krate_sorts_deps() {
     assert_eq!(deps[0].name, "dep-a");
     assert_eq!(deps[1].name, "dep-b");
 }
+
+#[test]
+fn empty_payload() {
+    let (_, _, user) = TestApp::full().with_user();
+
+    let response = user.put::<()>("/api/v1/crates/new", &[]);
+    assert_eq!(response.status(), StatusCode::OK);
+    assert_eq!(
+        response.into_json(),
+        json!({ "errors": [{ "detail": "invalid metadata length" }] })
+    );
+}