diff --git a/CHANGELOG.md b/CHANGELOG.md index e3963fa6..51687362 100644 --- 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 index 88fa4067..9cf6130f 100644 --- 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 index 69fe1f51..dce7b2cb 100644 --- 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 index 8b31a436..285c3764 100644 --- 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 + '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 + '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, &'b str)> { +) -> anyhow::Result<(std::cell::RefMut<'a, HashMap>, &'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 index b7665076..4f5fbd4f 100644 --- 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 { 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 index b63aa738..4d989760 100644 --- 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>> { 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 index 9468e11e..b96b8b59 100644 --- 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 index 8f9cbac1..da556a30 100644 --- a/src/webserver/http_request_info.rs +++ b/src/webserver/http_request_info.rs @@ -17,6 +17,7 @@ use actix_web_httpauth::headers::authorization::Authorization; use actix_web_httpauth::headers::authorization::Basic; use anyhow::anyhow; use anyhow::Context; +use std::cell::RefCell; use std::collections::HashMap; use std::net::IpAddr; use std::rc::Rc; @@ -32,8 +33,9 @@ pub struct RequestInfo { pub method: actix_web::http::Method, pub path: String, pub protocol: String, - pub get_variables: ParamMap, + pub url_params: ParamMap, pub post_variables: ParamMap, + pub set_variables: RefCell, pub uploaded_files: Rc>, pub headers: ParamMap, pub client_ip: Option, @@ -53,8 +55,9 @@ impl RequestInfo { method: self.method.clone(), path: self.path.clone(), protocol: self.protocol.clone(), - get_variables: ParamMap::new(), + url_params: ParamMap::new(), post_variables: ParamMap::new(), + set_variables: RefCell::new(ParamMap::new()), uploaded_files: self.uploaded_files.clone(), headers: self.headers.clone(), client_ip: self.client_ip, @@ -72,9 +75,12 @@ impl RequestInfo { impl Clone for RequestInfo { fn clone(&self) -> Self { let mut clone = self.clone_without_variables(); - clone.get_variables.clone_from(&self.get_variables); + clone.url_params.clone_from(&self.url_params); clone.post_variables.clone_from(&self.post_variables); clone + .set_variables + .replace(self.set_variables.borrow().clone()); + clone } } @@ -116,8 +122,9 @@ pub(crate) async fn extract_request_info( method, path: req.path().to_string(), headers: param_map(headers), - get_variables: param_map(get_variables), + url_params: param_map(get_variables), post_variables: param_map(post_variables), + set_variables: RefCell::new(ParamMap::new()), uploaded_files: Rc::new(HashMap::from_iter(uploaded_files)), client_ip, cookies: param_map(cookies), @@ -295,7 +302,7 @@ mod test { .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); + assert_eq!(request_info.url_params.len(), 0); } #[actix_web::test] @@ -326,7 +333,7 @@ mod test { ); assert_eq!(request_info.uploaded_files.len(), 0); assert_eq!( - request_info.get_variables, + request_info.url_params, vec![( "my_array".to_string(), SingleOrVec::Vec(vec!["5".to_string()]) @@ -374,8 +381,8 @@ mod test { assert_eq!(request_info.uploaded_files.len(), 1); let my_upload = &request_info.uploaded_files["my_uploaded_file"]; assert_eq!(my_upload.file_name.as_ref().unwrap(), "test.txt"); - assert_eq!(request_info.get_variables.len(), 0); + assert_eq!(request_info.url_params.len(), 0); assert_eq!(std::fs::read(&my_upload.file).unwrap(), b"Hello World"); - assert_eq!(request_info.get_variables.len(), 0); + assert_eq!(request_info.url_params.len(), 0); } } diff --git a/tests/sql_test_files/it_works_immutable_variables.sql b/tests/sql_test_files/it_works_immutable_variables.sql new file mode 100644 index 00000000..11b5d459 --- /dev/null +++ b/tests/sql_test_files/it_works_immutable_variables.sql @@ -0,0 +1,31 @@ +SET x = 'set_value'; +SET set_only = 'only_in_set'; + +SELECT 'text' AS component; + +SELECT CASE + WHEN $x = 'set_value' THEN 'It works !' + WHEN $x = '1' THEN 'FAIL: SET variable should shadow URL param' + ELSE 'FAIL: Unexpected value for $x: ' || COALESCE($x, 'NULL') +END AS contents; + +SELECT CASE + WHEN $set_only = 'only_in_set' THEN 'It works !' + ELSE 'FAIL: SET-only variable not found' +END AS contents; + +SELECT CASE + WHEN sqlpage.variables('get') = '{"x":"1"}' THEN 'It works !' + ELSE 'FAIL: variables(''get'') should return only URL parameters, got: ' || sqlpage.variables('get') +END AS contents; + +SELECT CASE + WHEN sqlpage.variables('set') = '{"x":"set_value","set_only":"only_in_set"}' THEN 'It works !' + ELSE 'FAIL: variables(''set'') should return only SET variables, got: ' || sqlpage.variables('set') +END AS contents; + +SELECT CASE + WHEN sqlpage.variables() = '{"x":"set_value","set_only":"only_in_set"}' THEN 'It works !' + ELSE 'FAIL: variables() should merge all with SET taking precedence, got: ' || sqlpage.variables() +END AS contents; +