From 16633e3c658e9b94edaffa3a80cca46268c39b5c Mon Sep 17 00:00:00 2001 From: AK-Khan02 Date: Wed, 10 Jun 2026 21:48:33 -0400 Subject: [PATCH] Return 400 for missing /api/serialize IDs (#1272) --- CHANGELOG.md | 2 ++ src/web/api.rs | 2 +- tests/rojo_test/serve_util.rs | 10 ++++------ tests/tests/serve.rs | 36 +++++++++++++++++++++++++++++------ 4 files changed, 37 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 00524693..339c0c6e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -44,6 +44,7 @@ Making a new release? Simply add the new header with the version and date undern * Fixed the sync fallback scrambling sibling order; replacements are now re-parented ancestors-first and in their original child order. ([#1265]) * Instances that share a name and class are now robustly matched on resync by comparing their properties, instead of relying on child order alone. ([#1266]) * Rojo now reports a clear error instead of panicking in several cases, including when the `serve` port is already in use, when a synced file is read-only or locked, when the filesystem watcher can't be created, and when the working directory is inaccessible. ([#1267]) +* Fixed `/api/serialize` returning success when a requested instance ID is missing from the serve session tree. ([#1272]) * `rojo serve` now validates the `Host`/`Origin` headers to protect the local/private server against DNS rebinding, gates `/api/open` to local clients, and warns when bound to a network-reachable address. The accepted hosts can be extended with the `--allowed-hosts` option or a project's `serveAllowedHosts` field, for example to reach a network-exposed server by hostname. ([#1270]) * Fixed syncback not removing stale `$properties` entries when Studio resets a property to its engine default. ([#1244]) @@ -60,6 +61,7 @@ Making a new release? Simply add the new header with the version and date undern [#1265]: https://github.com/rojo-rbx/rojo/pull/1265 [#1266]: https://github.com/rojo-rbx/rojo/pull/1266 [#1267]: https://github.com/rojo-rbx/rojo/pull/1267 +[#1272]: https://github.com/rojo-rbx/rojo/pull/1272 [#1270]: https://github.com/rojo-rbx/rojo/pull/1270 [#1244]: https://github.com/rojo-rbx/rojo/pull/1244 diff --git a/src/web/api.rs b/src/web/api.rs index 2875f2aa..4fd21c0a 100644 --- a/src/web/api.rs +++ b/src/web/api.rs @@ -276,7 +276,7 @@ impl ApiService { response_dom.transfer_within(child_ref, object_value); } else { - msgpack( + return msgpack( ErrorResponse::bad_request(format!("provided id {id} is not in the tree")), StatusCode::BAD_REQUEST, ); diff --git a/tests/rojo_test/serve_util.rs b/tests/rojo_test/serve_util.rs index 4a817a8a..458743e0 100644 --- a/tests/rojo_test/serve_util.rs +++ b/tests/rojo_test/serve_util.rs @@ -228,11 +228,11 @@ impl TestServeSession { } } - pub fn get_api_serialize( + pub fn post_api_serialize( &self, ids: &[Ref], session_id: SessionId, - ) -> Result { + ) -> Result { let client = reqwest::blocking::Client::new(); let url = format!("http://localhost:{}/api/serialize", self.port); let body = serialize_msgpack(&SerializeRequest { @@ -241,9 +241,7 @@ impl TestServeSession { }) .unwrap(); - let body = client.post(url).body(body).send()?.bytes()?; - - Ok(deserialize_msgpack(&body).expect("Server returned malformed response")) + client.post(url).body(body).send() } /// Sends a GET to `/api/rojo` with the given extra request headers and @@ -291,7 +289,7 @@ fn serialize_msgpack(value: T) -> Result, rmp_serde::encod Ok(serialized) } -fn deserialize_msgpack<'a, T: Deserialize<'a>>( +pub fn deserialize_msgpack<'a, T: Deserialize<'a>>( input: &'a [u8], ) -> Result { let mut deserializer = rmp_serde::Deserializer::new(input).with_human_readable(); diff --git a/tests/tests/serve.rs b/tests/tests/serve.rs index 490c13db..0d6f75cc 100644 --- a/tests/tests/serve.rs +++ b/tests/tests/serve.rs @@ -1,14 +1,16 @@ use std::fs; use insta::{assert_snapshot, assert_yaml_snapshot, with_settings}; +use rbx_dom_weak::types::Ref; +use reqwest::StatusCode; use tempfile::tempdir; use crate::rojo_test::{ internable::InternAndRedact, - serve_util::{run_serve_test, serialize_to_xml_model}, + serve_util::{deserialize_msgpack, run_serve_test, serialize_to_xml_model}, }; -use librojo::web_api::SocketPacketType; +use librojo::web_api::{SerializeResponse, SocketPacketType}; #[test] fn rejects_dns_rebinding_requests() { @@ -713,9 +715,13 @@ fn meshpart_with_id() { .find(|(_, inst)| inst.class_name == "ObjectValue") .unwrap(); - let serialize_response = session - .get_api_serialize(&[*meshpart, *objectvalue], info.session_id) + let body = session + .post_api_serialize(&[*meshpart, *objectvalue], info.session_id) + .unwrap() + .bytes() .unwrap(); + let serialize_response: SerializeResponse = + deserialize_msgpack(&body).expect("Server returned malformed response"); // We don't assert a snapshot on the SerializeResponse because the model includes the // Refs from the DOM as names, which means it will obviously be different every time @@ -727,6 +733,20 @@ fn meshpart_with_id() { }); } +#[test] +fn serialize_missing_id() { + run_serve_test("empty", |session, _| { + let info = session.get_api_rojo().unwrap(); + let missing_id = Ref::new(); + + let response = session + .post_api_serialize(&[missing_id], info.session_id) + .unwrap(); + + assert_eq!(response.status(), StatusCode::BAD_REQUEST); + }); +} + #[test] fn forced_parent() { run_serve_test("forced_parent", |session, mut redactions| { @@ -741,9 +761,13 @@ fn forced_parent() { read_response.intern_and_redact(&mut redactions, root_id) ); - let serialize_response = session - .get_api_serialize(&[root_id], info.session_id) + let body = session + .post_api_serialize(&[root_id], info.session_id) + .unwrap() + .bytes() .unwrap(); + let serialize_response: SerializeResponse = + deserialize_msgpack(&body).expect("Server returned malformed response"); assert_eq!(serialize_response.session_id, info.session_id);