diff --git a/CHANGELOG.md b/CHANGELOG.md index b1857771..925ee983 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,8 @@ ## 0.20.1 (unreleased) - More than 200 new icons, with [tabler icons v3](https://tabler.io/icons/changelog#3.0) + - New [`sqlpage.persist_uploaded_file`](https://sql.ophir.dev/functions.sql?function=persist_uploaded_file#function) function to save uploaded files to a permanent location on the local filesystem (where SQLPage is running). This is useful to store files uploaded by users in a safe location, and to serve them back to users later. + - Correct error handling for file uploads. SQLPage used to silently ignore file uploads that failed (because they exceeded [max_uploaded_file_size](./configuration.md), for instance), but now it displays a clear error message to the user. ## 0.20.0 (2024-03-12) diff --git a/Cargo.lock b/Cargo.lock index 841d31e4..508971c5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -296,9 +296,9 @@ dependencies = [ [[package]] name = "aho-corasick" -version = "1.1.2" +version = "1.1.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b2969dcb958b36655471fc61f7e416fa76033bdd4bfed0678d8fee1e2d07a1f0" +checksum = "8e60d3430d3a69478ad0993f19238d2df97c507009a52b3c10addcd7f6bcb916" dependencies = [ "memchr", ] @@ -672,9 +672,9 @@ dependencies = [ [[package]] name = "backtrace" -version = "0.3.69" +version = "0.3.71" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2089b7e3f35b9dd2d0ed921ead4f6d318c27680d4a5bd167b3ee120edb105837" +checksum = "26b05800d2e817c8b3b4b54abd461726265fa9789ae34330622f2db9ee696f9d" dependencies = [ "addr2line", "cc", @@ -793,9 +793,9 @@ checksum = "1fd0f2584146f6f2ef48085050886acf353beff7305ebd1ae69500e27c67f64b" [[package]] name = "bytes" -version = "1.5.0" +version = "1.6.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a2bd12c1caf447e69cd4528f47f94d203fd2582878ecb9e9465484c4148a8223" +checksum = "514de17de45fdb8dc022b1a7975556c53c86f9f0aa5f534b98977b171857c2c9" [[package]] name = "bytestring" @@ -1786,9 +1786,9 @@ dependencies = [ [[package]] name = "indexmap" -version = "2.2.5" +version = "2.2.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7b0b929d511467233429c45a44ac1dcaa21ba0f5ba11e4879e6ed28ddb4f9df4" +checksum = "168fb715dda47215e360912c096649d23d58bf392ac62f73919e831745e40f26" dependencies = [ "equivalent", "hashbrown 0.14.3", @@ -2531,9 +2531,9 @@ dependencies = [ [[package]] name = "regex" -version = "1.10.3" +version = "1.10.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b62dbe01f0b06f9d8dc7d49e05a0785f153b00b2c227856282f671e0318c9b15" +checksum = "c117dbdfde9c8308975b6a18d71f3f385c89461f7b3fb054288ecf2a2058ba4c" dependencies = [ "aho-corasick", "memchr", @@ -2676,9 +2676,9 @@ dependencies = [ [[package]] name = "rustix" -version = "0.38.31" +version = "0.38.32" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6ea3e1a662af26cd7a3ba09c0297a31af215563ecf42817c98df621387f4e949" +checksum = "65e04861e65f21776e67888bfbea442b3642beaa0138fdb1dd7a84a52dffdb89" dependencies = [ "bitflags 2.5.0", "errno", @@ -2905,9 +2905,9 @@ dependencies = [ [[package]] name = "smallvec" -version = "1.13.1" +version = "1.13.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e6ecd384b10a64542d77071bd64bd7b231f4ed5940fba55e98c3de13824cf3d7" +checksum = "3c5e1a9a646d36c3599cd173a41282daf47c44583ad367b8e6837255952e5c67" [[package]] name = "socket2" @@ -3217,7 +3217,7 @@ checksum = "85b77fafb263dd9d05cbeac119526425676db3784113aa9295c88498cbf8bff1" dependencies = [ "cfg-if", "fastrand 2.0.1", - "rustix 0.38.31", + "rustix 0.38.32", "windows-sys 0.52.0", ] @@ -3395,9 +3395,9 @@ dependencies = [ [[package]] name = "toml_edit" -version = "0.22.8" +version = "0.22.9" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c12219811e0c1ba077867254e5ad62ee2c9c190b0d957110750ac0cda1ae96cd" +checksum = "8e40bb779c5187258fd7aad0eb68cb8706a0a81fa712fbea808ab43c4b8374c4" dependencies = [ "indexmap", "serde", diff --git a/examples/image gallery with user uploads/.gitignore b/examples/image gallery with user uploads/.gitignore new file mode 100644 index 00000000..ba281509 --- /dev/null +++ b/examples/image gallery with user uploads/.gitignore @@ -0,0 +1 @@ +images/ \ No newline at end of file diff --git a/examples/image gallery with user uploads/sqlpage/sqlpage.json b/examples/image gallery with user uploads/sqlpage/sqlpage.json index 1828e4db..60322051 100644 --- a/examples/image gallery with user uploads/sqlpage/sqlpage.json +++ b/examples/image gallery with user uploads/sqlpage/sqlpage.json @@ -1,3 +1,3 @@ { - "max_uploaded_file_size": 500000 + "max_uploaded_file_size": 5000000 } \ No newline at end of file diff --git a/examples/image gallery with user uploads/upload.sql b/examples/image gallery with user uploads/upload.sql index d6f3c97c..aa8e8857 100644 --- a/examples/image gallery with user uploads/upload.sql +++ b/examples/image gallery with user uploads/upload.sql @@ -11,7 +11,10 @@ insert or ignore into image (title, description, image_url) values ( :Title, :Description, - sqlpage.read_file_as_data_url(sqlpage.uploaded_file_path('Image')) + -- Persist the uploaded file to the local "images" folder at the root of the website and return the path + sqlpage.persist_uploaded_file('Image', 'images', 'jpg,jpeg,png,gif') + -- alternatively, if the images are small, you could store them in the database directly with the following line + -- sqlpage.read_file_as_data_url(sqlpage.uploaded_file_path('Image')) ) returning 'redirect' as component, format('/?created_id=%d', id) as link; diff --git a/examples/official-site/sqlpage/migrations/23_uploaded_file_functions.sql b/examples/official-site/sqlpage/migrations/23_uploaded_file_functions.sql index 5200648f..c11c4b2f 100644 --- a/examples/official-site/sqlpage/migrations/23_uploaded_file_functions.sql +++ b/examples/official-site/sqlpage/migrations/23_uploaded_file_functions.sql @@ -48,27 +48,12 @@ insert into text_documents (title, path) values (:title, sqlpage.read_file_as_te When the uploaded file is larger than a few megabytes, it is not recommended to store it in the database. Instead, one can save the file to a permanent location on the server, and store the path to the file in the database. -You can move the file to a permanent location using the [`sqlpage.exec`](?function=exec#function) function: - -```sql -set file_name = sqlpage.random_string(10); -set exec_result = sqlpage.exec(''mv'', sqlpage.uploaded_file_path(''myfile''), ''/my_upload_directory/'' || $file_name); -insert into uploaded_files (title, path) values (:title, $file_name); -``` - -> *Notes*: -> - The `sqlpage.exec` function is disabled by default, and you need to enable it in the [configuration file](https://github.com/lovasoa/SQLpage/blob/main/configuration.md). -> - `mv` is specific to MacOS and Linux. On Windows, you can use [`move`](https://learn.microsoft.com/en-us/windows-server/administration/windows-commands/move) instead: -> - ```sql -> SET image_path = sqlpage.uploaded_file_path(''myfile''); -> SET exec_result = sqlpage.exec(''cmd'', ''/C'', ''move'', $image_path, ''C:\MyUploadDirectory''); -> ``` - +You can move the file to a permanent location using the [`sqlpage.persist_uploaded_file`](?function=persist_uploaded_file#function) function. ### Advanced file handling For more advanced file handling, such as uploading files to a cloud storage service, you can write a small script in your favorite programming language, -and call it using the `sqlpage.exec` function. +and call it using the [`sqlpage.exec`](?function=exec#function) function. For instance, one could save the following small bash script to `/usr/local/bin/upload_to_s3`: diff --git a/examples/official-site/sqlpage/migrations/39_persist_uploaded_file.sql b/examples/official-site/sqlpage/migrations/39_persist_uploaded_file.sql new file mode 100644 index 00000000..aeb72bd6 --- /dev/null +++ b/examples/official-site/sqlpage/migrations/39_persist_uploaded_file.sql @@ -0,0 +1,64 @@ +INSERT INTO sqlpage_functions ( + "name", + "introduced_in_version", + "icon", + "description_md" + ) +VALUES ( + 'persist_uploaded_file', + '0.20.1', + 'device-floppy', + 'Persists an uploaded file to the local filesystem, and returns its path. + +### Example + +#### User profile picture + +##### `upload_form.sql` + +```sql +select ''form'' as component, ''persist_uploaded_file.sql'' as action; +select ''file'' as type, ''profile_picture'' as name, ''Upload your profile picture'' as label; +``` + +##### `persist_uploaded_file.sql` + +```sql +update user +set profile_picture = sqlpage.persist_uploaded_file(''profile_picture'', ''profile_pictures'', ''jpg,jpeg,png,gif,webp'') +where id = ( + select user_id from session where session_id = sqlpage.cookie(''session_id'') +); +``` + +' + ); +INSERT INTO sqlpage_function_parameters ( + "function", + "index", + "name", + "description_md", + "type" + ) +VALUES ( + 'persist_uploaded_file', + 1, + 'file', + 'Name of the form field containing the uploaded file. The current page must be referenced in the `action` property of a `form` component that contains a file input field.', + 'TEXT' + ), + ( + 'persist_uploaded_file', + 2, + 'destination_folder', + 'Optional. Path to the folder where the file will be saved, relative to the web root (the root folder of your website files). By default, the file will be saved in the `uploads` folder.', + 'TEXT' + ), + ( + 'persist_uploaded_file', + 3, + 'allowed_extensions', + 'Optional. Comma-separated list of allowed file extensions. By default: jpg,jpeg,png,gif,bmp,webp,pdf,txt,doc,docx,xls,xlsx,csv,mp3,mp4,wav,avi,mov. +Changing this may be dangerous ! If you add "sql", "svg" or "html" to the list, an attacker could execute arbitrary SQL queries on your database, or impersonate other users.', + 'TEXT' + ); diff --git a/sqlpage/sqlpage.db b/sqlpage/sqlpage.db index 25af6df8..3ca97edb 100644 Binary files a/sqlpage/sqlpage.db and b/sqlpage/sqlpage.db differ diff --git a/src/webserver/database/sql_pseudofunctions.rs b/src/webserver/database/sql_pseudofunctions.rs index 68151e69..664d3a54 100644 --- a/src/webserver/database/sql_pseudofunctions.rs +++ b/src/webserver/database/sql_pseudofunctions.rs @@ -42,6 +42,11 @@ pub(super) enum StmtParam { Literal(String), UploadedFilePath(String), UploadedFileMimeType(String), + PersistUploadedFile { + field_name: Box, + folder: Option>, + allowed_extensions: Option>, + }, ReadFileAsText(Box), ReadFileAsDataUrl(Box), RunSql(Box), @@ -107,6 +112,25 @@ pub(super) fn func_call_to_param(func_name: &str, arguments: &mut [FunctionArg]) extract_single_quoted_string("uploaded_file_mime_type", arguments) .map_or_else(StmtParam::Error, StmtParam::UploadedFileMimeType) } + "persist_uploaded_file" => { + let field_name = Box::new(extract_variable_argument( + "persist_uploaded_file", + arguments, + )); + let folder = arguments + .get_mut(1) + .and_then(function_arg_to_stmt_param) + .map(Box::new); + let allowed_extensions = arguments + .get_mut(2) + .and_then(function_arg_to_stmt_param) + .map(Box::new); + StmtParam::PersistUploadedFile { + field_name, + folder, + allowed_extensions, + } + } "read_file_as_text" => StmtParam::ReadFileAsText(Box::new(extract_variable_argument( "read_file_as_text", arguments, @@ -135,10 +159,88 @@ pub(super) async fn extract_req_param<'a>( StmtParam::ReadFileAsText(inner) => read_file_as_text(inner, request).await?, StmtParam::ReadFileAsDataUrl(inner) => read_file_as_data_url(inner, request).await?, StmtParam::RunSql(inner) => run_sql(inner, request).await?, + StmtParam::PersistUploadedFile { + field_name, + folder, + allowed_extensions, + } => { + persist_uploaded_file( + field_name, + folder.as_deref(), + allowed_extensions.as_deref(), + request, + ) + .await? + } _ => extract_req_param_non_nested(param, request)?, }) } +const DEFAULT_ALLOWED_EXTENSIONS: &str = + "jpg,jpeg,png,gif,bmp,webp,pdf,txt,doc,docx,xls,xlsx,csv,mp3,mp4,wav,avi,mov"; + +async fn persist_uploaded_file<'a>( + field_name: &StmtParam, + folder: Option<&StmtParam>, + allowed_extensions: Option<&StmtParam>, + request: &'a RequestInfo, +) -> anyhow::Result>> { + let field_name = extract_req_param_non_nested(field_name, request)? + .ok_or_else(|| anyhow!("persist_uploaded_file: field_name is NULL"))?; + let folder = folder + .map_or_else(|| Ok(None), |x| extract_req_param_non_nested(x, request))? + .unwrap_or(Cow::Borrowed("uploads")); + let allowed_extensions_str = &allowed_extensions + .map_or_else(|| Ok(None), |x| extract_req_param_non_nested(x, request))? + .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!("persist_uploaded_file: no file uploaded with field name {field_name}. Uploaded files: {:?}", request.uploaded_files.keys()) + })?; + let file_name = &uploaded_file.file_name.as_deref().unwrap_or_default(); + let extension = file_name.split('.').last().unwrap_or_default(); + if !allowed_extensions + .clone() + .any(|x| x.eq_ignore_ascii_case(extension)) + { + let exts = allowed_extensions.collect::>().join(", "); + bail!( + "persist_uploaded_file: file extension {extension} is not allowed. Allowed extensions: {exts}" + ); + } + // resolve the folder path relative to the web root + let web_root = &request.app_state.config.web_root; + let target_folder = web_root.join(&*folder); + // create the folder if it doesn't exist + tokio::fs::create_dir_all(&target_folder) + .await + .with_context(|| { + format!("persist_uploaded_file: unable to create folder {target_folder:?}") + })?; + let date = chrono::Utc::now().format("%Y-%m-%d %Hh%Mm%Ss"); + let random_part = random_string(8); + let random_target_name = format!("{date} {random_part}.{extension}"); + let target_path = target_folder.join(&random_target_name); + tokio::fs::copy(&uploaded_file.file.path(), &target_path) + .await + .with_context(|| { + format!( + "persist_uploaded_file: unable to copy uploaded file {field_name:?} to {target_path:?}" + ) + })?; + // remove the WEB_ROOT prefix from the path, but keep the leading slash + let path = "/".to_string() + target_path + .strip_prefix(web_root)? + .to_str() + .with_context(|| { + format!("persist_uploaded_file: unable to convert path {target_path:?} to a string") + })?; + Ok(Some(Cow::Owned(path))) +} + fn url_encode<'a>( inner: &StmtParam, request: &'a RequestInfo, @@ -357,6 +459,9 @@ pub(super) fn extract_req_param_non_nested<'a>( .get(x) .and_then(|x| x.file.path().to_str()) .map(Cow::Borrowed), + StmtParam::PersistUploadedFile { .. } => { + bail!("Nested persist_uploaded_file() function not allowed") + } StmtParam::UploadedFileMimeType(x) => request .uploaded_files .get(x) diff --git a/src/webserver/http.rs b/src/webserver/http.rs index eb2affc0..15c73867 100644 --- a/src/webserver/http.rs +++ b/src/webserver/http.rs @@ -219,7 +219,9 @@ async fn render_sql( .clone() // Cheap reference count increase .into_inner(); - let mut req_param = extract_request_info(srv_req, Arc::clone(&app_state)).await; + let mut req_param = extract_request_info(srv_req, Arc::clone(&app_state)) + .await + .map_err(anyhow_err_to_actix)?; log::debug!("Received a request with the following parameters: {req_param:?}"); let (resp_send, resp_recv) = tokio::sync::oneshot::channel::(); diff --git a/src/webserver/http_request_info.rs b/src/webserver/http_request_info.rs index 7cf3be49..d216017d 100644 --- a/src/webserver/http_request_info.rs +++ b/src/webserver/http_request_info.rs @@ -15,6 +15,7 @@ use actix_web::HttpRequest; use actix_web_httpauth::headers::authorization::Authorization; use actix_web_httpauth::headers::authorization::Basic; use anyhow::anyhow; +use anyhow::Context; use std::collections::hash_map::Entry; use std::collections::HashMap; use std::net::IpAddr; @@ -58,12 +59,11 @@ impl Clone for RequestInfo { pub(crate) async fn extract_request_info( req: &mut ServiceRequest, app_state: Arc, -) -> RequestInfo { +) -> anyhow::Result { let (http_req, payload) = req.parts_mut(); let protocol = http_req.connection_info().scheme().to_string(); let config = &app_state.config; - let (post_variables, uploaded_files) = extract_post_data(http_req, payload, config).await; - + let (post_variables, uploaded_files) = extract_post_data(http_req, payload, config).await?; let headers = req.headers().iter().map(|(name, value)| { ( name.to_string(), @@ -85,7 +85,7 @@ pub(crate) async fn extract_request_info( .ok() .map(Authorization::into_scheme); - RequestInfo { + Ok(RequestInfo { path: req.path().to_string(), headers: param_map(headers), get_variables: param_map(get_variables), @@ -97,48 +97,39 @@ pub(crate) async fn extract_request_info( app_state, protocol, clone_depth: 0, - } + }) } async fn extract_post_data( http_req: &mut actix_web::HttpRequest, payload: &mut actix_web::dev::Payload, config: &crate::app_config::AppConfig, -) -> (Vec<(String, String)>, Vec<(String, TempFile)>) { +) -> anyhow::Result<(Vec<(String, String)>, Vec<(String, TempFile)>)> { let content_type = http_req .headers() .get(&CONTENT_TYPE) .map(AsRef::as_ref) .unwrap_or_default(); if content_type.starts_with(b"application/x-www-form-urlencoded") { - match extract_urlencoded_post_variables(http_req, payload).await { - Ok(post_variables) => (post_variables, Vec::new()), - Err(e) => { - log::error!("Could not read urlencoded POST request data: {}", e); - (Vec::new(), Vec::new()) - } - } + let vars = extract_urlencoded_post_variables(http_req, payload).await?; + Ok((vars, Vec::new())) } else if content_type.starts_with(b"multipart/form-data") { - extract_multipart_post_data(http_req, payload, config) - .await - .unwrap_or_else(|e| { - log::error!("Could not read request data: {}", e); - (Vec::new(), Vec::new()) - }) + extract_multipart_post_data(http_req, payload, config).await } else { let ct_str = String::from_utf8_lossy(content_type); log::debug!("Not parsing POST data from request without known content type {ct_str}"); - (Vec::new(), Vec::new()) + Ok((Vec::new(), Vec::new())) } } async fn extract_urlencoded_post_variables( http_req: &mut actix_web::HttpRequest, payload: &mut actix_web::dev::Payload, -) -> actix_web::Result> { +) -> anyhow::Result> { Form::>::from_request(http_req, payload) .await .map(Form::into_inner) + .map_err(|e| anyhow!("could not parse request as urlencoded form data: {e}")) } async fn extract_multipart_post_data( @@ -171,7 +162,9 @@ async fn extract_multipart_post_data( log::trace!("Parsing multipart field: {}", field_name); if let Some(filename) = filename { log::debug!("Extracting file: {field_name} ({filename})"); - let extracted = extract_file(http_req, field, &mut limits).await?; + let extracted = extract_file(http_req, field, &mut limits).await.with_context( + || format!("Failed to extract file {field_name:?}. Max file size: {:.3} MB", config.max_uploaded_file_size as f32 / 1_000_000.0), + )?; log::trace!("Extracted file {field_name} to {:?}", extracted.file.path()); uploaded_files.push((field_name, extracted)); } else { @@ -244,7 +237,7 @@ mod test { serde_json::from_str::(r#"{"listen_on": "localhost:1234"}"#).unwrap(); let mut service_request = TestRequest::default().to_srv_request(); let app_data = Arc::new(AppState::init(&config).await.unwrap()); - let request_info = extract_request_info(&mut service_request, app_data).await; + let request_info = extract_request_info(&mut service_request, app_data).await.unwrap(); assert_eq!(request_info.post_variables.len(), 0); assert_eq!(request_info.uploaded_files.len(), 0); assert_eq!(request_info.get_variables.len(), 0); @@ -260,7 +253,7 @@ mod test { .set_payload("my_array[]=3&my_array[]=Hello%20World&repeated=1&repeated=2") .to_srv_request(); let app_data = Arc::new(AppState::init(&config).await.unwrap()); - let request_info = extract_request_info(&mut service_request, app_data).await; + let request_info = extract_request_info(&mut service_request, app_data).await.unwrap(); assert_eq!( request_info.post_variables, vec![ @@ -307,7 +300,7 @@ mod test { ) .to_srv_request(); let app_data = Arc::new(AppState::init(&config).await.unwrap()); - let request_info = extract_request_info(&mut service_request, app_data).await; + let request_info = extract_request_info(&mut service_request, app_data).await.unwrap(); assert_eq!( request_info.post_variables, vec![(