From f05d0e03a10dd40145046d13bc87ef65c59981a6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Commaille?= Date: Tue, 10 Dec 2024 16:09:53 +0100 Subject: [PATCH] api: Disallow #[serde(flatten)] for single-body-fields of requests and responses `#[ruma_api(body)]` must be used instead. --- .../src/typing/create_typing_event.rs | 2 +- crates/ruma-common/CHANGELOG.md | 5 +++ crates/ruma-common/tests/it/api/ruma_api.rs | 2 ++ .../it/api/ui/serde-flatten-request-body.rs | 30 +++++++++++++++++ .../api/ui/serde-flatten-request-body.stderr | 6 ++++ .../it/api/ui/serde-flatten-response-body.rs | 30 +++++++++++++++++ .../api/ui/serde-flatten-response-body.stderr | 33 +++++++++++++++++++ crates/ruma-macros/src/api/request.rs | 21 +++++++++--- crates/ruma-macros/src/api/response.rs | 20 +++++++++-- crates/ruma-macros/src/util.rs | 27 ++++++++++++++- 10 files changed, 167 insertions(+), 9 deletions(-) create mode 100644 crates/ruma-common/tests/it/api/ui/serde-flatten-request-body.rs create mode 100644 crates/ruma-common/tests/it/api/ui/serde-flatten-request-body.stderr create mode 100644 crates/ruma-common/tests/it/api/ui/serde-flatten-response-body.rs create mode 100644 crates/ruma-common/tests/it/api/ui/serde-flatten-response-body.stderr diff --git a/crates/ruma-client-api/src/typing/create_typing_event.rs b/crates/ruma-client-api/src/typing/create_typing_event.rs index 87fae313..b0d7272a 100644 --- a/crates/ruma-client-api/src/typing/create_typing_event.rs +++ b/crates/ruma-client-api/src/typing/create_typing_event.rs @@ -37,7 +37,7 @@ pub mod v3 { pub user_id: OwnedUserId, /// Whether the user is typing within a length of time or not. - #[serde(flatten)] + #[ruma_api(body)] pub state: Typing, } diff --git a/crates/ruma-common/CHANGELOG.md b/crates/ruma-common/CHANGELOG.md index c1c2dc34..6ce6f663 100644 --- a/crates/ruma-common/CHANGELOG.md +++ b/crates/ruma-common/CHANGELOG.md @@ -1,5 +1,10 @@ # [unreleased] +Breaking changes: + +- `#[serde(flatten)]` on the only body field of a `#[request]` or `#[response]` + struct is disallowed. `#[ruma_api(body)]` must be used instead. + Improvements: - The `ruma_identifiers_storage` compile-time `cfg` setting can also be diff --git a/crates/ruma-common/tests/it/api/ruma_api.rs b/crates/ruma-common/tests/it/api/ruma_api.rs index f863050b..992b0686 100644 --- a/crates/ruma-common/tests/it/api/ruma_api.rs +++ b/crates/ruma-common/tests/it/api/ruma_api.rs @@ -7,4 +7,6 @@ fn ui() { t.pass("tests/it/api/ui/response-only.rs"); t.compile_fail("tests/it/api/ui/deprecated-without-added.rs"); t.compile_fail("tests/it/api/ui/removed-without-deprecated.rs"); + t.compile_fail("tests/it/api/ui/serde-flatten-request-body.rs"); + t.compile_fail("tests/it/api/ui/serde-flatten-response-body.rs"); } diff --git a/crates/ruma-common/tests/it/api/ui/serde-flatten-request-body.rs b/crates/ruma-common/tests/it/api/ui/serde-flatten-request-body.rs new file mode 100644 index 00000000..4542ea35 --- /dev/null +++ b/crates/ruma-common/tests/it/api/ui/serde-flatten-request-body.rs @@ -0,0 +1,30 @@ +use ruma_common::{ + api::{request, response, Metadata}, + metadata, +}; +use serde::{Deserialize, Serialize}; + +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct CustomRequestBody { + pub bar: String, +} + +const METADATA: Metadata = metadata! { + method: POST, // An `http::Method` constant. No imports required. + rate_limited: false, + authentication: None, + history: { + unstable => "/_matrix/some/endpoint", + } +}; + +#[request] +pub struct Request { + #[serde(flatten)] + pub foo: CustomRequestBody, +} + +#[response] +pub struct Response; + +fn main() {} diff --git a/crates/ruma-common/tests/it/api/ui/serde-flatten-request-body.stderr b/crates/ruma-common/tests/it/api/ui/serde-flatten-request-body.stderr new file mode 100644 index 00000000..1de3d9e8 --- /dev/null +++ b/crates/ruma-common/tests/it/api/ui/serde-flatten-request-body.stderr @@ -0,0 +1,6 @@ +error: Use `#[ruma_api(body)]` to represent the JSON body as a single field + --> tests/it/api/ui/serde-flatten-request-body.rs:23:5 + | +23 | / #[serde(flatten)] +24 | | pub foo: CustomRequestBody, + | |______________________________^ diff --git a/crates/ruma-common/tests/it/api/ui/serde-flatten-response-body.rs b/crates/ruma-common/tests/it/api/ui/serde-flatten-response-body.rs new file mode 100644 index 00000000..05a06693 --- /dev/null +++ b/crates/ruma-common/tests/it/api/ui/serde-flatten-response-body.rs @@ -0,0 +1,30 @@ +use ruma_common::{ + api::{request, response, Metadata}, + metadata, +}; +use serde::{Deserialize, Serialize}; + +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct CustomResponseBody { + pub bar: String, +} + +const METADATA: Metadata = metadata! { + method: GET, // An `http::Method` constant. No imports required. + rate_limited: false, + authentication: None, + history: { + unstable => "/_matrix/some/endpoint", + } +}; + +#[request] +pub struct Request; + +#[response] +pub struct Response { + #[serde(flatten)] + pub foo: CustomResponseBody, +} + +fn main() {} diff --git a/crates/ruma-common/tests/it/api/ui/serde-flatten-response-body.stderr b/crates/ruma-common/tests/it/api/ui/serde-flatten-response-body.stderr new file mode 100644 index 00000000..7ee85871 --- /dev/null +++ b/crates/ruma-common/tests/it/api/ui/serde-flatten-response-body.stderr @@ -0,0 +1,33 @@ +error: Use `#[ruma_api(body)]` to represent the JSON body as a single field + --> tests/it/api/ui/serde-flatten-response-body.rs:26:5 + | +26 | / #[serde(flatten)] +27 | | pub foo: CustomResponseBody, + | |_______________________________^ + +error[E0277]: the trait bound `Response: IncomingResponse` is not satisfied + --> tests/it/api/ui/serde-flatten-response-body.rs:21:1 + | +21 | #[request] + | ^^^^^^^^^^ the trait `IncomingResponse` is not implemented for `Response` + | +note: required by a bound in `ruma_common::api::OutgoingRequest::IncomingResponse` + --> src/api.rs + | + | type IncomingResponse: IncomingResponse; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `OutgoingRequest::IncomingResponse` + = note: this error originates in the derive macro `::ruma_common::exports::ruma_macros::Request` (in Nightly builds, run with -Z macro-backtrace for more info) + +error[E0277]: the trait bound `Response: OutgoingResponse` is not satisfied + --> tests/it/api/ui/serde-flatten-response-body.rs:21:1 + | +21 | #[request] + | ^^^^^^^^^^ the trait `OutgoingResponse` is not implemented for `Response` + | + = help: the trait `OutgoingResponse` is implemented for `MatrixError` +note: required by a bound in `ruma_common::api::IncomingRequest::OutgoingResponse` + --> src/api.rs + | + | type OutgoingResponse: OutgoingResponse; + | ^^^^^^^^^^^^^^^^ required by this bound in `IncomingRequest::OutgoingResponse` + = note: this error originates in the derive macro `::ruma_common::exports::ruma_macros::Request` (in Nightly builds, run with -Z macro-backtrace for more info) diff --git a/crates/ruma-macros/src/api/request.rs b/crates/ruma-macros/src/api/request.rs index 4081101a..61fe05e6 100644 --- a/crates/ruma-macros/src/api/request.rs +++ b/crates/ruma-macros/src/api/request.rs @@ -11,7 +11,7 @@ use super::{ attribute::{DeriveRequestMeta, RequestMeta}, ensure_feature_presence, }; -use crate::util::{import_ruma_common, PrivateField}; +use crate::util::{field_has_serde_flatten_attribute, import_ruma_common, PrivateField}; mod incoming; mod outgoing; @@ -251,9 +251,10 @@ impl Request { } }; - let has_body_fields = self.fields.iter().any(|f| matches!(&f.kind, RequestFieldKind::Body)); - let has_query_fields = - self.fields.iter().any(|f| matches!(&f.kind, RequestFieldKind::Query)); + let mut body_fields = + self.fields.iter().filter(|f| matches!(f.kind, RequestFieldKind::Body)); + let first_body_field = body_fields.next(); + let has_body_fields = first_body_field.is_some(); if has_newtype_body_field && has_body_fields { return Err(syn::Error::new_spanned( @@ -262,6 +263,18 @@ impl Request { )); } + if let Some(first_body_field) = first_body_field { + let is_single_body_field = body_fields.next().is_none(); + + if is_single_body_field && field_has_serde_flatten_attribute(&first_body_field.inner) { + return Err(syn::Error::new_spanned( + first_body_field, + "Use `#[ruma_api(body)]` to represent the JSON body as a single field", + )); + } + } + + let has_query_fields = self.has_query_fields(); if has_query_all_field && has_query_fields { return Err(syn::Error::new_spanned( &self.ident, diff --git a/crates/ruma-macros/src/api/response.rs b/crates/ruma-macros/src/api/response.rs index 5f28dbaa..c373668c 100644 --- a/crates/ruma-macros/src/api/response.rs +++ b/crates/ruma-macros/src/api/response.rs @@ -14,7 +14,7 @@ use super::{ attribute::{DeriveResponseMeta, ResponseMeta}, ensure_feature_presence, }; -use crate::util::{import_ruma_common, PrivateField}; +use crate::util::{field_has_serde_flatten_attribute, import_ruma_common, PrivateField}; mod incoming; mod outgoing; @@ -213,8 +213,11 @@ impl Response { } }; - let has_body_fields = - self.fields.iter().any(|f| matches!(&f.kind, ResponseFieldKind::Body)); + let mut body_fields = + self.fields.iter().filter(|f| matches!(f.kind, ResponseFieldKind::Body)); + let first_body_field = body_fields.next(); + let has_body_fields = first_body_field.is_some(); + if has_newtype_body_field && has_body_fields { return Err(syn::Error::new_spanned( &self.ident, @@ -222,6 +225,17 @@ impl Response { )); } + if let Some(first_body_field) = first_body_field { + let is_single_body_field = body_fields.next().is_none(); + + if is_single_body_field && field_has_serde_flatten_attribute(&first_body_field.inner) { + return Err(syn::Error::new_spanned( + first_body_field, + "Use `#[ruma_api(body)]` to represent the JSON body as a single field", + )); + } + } + Ok(()) } } diff --git a/crates/ruma-macros/src/util.rs b/crates/ruma-macros/src/util.rs index 9dd7e510..e7ea406d 100644 --- a/crates/ruma-macros/src/util.rs +++ b/crates/ruma-macros/src/util.rs @@ -1,7 +1,7 @@ use proc_macro2::TokenStream; use proc_macro_crate::{crate_name, FoundCrate}; use quote::{format_ident, quote, ToTokens}; -use syn::{Field, Ident, LitStr}; +use syn::{Attribute, Field, Ident, LitStr}; pub(crate) fn import_ruma_common() -> TokenStream { if let Ok(FoundCrate::Name(name)) = crate_name("ruma-common") { @@ -202,3 +202,28 @@ pub fn cfg_expand_struct(item: &mut syn::ItemStruct) { fields.named.push(field); } } + +/// Whether the given field has a `#[serde(flatten)]` attribute. +pub fn field_has_serde_flatten_attribute(field: &Field) -> bool { + field.attrs.iter().any(is_serde_flatten_attribute) +} + +/// Whether the given attribute is a `#[serde(flatten)]` attribute. +fn is_serde_flatten_attribute(attr: &Attribute) -> bool { + if !attr.path().is_ident("serde") { + return false; + } + + let mut contains_flatten = false; + let _ = attr.parse_nested_meta(|meta| { + if meta.path.is_ident("flatten") { + contains_flatten = true; + // Return an error to stop the parsing early. + return Err(meta.error("found")); + } + + Ok(()) + }); + + contains_flatten +}