From f5c299f23c997d7e5c6deac737fb3472bfe301f3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20Gali=C4=87?= Date: Fri, 14 Sep 2018 15:14:24 +0200 Subject: [PATCH 1/6] make blog/instance description a SafeString long_description & short_description's documentation say they can be Markdown, but they are String, not SafeString. This led to escaped strings being printed in the editor https://github.com/Plume-org/Plume/issues/220 --- plume-models/src/blogs.rs | 5 +++-- plume-models/src/instance.rs | 9 +++++---- plume-models/src/users.rs | 4 ++-- src/routes/instance.rs | 4 ++-- src/setup.rs | 5 +++-- 5 files changed, 15 insertions(+), 12 deletions(-) diff --git a/plume-models/src/blogs.rs b/plume-models/src/blogs.rs index a992f748..9c73a7d6 100644 --- a/plume-models/src/blogs.rs +++ b/plume-models/src/blogs.rs @@ -22,6 +22,7 @@ use plume_common::activity_pub::{ inbox::WithInbox, sign }; +use safe_string::SafeString; use instance::*; use users::User; use schema::blogs; @@ -142,8 +143,8 @@ impl Blog { name: inst.clone(), local: false, // We don't really care about all the following for remote instances - long_description: String::new(), - short_description: String::new(), + long_description: SafeString::new(&::new()), + short_description: SafeString::new(&::new()), default_license: String::new(), open_registrations: true, short_description_html: String::new(), diff --git a/plume-models/src/instance.rs b/plume-models/src/instance.rs index 52830aa6..a2503062 100644 --- a/plume-models/src/instance.rs +++ b/plume-models/src/instance.rs @@ -3,6 +3,7 @@ use diesel::{self, QueryDsl, RunQueryDsl, ExpressionMethods, PgConnection}; use std::iter::Iterator; use plume_common::utils::md_to_html; +use safe_string::SafeString; use ap_url; use users::User; use schema::{instances, users}; @@ -16,8 +17,8 @@ pub struct Instance { pub blocked: bool, pub creation_date: NaiveDateTime, pub open_registrations: bool, - pub short_description: String, - pub long_description: String, + pub short_description: SafeString, + pub long_description: SafeString, pub default_license : String, pub long_description_html: String, pub short_description_html: String @@ -30,8 +31,8 @@ pub struct NewInstance { pub name: String, pub local: bool, pub open_registrations: bool, - pub short_description: String, - pub long_description: String, + pub short_description: SafeString, + pub long_description: SafeString, pub default_license : String, pub long_description_html: String, pub short_description_html: String diff --git a/plume-models/src/users.rs b/plume-models/src/users.rs index 9cfad4e3..9c2c8113 100644 --- a/plume-models/src/users.rs +++ b/plume-models/src/users.rs @@ -205,8 +205,8 @@ impl User { public_domain: inst.clone(), local: false, // We don't really care about all the following for remote instances - long_description: String::new(), - short_description: String::new(), + long_description: SafeString::new(&::new()), + short_description: SafeString::new(&::new()), default_license: String::new(), open_registrations: true, short_description_html: String::new(), diff --git a/src/routes/instance.rs b/src/routes/instance.rs index 6905ecbc..6f2a970a 100644 --- a/src/routes/instance.rs +++ b/src/routes/instance.rs @@ -125,8 +125,8 @@ fn update_settings(conn: DbConn, admin: Admin, form: LenientForm::new()), + short_description: SafeString::new(&::new()), default_license: String::from("CC-0"), open_registrations: true, short_description_html: String::new(), From 65e213309bb130a02160c371408c1c745083c796 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20Gali=C4=87?= Date: Fri, 14 Sep 2018 18:24:27 +0200 Subject: [PATCH 2/6] do not allocate empty strings follow review from @pwoolcoc, and do not use SafeString::new(&::new()) since this makes an allocation which will then just be thrown away. Instead, we pass "" --- plume-models/src/blogs.rs | 4 ++-- plume-models/src/users.rs | 4 ++-- src/routes/instance.rs | 4 ++-- src/setup.rs | 4 ++-- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/plume-models/src/blogs.rs b/plume-models/src/blogs.rs index 9c73a7d6..c825f6aa 100644 --- a/plume-models/src/blogs.rs +++ b/plume-models/src/blogs.rs @@ -143,8 +143,8 @@ impl Blog { name: inst.clone(), local: false, // We don't really care about all the following for remote instances - long_description: SafeString::new(&::new()), - short_description: SafeString::new(&::new()), + long_description: SafeString::new(""), + short_description: SafeString::new(""), default_license: String::new(), open_registrations: true, short_description_html: String::new(), diff --git a/plume-models/src/users.rs b/plume-models/src/users.rs index 9c2c8113..d8addb19 100644 --- a/plume-models/src/users.rs +++ b/plume-models/src/users.rs @@ -205,8 +205,8 @@ impl User { public_domain: inst.clone(), local: false, // We don't really care about all the following for remote instances - long_description: SafeString::new(&::new()), - short_description: SafeString::new(&::new()), + long_description: SafeString::new(""), + short_description: SafeString::new(""), default_license: String::new(), open_registrations: true, short_description_html: String::new(), diff --git a/src/routes/instance.rs b/src/routes/instance.rs index 6f2a970a..6905ecbc 100644 --- a/src/routes/instance.rs +++ b/src/routes/instance.rs @@ -125,8 +125,8 @@ fn update_settings(conn: DbConn, admin: Admin, form: LenientForm::new()), - short_description: SafeString::new(&::new()), + long_description: SafeString::new(""), + short_description: SafeString::new(""), default_license: String::from("CC-0"), open_registrations: true, short_description_html: String::new(), From 0897088aa5071034f4099e1de4b6271ffb8fb423 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20Gali=C4=87?= Date: Fri, 14 Sep 2018 18:26:42 +0200 Subject: [PATCH 3/6] add implementation for FromFormValue for SafeString thanks again to @pwoolcoc for this! --- plume-models/src/safe_string.rs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/plume-models/src/safe_string.rs b/plume-models/src/safe_string.rs index 98897434..886e385b 100644 --- a/plume-models/src/safe_string.rs +++ b/plume-models/src/safe_string.rs @@ -101,3 +101,17 @@ impl AsRef for SafeString { &self.value } } + +use rocket::request::FromFormValue; +use rocket::http::RawStr; + +impl<'v> FromFormValue<'v> for SafeString { + type Error = &'v RawStr; + + fn from_form_value(form_value: &'v RawStr) -> Result { + let val = String::from_form_value(form_value)?; + Ok(SafeString { + value: val, + }) + } +} From d62c72dde01afd93aa2a693970d3a0fe9b4c8d11 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20Gali=C4=87?= Date: Fri, 14 Sep 2018 19:50:59 +0200 Subject: [PATCH 4/6] allocate new SafeString in FromFormValue impl thanks to @fdb-hiroshima for this review! --- plume-models/src/safe_string.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/plume-models/src/safe_string.rs b/plume-models/src/safe_string.rs index 886e385b..4cadbd64 100644 --- a/plume-models/src/safe_string.rs +++ b/plume-models/src/safe_string.rs @@ -110,8 +110,6 @@ impl<'v> FromFormValue<'v> for SafeString { fn from_form_value(form_value: &'v RawStr) -> Result { let val = String::from_form_value(form_value)?; - Ok(SafeString { - value: val, - }) + Ok(SafeString::new(&val)) } } From 06718a5c8a0af6c9facd10debfadae100fd25aa2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20Gali=C4=87?= Date: Fri, 14 Sep 2018 20:25:16 +0200 Subject: [PATCH 5/6] directly use SafeString in InstanceSettingsForm --- plume-models/src/instance.rs | 2 +- src/routes/instance.rs | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/plume-models/src/instance.rs b/plume-models/src/instance.rs index a2503062..8466fa68 100644 --- a/plume-models/src/instance.rs +++ b/plume-models/src/instance.rs @@ -115,7 +115,7 @@ impl Instance { )) } - pub fn update(&self, conn: &PgConnection, name: String, open_registrations: bool, short_description: String, long_description: String) -> Instance { + pub fn update(&self, conn: &PgConnection, name: String, open_registrations: bool, short_description: SafeString, long_description: SafeString) -> Instance { let (sd, _) = md_to_html(short_description.as_ref()); let (ld, _) = md_to_html(long_description.as_ref()); diesel::update(self) diff --git a/src/routes/instance.rs b/src/routes/instance.rs index 6905ecbc..25f56bb5 100644 --- a/src/routes/instance.rs +++ b/src/routes/instance.rs @@ -10,7 +10,9 @@ use plume_models::{ db_conn::DbConn, posts::Post, users::User, + safe_string::SafeString, instance::* + }; use inbox::Inbox; use routes::Page; @@ -110,8 +112,8 @@ struct InstanceSettingsForm { #[validate(length(min = "1"))] name: String, open_registrations: bool, - short_description: String, - long_description: String, + short_description: SafeString, + long_description: SafeString, #[validate(length(min = "1"))] default_license: String } From fb074e63442c35c8ec400789b47dab651398c361 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20Gali=C4=87?= Date: Fri, 14 Sep 2018 21:44:32 +0200 Subject: [PATCH 6/6] render SafeString thru |safe thanks again to @fdb-hiroshima for pointing me in the right direction! --- templates/instance/admin.html.tera | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/templates/instance/admin.html.tera b/templates/instance/admin.html.tera index b35ddcdd..a3fc592f 100644 --- a/templates/instance/admin.html.tera +++ b/templates/instance/admin.html.tera @@ -23,10 +23,10 @@ - + - + {{ macros::input(name="default_license", label="Default license", errors=errors, form=form, props='minlenght="1"', default=instance) }}