Make URL and POST parameters immutable (#1109)

lovasoa · web-flow · commit b11e7bf0ffed · 2025-11-19T03:10:54.000+01:00
* Make URL and POST parameters immutable, separate from SET variables - URL and POST parameters are now immutable after request initialization - SET command creates user-defined variables in separate namespace - Variable lookup: SET variables shadow request parameters - Added sqlpage.variables('set') to inspect user-defined variables - Simplified API: most functions now use &RequestInfo instead of &mut - All tests passing (151 total) * Restore deprecation warning for SET on POST variable names * Restore deprecation warnings for $var accessing POST variables - Warn when both URL and POST have same variable name - Warn when $var is used for POST-only variable (should use :var) * Simplify run_sql: always use clone_without_variables No need to branch on whether variables are provided since we clone in both cases anyway. * Revert "Simplify run_sql: always use clone_without_variables" This reverts commit 60f5a05. * Fix cross-database test compatibility for immutable variables Renamed test to run only on SQLite since json_extract() is SQLite-specific. Other databases (PostgreSQL, MySQL, MSSQL) have different JSON functions. * Fix test to work across all databases without json_extract PostgreSQL doesn't have json_extract, so compare the full JSON string instead. * Document variable system improvements in CHANGELOG * Make CHANGELOG more explicit about breaking changes with examples * Fix CHANGELOG: SET overwrites GET parameters, not POST * Add database-specific examples for accessing original URL parameters
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,6 +1,30 @@
 # CHANGELOG.md
 
 ## unrelease
+ - **Variable System Improvements**: URL and POST parameters are now immutable, preventing accidental modification. User-defined variables created with `SET` remain mutable.
+   - **BREAKING**: `$variable` no longer accesses POST parameters. Use `:variable` instead.
+     - **What changed**: Previously, `$x` would return a POST parameter value if no GET parameter named `x` existed.
+     - **Fix**: Replace `$x` with `:x` when you need to access form field values.
+     - **Example**: Change `SELECT $username` to `SELECT :username` when reading form submissions.
+   - **BREAKING**: `SET $name` no longer overwrites GET (URL) parameters when a URL parameter with the same name exists.
+     - **What changed**: `SET $name = 'value'` would previously overwrite the URL parameter `$name`. Now it creates an independent SET variable that shadows the URL parameter.
+     - **Fix**: This is generally the desired behavior. If you need to access the original URL parameter after setting a variable with the same name, extract it from the JSON returned by `sqlpage.variables('get')`.
+     - **Example**: If your URL is `page.sql?name=john`, and you do `SET $name = 'modified'`, then:
+       - `$name` will be `'modified'` (the SET variable)
+       - The original URL parameter is still preserved and accessible:
+         - PostgreSQL: `sqlpage.variables('get')->>'name'` returns `'john'`
+         - SQLite: `json_extract(sqlpage.variables('get'), '$.name')` returns `'john'`
+         - MySQL: `JSON_UNQUOTE(JSON_EXTRACT(sqlpage.variables('get'), '$.name'))` returns `'john'`
+   - **New behavior**: Variable lookup now follows this precedence:
+     - `$variable` checks SET variables first, then URL parameters
+     - `:variable` checks SET variables first, then POST parameters  
+     - SET variables always shadow URL/POST parameters with the same name
+   - **New sqlpage.variables() filters**:
+     - `sqlpage.variables('get')` returns only URL parameters as JSON
+     - `sqlpage.variables('post')` returns only POST parameters as JSON
+     - `sqlpage.variables('set')` returns only user-defined SET variables as JSON
+     - `sqlpage.variables()` returns all variables merged together, with SET variables taking precedence
+   - **Deprecation warnings**: Using `$var` when both a URL parameter and POST parameter exist with the same name now shows a warning. In a future version, you'll need to explicitly choose between `$var` (URL) and `:var` (POST).
  - add support for postgres range types
 
 ## v0.39.1 (2025-11-08)
diff --git a/examples/official-site/extensions-to-sql.md b/examples/official-site/extensions-to-sql.md
@@ -75,10 +75,15 @@ SELECT (select 1) AS one;
 ## Variables
 
 SQLPage communicates information about incoming HTTP requests to your SQL code through prepared statement variables.
-You can use 
- - `$var` to reference a GET variable (an URL parameter),
- - `:var` to reference a POST variable (a value filled by an user in a form field),
- - `set var = ...` to set the value of `$var`.
+
+### Variable Types and Mutability
+
+There are two types of variables in SQLPage:
+
+1. **Request parameters** (immutable): URL parameters and form data from the HTTP request
+2. **User-defined variables** (mutable): Variables created with the `SET` command
+
+Request parameters cannot be modified after the request is received. This ensures the original request data remains intact throughout request processing.
 
 ### POST parameters
 
@@ -111,20 +116,30 @@ When a URL parameter is not set, its value is `NULL`.
 
 ### The SET command
 
-`SET` stores a value in SQLPage (not in the database). Only strings and `NULL` are stored.
+`SET` creates or updates a user-defined variable in SQLPage (not in the database). Only strings and `NULL` are stored.
 
 ```sql
 -- Give a default value to a variable
 SET post_id = COALESCE($post_id, 0);
+
+-- User-defined variables shadow URL parameters with the same name
+SET my_var = 'custom value';  -- This value takes precedence over ?my_var=...
 ```
 
+**Variable Lookup Precedence:**
+- `$var`: checks user-defined variables first, then URL parameters
+- `:var`: checks user-defined variables first, then POST parameters
+
+This means `SET` variables always take precedence over request parameters when using `$var` or `:var` syntax.
+
+**How SET works:**
 - If the right-hand side is purely literals/variables, SQLPage computes it directly. See the section about *static simple select* above.
 - If it needs the database (for example, calls a database function), SQLPage runs an internal `SELECT` to compute it and stores the first column of the first row of results.
 
 Only a single textual value (**string or `NULL`**) is stored.
-`set id = 1` will store the string `'1'`, not the number `1`.
+`SET id = 1` will store the string `'1'`, not the number `1`.
 
-On databases with a strict type system, such as PostgreSQL, if you need a number, you will need to cast your variables: `select * from post where id = $id::int`.
+On databases with a strict type system, such as PostgreSQL, if you need a number, you will need to cast your variables: `SELECT * FROM post WHERE id = $id::int`.
 
 Complex structures can be stored as json strings.
 
diff --git a/examples/official-site/sqlpage/migrations/20_variables_function.sql b/examples/official-site/sqlpage/migrations/20_variables_function.sql
@@ -9,10 +9,27 @@ VALUES (
     'variables',
     '0.15.0',
     'variable',
-    'Returns a JSON string containing all variables passed as URL parameters or posted through a form.
+    'Returns a JSON string containing variables from the HTTP request and user-defined variables.
 
 The database''s json handling functions can then be used to process the data.
 
+## Variable Types
+
+SQLPage distinguishes between three types of variables:
+
+- **GET variables**: URL parameters from the query string (immutable)
+- **POST variables**: Form data from POST requests (immutable)  
+- **SET variables**: User-defined variables created with the `SET` command (mutable)
+
+## Usage
+
+- `sqlpage.variables()` - returns all variables (GET, POST, and SET combined, with SET variables taking precedence)
+- `sqlpage.variables(''get'')` - returns only URL parameters
+- `sqlpage.variables(''post'')` - returns only POST form data
+- `sqlpage.variables(''set'')` - returns only user-defined variables created with `SET`
+
+When a SET variable has the same name as a GET or POST variable, the SET variable takes precedence in the combined result.
+
 ## Example: a form with a variable number of fields
 
 ### Making a form based on questions in a database table
@@ -95,6 +112,6 @@ VALUES (
     'variables',
     1,
     'method',
-    'Optional. The HTTP request method (GET or POST). Must be a literal string. When not provided, all variables are returned.',
+    'Optional. Filter variables by source: ''get'' (URL parameters), ''post'' (form data), or ''set'' (user-defined variables). When not provided, all variables are returned with SET variables taking precedence over request parameters.',
     'TEXT'
 );
diff --git a/src/webserver/database/execute_queries.rs b/src/webserver/database/execute_queries.rs
@@ -44,7 +44,7 @@ impl Database {
 
 pub fn stream_query_results_with_conn<'a>(
     sql_file: &'a ParsedSqlFile,
-    request: &'a mut RequestInfo,
+    request: &'a RequestInfo,
     db_connection: &'a mut DbConn,
 ) -> impl Stream<Item = DbItem> + 'a {
     let source_file = &sql_file.source_path;
@@ -175,7 +175,7 @@ async fn extract_req_param_as_json(
 /// This allows recursive calls.
 pub fn stream_query_results_boxed<'a>(
     sql_file: &'a ParsedSqlFile,
-    request: &'a mut RequestInfo,
+    request: &'a RequestInfo,
     db_connection: &'a mut DbConn,
 ) -> Pin<Box<dyn Stream<Item = DbItem> + 'a>> {
     Box::pin(stream_query_results_with_conn(
@@ -187,7 +187,7 @@ pub fn stream_query_results_boxed<'a>(
 
 async fn execute_set_variable_query<'a>(
     db_connection: &'a mut DbConn,
-    request: &'a mut RequestInfo,
+    request: &'a RequestInfo,
     variable: &StmtParam,
     statement: &StmtWithParams,
     source_file: &Path,
@@ -209,7 +209,7 @@ async fn execute_set_variable_query<'a>(
         }
     };
 
-    let (vars, name) = vars_and_name(request, variable)?;
+    let (mut vars, name) = vars_and_name(request, variable)?;
 
     if let Some(value) = value {
         log::debug!("Setting variable {name} to {value:?}");
@@ -223,7 +223,7 @@ async fn execute_set_variable_query<'a>(
 
 async fn execute_set_simple_static<'a>(
     db_connection: &'a mut DbConn,
-    request: &'a mut RequestInfo,
+    request: &'a RequestInfo,
     variable: &StmtParam,
     value: &SimpleSelectValue,
     _source_file: &Path,
@@ -241,7 +241,7 @@ async fn execute_set_simple_static<'a>(
         }
     };
 
-    let (vars, name) = vars_and_name(request, variable)?;
+    let (mut vars, name) = vars_and_name(request, variable)?;
 
     if let Some(value) = value_str {
         log::debug!("Setting variable {name} to static value {value:?}");
@@ -254,20 +254,17 @@ async fn execute_set_simple_static<'a>(
 }
 
 fn vars_and_name<'a, 'b>(
-    request: &'a mut RequestInfo,
+    request: &'a RequestInfo,
     variable: &'b StmtParam,
-) -> anyhow::Result<(&'a mut HashMap<String, SingleOrVec>, &'b str)> {
+) -> anyhow::Result<(std::cell::RefMut<'a, HashMap<String, SingleOrVec>>, &'b str)> {
     match variable {
-        StmtParam::PostOrGet(name) => {
+        StmtParam::PostOrGet(name) | StmtParam::Get(name) => {
             if request.post_variables.contains_key(name) {
                 log::warn!("Deprecation warning! Setting the value of ${name}, but there is already a form field named :{name}. This will stop working soon. Please rename the variable, or use :{name} directly if you intended to overwrite the posted form field value.");
-                Ok((&mut request.post_variables, name))
-            } else {
-                Ok((&mut request.get_variables, name))
             }
+            Ok((request.set_variables.borrow_mut(), name))
         }
-        StmtParam::Get(name) => Ok((&mut request.get_variables, name)),
-        StmtParam::Post(name) => Ok((&mut request.post_variables, name)),
+        StmtParam::Post(name) => Ok((request.set_variables.borrow_mut(), name)),
         _ => Err(anyhow!(
             "Only GET and POST variables can be set, not {variable:?}"
         )),
diff --git a/src/webserver/database/sqlpage_functions/functions.rs b/src/webserver/database/sqlpage_functions/functions.rs
@@ -569,8 +569,8 @@ async fn run_sql<'a>(
         )
         .await
         .with_context(|| format!("run_sql: invalid path {sql_file_path:?}"))?;
-    let mut tmp_req = if let Some(variables) = variables {
-        let mut tmp_req = request.clone_without_variables();
+    let tmp_req = if let Some(variables) = variables {
+        let tmp_req = request.clone_without_variables();
         let variables: ParamMap = serde_json::from_str(&variables).map_err(|err| {
             let context = format!(
                 "run_sql: unable to parse the variables argument (line {}, column {})",
@@ -579,7 +579,7 @@ async fn run_sql<'a>(
             );
             anyhow::Error::new(err).context(context)
         })?;
-        tmp_req.get_variables = variables;
+        tmp_req.set_variables.replace(variables);
         tmp_req
     } else {
         request.clone()
@@ -596,7 +596,7 @@ async fn run_sql<'a>(
     let mut results_stream =
         crate::webserver::database::execute_queries::stream_query_results_boxed(
             &sql_file,
-            &mut tmp_req,
+            &tmp_req,
             db_connection,
         );
     let mut json_results_bytes = Vec::new();
@@ -691,22 +691,30 @@ async fn variables<'a>(
 ) -> anyhow::Result<String> {
     Ok(if let Some(get_or_post) = get_or_post {
         if get_or_post.eq_ignore_ascii_case("get") {
-            serde_json::to_string(&request.get_variables)?
+            serde_json::to_string(&request.url_params)?
         } else if get_or_post.eq_ignore_ascii_case("post") {
             serde_json::to_string(&request.post_variables)?
+        } else if get_or_post.eq_ignore_ascii_case("set") {
+            serde_json::to_string(&*request.set_variables.borrow())?
         } else {
             return Err(anyhow!(
-                "Expected 'get' or 'post' as the argument to sqlpage.all_variables"
+                "Expected 'get', 'post', or 'set' as the argument to sqlpage.variables"
             ));
         }
     } else {
         use serde::{ser::SerializeMap, Serializer};
         let mut res = Vec::new();
         let mut serializer = serde_json::Serializer::new(&mut res);
-        let len = request.get_variables.len() + request.post_variables.len();
+        let set_vars = request.set_variables.borrow();
+        let len = request.url_params.len() + request.post_variables.len() + set_vars.len();
         let mut ser = serializer.serialize_map(Some(len))?;
-        let iter = request.get_variables.iter().chain(&request.post_variables);
-        for (k, v) in iter {
+        for (k, v) in &request.url_params {
+            ser.serialize_entry(k, v)?;
+        }
+        for (k, v) in &request.post_variables {
+            ser.serialize_entry(k, v)?;
+        }
+        for (k, v) in &*set_vars {
             ser.serialize_entry(k, v)?;
         }
         ser.end()?;
diff --git a/src/webserver/database/syntax_tree.rs b/src/webserver/database/syntax_tree.rs
@@ -156,24 +156,34 @@ pub(super) async fn extract_req_param<'a>(
 ) -> anyhow::Result<Option<Cow<'a, str>>> {
     Ok(match param {
         // sync functions
-        StmtParam::Get(x) => request.get_variables.get(x).map(SingleOrVec::as_json_str),
-        StmtParam::Post(x) => request.post_variables.get(x).map(SingleOrVec::as_json_str),
+        StmtParam::Get(x) => request.url_params.get(x).map(SingleOrVec::as_json_str),
+        StmtParam::Post(x) => {
+            if let Some(val) = request.set_variables.borrow().get(x) {
+                Some(Cow::Owned(val.as_json_str().into_owned()))
+            } else {
+                request.post_variables.get(x).map(SingleOrVec::as_json_str)
+            }
+        }
         StmtParam::PostOrGet(x) => {
-            let post_val = request.post_variables.get(x);
-            let get_val = request.get_variables.get(x);
-            if let Some(v) = post_val {
-                if let Some(get_val) = get_val {
-                    log::warn!(
-                        "Deprecation warning! There is both a URL parameter named '{x}' with value '{get_val}' and a form field named '{x}' with value '{v}'. \
-                        SQLPage is using the value from the form submission, but this is ambiguous, can lead to unexpected behavior, and will stop working in a future version of SQLPage. \
-                        To fix this, please rename the URL parameter to something else, and reference the form field with :{x}."
-                    );
+            if let Some(val) = request.set_variables.borrow().get(x) {
+                Some(Cow::Owned(val.as_json_str().into_owned()))
+            } else {
+                let url_val = request.url_params.get(x);
+                let post_val = request.post_variables.get(x);
+                if let Some(post_val) = post_val {
+                    if let Some(url_val) = url_val {
+                        log::warn!(
+                            "Deprecation warning! There is both a URL parameter named '{x}' with value '{url_val}' and a form field named '{x}' with value '{post_val}'. \
+                            SQLPage is using the value from the form submission, but this is ambiguous, can lead to unexpected behavior, and will stop working in a future version of SQLPage. \
+                            To fix this, please rename the URL parameter to something else, and reference the form field with :{x}."
+                        );
+                    } else {
+                        log::warn!("Deprecation warning! ${x} was used to reference a form field value (a POST variable) instead of a URL parameter. This will stop working soon. Please use :{x} instead.");
+                    }
+                    Some(post_val.as_json_str())
                 } else {
-                    log::warn!("Deprecation warning! ${x} was used to reference a form field value (a POST variable) instead of a URL parameter. This will stop working soon. Please use :{x} instead.");
+                    url_val.map(SingleOrVec::as_json_str)
                 }
-                Some(v.as_json_str())
-            } else {
-                get_val.map(SingleOrVec::as_json_str)
             }
         }
         StmtParam::Error(x) => anyhow::bail!("{x}"),
diff --git a/src/webserver/http.rs b/src/webserver/http.rs
@@ -174,7 +174,7 @@ async fn render_sql(
         .clone()
         .into_inner();
 
-    let mut req_param = extract_request_info(srv_req, Arc::clone(&app_state), server_timing)
+    let req_param = extract_request_info(srv_req, Arc::clone(&app_state), server_timing)
         .await
         .map_err(|e| anyhow_err_to_actix(e, &app_state))?;
     log::debug!("Received a request with the following parameters: {req_param:?}");
@@ -185,14 +185,14 @@ async fn render_sql(
     let source_path: PathBuf = sql_file.source_path.clone();
     actix_web::rt::spawn(async move {
         let request_context = RequestContext {
-            is_embedded: req_param.get_variables.contains_key("_sqlpage_embed"),
+            is_embedded: req_param.url_params.contains_key("_sqlpage_embed"),
             source_path,
             content_security_policy: ContentSecurityPolicy::with_random_nonce(),
             server_timing: Arc::clone(&req_param.server_timing),
         };
         let mut conn = None;
         let database_entries_stream =
-            stream_query_results_with_conn(&sql_file, &mut req_param, &mut conn);
+            stream_query_results_with_conn(&sql_file, &req_param, &mut conn);
         let database_entries_stream = stop_at_first_error(database_entries_stream);
         let response_with_writer = build_response_header_and_stream(
             Arc::clone(&app_state),
diff --git a/src/webserver/http_request_info.rs b/src/webserver/http_request_info.rs
diff --git a/tests/sql_test_files/it_works_immutable_variables.sql b/tests/sql_test_files/it_works_immutable_variables.sql
