From 56775be0944263122ab94e5c4c33a1bc335f9a2a Mon Sep 17 00:00:00 2001 From: Ophir LOJKINE Date: Wed, 23 Sep 2026 17:06:47 +0200 Subject: [PATCH 1/2] fix(mssql) :: support JSON_OBJECT key:value arguments (#1480) --- CHANGELOG.md | 1 + src/webserver/database/sql.rs | 69 ++++++++++++++++- .../sql/rewrite/projection_partitioning.rs | 39 ++++------ .../sql/rewrite/sqlpage_expression.rs | 77 ++++++++++++------- 4 files changed, 134 insertions(+), 52 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4ae2dbfac..fc823a755 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,7 @@ # CHANGELOG.md ## v0.47.0 (unreleased) +- Fixed MSSQL `JSON_OBJECT('key': value)` expressions being rejected by SQLPage's parser, including when used in `SET` statements or nested in `sqlpage.*` function calls. - OIDC now checks both normalized request paths and their resolved SQL files against protected prefixes, closing authentication bypasses through path and clean-URL aliases. Nonce verification also rejects provider-returned Argon2 parameters outside SQLPage's fixed low-cost profile before hashing. - `cargo install sqlpage`, and any build from the crates.io tarball, no longer needs internet access. The browser libraries now come from npm and ship inside the published crate. Building from a git checkout needs `npm ci` first. Pre-built binaries and the Docker image are unaffected. - The browser libraries are now part of the browser scripts. SQLPage no longer defines the `window.tabler` and `window.bootstrap` globals; custom scripts that reached for them should load their own copy of Bootstrap. diff --git a/src/webserver/database/sql.rs b/src/webserver/database/sql.rs index 58e4162f0..c9e85325b 100644 --- a/src/webserver/database/sql.rs +++ b/src/webserver/database/sql.rs @@ -269,7 +269,7 @@ mod tests { ConcatNullBehavior, RowInputId, SqlPageExpr, VariableRef, VariableSource, }; use crate::webserver::database::sqlpage_functions::functions::SqlPageFunctionName; - use sqlparser::dialect::{MySqlDialect, PostgreSqlDialect}; + use sqlparser::dialect::{MsSqlDialect, MySqlDialect, PostgreSqlDialect}; use sqlx::any::AnyKind; fn database(database_type: SupportedDatabase) -> DbInfo { @@ -299,6 +299,14 @@ mod tests { .unwrap() } + fn one_mssql(sql: &str) -> FileStatement { + let database = database(SupportedDatabase::Mssql); + parse_sql(&database, &MsSqlDialect {}, sql) + .unwrap() + .next() + .unwrap() + } + fn rewrite_database(sql: &str) -> DatabaseQuery { let FileStatement::Query(Query { body: QueryBody::Database(query), @@ -696,6 +704,65 @@ mod tests { assert_eq!(query.columns.len(), 1); } + #[test] + fn mssql_json_object_colon_arguments_work_in_sqlpage_expressions() { + let json_object = SqlPageExpr::JsonObject(Box::new([(text("a"), text("b"))])); + + let FileStatement::Query(Query { + body: QueryBody::StaticSimpleSelect(query), + .. + }) = one_mssql("select 'text' as component, json_object('a':'b') as contents") + else { + panic!("expected a static SQLPage-owned row"); + }; + assert_eq!(query.columns[1].value, json_object); + + let FileStatement::SetVariable { + target, + value: + Query { + body: QueryBody::StaticSimpleSelect(query), + .. + }, + } = one_mssql("set x = json_object('a':'b')") + else { + panic!("expected a SQLPage-owned SET value"); + }; + assert_eq!(target.0, "x"); + assert_eq!(query.columns[0].value, json_object); + + let FileStatement::Query(Query { + body: QueryBody::StaticSimpleSelect(query), + .. + }) = one_mssql( + "select 'dynamic' as component, sqlpage.run_sql('frag.sql', json_object('a':'b')) as properties", + ) + else { + panic!("expected a static SQLPage-owned row"); + }; + assert_eq!( + query.columns[1].value, + call( + SqlPageFunctionName::run_sql, + [text("frag.sql"), json_object] + ) + ); + } + + #[test] + fn mssql_json_object_colon_arguments_do_not_fail_projection_analysis() { + let FileStatement::Query(Query { + body: QueryBody::Database(query), + .. + }) = one_mssql("select json_object('a': value) as contents from items") + else { + panic!("expected a database query"); + }; + assert!(query.row_input_json.is_empty()); + assert!(query.computed_columns.is_empty()); + assert!(query.sql.contains("json_object('a' : value)")); + } + #[test] fn boolean_literal_stays_a_literal_in_the_static_simple_select() { let FileStatement::Query(Query { diff --git a/src/webserver/database/sql/rewrite/projection_partitioning.rs b/src/webserver/database/sql/rewrite/projection_partitioning.rs index 96c414d75..84747f728 100644 --- a/src/webserver/database/sql/rewrite/projection_partitioning.rs +++ b/src/webserver/database/sql/rewrite/projection_partitioning.rs @@ -1,13 +1,13 @@ //! Assigns selected expressions to the database or per-row `SQLPage` evaluation. use sqlparser::ast::{ - BinaryOperator, DataType, Expr as SqlExpr, FunctionArg, FunctionArgExpr, FunctionArguments, - Ident, ObjectNamePart, SelectItem, SetExpr, Statement as SqlStatement, + BinaryOperator, DataType, Expr as SqlExpr, Ident, ObjectNamePart, SelectItem, SetExpr, + Statement as SqlStatement, }; use super::sqlpage_expression::{ - SqlPageExpressionContext, build_emulated, build_sqlpage_expr, emulated_function, - recognize_sqlpage_function, take_expression_arguments, + EmulatedFunction, SqlPageExpressionContext, build_emulated, build_sqlpage_expr, + emulated_function, expression_arguments, recognize_sqlpage_function, }; use crate::webserver::database::sqlpage_expr::{RowExpr, RowInputId, SqlPageExpr}; use crate::webserver::database::{DbInfo, SupportedDatabase}; @@ -77,13 +77,15 @@ impl<'a> ProjectionPartitioner<'a> { } let kind = emulated_function(&function) .expect("per-row function ownership was already classified"); - let arguments = take_expression_arguments(function)? - .into_iter() - .map(|argument| { - let projection = self.partition_projection(argument)?; - self.projection_into_row_expr(projection) - }) - .collect::>>()?; + let arguments = + expression_arguments(&function, matches!(kind, EmulatedFunction::JsonObject))? + .into_iter() + .cloned() + .map(|argument| { + let projection = self.partition_projection(argument)?; + self.projection_into_row_expr(projection) + }) + .collect::>>()?; Ok(PartitionedProjection::PerRow(build_emulated( kind, arguments, @@ -145,19 +147,12 @@ fn projection_is_per_row(expression: &SqlExpr) -> anyhow::Result { if recognize_sqlpage_function(function)?.is_some() { return Ok(true); } - if emulated_function(function).is_none() { + let Some(kind) = emulated_function(function) else { return Ok(false); - } - let FunctionArguments::List(arguments) = &function.args else { - anyhow::bail!("Unsupported arguments to {}", function.name); }; - if arguments.duplicate_treatment.is_some() || !arguments.clauses.is_empty() { - anyhow::bail!("Unsupported arguments to {}", function.name); - } - for argument in &arguments.args { - let FunctionArg::Unnamed(FunctionArgExpr::Expr(expression)) = argument else { - anyhow::bail!("Named and wildcard function arguments are not supported"); - }; + let arguments = + expression_arguments(function, matches!(kind, EmulatedFunction::JsonObject))?; + for expression in arguments { if projection_is_per_row(expression)? { return Ok(true); } diff --git a/src/webserver/database/sql/rewrite/sqlpage_expression.rs b/src/webserver/database/sql/rewrite/sqlpage_expression.rs index 8df150b4b..581f92e57 100644 --- a/src/webserver/database/sql/rewrite/sqlpage_expression.rs +++ b/src/webserver/database/sql/rewrite/sqlpage_expression.rs @@ -5,8 +5,9 @@ use std::str::FromStr as _; use anyhow::{Context as _, anyhow}; use serde_json::Value as JsonValue; use sqlparser::ast::{ - BinaryOperator, Expr as SqlExpr, Function, FunctionArg, FunctionArgExpr, FunctionArgumentList, - FunctionArguments, Ident, ObjectName, ObjectNamePart, Value, ValueWithSpan, + BinaryOperator, Expr as SqlExpr, Function, FunctionArg, FunctionArgExpr, FunctionArgOperator, + FunctionArgumentList, FunctionArguments, Ident, ObjectName, ObjectNamePart, Value, + ValueWithSpan, }; use crate::webserver::database::sqlpage_expr::{ @@ -71,18 +72,16 @@ pub(super) fn is_static_simple_select_expression(expression: &SqlExpr) -> anyhow }) => Ok(true), SqlExpr::Identifier(identifier) => Ok(variable_from_ident(identifier).is_some()), SqlExpr::Function(function) => { - if recognize_sqlpage_function(function)?.is_none() - && emulated_function(function).is_none() - { + let sqlpage_function = recognize_sqlpage_function(function)?; + let emulated = emulated_function(function); + if sqlpage_function.is_none() && emulated.is_none() { return Ok(false); } - let FunctionArguments::List(arguments) = &function.args else { - return Ok(false); - }; - for argument in &arguments.args { - let FunctionArg::Unnamed(FunctionArgExpr::Expr(expression)) = argument else { - return Ok(false); - }; + let arguments = expression_arguments( + function, + matches!(emulated, Some(EmulatedFunction::JsonObject)), + )?; + for expression in arguments { if !is_static_simple_select_expression(expression)? { return Ok(false); } @@ -126,8 +125,9 @@ pub(super) fn build_sqlpage_expr( ), SqlExpr::Function(function) => { if let Some(function_name) = recognize_sqlpage_function(&function)? { - let arguments = take_expression_arguments(function)? + let arguments = expression_arguments(&function, false)? .into_iter() + .cloned() .map(|argument| build_sqlpage_expr(database, context, argument)) .collect::>>()?; Ok(SqlPageExpr::Call { @@ -135,10 +135,12 @@ pub(super) fn build_sqlpage_expr( arguments: arguments.into_boxed_slice(), }) } else if let Some(kind) = emulated_function(&function) { - let arguments = take_expression_arguments(function)? - .into_iter() - .map(|argument| build_sqlpage_expr(database, context, argument)) - .collect::>>()?; + let arguments = + expression_arguments(&function, matches!(kind, EmulatedFunction::JsonObject))? + .into_iter() + .cloned() + .map(|argument| build_sqlpage_expr(database, context, argument)) + .collect::>>()?; build_emulated(kind, arguments, database.database_type) } else { context.use_database_expr(SqlExpr::Function(function)) @@ -270,23 +272,40 @@ pub(super) fn emulated_function(function: &Function) -> Option } } -pub(super) fn take_expression_arguments(function: Function) -> anyhow::Result> { - let FunctionArguments::List(arguments) = function.args else { +pub(super) fn expression_arguments( + function: &Function, + json_object_key_value_pairs: bool, +) -> anyhow::Result> { + let FunctionArguments::List(arguments) = &function.args else { anyhow::bail!("Unsupported arguments to {}", function.name); }; if arguments.duplicate_treatment.is_some() || !arguments.clauses.is_empty() { anyhow::bail!("Unsupported arguments to {}", function.name); } - arguments - .args - .into_iter() - .map(|argument| match argument { - FunctionArg::Unnamed(FunctionArgExpr::Expr(expression)) => Ok(expression), - _ => Err(anyhow!( - "Named and wildcard function arguments are not supported" - )), - }) - .collect() + let mut expressions = Vec::with_capacity(arguments.args.len()); + for argument in &arguments.args { + match argument { + FunctionArg::Unnamed(FunctionArgExpr::Expr(expression)) => { + expressions.push(expression); + } + FunctionArg::ExprNamed { + name, + arg: FunctionArgExpr::Expr(expression), + operator: FunctionArgOperator::Colon | FunctionArgOperator::Value, + } if json_object_key_value_pairs => { + // MSSQL and PostgreSQL allow JSON_OBJECT(key: value) / (key VALUE value). + // SQLPage's emulation stores object arguments as alternating key/value + // expressions, so expand this syntax into that representation here. + // Keep the expression in the argument name as the key, in source order. + expressions.push(name); + expressions.push(expression); + } + _ => { + anyhow::bail!("Named and wildcard function arguments are not supported"); + } + } + } + Ok(expressions) } pub(super) fn variable_from_expr(expression: &SqlExpr) -> Option { From 9af3ef778278a233f8b0840e9658e6e86094da29 Mon Sep 17 00:00:00 2001 From: Ophir Lojkine Date: Wed, 23 Sep 2026 16:25:12 +0000 Subject: [PATCH 2/2] test(mssql) :: cover JSON_OBJECT syntax on postgres too --- src/webserver/database/sql.rs | 121 ++++++++++-------- .../sql/rewrite/sqlpage_expression.rs | 4 - 2 files changed, 68 insertions(+), 57 deletions(-) diff --git a/src/webserver/database/sql.rs b/src/webserver/database/sql.rs index c9e85325b..9cefc8787 100644 --- a/src/webserver/database/sql.rs +++ b/src/webserver/database/sql.rs @@ -354,6 +354,50 @@ mod tests { SqlPageExpr::Literal(serde_json::Value::String(value.into())) } + fn assert_json_object_colon_arguments_work(parse: fn(&str) -> FileStatement) { + let json_object = SqlPageExpr::JsonObject(Box::new([(text("a"), text("b"))])); + + let FileStatement::Query(Query { + body: QueryBody::StaticSimpleSelect(query), + .. + }) = parse("select 'text' as component, json_object('a':'b') as contents") + else { + panic!("expected a static SQLPage-owned row"); + }; + assert_eq!(query.columns[1].value, json_object); + + let FileStatement::SetVariable { + target, + value: + Query { + body: QueryBody::StaticSimpleSelect(query), + .. + }, + } = parse("set x = json_object('a':'b')") + else { + panic!("expected a SQLPage-owned SET value"); + }; + assert_eq!(target.0, "x"); + assert_eq!(query.columns[0].value, json_object); + + let FileStatement::Query(Query { + body: QueryBody::StaticSimpleSelect(query), + .. + }) = parse( + "select 'dynamic' as component, sqlpage.run_sql('frag.sql', json_object('a':'b')) as properties", + ) + else { + panic!("expected a static SQLPage-owned row"); + }; + assert_eq!( + query.columns[1].value, + call( + SqlPageFunctionName::run_sql, + [text("frag.sql"), json_object] + ) + ); + } + #[test] fn database_only_parent_forces_binding() { let FileStatement::Query(Query { @@ -705,62 +749,33 @@ mod tests { } #[test] - fn mssql_json_object_colon_arguments_work_in_sqlpage_expressions() { - let json_object = SqlPageExpr::JsonObject(Box::new([(text("a"), text("b"))])); - - let FileStatement::Query(Query { - body: QueryBody::StaticSimpleSelect(query), - .. - }) = one_mssql("select 'text' as component, json_object('a':'b') as contents") - else { - panic!("expected a static SQLPage-owned row"); - }; - assert_eq!(query.columns[1].value, json_object); - - let FileStatement::SetVariable { - target, - value: - Query { - body: QueryBody::StaticSimpleSelect(query), - .. - }, - } = one_mssql("set x = json_object('a':'b')") - else { - panic!("expected a SQLPage-owned SET value"); - }; - assert_eq!(target.0, "x"); - assert_eq!(query.columns[0].value, json_object); - - let FileStatement::Query(Query { - body: QueryBody::StaticSimpleSelect(query), - .. - }) = one_mssql( - "select 'dynamic' as component, sqlpage.run_sql('frag.sql', json_object('a':'b')) as properties", - ) - else { - panic!("expected a static SQLPage-owned row"); - }; - assert_eq!( - query.columns[1].value, - call( - SqlPageFunctionName::run_sql, - [text("frag.sql"), json_object] - ) - ); + fn json_object_colon_arguments_work_in_sqlpage_expressions() { + assert_json_object_colon_arguments_work(one); + assert_json_object_colon_arguments_work(one_mssql); } #[test] - fn mssql_json_object_colon_arguments_do_not_fail_projection_analysis() { - let FileStatement::Query(Query { - body: QueryBody::Database(query), - .. - }) = one_mssql("select json_object('a': value) as contents from items") - else { - panic!("expected a database query"); - }; - assert!(query.row_input_json.is_empty()); - assert!(query.computed_columns.is_empty()); - assert!(query.sql.contains("json_object('a' : value)")); + fn json_object_colon_arguments_do_not_fail_projection_analysis() { + for statement in [ + one("select json_object('a': value) as contents from items"), + one_mssql("select json_object('a': value) as contents from items"), + ] { + let FileStatement::Query(Query { + body: QueryBody::Database(query), + .. + }) = statement + else { + panic!("expected a database query"); + }; + assert!(query.row_input_json.is_empty()); + assert!(query.computed_columns.is_empty()); + assert!( + query + .sql + .to_ascii_lowercase() + .contains("json_object('a' : value)") + ); + } } #[test] diff --git a/src/webserver/database/sql/rewrite/sqlpage_expression.rs b/src/webserver/database/sql/rewrite/sqlpage_expression.rs index 581f92e57..f52881d33 100644 --- a/src/webserver/database/sql/rewrite/sqlpage_expression.rs +++ b/src/webserver/database/sql/rewrite/sqlpage_expression.rs @@ -293,10 +293,6 @@ pub(super) fn expression_arguments( arg: FunctionArgExpr::Expr(expression), operator: FunctionArgOperator::Colon | FunctionArgOperator::Value, } if json_object_key_value_pairs => { - // MSSQL and PostgreSQL allow JSON_OBJECT(key: value) / (key VALUE value). - // SQLPage's emulation stores object arguments as alternating key/value - // expressions, so expand this syntax into that representation here. - // Keep the expression in the argument name as the key, in source order. expressions.push(name); expressions.push(expression); }