Bind primary key more strictly in record update requests.

This commit is contained in:
Sebastian Jeltsch
2025-08-07 21:47:02 +02:00
parent fcfc9b8480
commit 70c8e00b08
8 changed files with 288 additions and 148 deletions

View File

@@ -44,7 +44,7 @@ pub(crate) async fn insert_row(
schema_metadata.json_metadata.has_file_columns(),
// NOTE: We "fancy" parse JSON string values, since the UI currently ships everything as a
// string. We could consider pushing some more type-awareness into the ui.
Params::from(&*schema_metadata, json_row, None, true)?,
Params::from(&*schema_metadata, json_row, None, None, true)?,
)
.await?;

View File

@@ -6,7 +6,7 @@ use ts_rs::TS;
use crate::admin::AdminError as Error;
use crate::app_state::AppState;
use crate::records::params::{JsonRow, Params};
use crate::records::params::{JsonRow, Params, PrimaryKeyParam};
use crate::records::query_builder::UpdateQueryBuilder;
#[derive(Debug, Serialize, Deserialize, Default, TS)]
@@ -58,12 +58,23 @@ pub async fn update_row_handler(
UpdateQueryBuilder::run(
&state,
&QualifiedNameEscaped::new(&schema_metadata.schema.name),
&column.name,
trailbase_schema::json::flat_json_to_value(column.data_type, request.primary_key_value, true)?,
schema_metadata.json_metadata.has_file_columns(),
// NOTE: We "fancy" parse JSON string values, since the UI currently ships everything as a
// string. We could consider pushing some more type-awareness into the ui.
Params::from(&*schema_metadata, request.row, None, true)?,
Params::from(
&*schema_metadata,
request.row,
None,
Some(PrimaryKeyParam {
column_name: pk_col.clone(),
// NOTE: We "fancy" parse JSON string values, since the UI currently ships everything as a
// string. We could consider pushing some more type-awareness into the ui.
value: trailbase_schema::json::flat_json_to_value(
column.data_type,
request.primary_key_value,
true,
)?,
}),
true,
)?,
)
.await?;

View File

@@ -87,7 +87,7 @@ pub async fn create_avatar_handler(
serde_json::Value::String(uuid_to_b64(&user.uuid)),
)]);
let lazy_params = LazyParams::new(&*table, record, Some(files));
let lazy_params = LazyParams::for_insert(&*table, record, Some(files));
let params = lazy_params
.consume()
.map_err(|_| AuthError::BadRequest("parameter conversion"))?;

View File

@@ -129,7 +129,7 @@ pub async fn create_record_handler(
}
}
let mut lazy_params = LazyParams::new(&api, record, files);
let mut lazy_params = LazyParams::for_insert(&api, record, files);
// NOTE: We're currently serializing the async checks, we could parallelize them however it's
// unclear if this would be much faster.

View File

@@ -112,10 +112,18 @@ impl SchemaAccessor for RecordApi {
}
}
pub struct PrimaryKeyParam {
pub column_name: String,
pub value: Value,
}
/// Represents a record provided by the user via request, i.e. a create or update record request.
///
/// To construct a `Params`, the request will be transformed, i.e. fields for unknown columns will
/// be filtered out and the json values will be translated into SQLite values.
///
/// NOTE: We could probably have two different implementations: InsertParams and UpateParams,
/// however this would bubble all the way up to LazyParams.
pub struct Params {
/// List of named params with their respective placeholders, e.g.:
/// '(":col_name": Value::Text("hi"))'.
@@ -127,6 +135,8 @@ pub struct Params {
/// Metadata for mapping `named_params` back to SQL schema to construct Insert/Update queries.
pub(super) column_names: Vec<String>,
pub(super) column_indexes: Vec<usize>,
pub(super) pk_column_name: Option<String>,
}
impl Params {
@@ -152,111 +162,89 @@ impl Params {
accessor: &S,
json: JsonRow,
multipart_files: Option<Vec<FileUploadInput>>,
primary_key: Option<PrimaryKeyParam>,
fancy_parse_string: bool,
) -> Result<Self, ParamsError> {
let len = json.len();
let mut params = Params {
named_params: NamedParams::with_capacity(len),
files: FileMetadataContents::default(),
column_names: Vec::with_capacity(len),
column_indexes: Vec::with_capacity(len),
};
let mut named_params = NamedParams::with_capacity(json.len() + 1);
let mut column_names = Vec::with_capacity(json.len() + 1);
let mut column_indexes = Vec::with_capacity(json.len() + 1);
for (key, value) in json {
// We simply skip unknown columns, this could simply be malformed input or version skew. This
// is similar in spirit to protobuf's unknown fields behavior.
let Some((index, col, json_meta)) = accessor.column_by_name(&key) else {
continue;
};
let mut files: FileMetadataContents = vec![];
let (param, mut json_files) =
extract_params_and_files_from_json(col, json_meta, value, fancy_parse_string)?;
if let Some(json_files) = json_files.as_mut() {
// Note: files provided as a multipart form upload are handled below. They need more
// special handling to establish the field.name to column mapping.
params.files.append(json_files);
let pk_column_name = if let Some(primary_key) = primary_key {
// Update parameters case.
for (key, value) in json {
// We simply skip unknown columns, this could simply be malformed input or version skew. This
// is similar in spirit to protobuf's unknown fields behavior.
let Some((index, col, json_meta)) = accessor.column_by_name(&key) else {
continue;
};
let (param, json_files) =
extract_params_and_files_from_json(col, json_meta, value, fancy_parse_string)?;
if let Some(json_files) = json_files {
// Note: files provided as a multipart form upload are handled below. They need more
// special handling to establish the field.name to column mapping.
files.extend(json_files);
}
if key == primary_key.column_name && primary_key.value != param {
return Err(ParamsError::Column(
"Primary key mismatch in update request",
));
}
named_params.push((prefix_colon(&key).into(), param));
column_names.push(key);
column_indexes.push(index);
}
params.named_params.push((prefix_colon(&key).into(), param));
params.column_names.push(key);
params.column_indexes.push(index);
}
// Inject the pk_value. It may already be present, if redundantly provided both in the API path
// *and* the request. In most cases it probably wont and duplication is not an issue.
named_params.push((":__pk_value".into(), primary_key.value));
Some(primary_key.column_name)
} else {
// Insert parameters case.
for (key, value) in json {
// We simply skip unknown columns, this could simply be malformed input or version skew. This
// is similar in spirit to protobuf's unknown fields behavior.
let Some((index, col, json_meta)) = accessor.column_by_name(&key) else {
continue;
};
let (param, json_files) =
extract_params_and_files_from_json(col, json_meta, value, fancy_parse_string)?;
if let Some(json_files) = json_files {
// Note: files provided as a multipart form upload are handled below. They need more
// special handling to establish the field.name to column mapping.
files.extend(json_files);
}
named_params.push((prefix_colon(&key).into(), param));
column_names.push(key);
column_indexes.push(index);
}
None
};
// Note: files provided as part of a JSON request are handled above.
if let Some(multipart_files) = multipart_files {
params.append_multipart_files(accessor, multipart_files)?;
files.extend(extract_files_from_multipart(
accessor,
multipart_files,
&mut named_params,
&mut column_names,
&mut column_indexes,
)?);
}
return Ok(params);
}
fn append_multipart_files<S: SchemaAccessor>(
&mut self,
accessor: &S,
multipart_files: Vec<FileUploadInput>,
) -> Result<(), ParamsError> {
let files: Vec<(String, FileUpload, Vec<u8>)> = multipart_files
.into_iter()
.map(|file| {
let (col_name, file_metadata, content) = file.consume()?;
return match col_name {
Some(col_name) => Ok((col_name, file_metadata, content)),
None => Err(ParamsError::Column(
"Multipart form upload missing name property",
)),
};
})
.collect::<Result<_, ParamsError>>()?;
// Validate and organize by type;
let mut uploaded_files = HashSet::<&'static str>::new();
for (field_name, file_metadata, _content) in &files {
// We simply skip unknown columns, this could simply be malformed input or version skew. This
// is similar in spirit to protobuf's unknown fields behavior.
let Some((index, col, json_meta)) = accessor.column_by_name(field_name) else {
continue;
};
let Some(JsonColumnMetadata::SchemaName(schema_name)) = &json_meta else {
return Err(ParamsError::Column("Expected json column"));
};
let value = Value::Text(serde_json::to_string(&file_metadata)?);
match schema_name.as_str() {
"std.FileUpload" => {
if !uploaded_files.insert(&col.name) {
return Err(ParamsError::Column(
"Collision: too many files for std.FileUpload",
));
}
self
.named_params
.push((prefix_colon(&col.name).into(), value));
self.column_names.push(col.name.to_string());
self.column_indexes.push(index);
}
"std.FileUploads" => {
self
.named_params
.push((prefix_colon(&col.name).into(), value));
self.column_names.push(col.name.to_string());
self.column_indexes.push(index);
}
_ => {
return Err(ParamsError::Column("Mismatching JSON schema"));
}
}
}
self.files.append(
&mut files
.into_iter()
.map(|(_, file_metadata, content)| (file_metadata, content))
.collect(),
);
return Ok(());
return Ok(Params {
named_params,
files,
column_names,
column_indexes,
pk_column_name,
});
}
}
@@ -273,13 +261,14 @@ pub struct LazyParams<'a, S: SchemaAccessor> {
accessor: &'a S,
json_row: JsonRow,
multipart_files: Option<Vec<FileUploadInput>>,
primary_key: Option<PrimaryKeyParam>,
// Cached evaluate params. We could use a OnceCell but we don't need the synchronisation.
result: Option<Result<Params, ParamsError>>,
}
impl<'a, S: SchemaAccessor> LazyParams<'a, S> {
pub fn new(
pub fn for_insert(
accessor: &'a S,
json_row: JsonRow,
multipart_files: Option<Vec<FileUploadInput>>,
@@ -287,6 +276,26 @@ impl<'a, S: SchemaAccessor> LazyParams<'a, S> {
LazyParams {
accessor,
json_row,
primary_key: None,
multipart_files,
result: None,
}
}
pub fn for_update(
accessor: &'a S,
json_row: JsonRow,
multipart_files: Option<Vec<FileUploadInput>>,
primary_key_column: String,
primary_key_value: Value,
) -> Self {
LazyParams {
accessor,
json_row,
primary_key: Some(PrimaryKeyParam {
column_name: primary_key_column,
value: primary_key_value,
}),
multipart_files,
result: None,
}
@@ -298,6 +307,7 @@ impl<'a, S: SchemaAccessor> LazyParams<'a, S> {
self.accessor,
std::mem::take(&mut self.json_row),
std::mem::take(&mut self.multipart_files),
std::mem::take(&mut self.primary_key),
false,
)
});
@@ -306,32 +316,101 @@ impl<'a, S: SchemaAccessor> LazyParams<'a, S> {
}
pub fn consume(mut self) -> Result<Params, ParamsError> {
return self
.result
.take()
.unwrap_or_else(|| Params::from(self.accessor, self.json_row, self.multipart_files, false));
return self.result.take().unwrap_or_else(|| {
Params::from(
self.accessor,
self.json_row,
self.multipart_files,
self.primary_key,
false,
)
});
}
}
fn extract_files_from_multipart<S: SchemaAccessor>(
accessor: &S,
multipart_files: Vec<FileUploadInput>,
named_params: &mut NamedParams,
column_names: &mut Vec<String>,
column_indexes: &mut Vec<usize>,
) -> Result<FileMetadataContents, ParamsError> {
let files: Vec<(String, FileUpload, Vec<u8>)> = multipart_files
.into_iter()
.map(|file| {
let (col_name, file_metadata, content) = file.consume()?;
let Some(col_name) = col_name else {
return Err(ParamsError::Column(
"Multipart form upload missing name property",
));
};
return Ok((col_name, file_metadata, content));
})
.collect::<Result<_, ParamsError>>()?;
// Validate and organize by type;
let mut uploaded_files = HashSet::<&'static str>::new();
for (field_name, file_metadata, _content) in &files {
// We simply skip unknown columns, this could simply be malformed input or version skew. This
// is similar in spirit to protobuf's unknown fields behavior.
let Some((index, col, json_meta)) = accessor.column_by_name(field_name) else {
continue;
};
let Some(JsonColumnMetadata::SchemaName(schema_name)) = &json_meta else {
return Err(ParamsError::Column("Expected json column"));
};
let value = Value::Text(serde_json::to_string(&file_metadata)?);
match schema_name.as_str() {
"std.FileUpload" => {
if !uploaded_files.insert(&col.name) {
return Err(ParamsError::Column(
"Collision: too many files for std.FileUpload",
));
}
named_params.push((prefix_colon(&col.name).into(), value));
column_names.push(col.name.to_string());
column_indexes.push(index);
}
"std.FileUploads" => {
named_params.push((prefix_colon(&col.name).into(), value));
column_names.push(col.name.to_string());
column_indexes.push(index);
}
_ => {
return Err(ParamsError::Column("Mismatching JSON schema"));
}
}
}
return Ok(
files
.into_iter()
.map(|(_, file_metadata, content)| (file_metadata, content))
.collect(),
);
}
fn extract_params_and_files_from_json(
col: &Column,
json_meta: Option<&JsonColumnMetadata>,
value: serde_json::Value,
fancy_parse_string: bool,
) -> Result<(Value, Option<FileMetadataContents>), ParamsError> {
let col_name = &col.name;
match value {
return match value {
serde_json::Value::Object(ref _map) => {
// Only text columns are allowed to store nested JSON as text.
if col.data_type != ColumnDataType::Text {
return Err(ParamsError::NestedObject(format!(
"Column data mismatch for: {col_name}"
"Column data mismatch for: {col:?}",
)));
}
let Some(ref json) = json_meta else {
return Err(ParamsError::NestedObject(format!(
"Plain text column w/o JSON: {col_name}"
"Missing JSON metadata for: {col:?}",
)));
};
@@ -345,11 +424,11 @@ fn extract_params_and_files_from_json(
let (_col_name, metadata, content) = file_upload.consume()?;
let param = Value::Text(serde_json::to_string(&metadata)?);
return Ok((param, Some(vec![(metadata, content)])));
Ok((param, Some(vec![(metadata, content)])))
}
_ => {
json.validate(&value)?;
return Ok((Value::Text(value.to_string()), None));
Ok((Value::Text(value.to_string()), None))
}
}
}
@@ -387,16 +466,14 @@ fn extract_params_and_files_from_json(
_ => {}
}
return Err(ParamsError::NestedArray(format!(
"Received nested array for unsuitable column: {col_name}"
)));
}
x => {
return Ok((
flat_json_to_value(col.data_type, x, fancy_parse_string)?,
None,
));
Err(ParamsError::NestedArray(format!(
"Received nested array for column: {col:?}",
)))
}
x => Ok((
flat_json_to_value(col.data_type, x, fancy_parse_string)?,
None,
)),
};
}
@@ -513,7 +590,14 @@ mod tests {
});
assert_params(
Params::from(&metadata, json_row_from_value(value).unwrap(), None, false).unwrap(),
Params::from(
&metadata,
json_row_from_value(value).unwrap(),
None,
None,
false,
)
.unwrap(),
);
}
@@ -528,7 +612,14 @@ mod tests {
});
assert_params(
Params::from(&metadata, json_row_from_value(value).unwrap(), None, false).unwrap(),
Params::from(
&metadata,
json_row_from_value(value).unwrap(),
None,
None,
false,
)
.unwrap(),
);
let value = json!({
@@ -544,12 +635,20 @@ mod tests {
&metadata,
json_row_from_value(value.clone()).unwrap(),
None,
None,
false
)
.is_err()
);
assert_params(
Params::from(&metadata, json_row_from_value(value).unwrap(), None, true).unwrap(),
Params::from(
&metadata,
json_row_from_value(value).unwrap(),
None,
None,
true,
)
.unwrap(),
);
}
@@ -564,7 +663,16 @@ mod tests {
"real": 3,
});
assert!(Params::from(&metadata, json_row_from_value(value).unwrap(), None, false).is_err());
assert!(
Params::from(
&metadata,
json_row_from_value(value).unwrap(),
None,
None,
false
)
.is_err()
);
// Test that nested JSON object can be passed.
let value = json!({
@@ -578,8 +686,14 @@ mod tests {
"real": 3,
});
let params =
Params::from(&metadata, json_row_from_value(value).unwrap(), None, false).unwrap();
let params = Params::from(
&metadata,
json_row_from_value(value).unwrap(),
None,
None,
false,
)
.unwrap();
assert_params(params);
}
@@ -592,7 +706,16 @@ mod tests {
"real": 3,
});
assert!(Params::from(&metadata, json_row_from_value(value).unwrap(), None, false).is_err());
assert!(
Params::from(
&metadata,
json_row_from_value(value).unwrap(),
None,
None,
false
)
.is_err()
);
// Test that nested JSON array can be passed.
let nested_json_blob: Vec<u8> = vec![65, 66, 67, 68];
@@ -609,8 +732,14 @@ mod tests {
"real": 3,
});
let params =
Params::from(&metadata, json_row_from_value(value).unwrap(), None, false).unwrap();
let params = Params::from(
&metadata,
json_row_from_value(value).unwrap(),
None,
None,
false,
)
.unwrap();
let json_col: Vec<Value> = params
.named_params
@@ -623,6 +752,8 @@ mod tests {
})
.collect();
assert_params(params);
assert_eq!(json_col.len(), 1);
let Value::Text(ref text) = json_col[0] else {
panic!("Unexpected param type: {:?}", json_col[0]);
@@ -637,8 +768,6 @@ mod tests {
"text": "test",
}),
);
assert_params(params);
}
}
}

View File

@@ -11,7 +11,7 @@ use crate::AppState;
use crate::config::proto::ConflictResolutionStrategy;
use crate::records::error::RecordError;
use crate::records::files::{FileError, FileManager, delete_files_marked_for_deletion};
use crate::records::params::{FileMetadataContents, Params, prefix_colon};
use crate::records::params::{FileMetadataContents, Params};
use crate::schema_metadata::{JsonColumnMetadata, SchemaMetadataCache, TableMetadata};
#[derive(Debug, Error)]
@@ -431,8 +431,6 @@ impl UpdateQueryBuilder {
pub(crate) async fn run(
state: &AppState,
table_name: &QualifiedNameEscaped,
pk_column: &str,
pk_value: Value,
has_file_columns: bool,
mut params: Params,
) -> Result<(), QueryError> {
@@ -440,6 +438,9 @@ impl UpdateQueryBuilder {
// Nothing to do.
return Ok(());
}
let Some(pk_column_name) = params.pk_column_name else {
return Err(QueryError::Precondition("Missing pk params"));
};
// We're storing any files to the object store first to make sure the DB entry is valid right
// after commit and not racily pointing to soon-to-be-written files.
@@ -453,18 +454,12 @@ impl UpdateQueryBuilder {
let query = UpdateRecordQueryTemplate {
table_name,
column_names: &params.column_names,
pk_column_name: pk_column,
pk_column_name: &pk_column_name,
returning: Some("_rowid_"),
}
.render()
.map_err(|err| QueryError::Internal(err.into()))?;
// Inject the pk_value. It may already be present, if redundantly provided both in the API path
// *and* the request. In most cases it probably wont and duplication is not an issue.
params
.named_params
.push((prefix_colon(pk_column).into(), pk_value));
let rowid: Option<i64> = state
.conn()
.query_row_f(query, params.named_params, |row| row.get(0))

View File

@@ -40,7 +40,14 @@ pub async fn update_record_handler(
let record_id = api.primary_key_to_value(record)?;
let (_index, pk_column) = api.record_pk_column();
let mut lazy_params = LazyParams::new(&api, request, multipart_files);
let mut lazy_params = LazyParams::for_update(
&api,
request,
multipart_files,
pk_column.name.clone(),
record_id.clone(),
);
api
.check_record_level_access(
Permission::Update,
@@ -53,8 +60,6 @@ pub async fn update_record_handler(
UpdateQueryBuilder::run(
&state,
api.table_name(),
&pk_column.name,
record_id,
api.has_file_columns(),
lazy_params
.consume()

View File

@@ -2,7 +2,7 @@ UPDATE {{ table_name }} SET
{%- for name in column_names -%}
{%- if !loop.first %},{% endif %} "{{ name }}" = :{{ name }}
{%- endfor %}
WHERE "{{ pk_column_name }}" = :{{ pk_column_name }}
WHERE "{{ pk_column_name }}" = :__pk_value
{%- match returning -%}
{%- when Some with ("*") %} RETURNING *
{%- when Some with (value) %} RETURNING "{{ value }}"