fix a bug making it impossible to call sqlpage pseudo-functions with url parameters as arguments on some databases

lovasoa · lovasoa · commit a17bf01c0449 · 2023-09-19T03:00:54.000+02:00
diff --git a/examples/user-authentication/index.sql b/examples/user-authentication/index.sql
@@ -1,18 +1,11 @@
-WITH logged_in AS (
-    SELECT COALESCE(sqlpage.cookie('session'), '') <> '' AS logged_in
-)
 SELECT 'shell' AS component,
     'User Management App' AS title,
     'user' AS icon,
     '/' AS link,
-    json_agg(menu_items.link) AS menu_item
-FROM (
-    SELECT 'signin' AS link FROM logged_in WHERE NOT logged_in
-    UNION ALL
-    SELECT 'signup' FROM logged_in WHERE NOT logged_in
-    UNION ALL
-    SELECT 'logout' FROM logged_in WHERE logged_in
-) AS menu_items;
+    CASE COALESCE(sqlpage.cookie('session'), '')
+        WHEN '' THEN '["signin", "signup"]'::json
+        ELSE '["logout"]'::json
+    END AS menu_item;
 
 SELECT 'hero' AS component,
     'SQLPage Authentication Demo' AS title,
diff --git a/src/webserver/database/sql.rs b/src/webserver/database/sql.rs
@@ -395,6 +395,16 @@ pub(super) fn extract_variable_argument(
         Some(Expr::Value(Value::Placeholder(placeholder))) => {
             Ok(map_param(std::mem::take(placeholder)))
         }
+        Some(Expr::Identifier(ident)) => {
+            if let Some(param) = extract_ident_param(ident) {
+                Ok(param)
+            } else {
+                Err(format!(
+                    "{func_name}({}) is not a valid call. The argument must be a placeholder or a variable name.",
+                    FormatArguments(arguments)
+                ))
+            }
+        }
         Some(Expr::Function(Function {
             name: ObjectName(func_name_parts),
             args,
@@ -519,7 +529,7 @@ fn sqlpage_func_name(func_name_parts: &[Ident]) -> &str {
 mod test {
     use super::*;
 
-    fn parse_stmt<D: Dialect>(sql: &str, dialect: &D) -> Statement {
+    fn parse_stmt(sql: &str, dialect: &dyn Dialect) -> Statement {
         let mut ast = Parser::parse_sql(dialect, sql).unwrap();
         assert_eq!(ast.len(), 1);
         ast.pop().unwrap()
@@ -567,6 +577,28 @@ mod test {
         );
     }
 
+    const ALL_DIALECTS: &[(&dyn Dialect, AnyKind)] = &[
+        (&PostgreSqlDialect {}, AnyKind::Postgres),
+        (&MsSqlDialect {}, AnyKind::Mssql),
+        (&MySqlDialect {}, AnyKind::MySql),
+        (&SQLiteDialect {}, AnyKind::Sqlite),
+    ];
+
+    #[test]
+    fn test_sqlpage_function_with_argument() {
+        for &(dialect, kind) in ALL_DIALECTS {
+            let mut ast = parse_stmt("select sqlpage.hash_password($x)", dialect);
+            let parameters = ParameterExtractor::extract_parameters(&mut ast, kind);
+            assert_eq!(
+                parameters,
+                [StmtParam::HashPassword(Box::new(StmtParam::GetOrPost(
+                    "x".to_string()
+                )))],
+                "Failed for dialect {dialect:?}"
+            );
+        }
+    }
+
     #[test]
     fn is_own_placeholder() {
         assert!(ParameterExtractor {
diff --git a/tests/index.rs b/tests/index.rs
@@ -44,17 +44,26 @@ async fn test_404() {
 }
 
 #[actix_web::test]
-async fn test_set_variable() {
-    let resp = req_path("/tests/test_set_variable.sql").await.unwrap();
-    let body = test::read_body(resp).await;
-    assert!(body.starts_with(b"<!DOCTYPE html>"));
-    // the body should contain the strint "It works!" and should not contain the string "error"
-    let body = String::from_utf8(body.to_vec()).unwrap();
-    assert!(body.contains("Hello John Doe !"), "{body}");
-    assert!(body.contains("How are you John Doe ?"), "{body}");
-    assert!(!body.contains("error"));
+async fn test_files_it_works() {
+    // Iterate over all the sql test files in the tests/ directory
+    for entry in std::fs::read_dir("tests").unwrap() {
+        let entry = entry.unwrap();
+        let path = entry.path();
+        if path.extension().unwrap_or_default() != "sql" {
+            continue;
+        }
+        let path = format!("/{}?x=1", path.display());
+        let resp = req_path(&path).await.unwrap();
+        let body = test::read_body(resp).await;
+        assert!(body.starts_with(b"<!DOCTYPE html>"));
+        // the body should contain the strint "It works!" and should not contain the string "error"
+        let body = String::from_utf8(body.to_vec()).unwrap();
+        assert!(body.contains("It works !"), "{path}: {body}");
+        assert!(!body.contains("error"), "{body}");
+    }
 }
 
+
 async fn req_path(path: &str) -> Result<actix_web::dev::ServiceResponse, actix_web::Error> {
     init_log();
     let config = test_config();
diff --git a/tests/test_hash_password.sql b/tests/test_hash_password.sql
@@ -0,0 +1 @@
+SELECT 'text' as component, 'It works ! The hashed password is: ' || sqlpage.hash_password($x) as contents;
diff --git a/tests/test_set_variable.sql b/tests/test_set_variable.sql
@@ -1,3 +1,2 @@
-set $person = 'John' || ' ' || 'Doe';
-select 'text' as component, 'Hello ' || $person || ' !' as contents;
-select 'text' as component, 'How are you ' || $person || ' ?' as contents;
+set $what_does_it_do = 'wo' || 'rks';
+select 'text' as component, 'It ' || $what_does_it_do || ' !' as contents;

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1 @@`
	`1`	`+SELECT 'text' as component, 'It works ! The hashed password is: ' \|\| sqlpage.hash_password($x) as contents;`