Merge pull request #2194 from scpwiki/WJ-1288-filename-restrictions

[WJ-1288] Add restrictions to filenames

deepwell/Cargo.toml

@@ -8,7 +8,7 @@ keywords = ["wikijump", "api", "backend", "wiki"]
 categories = ["asynchronous", "database", "web-programming::http-server"]
 exclude = [".gitignore", ".editorconfig"]
 
-version = "2024.11.29"
+version = "2024.11.30"
 authors = ["Emmie Smith <[email protected]>"]
 edition = "2021"
 

deepwell/src/services/error.rs

@@ -145,7 +145,10 @@ pub enum Error {
     FileNameEmpty,
 
     #[error("File name too long")]
-    FileNameTooLong,
+    FileNameTooLong { length: usize, maximum: usize },
+
+    #[error("File name contains invalid characters (control chars or slashes)")]
+    FileNameInvalidCharacters,
 
     #[error("File MIME type cannot be empty")]
     FileMimeEmpty,
@@ -381,28 +384,29 @@ impl Error {
             Error::FilterRegexInvalid(_) => 4005,
             Error::FilterNotDeleted => 4006,
             Error::FileNameEmpty => 4007,
-            Error::FileNameTooLong => 4008,
-            Error::FileMimeEmpty => 4009,
-            Error::FileNotDeleted => 4010,
-            Error::PageNotDeleted => 4011,
-            Error::PageSlugEmpty => 4012,
-            Error::SiteSlugEmpty => 4013,
-            Error::UserNameTooShort => 4014,
-            Error::UserSlugEmpty => 4015,
-            Error::UserEmailEmpty => 4022,
-            Error::MessageSubjectEmpty => 4016,
-            Error::MessageSubjectTooLong => 4017,
-            Error::MessageBodyEmpty => 4018,
-            Error::MessageBodyTooLong => 4019,
-            Error::MessageNoRecipients => 4020,
-            Error::MessageTooManyRecipients => 4021,
-            Error::BlobWrongUser => 4022,
-            Error::BlobTooBig => 4023,
-            Error::BlobNotUploaded => 4024,
-            Error::BlobSizeMismatch { .. } => 4025,
-            Error::BlobBlacklisted(_) => 4026,
-            Error::BlobCannotBlacklistExisting => 4027,
-            Error::NotLatestRevisionId => 4028,
+            Error::FileNameTooLong { .. } => 4008,
+            Error::FileNameInvalidCharacters => 4009,
+            Error::FileMimeEmpty => 4010,
+            Error::FileNotDeleted => 4011,
+            Error::PageNotDeleted => 4012,
+            Error::PageSlugEmpty => 4013,
+            Error::SiteSlugEmpty => 4014,
+            Error::UserNameTooShort => 4015,
+            Error::UserSlugEmpty => 4016,
+            Error::UserEmailEmpty => 4017,
+            Error::MessageSubjectEmpty => 4018,
+            Error::MessageSubjectTooLong => 4019,
+            Error::MessageBodyEmpty => 4020,
+            Error::MessageBodyTooLong => 4021,
+            Error::MessageNoRecipients => 4022,
+            Error::MessageTooManyRecipients => 4023,
+            Error::BlobWrongUser => 4024,
+            Error::BlobTooBig => 4025,
+            Error::BlobNotUploaded => 4026,
+            Error::BlobSizeMismatch { .. } => 4027,
+            Error::BlobBlacklisted(_) => 4028,
+            Error::BlobCannotBlacklistExisting => 4029,
+            Error::NotLatestRevisionId => 4030,
 
             // 4100 -- Localization
             Error::LocaleInvalid(_) => 4100,
@@ -456,6 +460,10 @@ impl Error {
                 "expected": expected,
                 "actual": actual,
             }),
+            Error::FileNameTooLong { length, maximum } => json!({
+                "length": length,
+                "maximum": maximum,
+            }),
 
             // Emit as-is
             Error::EmailVerification(value) => json!(value),

deepwell/src/services/file/service.rs

@@ -34,8 +34,16 @@ use crate::services::file_revision::{
 use crate::services::filter::{FilterClass, FilterType};
 use crate::services::{BlobService, FileRevisionService, FilterService};
 use crate::types::FileOrder;
+use crate::utils::regex_replace_in_place;
+use once_cell::sync::Lazy;
+use regex::Regex;
 use sea_orm::ActiveValue;
 
+pub const MAXIMUM_FILE_NAME_LENGTH: usize = 256;
+
+static LEADING_TRAILING_SPACES: Lazy<Regex> =
+    Lazy::new(|| Regex::new(r"(^\s+)|(\s+$)").unwrap());
+
 #[derive(Debug)]
 pub struct FileService;
 
@@ -51,7 +59,7 @@ impl FileService {
         CreateFile {
             site_id,
             page_id,
-            name,
+            mut name,
             uploaded_blob_id,
             direct_upload,
             revision_comments,
@@ -64,7 +72,7 @@ impl FileService {
         let txn = ctx.transaction();
 
         // Verify filename is valid
-        check_file_name(&name)?;
+        check_file_name(&mut name)?;
 
         // Ensure row consistency
         Self::check_conflicts(ctx, page_id, &name, "create").await?;
@@ -145,7 +153,7 @@ impl FileService {
         check_last_revision(&last_revision, last_revision_id)?;
 
         let EditFileBody {
-            name,
+            mut name,
             licensing,
             uploaded_blob_id,
             direct_upload,
@@ -157,7 +165,7 @@ impl FileService {
         //
         // If the name isn't changing, then we already verified this
         // when the file was originally created.
-        if let Maybe::Set(ref name) = name {
+        if let Maybe::Set(ref mut name) = name {
             check_file_name(name)?;
             Self::check_conflicts(ctx, page_id, name, "update").await?;
             new_name = ActiveValue::Set(name.clone());
@@ -255,15 +263,15 @@ impl FileService {
         check_last_revision(&last_revision, last_revision_id)?;
 
         // Get destination filename
-        let name = name.unwrap_or_else(|| last_revision.name.clone());
+        let mut name = name.unwrap_or_else(|| last_revision.name.clone());
 
         info!(
             "Moving file with ID {} from page ID {} to {}",
             file_id, current_page_id, destination_page_id,
         );
 
         // Verify filename is valid
-        check_file_name(&name)?;
+        check_file_name(&mut name)?;
 
         // Ensure there isn't a file with this name on the destination page
         Self::check_conflicts(ctx, destination_page_id, &name, "move").await?;
@@ -772,8 +780,47 @@ impl FileService {
 }
 
 /// Verifies that this filename is valid.
-fn check_file_name(_name: &str) -> Result<()> {
-    // TODO https://scuttle.atlassian.net/browse/WJ-1288
+///
+/// This helper function is generally read-only, but if
+/// it finds a name which has leading or trailing whitespace,
+/// then it trims that off in-place.
+fn check_file_name(name: &mut String) -> Result<()> {
+    // Removes leading or trailing whitespace
+    regex_replace_in_place(name, &LEADING_TRAILING_SPACES, "");
+    debug!("Trimmed file name: '{name}'");
+
+    // Disallow empty filenames
+    if name.is_empty() {
+        error!("File name is empty");
+        return Err(Error::FileNameEmpty);
+    }
+
+    // Limit filename length
+    if name.len() >= MAXIMUM_FILE_NAME_LENGTH {
+        error!(
+            "File name of invalid length: {} > {}",
+            name.len(),
+            MAXIMUM_FILE_NAME_LENGTH,
+        );
+        return Err(Error::FileNameTooLong {
+            length: name.len(),
+            maximum: MAXIMUM_FILE_NAME_LENGTH,
+        });
+    }
+
+    // Makes sure there aren't any control characters or slashes.
+    //
+    // Rust considers null bytes, newlines, tabs and the various unprintables to be 'control'.
+    // See https://doc.rust-lang.org/stable/std/primitive.char.html#method.is_control
+    if name
+        .chars()
+        .any(|c| c.is_control() || c == '/' || c == '\\')
+    {
+        error!("File name contains control characters or slashes");
+        return Err(Error::FileNameInvalidCharacters);
+    }
+
+    // Looks good
     Ok(())
 }
 

deepwell/src/services/file_revision/service.rs

@@ -31,8 +31,6 @@ use once_cell::sync::Lazy;
 use sea_orm::{prelude::*, FromQueryResult};
 use std::num::NonZeroI32;
 
-pub const MAXIMUM_FILE_NAME_LENGTH: usize = 256;
-
 /// The changes for the first revision.
 /// The first revision is always considered to have changed everything.
 ///
@@ -141,19 +139,7 @@ impl FileRevisionService {
         }
 
         // Validate inputs
-        if name.is_empty() {
-            error!("File name is empty");
-            return Err(Error::FileNameEmpty);
-        }
-
-        if name.len() >= MAXIMUM_FILE_NAME_LENGTH {
-            error!(
-                "File name of invalid length: {} > {}",
-                name.len(),
-                MAXIMUM_FILE_NAME_LENGTH,
-            );
-            return Err(Error::FileNameTooLong);
-        }
+        // (Note that filename checks are done in FileService)
 
         if mime_hint.is_empty() {
             error!("MIME type hint is empty");

deepwell/src/services/user/service.rs

@@ -54,10 +54,10 @@ impl UserService {
         let txn = ctx.transaction();
         let slug = get_user_slug(&name, user_type);
 
-        debug!("Normalizing user data (name '{}', slug '{}')", name, slug,);
+        debug!("Normalizing user data (name '{name}', slug '{slug}')");
         regex_replace_in_place(&mut name, &LEADING_TRAILING_CHARS, "");
 
-        info!("Attempting to create user '{}' ('{}')", name, slug);
+        info!("Attempting to create user '{name}' ('{slug}')");
 
         // Empty slug check
         if slug.is_empty() {