add support for large form submissions

fixes #705

Author: lovasoa

CHANGELOG.md

@@ -47,6 +47,10 @@
     - Encryption is supported on both sides, but disabled on either side. You can enable this mode in this library by setting `encrypt=off` in the connection string. In this mode, the login phase will be encrypted, but data packets will be sent in plaintext.
     - Encryption is supported and enabled on both sides. You can enable this mode in this library by setting `encrypt=strict` in the connection string. In this mode, both the login phase and data packets will be encrypted.
 - Improved logging in the mssql driver login phase
+- Improved handling of very large form submissions
+  - The was a fixed 16kB limit on the size of form submissions.
+  - The size is now limited by the `max_uploaded_file_size` configuration option, which defaults to 5MB.
+  - 
 
 ## 0.30.1 (2024-10-31)
 - fix a bug where table sorting would break if table search was not also enabled.

configuration.md

@@ -23,7 +23,7 @@ Here are the available configuration options and their default values:
 | `site_prefix`                                 | `/`                                                         | Base path of the site. If you want to host SQLPage at `https://example.com/sqlpage/`, set this to `/sqlpage/`. When using a reverse proxy, this allows hosting SQLPage together with other applications on the same subdomain. |
 | `configuration_directory`                     | `./sqlpage/`                                                | The directory where the `sqlpage.json` file is located. This is used to find the path to [`templates/`](https://sql.datapage.app/custom_components.sql), [`migrations/`](https://sql.datapage.app/your-first-sql-website/migrations.sql), and `on_connect.sql`. Obviously, this configuration parameter can be set only through environment variables, not through the `sqlpage.json` file itself in order to find the `sqlpage.json` file. Be careful not to use a path that is accessible from the public WEB_ROOT |
 | `allow_exec`                                  | false                                                       | Allow usage of the `sqlpage.exec` function. Do this only if all users with write access to sqlpage query files and to the optional `sqlpage_files` table on the database are trusted.                                                                  |
-| `max_uploaded_file_size`                      | 5242880                                                     | Maximum size of uploaded files in bytes. Defaults to 5 MiB.                                                                                                                                                                                            |
+| `max_uploaded_file_size`                      | 5242880                                                     | Maximum size of forms and uploaded files in bytes. Defaults to 5 MiB.                                                                                                                                                                                            |
 | `max_pending_rows`                            | 256                                                         | Maximum number of rendered rows that can be queued up in memory when a client is slow to receive them. |
 | `compress_responses`                          | true                                                        | When the client supports it, compress the http response body. This can save bandwidth and speed up page loading on slow connections, but can also increase CPU usage and cause rendering delays on pages that take time to render (because streaming responses are buffered for longer than necessary). |
 | `https_domain`                                |                                                             | Domain name to request a certificate for. Setting this parameter will automatically make SQLPage listen on port 443 and request an SSL certificate. The server will take a little bit longer to start the first time it has to request a certificate.  |

src/webserver/http.rs

@@ -160,7 +160,7 @@ async fn render_sql(
 
     let mut req_param = extract_request_info(srv_req, Arc::clone(&app_state))
         .await
-        .map_err(anyhow_err_to_actix)?;
+        .map_err(|e| anyhow_err_to_actix(e, app_state.config.environment))?;
     log::debug!("Received a request with the following parameters: {req_param:?}");
 
     let (resp_send, resp_recv) = tokio::sync::oneshot::channel::<HttpResponse>();
@@ -203,14 +203,12 @@ async fn render_sql(
     resp_recv.await.map_err(ErrorInternalServerError)
 }
 
-fn send_anyhow_error(
-    e: &anyhow::Error,
-    resp_send: tokio::sync::oneshot::Sender<HttpResponse>,
-    env: app_config::DevOrProd,
-) {
-    log::error!("An error occurred before starting to send the response body: {e:#}");
+fn anyhow_err_to_actix_resp(e: &anyhow::Error, env: app_config::DevOrProd) -> HttpResponse {
     let mut resp = HttpResponseBuilder::new(StatusCode::INTERNAL_SERVER_ERROR);
-    let mut body = "Sorry, but we were not able to process your request. \n\n".to_owned();
+    let mut body = "Sorry, but we were not able to process your request. \n\n\
+        Below are detailed debugging information which may contain sensitive data. \
+        Set environment to \"prod\" in the configuration file to hide this information. \n\n"
+        .to_owned();
     if env.is_prod() {
         body.push_str("Contact the administrator for more information. A detailed error message has been logged.");
     } else {
@@ -221,23 +219,41 @@ fn send_anyhow_error(
         header::CONTENT_TYPE,
         header::HeaderValue::from_static("text/plain"),
     ));
-    let resp = if let Some(e @ &ErrorWithStatus { .. }) = e.downcast_ref() {
-        e.error_response()
+
+    if let Some(status_err @ &ErrorWithStatus { .. }) = e.downcast_ref() {
+        status_err
+            .error_response()
+            .set_body(actix_web::body::BoxBody::new(body))
     } else if let Some(sqlx::Error::PoolTimedOut) = e.downcast_ref() {
-        // People are HTTP connections faster than we can open SQL connections. Ask them to slow down politely.
         use rand::Rng;
-        resp.status(StatusCode::SERVICE_UNAVAILABLE).insert_header((
-            header::RETRY_AFTER,
-            header::HeaderValue::from(rand::thread_rng().gen_range(1..=15)),
-        )).body("The database is currently too busy to handle your request. Please try again later.\n\n".to_owned() + &body)
+        resp.status(StatusCode::TOO_MANY_REQUESTS)
+            .insert_header((
+                header::RETRY_AFTER,
+                header::HeaderValue::from(rand::thread_rng().gen_range(1..=15)),
+            ))
+            .body("The database is currently too busy to handle your request. Please try again later.\n\n".to_owned() + &body)
     } else {
         resp.body(body)
-    };
+    }
+}
+
+fn send_anyhow_error(
+    e: &anyhow::Error,
+    resp_send: tokio::sync::oneshot::Sender<HttpResponse>,
+    env: app_config::DevOrProd,
+) {
+    log::error!("An error occurred before starting to send the response body: {e:#}");
     resp_send
-        .send(resp)
+        .send(anyhow_err_to_actix_resp(e, env))
         .unwrap_or_else(|_| log::error!("could not send headers"));
 }
 
+fn anyhow_err_to_actix(e: anyhow::Error, env: app_config::DevOrProd) -> actix_web::Error {
+    log::error!("{e:#}");
+    let resp = anyhow_err_to_actix_resp(&e, env);
+    actix_web::error::InternalError::from_response(e, resp).into()
+}
+
 #[derive(Debug, serde::Serialize, serde::Deserialize, PartialEq, Clone)]
 #[serde(untagged)]
 pub enum SingleOrVec {
@@ -295,20 +311,10 @@ async fn process_sql_request(
         .get_with_privilege(app_state, &sql_path, false)
         .await
         .with_context(|| format!("Unable to get SQL file {sql_path:?}"))
-        .map_err(anyhow_err_to_actix)?;
+        .map_err(|e| anyhow_err_to_actix(e, app_state.config.environment))?;
     render_sql(req, sql_file).await
 }
 
-fn anyhow_err_to_actix(e: anyhow::Error) -> actix_web::Error {
-    log::error!("{e:#}");
-    match e.downcast::<ErrorWithStatus>() {
-        Ok(err) => actix_web::Error::from(err),
-        Err(e) => ErrorInternalServerError(format!(
-            "An error occurred while trying to handle your request: {e:#}"
-        )),
-    }
-}
-
 async fn serve_file(
     path: &str,
     state: &AppState,
@@ -322,7 +328,7 @@ async fn serve_file(
             .modified_since(state, path.as_ref(), since, false)
             .await
             .with_context(|| format!("Unable to get modification time of file {path:?}"))
-            .map_err(anyhow_err_to_actix)?;
+            .map_err(|e| anyhow_err_to_actix(e, state.config.environment))?;
         if !modified {
             return Ok(HttpResponse::NotModified().finish());
         }
@@ -332,7 +338,7 @@ async fn serve_file(
         .read_file(state, path.as_ref(), false)
         .await
         .with_context(|| format!("Unable to read file {path:?}"))
-        .map_err(anyhow_err_to_actix)
+        .map_err(|e| anyhow_err_to_actix(e, state.config.environment))
         .map(|b| {
             HttpResponse::Ok()
                 .insert_header(
@@ -392,7 +398,7 @@ async fn serve_fallback(
                 return render_sql(service_request, sql_file).await;
             }
             Err(e) => {
-                let actix_web_err = anyhow_err_to_actix(e);
+                let actix_web_err = anyhow_err_to_actix(e, app_state.config.environment);
 
                 // `maybe_fallback_path` does not exist, continue search in parent dir.
                 if actix_web_err.as_response_error().status_code() == StatusCode::NOT_FOUND {
@@ -553,10 +559,35 @@ pub fn create_app(
         .wrap(middleware::NormalizePath::new(
             middleware::TrailingSlash::MergeOnly,
         ))
-        .app_data(PayloadConfig::default().limit(app_state.config.max_uploaded_file_size * 2))
+        .app_data(payload_config(&app_state))
+        .app_data(form_config(&app_state))
         .app_data(app_state)
 }
 
+#[must_use]
+pub fn form_config(app_state: &web::Data<AppState>) -> web::FormConfig {
+    web::FormConfig::default()
+        .limit(app_state.config.max_uploaded_file_size)
+        .error_handler(|decode_err, _req| {
+            match decode_err {
+                actix_web::error::UrlencodedError::Overflow { size, limit } => {
+                    actix_web::error::ErrorPayloadTooLarge(
+                        format!(
+                            "The submitted form data size ({size} bytes) exceeds the maximum allowed upload size ({limit} bytes). \
+                            You can increase this limit by setting max_uploaded_file_size in the configuration file.",
+                        ),
+                    )
+                }
+                _ => actix_web::Error::from(decode_err),
+            }
+        })
+}
+
+#[must_use]
+pub fn payload_config(app_state: &web::Data<AppState>) -> PayloadConfig {
+    PayloadConfig::default().limit(app_state.config.max_uploaded_file_size * 2)
+}
+
 fn default_headers(app_state: &web::Data<AppState>) -> middleware::DefaultHeaders {
     let server_header = format!("{} v{}", env!("CARGO_PKG_NAME"), env!("CARGO_PKG_VERSION"));
     let mut headers = middleware::DefaultHeaders::new().add(("Server", server_header));

src/webserver/http_request_info.rs

@@ -144,7 +144,14 @@ async fn extract_urlencoded_post_variables(
     Form::<Vec<(String, String)>>::from_request(http_req, payload)
         .await
         .map(Form::into_inner)
-        .map_err(|e| anyhow!("could not parse request as urlencoded form data: {e}"))
+        .map_err(|e| {
+            anyhow!(super::ErrorWithStatus {
+                status: actix_web::http::StatusCode::BAD_REQUEST,
+            })
+            .context(format!(
+                "could not parse request as urlencoded form data: {e}"
+            ))
+        })
 }
 
 async fn extract_multipart_post_data(

tests/display_form_field.sql

@@ -0,0 +1,4 @@
+-- This test checks that the size of the form field can successfully roundtrip,
+-- from POST variable to sqlpage variable to handlebars, back to the client
+set x = :x;
+select 'text' as component, $x as contents;

tests/index.rs

@@ -13,7 +13,10 @@ use actix_web::{
 };
 use sqlpage::{
     app_config::{test_database_url, AppConfig},
-    webserver::{self, http::main_handler},
+    webserver::{
+        self,
+        http::{form_config, main_handler, payload_config},
+    },
     AppState,
 };
 
@@ -341,7 +344,7 @@ async fn test_blank_file_upload_field() -> actix_web::Result<()> {
 
 #[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
+    // Files larger than 123456 bytes should be rejected as per the test_config
     let req = get_request_to("/tests/upload_file_test.sql")
         .await?
         .insert_header(("content-type", "multipart/form-data; boundary=1234567890"))
@@ -352,7 +355,7 @@ async fn test_file_upload_too_large() -> actix_web::Result<()> {
             \r\n\
             "
             .to_string()
-                + "a".repeat(12346).as_str()
+                + "a".repeat(123457).as_str()
                 + "\r\n\
             --1234567890--\r\n",
         )
@@ -368,6 +371,30 @@ async fn test_file_upload_too_large() -> actix_web::Result<()> {
     Ok(())
 }
 
+#[actix_web::test]
+async fn test_large_form_field_roundtrip() -> actix_web::Result<()> {
+    // POST payloads smaller than 123456 bytes should be accepted
+    let long_string = "a".repeat(123454);
+    let req = get_request_to("/tests/display_form_field.sql")
+        .await?
+        .insert_header(("content-type", "application/x-www-form-urlencoded"))
+        .set_payload(["x=", &long_string].concat()) // total size is 123454 + 2 = 123456 bytes
+        .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("error"),
+        "{body_str}\nshouldn't have errors"
+    );
+    assert!(
+        body_str.contains(&long_string),
+        "{body_str}\nexpected to contain long string submitted"
+    );
+    Ok(())
+}
+
 #[actix_web::test]
 async fn test_upload_file_data_url() -> actix_web::Result<()> {
     let req = get_request_to("/tests/upload_file_data_url_test.sql")
@@ -539,8 +566,11 @@ async fn get_request_to(path: &str) -> actix_web::Result<TestRequest> {
     Ok(test::TestRequest::get()
         .uri(path)
         .insert_header(ContentType::plaintext())
+        .app_data(payload_config(&data))
+        .app_data(form_config(&data))
         .app_data(data))
 }
+
 async fn make_app_data_from_config(config: AppConfig) -> actix_web::web::Data<AppState> {
     let state = AppState::init(&config).await.unwrap();
     actix_web::web::Data::new(state)
@@ -587,7 +617,7 @@ pub fn test_config() -> AppConfig {
         "database_connection_retries": 3,
         "database_connection_acquire_timeout_seconds": 15,
         "allow_exec": true,
-        "max_uploaded_file_size": 12345,
+        "max_uploaded_file_size": 123456,
         "listen_on": "111.111.111.111:1",
         "system_root_ca_certificates" : false
     }}"#,