see sqlpage#278

Author: lovasoa

CHANGELOG.md

@@ -10,6 +10,7 @@
  - Fixed a bug in the cookie component where removing a cookie from a subdirectory would not work.
  - [Updated SQL parser](https://github.com/sqlparser-rs/sqlparser-rs/blob/main/CHANGELOG.md#0470-2024-06-01). Fixes support for `AT TIME ZONE` in postgres. Fixes `GROUP_CONCAT()` in MySQL.
  - Add a new warning message in the logs when trying to use `SET $x = ` when there is already a form field named `x`.
+ - **Empty Uploaded files**: when a form contains an optional file upload field, and the user does not upload a file, the field used to still be accessible to SQLPage file-related functions such as `sqlpage.uploaded_file_path` and `sqlpage.uploaded_file_mime_type`. This is now fixed, and these functions will return `NULL` when the user does not upload a file. `sqlpage.persist_uploaded_file` will not create an empty file in the target directory when the user does not upload a file, instead it will do nothing and return `NULL`.
 
 ## 0.22.0 (2024-05-29)
  -  **Important Security Fix:** The behavior of `SET $x` has been modified to match `SELECT $x`.

examples/official-site/sqlpage/migrations/39_persist_uploaded_file.sql

@@ -9,6 +9,7 @@ VALUES (
         '0.20.1',
         'device-floppy',
         'Persists an uploaded file to the local filesystem, and returns its path.
+If the file input field is empty, the function returns NULL.
 
 ### Example
 

src/webserver/database/sqlpage_functions/functions.rs

@@ -2,6 +2,7 @@ use super::RequestInfo;
 use crate::webserver::{database::execute_queries::DbConn, http::SingleOrVec, ErrorWithStatus};
 use anyhow::{anyhow, Context};
 use futures_util::StreamExt;
+use mime_guess::mime;
 use std::{borrow::Cow, ffi::OsStr, str::FromStr};
 
 super::function_definition_macro::sqlpage_functions! {
@@ -194,20 +195,14 @@ async fn persist_uploaded_file<'a>(
     field_name: Cow<'a, str>,
     folder: Option<Cow<'a, str>>,
     allowed_extensions: Option<Cow<'a, str>>,
-) -> anyhow::Result<String> {
+) -> anyhow::Result<Option<String>> {
     let folder = folder.unwrap_or(Cow::Borrowed("uploads"));
     let allowed_extensions_str =
         allowed_extensions.unwrap_or(Cow::Borrowed(DEFAULT_ALLOWED_EXTENSIONS));
     let allowed_extensions = allowed_extensions_str.split(',');
-    let uploaded_file = request
-        .uploaded_files
-        .get(&field_name.to_string())
-        .ok_or_else(|| {
-            anyhow!(
-                "no file uploaded with field name {field_name}. Uploaded files: {:?}",
-                request.uploaded_files.keys()
-            )
-        })?;
+    let Some(uploaded_file) = request.uploaded_files.get(&field_name.to_string()) else {
+        return Ok(None);
+    };
     let file_name = uploaded_file.file_name.as_deref().unwrap_or_default();
     let extension = file_name.split('.').last().unwrap_or_default();
     if !allowed_extensions
@@ -239,7 +234,7 @@ async fn persist_uploaded_file<'a>(
             .strip_prefix(web_root)?
             .to_str()
             .with_context(|| format!("unable to convert path {target_path:?} to a string"))?;
-    Ok(path)
+    Ok(Some(path))
 }
 
 /// Returns the protocol of the current request (http or https).
@@ -339,7 +334,7 @@ fn mime_from_upload_path<'a>(request: &'a RequestInfo, path: &str) -> Option<&'a
 
 fn mime_guess_from_filename(filename: &str) -> mime_guess::Mime {
     let maybe_mime = mime_guess::from_path(filename).first();
-    maybe_mime.unwrap_or(mime_guess::mime::APPLICATION_OCTET_STREAM)
+    maybe_mime.unwrap_or(mime::APPLICATION_OCTET_STREAM)
 }
 
 async fn request_method(request: &RequestInfo) -> String {

src/webserver/http_request_info.rs

@@ -176,6 +176,10 @@ async fn extract_multipart_post_data(
                     )
                 })?;
             log::trace!("Extracted file {field_name} to {:?}", extracted.file.path());
+            if is_file_field_empty(&extracted).await? {
+                log::debug!("Ignoring empty file field: {field_name}");
+                continue;
+            }
             uploaded_files.push((field_name, extracted));
         } else {
             let text_contents = extract_text(http_req, field, &mut limits).await?;
@@ -211,6 +215,22 @@ async fn extract_file(
     Ok(file)
 }
 
+/// file upload form fields that are left blank result in the browser sending an empty file, with a mime type of application/octet-stream.
+/// We don't want to treat this the same as actual empty files, so we check for this case.
+async fn is_file_field_empty(
+    uploaded_file: &actix_multipart::form::tempfile::TempFile,
+) -> anyhow::Result<bool> {
+    Ok(
+        uploaded_file.content_type == Some(mime_guess::mime::APPLICATION_OCTET_STREAM)
+            && uploaded_file
+                .file_name
+                .as_ref()
+                .filter(|x| !x.is_empty())
+                .is_none()
+            && tokio::fs::metadata(&uploaded_file.file.path()).await?.len() == 0,
+    )
+}
+
 #[cfg(test)]
 mod test {
     use super::super::http::SingleOrVec;

tests/index.rs

@@ -231,6 +231,33 @@ async fn test_file_upload_through_runsql() -> actix_web::Result<()> {
     test_file_upload("/tests/upload_file_runsql_test.sql").await
 }
 
+// Diabled because of
+#[actix_web::test]
+async fn test_blank_file_upload_field() -> actix_web::Result<()> {
+    let req = get_request_to("/tests/upload_file_test.sql")
+        .await?
+        .insert_header(("content-type", "multipart/form-data; boundary=1234567890"))
+        .set_payload(
+            "--1234567890\r\n\
+            Content-Disposition: form-data; name=\"my_file\"; filename=\"\"\r\n\
+            Content-Type: application/octet-stream\r\n\
+            \r\n\
+            \r\n\
+            --1234567890--\r\n",
+        )
+        .to_srv_request();
+    let resp = main_handler(req).await?;
+
+    assert_eq!(resp.status(), StatusCode::OK);
+    let body = test::read_body(resp).await;
+    let body_str = String::from_utf8(body.to_vec()).unwrap();
+    assert!(
+        body_str.contains("No file uploaded"),
+        "{body_str}\nexpected to contain: No file uploaded"
+    );
+    Ok(())
+}
+
 #[actix_web::test]
 async fn test_file_upload_too_large() -> actix_web::Result<()> {
     // Files larger than 12345 bytes should be rejected as per the test_config

tests/upload_file_test.sql

@@ -1,2 +1,5 @@
 select 'text' as component,
-    sqlpage.read_file_as_text(sqlpage.uploaded_file_path('my_file')) as contents;
+    COALESCE(
+        sqlpage.read_file_as_text(sqlpage.uploaded_file_path('my_file')),
+        'No file uploaded'
+     ) as contents;