From 33fd31754af6cc55436ec65061c6a38cbd2b6d68 Mon Sep 17 00:00:00 2001 From: Dessalines Date: Sun, 4 Aug 2024 09:45:53 -0400 Subject: [PATCH] Adding a URL max length lemmy error. (#4960) * Adding a URL max length error. - Also increasing the post.url max length to 2000 (seems standard) - I ran into this when fixing torrent support, which often use longer urls. * Fixing sql_format. --- crates/api_crud/src/post/create.rs | 6 +-- crates/api_crud/src/post/update.rs | 6 +-- crates/apub/src/objects/post.rs | 4 +- crates/db_schema/src/schema.rs | 2 +- crates/utils/src/error.rs | 1 + crates/utils/src/utils/validation.rs | 38 ++++++++++++++----- .../down.sql | 3 ++ .../up.sql | 5 +++ 8 files changed, 47 insertions(+), 18 deletions(-) create mode 100644 migrations/2024-08-03-155932_increase_post_url_max_length/down.sql create mode 100644 migrations/2024-08-03-155932_increase_post_url_max_length/up.sql diff --git a/crates/api_crud/src/post/create.rs b/crates/api_crud/src/post/create.rs index 0f6abc2aa..a0f0b7525 100644 --- a/crates/api_crud/src/post/create.rs +++ b/crates/api_crud/src/post/create.rs @@ -35,11 +35,11 @@ use lemmy_utils::{ utils::{ slurs::check_slurs, validation::{ - check_url_scheme, is_url_blocked, is_valid_alt_text_field, is_valid_body_field, is_valid_post_title, + is_valid_url, }, }, }; @@ -69,11 +69,11 @@ pub async fn create_post( if let Some(url) = &url { is_url_blocked(url, &url_blocklist)?; - check_url_scheme(url)?; + is_valid_url(url)?; } if let Some(custom_thumbnail) = &custom_thumbnail { - check_url_scheme(custom_thumbnail)?; + is_valid_url(custom_thumbnail)?; } if let Some(alt_text) = &data.alt_text { diff --git a/crates/api_crud/src/post/update.rs b/crates/api_crud/src/post/update.rs index cd4b2d41b..835398596 100644 --- a/crates/api_crud/src/post/update.rs +++ b/crates/api_crud/src/post/update.rs @@ -28,11 +28,11 @@ use lemmy_utils::{ utils::{ slurs::check_slurs, validation::{ - check_url_scheme, is_url_blocked, is_valid_alt_text_field, is_valid_body_field, is_valid_post_title, + is_valid_url, }, }, }; @@ -77,11 +77,11 @@ pub async fn update_post( if let Some(Some(url)) = &url { is_url_blocked(url, &url_blocklist)?; - check_url_scheme(url)?; + is_valid_url(url)?; } if let Some(Some(custom_thumbnail)) = &custom_thumbnail { - check_url_scheme(custom_thumbnail)?; + is_valid_url(custom_thumbnail)?; } let post_id = data.post_id; diff --git a/crates/apub/src/objects/post.rs b/crates/apub/src/objects/post.rs index 7e4254840..0364039bb 100644 --- a/crates/apub/src/objects/post.rs +++ b/crates/apub/src/objects/post.rs @@ -41,7 +41,7 @@ use lemmy_db_views_actor::structs::CommunityModeratorView; use lemmy_utils::{ error::{LemmyError, LemmyErrorType, LemmyResult}, spawn_try_task, - utils::{markdown::markdown_to_html, slurs::check_slurs_opt, validation::check_url_scheme}, + utils::{markdown::markdown_to_html, slurs::check_slurs_opt, validation::is_valid_url}, }; use std::ops::Deref; use stringreader::StringReader; @@ -221,7 +221,7 @@ impl Object for ApubPost { }; if let Some(url) = &url { - check_url_scheme(url)?; + is_valid_url(url)?; } let alt_text = first_attachment.cloned().and_then(Attachment::alt_text); diff --git a/crates/db_schema/src/schema.rs b/crates/db_schema/src/schema.rs index c3102b578..fc418ec28 100644 --- a/crates/db_schema/src/schema.rs +++ b/crates/db_schema/src/schema.rs @@ -711,7 +711,7 @@ diesel::table! { id -> Int4, #[max_length = 200] name -> Varchar, - #[max_length = 512] + #[max_length = 2000] url -> Nullable, body -> Nullable, creator_id -> Int4, diff --git a/crates/utils/src/error.rs b/crates/utils/src/error.rs index 542888612..860dad6fd 100644 --- a/crates/utils/src/error.rs +++ b/crates/utils/src/error.rs @@ -180,6 +180,7 @@ pub enum LemmyErrorType { InboxTimeout, Unknown(String), CantDeleteSite, + UrlLengthOverflow, } cfg_if! { diff --git a/crates/utils/src/utils/validation.rs b/crates/utils/src/utils/validation.rs index 2f9e27f5c..0a59e2fea 100644 --- a/crates/utils/src/utils/validation.rs +++ b/crates/utils/src/utils/validation.rs @@ -21,6 +21,7 @@ const ALLOWED_POST_URL_SCHEMES: [&str; 3] = ["http", "https", "magnet"]; const BODY_MAX_LENGTH: usize = 10000; const POST_BODY_MAX_LENGTH: usize = 50000; const BIO_MAX_LENGTH: usize = 300; +const URL_MAX_LENGTH: usize = 2000; const ALT_TEXT_MAX_LENGTH: usize = 1500; const SITE_NAME_MAX_LENGTH: usize = 20; const SITE_NAME_MIN_LENGTH: usize = 1; @@ -284,11 +285,17 @@ pub fn check_site_visibility_valid( } } -pub fn check_url_scheme(url: &Url) -> LemmyResult<()> { +pub fn is_valid_url(url: &Url) -> LemmyResult<()> { if !ALLOWED_POST_URL_SCHEMES.contains(&url.scheme()) { Err(LemmyErrorType::InvalidUrlScheme)? } + max_length_check( + url.as_str(), + URL_MAX_LENGTH, + LemmyErrorType::UrlLengthOverflow, + )?; + Ok(()) } @@ -349,7 +356,6 @@ mod tests { utils::validation::{ build_and_check_regex, check_site_visibility_valid, - check_url_scheme, check_urls_are_valid, clean_url_params, is_url_blocked, @@ -358,11 +364,13 @@ mod tests { is_valid_display_name, is_valid_matrix_id, is_valid_post_title, + is_valid_url, site_description_length_check, site_name_length_check, BIO_MAX_LENGTH, SITE_DESCRIPTION_MAX_LENGTH, SITE_NAME_MAX_LENGTH, + URL_MAX_LENGTH, }, }; use pretty_assertions::assert_eq; @@ -580,15 +588,27 @@ mod tests { } #[test] - fn test_check_url_scheme() -> LemmyResult<()> { - assert!(check_url_scheme(&Url::parse("http://example.com")?).is_ok()); - assert!(check_url_scheme(&Url::parse("https://example.com")?).is_ok()); - assert!(check_url_scheme(&Url::parse("https://example.com")?).is_ok()); - assert!(check_url_scheme(&Url::parse("ftp://example.com")?).is_err()); - assert!(check_url_scheme(&Url::parse("javascript:void")?).is_err()); + fn test_check_url_valid() -> LemmyResult<()> { + assert!(is_valid_url(&Url::parse("http://example.com")?).is_ok()); + assert!(is_valid_url(&Url::parse("https://example.com")?).is_ok()); + assert!(is_valid_url(&Url::parse("https://example.com")?).is_ok()); + assert!(is_valid_url(&Url::parse("ftp://example.com")?) + .is_err_and(|e| e.error_type.eq(&LemmyErrorType::InvalidUrlScheme))); + assert!(is_valid_url(&Url::parse("javascript:void")?) + .is_err_and(|e| e.error_type.eq(&LemmyErrorType::InvalidUrlScheme))); let magnet_link="magnet:?xt=urn:btih:4b390af3891e323778959d5abfff4b726510f14c&dn=Ravel%20Complete%20Piano%20Sheet%20Music%20-%20Public%20Domain&tr=udp%3A%2F%2Fopen.tracker.cl%3A1337%2Fannounce"; - assert!(check_url_scheme(&Url::parse(magnet_link)?).is_ok()); + assert!(is_valid_url(&Url::parse(magnet_link)?).is_ok()); + + // Also make sure the length overflow hits an error + let mut long_str = "http://example.com/test=".to_string(); + for _ in 1..URL_MAX_LENGTH { + long_str.push('X'); + } + let long_url = Url::parse(&long_str)?; + assert!( + is_valid_url(&long_url).is_err_and(|e| e.error_type.eq(&LemmyErrorType::UrlLengthOverflow)) + ); Ok(()) } diff --git a/migrations/2024-08-03-155932_increase_post_url_max_length/down.sql b/migrations/2024-08-03-155932_increase_post_url_max_length/down.sql new file mode 100644 index 000000000..d25918578 --- /dev/null +++ b/migrations/2024-08-03-155932_increase_post_url_max_length/down.sql @@ -0,0 +1,3 @@ +ALTER TABLE post + ALTER COLUMN url TYPE varchar(512); + diff --git a/migrations/2024-08-03-155932_increase_post_url_max_length/up.sql b/migrations/2024-08-03-155932_increase_post_url_max_length/up.sql new file mode 100644 index 000000000..7c6818d22 --- /dev/null +++ b/migrations/2024-08-03-155932_increase_post_url_max_length/up.sql @@ -0,0 +1,5 @@ +-- Change the post url max limit to 2000 +-- From here: https://stackoverflow.com/questions/417142/what-is-the-maximum-length-of-a-url-in-different-browsers#417184 +ALTER TABLE post + ALTER COLUMN url TYPE varchar(2000); +