From 2c60c641814dc36be10b4e813528a07904aae200 Mon Sep 17 00:00:00 2001 From: Bad Manners Date: Sat, 28 Sep 2024 11:00:42 -0300 Subject: [PATCH 1/6] Allow splices in attribute names Closes #444 --- docs/content/splices-toggles.md | 17 +++++- maud/tests/splices.rs | 10 ++++ maud_macros/src/ast.rs | 29 ++++++++- maud_macros/src/generate.rs | 19 ++++-- maud_macros/src/parse.rs | 103 ++++++++++++++++++-------------- 5 files changed, 126 insertions(+), 52 deletions(-) diff --git a/docs/content/splices-toggles.md b/docs/content/splices-toggles.md index 84f8bf3c..35bb5264 100644 --- a/docs/content/splices-toggles.md +++ b/docs/content/splices-toggles.md @@ -90,6 +90,21 @@ html! { # ; ``` +### Splices in attribute name + +You can also use splices in the attribute name: + +```rust +let tuple = ("hx-get", "/pony"); +# let _ = maud:: +html! { + button (tuple.0)=(tuple.1) { + "Get a pony!" + } +} +# ; +``` + ### What can be spliced? You can splice any value that implements [`Render`][Render]. @@ -145,7 +160,7 @@ html! { ### Optional attributes with values: `title=[Some("value")]` -Add optional attributes to an element using `attr=[value]` syntax, with *square* brackets. +Add optional attributes to an element using `attr=[value]` syntax, with _square_ brackets. These are only rendered if the value is `Some`, and entirely omitted if the value is `None`. ```rust diff --git a/maud/tests/splices.rs b/maud/tests/splices.rs index 8665e84b..7e2b6d0c 100644 --- a/maud/tests/splices.rs +++ b/maud/tests/splices.rs @@ -73,6 +73,16 @@ fn locals() { assert_eq!(result.into_string(), "Pinkie Pie"); } +#[test] +fn attribute_name() { + let tuple = ("hx-get", "/pony"); + let result = html! { button (tuple.0)=(tuple.1) { "Get a pony!" } }; + assert_eq!( + result.into_string(), + r#""# + ); +} + /// An example struct, for testing purposes only struct Creature { name: &'static str, diff --git a/maud_macros/src/ast.rs b/maud_macros/src/ast.rs index b95665ec..61a5c166 100644 --- a/maud_macros/src/ast.rs +++ b/maud_macros/src/ast.rs @@ -153,13 +153,13 @@ impl Special { #[derive(Debug)] pub struct NamedAttr { - pub name: TokenStream, + pub name: AttrName, pub attr_type: AttrType, } impl NamedAttr { fn span(&self) -> SpanRange { - let name_span = span_tokens(self.name.clone()); + let name_span = span_tokens(self.name.tokens()); if let Some(attr_type_span) = self.attr_type.span() { name_span.join_range(attr_type_span) } else { @@ -168,6 +168,31 @@ impl NamedAttr { } } +#[derive(Debug, Clone)] +pub enum AttrName { + Fixed { value: TokenStream }, + Splice { expr: TokenStream }, +} + +impl AttrName { + pub fn tokens(&self) -> TokenStream { + match self { + AttrName::Fixed { value } => value.clone(), + AttrName::Splice { expr, .. } => expr.clone(), + } + } +} + +impl std::fmt::Display for AttrName { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + AttrName::Fixed { value } => f.write_str(&value.to_string())?, + AttrName::Splice { expr, .. } => f.write_str(&expr.to_string())?, + }; + Ok(()) + } +} + #[derive(Debug)] pub enum AttrType { Normal { value: Markup }, diff --git a/maud_macros/src/generate.rs b/maud_macros/src/generate.rs index ee27a55d..f432a500 100644 --- a/maud_macros/src/generate.rs +++ b/maud_macros/src/generate.rs @@ -123,12 +123,19 @@ impl Generator { build.push_escaped(&name_to_string(name)); } + fn attr_name(&self, name: AttrName, build: &mut Builder) { + match name { + AttrName::Fixed { value } => self.name(value, build), + AttrName::Splice { expr, .. } => self.splice(expr, build), + } + } + fn attrs(&self, attrs: Vec, build: &mut Builder) { for NamedAttr { name, attr_type } in desugar_attrs(attrs) { match attr_type { AttrType::Normal { value } => { build.push_str(" "); - self.name(name, build); + self.attr_name(name, build); build.push_str("=\""); self.markup(value, build); build.push_str("\""); @@ -140,7 +147,7 @@ impl Generator { let body = { let mut build = self.builder(); build.push_str(" "); - self.name(name, &mut build); + self.attr_name(name, &mut build); build.push_str("=\""); self.splice(inner_value.clone(), &mut build); build.push_str("\""); @@ -150,7 +157,7 @@ impl Generator { } AttrType::Empty { toggler: None } => { build.push_str(" "); - self.name(name, build); + self.attr_name(name, build); } AttrType::Empty { toggler: Some(Toggler { cond, .. }), @@ -158,7 +165,7 @@ impl Generator { let body = { let mut build = self.builder(); build.push_str(" "); - self.name(name, &mut build); + self.attr_name(name, &mut build); build.finish() }; build.push_tokens(quote!(if (#cond) { #body })); @@ -224,7 +231,9 @@ fn desugar_classes_or_ids( }); } Some(NamedAttr { - name: TokenStream::from(TokenTree::Ident(Ident::new(attr_name, Span::call_site()))), + name: AttrName::Fixed { + value: TokenStream::from(TokenTree::Ident(Ident::new(attr_name, Span::call_site()))), + }, attr_type: AttrType::Normal { value: Markup::Block(Block { markups, diff --git a/maud_macros/src/parse.rs b/maud_macros/src/parse.rs index 05af2894..ffb4e04b 100644 --- a/maud_macros/src/parse.rs +++ b/maud_macros/src/parse.rs @@ -13,7 +13,7 @@ pub fn parse(input: TokenStream) -> Vec { #[derive(Clone)] struct Parser { /// If we're inside an attribute, then this contains the attribute name. - current_attr: Option, + current_attr: Option, input: ::IntoIter, } @@ -580,48 +580,7 @@ impl Parser { let mut attrs = Vec::new(); loop { if let Some(name) = self.try_namespaced_name() { - // Attribute - match self.peek() { - // Non-empty attribute - Some(TokenTree::Punct(ref punct)) if punct.as_char() == '=' => { - self.advance(); - // Parse a value under an attribute context - assert!(self.current_attr.is_none()); - self.current_attr = Some(ast::name_to_string(name.clone())); - let attr_type = match self.attr_toggler() { - Some(toggler) => ast::AttrType::Optional { toggler }, - None => { - let value = self.markup(); - ast::AttrType::Normal { value } - } - }; - self.current_attr = None; - attrs.push(ast::Attr::Named { - named_attr: ast::NamedAttr { name, attr_type }, - }); - } - // Empty attribute (legacy syntax) - Some(TokenTree::Punct(ref punct)) if punct.as_char() == '?' => { - self.advance(); - let toggler = self.attr_toggler(); - attrs.push(ast::Attr::Named { - named_attr: ast::NamedAttr { - name: name.clone(), - attr_type: ast::AttrType::Empty { toggler }, - }, - }); - } - // Empty attribute (new syntax) - _ => { - let toggler = self.attr_toggler(); - attrs.push(ast::Attr::Named { - named_attr: ast::NamedAttr { - name: name.clone(), - attr_type: ast::AttrType::Empty { toggler }, - }, - }); - } - } + attrs.push(self.attr(ast::AttrName::Fixed { value: name })); } else { match self.peek() { // Class shorthand @@ -644,6 +603,18 @@ impl Parser { name, }); } + // Spliced attribute name + Some(TokenTree::Group(ref group)) + if group.delimiter() == Delimiter::Parenthesis => + { + match self.markup() { + ast::Markup::Splice { expr, .. } => { + attrs.push(self.attr(ast::AttrName::Splice { expr })); + } + // If it's not a splice, backtrack and bail out + _ => break, + } + } // If it's not a valid attribute, backtrack and bail out _ => break, } @@ -665,7 +636,7 @@ impl Parser { ast::Attr::Id { .. } => "id".to_string(), ast::Attr::Named { named_attr } => named_attr .name - .clone() + .tokens() .into_iter() .map(|token| token.to_string()) .collect(), @@ -685,6 +656,50 @@ impl Parser { attrs } + fn attr(&mut self, name: ast::AttrName) -> ast::Attr { + match self.peek() { + // Non-empty attribute + Some(TokenTree::Punct(ref punct)) if punct.as_char() == '=' => { + self.advance(); + // Parse a value under an attribute context + assert!(self.current_attr.is_none()); + self.current_attr = Some(name.clone()); + let attr_type = match self.attr_toggler() { + Some(toggler) => ast::AttrType::Optional { toggler }, + None => { + let value = self.markup(); + ast::AttrType::Normal { value } + } + }; + self.current_attr = None; + ast::Attr::Named { + named_attr: ast::NamedAttr { name, attr_type }, + } + } + // Empty attribute (legacy syntax) + Some(TokenTree::Punct(ref punct)) if punct.as_char() == '?' => { + self.advance(); + let toggler = self.attr_toggler(); + ast::Attr::Named { + named_attr: ast::NamedAttr { + name, + attr_type: ast::AttrType::Empty { toggler }, + }, + } + } + // Empty attribute (new syntax) + _ => { + let toggler = self.attr_toggler(); + ast::Attr::Named { + named_attr: ast::NamedAttr { + name, + attr_type: ast::AttrType::Empty { toggler }, + }, + } + } + } + } + /// Parses the name of a class or ID. fn class_or_id_name(&mut self) -> ast::Markup { if let Some(symbol) = self.try_name() { From eaac99f70f50953c7c1921334f1de5de61cb5395 Mon Sep 17 00:00:00 2001 From: Bad Manners Date: Mon, 28 Oct 2024 22:52:29 -0300 Subject: [PATCH 2/6] Hacky fix to prevent XSS --- maud/src/lib.rs | 61 +++++++++++++++++++++++++++++++++++++ maud/tests/splices.rs | 14 +++++++++ maud_macros/src/generate.rs | 9 +++++- 3 files changed, 83 insertions(+), 1 deletion(-) diff --git a/maud/src/lib.rs b/maud/src/lib.rs index beaaea23..cd7d13c3 100644 --- a/maud/src/lib.rs +++ b/maud/src/lib.rs @@ -418,6 +418,67 @@ pub mod macro_private { use alloc::string::String; use core::fmt::Display; + pub fn strip_to_attr_name(input: &str, output: &mut String) { + for c in input.chars() { + match c { + ' ' + | '"' + | '\'' + | '>' + | '/' + | '=' + | '\u{0000}'..='\u{001F}' + | '\u{FDD0}'..='\u{FDEF}' + | '\u{FFFE}' + | '\u{FFFF}' + | '\u{1FFFE}' + | '\u{1FFFF}' + | '\u{2FFFE}' + | '\u{2FFFF}' + | '\u{3FFFE}' + | '\u{3FFFF}' + | '\u{4FFFE}' + | '\u{4FFFF}' + | '\u{5FFFE}' + | '\u{5FFFF}' + | '\u{6FFFE}' + | '\u{6FFFF}' + | '\u{7FFFE}' + | '\u{7FFFF}' + | '\u{8FFFE}' + | '\u{8FFFF}' + | '\u{9FFFE}' + | '\u{9FFFF}' + | '\u{AFFFE}' + | '\u{AFFFF}' + | '\u{BFFFE}' + | '\u{BFFFF}' + | '\u{CFFFE}' + | '\u{CFFFF}' + | '\u{DFFFE}' + | '\u{DFFFF}' + | '\u{EFFFE}' + | '\u{EFFFF}' + | '\u{FFFFE}' + | '\u{FFFFF}' + | '\u{10FFFE}' + | '\u{10FFFF}' => (), + _ => output.push(c), + } + } + } + + #[doc(hidden)] + #[macro_export] + macro_rules! render_attr_name { + ($x:expr, $buffer:expr) => {{ + use $crate::macro_private::strip_to_attr_name; + strip_to_attr_name(($x), $buffer); + }}; + } + + pub use render_attr_name; + #[doc(hidden)] #[macro_export] macro_rules! render_to { diff --git a/maud/tests/splices.rs b/maud/tests/splices.rs index 7e2b6d0c..3769d610 100644 --- a/maud/tests/splices.rs +++ b/maud/tests/splices.rs @@ -83,6 +83,20 @@ fn attribute_name() { ); } +#[test] +fn no_xss_from_spliced_attributes() { + let evil_tuple = ( + "x onclick=\"alert(42);\" x", + "\" onclick=alert(24); href=\"", + ); + let result = + html! { button (format!("data-{}", evil_tuple.0))=(evil_tuple.1) { "XSS be gone!" } }; + assert_eq!( + result.into_string(), + r#""# + ); +} + /// An example struct, for testing purposes only struct Creature { name: &'static str, diff --git a/maud_macros/src/generate.rs b/maud_macros/src/generate.rs index f432a500..d00157c3 100644 --- a/maud_macros/src/generate.rs +++ b/maud_macros/src/generate.rs @@ -106,6 +106,13 @@ impl Generator { build.push_tokens(quote!(maud::macro_private::render_to!(&(#expr), &mut #output_ident);)); } + fn splice_attr_name(&self, expr: TokenStream, build: &mut Builder) { + let output_ident = self.output_ident.clone(); + build.push_tokens( + quote!(maud::macro_private::render_attr_name!(&(#expr), &mut #output_ident);), + ); + } + fn element(&self, name: TokenStream, attrs: Vec, body: ElementBody, build: &mut Builder) { build.push_str("<"); self.name(name.clone(), build); @@ -126,7 +133,7 @@ impl Generator { fn attr_name(&self, name: AttrName, build: &mut Builder) { match name { AttrName::Fixed { value } => self.name(value, build), - AttrName::Splice { expr, .. } => self.splice(expr, build), + AttrName::Splice { expr, .. } => self.splice_attr_name(expr, build), } } From 8d721fb430c2347f5e618fb0e523df4d5371b00f Mon Sep 17 00:00:00 2001 From: Bad Manners Date: Tue, 29 Oct 2024 18:31:39 -0300 Subject: [PATCH 3/6] Undo autoformatting change --- docs/content/splices-toggles.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/content/splices-toggles.md b/docs/content/splices-toggles.md index 35bb5264..a661794e 100644 --- a/docs/content/splices-toggles.md +++ b/docs/content/splices-toggles.md @@ -160,7 +160,7 @@ html! { ### Optional attributes with values: `title=[Some("value")]` -Add optional attributes to an element using `attr=[value]` syntax, with _square_ brackets. +Add optional attributes to an element using `attr=[value]` syntax, with *square* brackets. These are only rendered if the value is `Some`, and entirely omitted if the value is `None`. ```rust From 59c6688c6dd105e0d93daee043ba1cd69ef67a43 Mon Sep 17 00:00:00 2001 From: Bad Manners Date: Wed, 30 Oct 2024 19:53:02 -0300 Subject: [PATCH 4/6] Less restrictive strip_to_attr_name --- maud/src/lib.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/maud/src/lib.rs b/maud/src/lib.rs index cd7d13c3..91766164 100644 --- a/maud/src/lib.rs +++ b/maud/src/lib.rs @@ -418,8 +418,8 @@ pub mod macro_private { use alloc::string::String; use core::fmt::Display; - pub fn strip_to_attr_name(input: &str, output: &mut String) { - for c in input.chars() { + pub fn strip_to_attr_name(input: impl Display, output: &mut String) { + for c in alloc::format!("{}", input).chars() { match c { ' ' | '"' @@ -473,7 +473,7 @@ pub mod macro_private { macro_rules! render_attr_name { ($x:expr, $buffer:expr) => {{ use $crate::macro_private::strip_to_attr_name; - strip_to_attr_name(($x), $buffer); + strip_to_attr_name($x, $buffer); }}; } From a7386e0c385b7a67ee89aec89fd630c4c3601cb0 Mon Sep 17 00:00:00 2001 From: Bad Manners Date: Wed, 30 Oct 2024 20:28:42 -0300 Subject: [PATCH 5/6] Display => AsRef --- maud/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/maud/src/lib.rs b/maud/src/lib.rs index 91766164..6cb17e58 100644 --- a/maud/src/lib.rs +++ b/maud/src/lib.rs @@ -418,8 +418,8 @@ pub mod macro_private { use alloc::string::String; use core::fmt::Display; - pub fn strip_to_attr_name(input: impl Display, output: &mut String) { - for c in alloc::format!("{}", input).chars() { + pub fn strip_to_attr_name(input: impl AsRef, output: &mut String) { + for c in input.as_ref().chars() { match c { ' ' | '"' From c9d097f6466a6ebbb5b9f8eddf58647466a1ca72 Mon Sep 17 00:00:00 2001 From: Bad Manners Date: Wed, 30 Oct 2024 22:00:28 -0300 Subject: [PATCH 6/6] Missing control characters --- maud/src/lib.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/maud/src/lib.rs b/maud/src/lib.rs index 6cb17e58..6982c188 100644 --- a/maud/src/lib.rs +++ b/maud/src/lib.rs @@ -428,6 +428,7 @@ pub mod macro_private { | '/' | '=' | '\u{0000}'..='\u{001F}' + | '\u{007F}'..='\u{009F}' | '\u{FDD0}'..='\u{FDEF}' | '\u{FFFE}' | '\u{FFFF}'