starred/windmill
1
0
Fork 0
mirror of https://github.com/windmill-labs/windmill.git synced 2026-10-20 08:03:28 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: reject out-of-range integer args to narrow SQL types instead of wrapping (#11864)

Browse Source 
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Ruben Fiszel
2026-10-18 12:27:06 +02:00
committed by GitHub
co-authored by Claude Opus 5.5
parent 578437885a
commit 19ec5d3cd8
3 changed files with 105 additions and 45 deletions
Download Patch File Download Diff File Expand all files Collapse all files
backend/windmill-worker/src/common.rs
+10
View File
@@ -92,6 +92,16 @@ pub(crate) fn raw_to_string(x: &str) -> String {
}
}
/// Narrow a JSON integer to the Rust type bound for a SQL argument, erroring the way the
/// database would rather than letting an `as` cast wrap it into a different value.
pub(crate) fn narrow_sql_int<T, N>(n: N, sql_type: &str) -> anyhow::Result<T>
where
T: TryFrom<N>,
N: std::fmt::Display + Copy,
{
T::try_from(n).map_err(|_| anyhow!("value \"{n}\" is out of range for type {sql_type}"))
}
pub async fn build_args_map<'a>(
job: &'a MiniPulledJob,
client: &AuthedClient,
backend/windmill-worker/src/mssql_executor.rs
+4 -4
View File
@@ -26,7 +26,7 @@ use windmill_queue::MiniPulledJob;
use windmill_queue::{append_logs, CanceledBy};
use crate::common::{
build_args_values, get_reserved_variables, s3_mode_args_to_worker_data,
build_args_values, get_reserved_variables, narrow_sql_int, s3_mode_args_to_worker_data,
s3_stream_and_upload_with_logs, OccupancyMetrics,
};
use crate::handle_child::run_future_with_polling_update_job_poller;
@@ -398,13 +398,13 @@ fn json_value_to_sql<'a>(
query.bind(b.to_owned());
}
Value::Number(n) if n.is_u64() && arg_t == "tinyint" => {
query.bind(n.as_u64().unwrap().to_owned() as u8)
query.bind(narrow_sql_int::<u8, _>(n.as_u64().unwrap(), "tinyint")?)
}
Value::Number(n) if n.is_i64() && arg_t == "smallint" => {
query.bind(n.as_i64().unwrap().to_owned() as i16)
query.bind(narrow_sql_int::<i16, _>(n.as_i64().unwrap(), "smallint")?)
}
Value::Number(n) if n.is_i64() && arg_t == "int" => {
query.bind(n.as_i64().unwrap().to_owned() as i32)
query.bind(narrow_sql_int::<i32, _>(n.as_i64().unwrap(), "int")?)
}
Value::Number(n) if n.is_i64() && arg_t == "bigint" => {
query.bind(n.as_i64().unwrap().to_owned() as i64)
backend/windmill-worker/src/pg_executor.rs
+91 -41
View File
@@ -38,7 +38,7 @@ use windmill_queue::{CanceledBy, MiniPulledJob};
use crate::agent_workers::get_datatable_resource_from_agent_http;
use crate::common::{
build_args_values, get_reserved_variables, s3_mode_args_to_worker_data,
build_args_values, get_reserved_variables, narrow_sql_int, s3_mode_args_to_worker_data,
s3_stream_and_upload_with_logs, sizeof_val, OccupancyMetrics, S3ModeWorkerData,
};
use crate::handle_child::run_future_with_polling_update_job_poller;
@@ -1768,23 +1768,30 @@ fn parse_datetime_utc(s: &str) -> Result<chrono::DateTime<Utc>, chrono::ParseErr
fn map_as_single_type<T>(
vec: Option<&Vec<Value>>,
f: impl Fn(&Value) -> Option<T>,
) -> anyhow::Result<Option<Vec<Option<T>>>> {
try_map_as_single_type(vec, |v| Ok(f(v)))
}
/// Like `map_as_single_type`, but `f` can reject an element of the right type
/// (e.g. an out-of-range integer) with its own error.
fn try_map_as_single_type<T>(
vec: Option<&Vec<Value>>,
f: impl Fn(&Value) -> anyhow::Result<Option<T>>,
) -> anyhow::Result<Option<Vec<Option<T>>>> {
if let Some(vec) = vec {
Ok(Some(
vec.into_iter()
.map(|v| {
// first option is if the value is of the right type (if none, will stop the collection and throw error)
// second option is if the value is null
// allow nulls in arrays
if matches!(v, Value::Null) {
Some(None)
} else {
f(v).map(Some)
}
})
.collect::<Option<Vec<Option<T>>>>()
.ok_or_else(|| anyhow::anyhow!("Mixed types in array"))?,
))
let mut out = Vec::with_capacity(vec.len());
for v in vec {
// allow nulls in arrays
if matches!(v, Value::Null) {
out.push(None);
} else {
match f(v)? {
Some(x) => out.push(Some(x)),
None => return Err(anyhow::anyhow!("Mixed types in array")),
}
}
}
Ok(Some(out))
} else {
Ok(None)
}
@@ -1819,26 +1826,23 @@ fn convert_vec_val(
Type::BOOL_ARRAY,
)),
"char" | "character" => Ok((
Box::new(map_as_single_type(vec, |v| {
v.as_i64()
.map(|x| x as i8)
.or_else(|| v.as_str().and_then(|s| s.parse::<i8>().ok()))
Box::new(try_map_as_single_type(vec, |v| match v.as_i64() {
Some(x) => narrow_sql_int::<i8, _>(x, "\"char\"").map(Some),
None => Ok(v.as_str().and_then(|s| s.parse::<i8>().ok())),
})?),
Type::CHAR_ARRAY,
)),
"smallint" | "smallserial" | "int2" | "serial2" => Ok((
Box::new(map_as_single_type(vec, |v| {
v.as_i64()
.map(|x| x as i16)
.or_else(|| v.as_str().and_then(|s| s.parse::<i16>().ok()))
Box::new(try_map_as_single_type(vec, |v| match v.as_i64() {
Some(x) => narrow_sql_int::<i16, _>(x, "smallint").map(Some),
None => Ok(v.as_str().and_then(|s| s.parse::<i16>().ok())),
})?),
Type::INT2_ARRAY,
)),
"int" | "integer" | "int4" | "serial" => Ok((
Box::new(map_as_single_type(vec, |v| {
v.as_i64()
.map(|x| x as i32)
.or_else(|| v.as_str().and_then(|s| s.parse::<i32>().ok()))
Box::new(try_map_as_single_type(vec, |v| match v.as_i64() {
Some(x) => narrow_sql_int::<i32, _>(x, "integer").map(Some),
None => Ok(v.as_str().and_then(|s| s.parse::<i32>().ok())),
})?),
Type::INT4_ARRAY,
)),
@@ -1859,18 +1863,18 @@ fn convert_vec_val(
Type::NUMERIC_ARRAY,
)),
"oid" => Ok((
Box::new(map_as_single_type(vec, |v| {
v.as_u64()
.map(|x| x as u32)
.or_else(|| v.as_str().and_then(|s| s.parse::<u32>().ok()))
Box::new(try_map_as_single_type(vec, |v| match v.as_u64() {
Some(x) => narrow_sql_int::<u32, _>(x, "oid").map(Some),
None => Ok(v.as_str().and_then(|s| s.parse::<u32>().ok())),
})?),
Type::OID_ARRAY,
)),
"bigint" | "bigserial" | "int8" | "serial8" => Ok((
Box::new(map_as_single_type(vec, |v| {
v.as_i64()
.or_else(|| v.as_u64().map(|x| x as i64))
.or_else(|| v.as_str().and_then(|s| s.parse::<i64>().ok()))
Box::new(try_map_as_single_type(vec, |v| match v.as_u64() {
Some(x) => narrow_sql_int::<i64, _>(x, "bigint").map(Some),
None => Ok(v
.as_i64()
.or_else(|| v.as_str().and_then(|s| s.parse::<i64>().ok()))),
})?),
Type::INT8_ARRAY,
)),
@@ -2074,7 +2078,8 @@ fn convert_val(
Ok((Box::new(n.to_string()), t))
}
Value::Number(n) if arg_t == "char" && n.is_i64() => {
Ok((Box::new(n.as_i64().unwrap() as i8), Type::CHAR))
let n = narrow_sql_int::<i8, _>(n.as_i64().unwrap(), "\"char\"")?;
Ok((Box::new(n), Type::CHAR))
}
Value::Number(n)
if (arg_t == "smallint"
@@ -2083,13 +2088,15 @@ fn convert_val(
|| arg_t == "serial2")
&& n.is_i64() =>
{
Ok((Box::new(n.as_i64().unwrap() as i16), Type::INT2))
let n = narrow_sql_int::<i16, _>(n.as_i64().unwrap(), "smallint")?;
Ok((Box::new(n), Type::INT2))
}
Value::Number(n)
if (arg_t == "int" || arg_t == "integer" || arg_t == "int4" || arg_t == "serial")
&& n.is_i64() =>
{
Ok((Box::new(n.as_i64().unwrap() as i32), Type::INT4))
let n = narrow_sql_int::<i32, _>(n.as_i64().unwrap(), "integer")?;
Ok((Box::new(n), Type::INT4))
}
Value::Number(n) if (arg_t == "real" || arg_t == "float4") && n.as_f64().is_some() => {
Ok((Box::new(n.as_f64().unwrap() as f32), Type::FLOAT4))
@@ -2109,7 +2116,8 @@ fn convert_val(
Type::NUMERIC,
)),
Value::Number(n) if arg_t == "oid" && n.is_u64() => {
Ok((Box::new(n.as_u64().unwrap() as u32), Type::OID))
let n = narrow_sql_int::<u32, _>(n.as_u64().unwrap(), "oid")?;
Ok((Box::new(n), Type::OID))
}
Value::Number(n)
if (arg_t == "bigint"
@@ -2118,7 +2126,8 @@ fn convert_val(
|| arg_t == "serial8")
&& n.is_u64() =>
{
Ok((Box::new(n.as_u64().unwrap() as i64), Type::INT8))
let n = narrow_sql_int::<i64, _>(n.as_u64().unwrap(), "bigint")?;
Ok((Box::new(n), Type::INT8))
}
Value::Number(n) if n.is_i64() => Ok((Box::new(n.as_i64().unwrap()), Type::INT8)),
Value::Number(n) => Ok((Box::new(n.as_f64().unwrap()), Type::FLOAT8)),
@@ -3313,6 +3322,47 @@ mod tests {
assert_convert_val_consistent("Number(0)/oid", json!(0), "oid", typ_for("oid"), Type::OID);
}
#[test]
fn convert_val_rejects_out_of_range_integers() {
let convert = |value: Value, arg_t: &str| {
convert_val(&value, &arg_t.to_string(), &typ_for(arg_t), false)
.map(|_| ())
.map_err(|e| e.to_string())
};
for (value, arg_t, sql_type) in [
(json!(200), "char", "\"char\""),
(json!(70000), "smallint", "smallint"),
(json!(-40000), "int2", "smallint"),
(json!(4294967297u64), "int4", "integer"),
(json!(4294967296u64), "oid", "oid"),
(json!(u64::MAX), "bigint", "bigint"),
(json!([1, 70000]), "smallint[]", "smallint"),
(json!([u64::MAX]), "int8[]", "bigint"),
] {
let err = convert(value.clone(), arg_t).expect_err(&format!("{value}/{arg_t}"));
let n = value.as_array().map_or(&value, |a| a.last().unwrap());
assert!(
err.contains(&format!(
"value \"{n}\" is out of range for type {sql_type}"
)),
"{value}/{arg_t}: {err}"
);
}
for (value, arg_t) in [
(json!(127), "char"),
(json!(-128), "char"),
(json!(32767), "smallint"),
(json!(-32768), "smallint"),
(json!(2147483647), "int"),
(json!(-2147483648i64), "int"),
(json!(4294967295u64), "oid"),
(json!(i64::MAX), "bigint"),
(json!([null, -32768, 32767]), "smallint[]"),
] {
convert(value.clone(), arg_t).unwrap_or_else(|e| panic!("{value}/{arg_t}: {e}"));
}
}
#[test]
fn convert_val_float_number_against_every_arg_t() {
let cases: &[(&str, Type)] = &[