From f88ed6ecd52c167485f50f1c1bd9dea2d2d8fc04 Mon Sep 17 00:00:00 2001 From: Douglas <32344964+NotTheDr01ds@users.noreply.github.com> Date: Sun, 26 Jan 2025 08:20:39 -0500 Subject: [PATCH] Fix improperly escaped strings in stor update (#14921) # Description Fixes #14909 with the same technique used in #12820 for `stor insert`. Single quotes (and others) now work properly in strings passed to `stor update`. Also did some minor refactoring on `stor insert` so it matches the changes in `stor update`. # User-Facing Changes Bug-fix. # Tests + Formatting Test added for this scenario. - :green_circle: `toolkit fmt` - :green_circle: `toolkit clippy` - :green_circle: `toolkit test` - :green_circle: `toolkit test stdlib` # After Submitting N/A --- crates/nu-command/src/stor/insert.rs | 24 ++++++-------- crates/nu-command/src/stor/update.rs | 42 +++++++----------------- crates/nu-command/tests/commands/mod.rs | 1 + crates/nu-command/tests/commands/stor.rs | 37 +++++++++++++++++++++ 4 files changed, 59 insertions(+), 45 deletions(-) create mode 100644 crates/nu-command/tests/commands/stor.rs diff --git a/crates/nu-command/src/stor/insert.rs b/crates/nu-command/src/stor/insert.rs index 94b5bdf940..4a2ebf4928 100644 --- a/crates/nu-command/src/stor/insert.rs +++ b/crates/nu-command/src/stor/insert.rs @@ -161,27 +161,23 @@ fn process( let new_table_name = table_name.unwrap_or("table".into()); if let Ok(conn) = db.open_connection() { - let mut create_stmt = format!("INSERT INTO {} ( ", new_table_name); + let mut create_stmt = format!("INSERT INTO {} (", new_table_name); + let mut column_placeholders: Vec = Vec::new(); + let cols = record.columns(); cols.for_each(|col| { - create_stmt.push_str(&format!("{}, ", col)); + column_placeholders.push(col.to_string()); }); - if create_stmt.ends_with(", ") { - create_stmt.pop(); - create_stmt.pop(); - } + + create_stmt.push_str(&column_placeholders.join(", ")); // Values are set as placeholders. - create_stmt.push_str(") VALUES ( "); + create_stmt.push_str(") VALUES ("); + let mut value_placeholders: Vec = Vec::new(); for (index, _) in record.columns().enumerate() { - create_stmt.push_str(&format!("?{}, ", index + 1)); + value_placeholders.push(format!("?{}", index + 1)); } - - if create_stmt.ends_with(", ") { - create_stmt.pop(); - create_stmt.pop(); - } - + create_stmt.push_str(&value_placeholders.join(", ")); create_stmt.push(')'); // dbg!(&create_stmt); diff --git a/crates/nu-command/src/stor/update.rs b/crates/nu-command/src/stor/update.rs index a158e0f7d0..8a77f41d51 100644 --- a/crates/nu-command/src/stor/update.rs +++ b/crates/nu-command/src/stor/update.rs @@ -1,6 +1,7 @@ -use crate::database::{SQLiteDatabase, MEMORY_DB}; +use crate::database::{values_to_sql, SQLiteDatabase, MEMORY_DB}; use nu_engine::command_prelude::*; use nu_protocol::Signals; +use rusqlite::params_from_iter; #[derive(Clone)] pub struct StorUpdate; @@ -163,36 +164,12 @@ fn process( let mut update_stmt = format!("UPDATE {} ", new_table_name); update_stmt.push_str("SET "); - let vals = record.iter(); - vals.for_each(|(key, val)| match val { - Value::Int { val, .. } => { - update_stmt.push_str(&format!("{} = {}, ", key, val)); - } - Value::Float { val, .. } => { - update_stmt.push_str(&format!("{} = {}, ", key, val)); - } - Value::String { val, .. } => { - update_stmt.push_str(&format!("{} = '{}', ", key, val)); - } - Value::Date { val, .. } => { - update_stmt.push_str(&format!("{} = '{}', ", key, val)); - } - Value::Bool { val, .. } => { - update_stmt.push_str(&format!("{} = {}, ", key, val)); - } - _ => { - // return Err(ShellError::UnsupportedInput { - // msg: format!("{} is not a valid datepart, expected one of year, month, day, hour, minute, second, millisecond, microsecond, nanosecond", part.item), - // input: "value originates from here".to_string(), - // msg_span: span, - // input_span: val.span(), - // }); - } - }); - if update_stmt.ends_with(", ") { - update_stmt.pop(); - update_stmt.pop(); + let mut placeholders: Vec = Vec::new(); + + for (index, (key, _)) in record.iter().enumerate() { + placeholders.push(format!("{} = ?{}", key, index + 1)); } + update_stmt.push_str(&placeholders.join(", ")); // Yup, this is a bit janky, but I'm not sure a better way to do this without having // --and and --or flags as well as supporting ==, !=, <>, is null, is not null, etc. @@ -202,7 +179,10 @@ fn process( } // dbg!(&update_stmt); - conn.execute(&update_stmt, []) + // Get the params from the passed values + let params = values_to_sql(record.values().cloned())?; + + conn.execute(&update_stmt, params_from_iter(params)) .map_err(|err| ShellError::GenericError { error: "Failed to open SQLite connection in memory from update".into(), msg: err.to_string(), diff --git a/crates/nu-command/tests/commands/mod.rs b/crates/nu-command/tests/commands/mod.rs index 73d5d4577a..3b47b0eb90 100644 --- a/crates/nu-command/tests/commands/mod.rs +++ b/crates/nu-command/tests/commands/mod.rs @@ -105,6 +105,7 @@ mod sort_by; mod source_env; mod split_column; mod split_row; +mod stor; mod str_; mod table; mod take; diff --git a/crates/nu-command/tests/commands/stor.rs b/crates/nu-command/tests/commands/stor.rs new file mode 100644 index 0000000000..4ed0c5dae0 --- /dev/null +++ b/crates/nu-command/tests/commands/stor.rs @@ -0,0 +1,37 @@ +use nu_test_support::{nu, pipeline}; + +#[test] +fn stor_insert() { + let actual = nu!(pipeline( + r#" + stor create --table-name test_table --columns { id: int, value: str }; + stor insert -t test_table --data-record { + id: 1 + value: "'Initial value'" + }; + stor open | query db 'select value from test_table' | get 0.value + "# + )); + + assert_eq!(actual.out, "'Initial value'"); +} + +#[test] +fn stor_update_with_quote() { + let actual = nu!(pipeline( + r#" + stor create --table-name test_table --columns { id: int, value: str }; + stor insert -t test_table --data-record { + id: 1 + value: "'Initial value'" + }; + stor update -t test_table --where-clause 'id = 1' --update-record { + id: 1 + value: "This didn't work, but should now." + }; + stor open | query db 'select value from test_table' | get 0.value + "# + )); + + assert_eq!(actual.out, "This didn't work, but should now."); +}