diff --git a/nostr/CHANGELOG.md b/nostr/CHANGELOG.md index 0e816c8f3..e7b500f9e 100644 --- a/nostr/CHANGELOG.md +++ b/nostr/CHANGELOG.md @@ -118,6 +118,7 @@ - Hardware-accelerate SHA-256 where available (https://github.com/nostrdevkit/nostr/pull/1419) - Reduce allocations and redundant key parsing in the NIP-44 v2 path (https://github.com/nostrdevkit/nostr/pull/1421) - Serialize and deserialize `RelayMessage` without a `serde_json::Value` tree (https://github.com/nostrdevkit/nostr/pull/1425) +- Avoid allocating a `String` per generic tag key when serializing and deserializing `Filter` (https://github.com/nostrdevkit/nostr/pull/1427) ### Security diff --git a/nostr/src/filter/mod.rs b/nostr/src/filter/mod.rs index e5843b3e0..75e9452ea 100644 --- a/nostr/src/filter/mod.rs +++ b/nostr/src/filter/mod.rs @@ -695,12 +695,60 @@ where S: Serializer, { let mut map = serializer.serialize_map(Some(generic_tags.len()))?; + + // The key is always `#` followed by a single ASCII letter, so it fits in a + // fixed buffer. `format!` would allocate a `String` for every entry. + let mut key: [u8; 2] = [b'#', 0]; + for (tag, values) in generic_tags.iter() { - map.serialize_entry(&format!("#{tag}"), values)?; + key[1] = tag.as_byte(); + let key: &str = core::str::from_utf8(&key).map_err(serde::ser::Error::custom)?; + map.serialize_entry(key, values)?; } + map.end() } +/// A generic tag query key. +/// +/// `Some` when the key is `#` followed by a single letter, `None` for any other +/// key, which is ignored. Deserializing through this avoids allocating a +/// `String` for every key in the map. +struct TagKey(Option); + +impl<'de> Deserialize<'de> for TagKey { + fn deserialize(deserializer: D) -> Result + where + D: Deserializer<'de>, + { + struct TagKeyVisitor; + + impl Visitor<'_> for TagKeyVisitor { + type Value = TagKey; + + fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result { + formatter.write_str("a map key") + } + + fn visit_str(self, v: &str) -> Result + where + E: serde::de::Error, + { + let mut chars = v.chars(); + if let (Some('#'), Some(ch), None) = (chars.next(), chars.next(), chars.next()) { + let tag: SingleLetterTag = + SingleLetterTag::from_char(ch).map_err(serde::de::Error::custom)?; + Ok(TagKey(Some(tag))) + } else { + Ok(TagKey(None)) + } + } + } + + deserializer.deserialize_str(TagKeyVisitor) + } +} + fn deserialize_generic_tags<'de, D>(deserializer: D) -> Result where D: Deserializer<'de>, @@ -719,15 +767,16 @@ where M: MapAccess<'de>, { let mut generic_tags = BTreeMap::new(); - while let Some(key) = map.next_key::()? { - let mut chars = key.chars(); - if let (Some('#'), Some(ch), None) = (chars.next(), chars.next(), chars.next()) { - let tag: SingleLetterTag = - SingleLetterTag::from_char(ch).map_err(serde::de::Error::custom)?; - let values: BTreeSet = map.next_value()?; - generic_tags.insert(tag, values); - } else { - map.next_value::()?; + while let Some(TagKey(tag)) = map.next_key::()? { + match tag { + Some(tag) => { + let values: BTreeSet = map.next_value()?; + generic_tags.insert(tag, values); + } + // Not a `#X` key, so it isn't a generic tag query. + None => { + map.next_value::()?; + } } } Ok(generic_tags) @@ -917,6 +966,38 @@ mod tests { assert_eq!(filter, Filter::new().search("test")); } + /// A key is a generic tag query only when it is `#` followed by exactly one + /// letter. Anything else is ignored, except a single non-letter character, + /// which is rejected. + #[test] + fn test_generic_tag_key_shapes() { + // Ignored: not `#` + exactly one character. + for json in [ + r##"{"#ab":["x"],"search":"test"}"##, + r##"{"#":["x"],"search":"test"}"##, + r##"{"t":["x"],"search":"test"}"##, + r##"{"":["x"],"search":"test"}"##, + ] { + let filter = Filter::from_json(json).unwrap(); + assert_eq!(filter, Filter::new().search("test"), "{json}"); + } + + // Rejected: `#` followed by a single non-letter. + for json in [ + r##"{"#1":["x"]}"##, + r##"{"#_":["x"]}"##, + r##"{"# ":["x"]}"##, + r###"{"##":["x"]}"###, + ] { + assert!(Filter::from_json(json).is_err(), "{json}"); + } + + // Accepted, both cases. + let filter = Filter::from_json(r##"{"#t":["x"],"#T":["y"]}"##).unwrap(); + assert_eq!(filter.generic_tags.len(), 2); + assert_eq!(filter.as_json(), r##"{"#t":["x"],"#T":["y"]}"##); + } + #[test] fn test_filter_is_empty() { let filter = Filter::new().identifier("test");