Skip to content

Commit 0f4692f

Browse files
authored
Merge 'Validate sqlite_schema before AUTOINCREMENT schema writes' from Kumar Ujjawal
- Hardened `translate_create_table` to verify `sqlite_schema` exists before using it. - Replaced panic-prone `unwrap()` calls with: - `bail_parse_error!("sqlite_schema table not found in schema")` Issue #3764 called out a possible SQLite parity edge case around AUTOINCREMENT behavior when schema metadata is unusual/corrupted (notably relevant to future `PRAGMA writable_schema` support). Reviewed-by: Nikita Sivukhin (@sivukhin) Closes #5683
2 parents 2bac3f7 + e614db5 commit 0f4692f

2 files changed

Lines changed: 121 additions & 17 deletions

File tree

core/translate/insert.rs

Lines changed: 42 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
use crate::{
22
error::{SQLITE_CONSTRAINT_NOTNULL, SQLITE_CONSTRAINT_PRIMARYKEY, SQLITE_CONSTRAINT_UNIQUE},
3-
schema::{self, BTreeTable, ColDef, Column, Index, IndexColumn, ResolvedFkRef, Table},
3+
schema::{
4+
self, BTreeTable, ColDef, Column, Index, IndexColumn, ResolvedFkRef, Table,
5+
SQLITE_SEQUENCE_TABLE_NAME,
6+
},
47
sync::Arc,
58
translate::{
69
emitter::{
@@ -1274,16 +1277,44 @@ fn resolve_upserts(
12741277
Ok(())
12751278
}
12761279

1280+
fn get_valid_sqlite_sequence_table(
1281+
resolver: &Resolver,
1282+
database_id: usize,
1283+
) -> Result<Arc<BTreeTable>> {
1284+
let Some(seq_table) = resolver.with_schema(database_id, |s| {
1285+
s.get_btree_table(SQLITE_SEQUENCE_TABLE_NAME)
1286+
}) else {
1287+
crate::bail_corrupt_error!("missing sqlite_sequence table");
1288+
};
1289+
1290+
if !seq_table.has_rowid {
1291+
crate::bail_corrupt_error!("malformed sqlite_sequence: table must have rowid");
1292+
}
1293+
1294+
if seq_table.columns.len() != 2 {
1295+
crate::bail_corrupt_error!(
1296+
"malformed sqlite_sequence: expected 2 columns, got {}",
1297+
seq_table.columns.len()
1298+
);
1299+
}
1300+
1301+
let col0_name = seq_table.columns[0].name.as_deref();
1302+
let col1_name = seq_table.columns[1].name.as_deref();
1303+
if !matches!(col0_name, Some(name) if name.eq_ignore_ascii_case("name"))
1304+
|| !matches!(col1_name, Some(name) if name.eq_ignore_ascii_case("seq"))
1305+
{
1306+
crate::bail_corrupt_error!("malformed sqlite_sequence: expected columns (name, seq)");
1307+
}
1308+
1309+
Ok(seq_table)
1310+
}
1311+
12771312
fn init_autoincrement(
12781313
program: &mut ProgramBuilder,
12791314
ctx: &mut InsertEmitCtx,
12801315
resolver: &Resolver,
12811316
) -> Result<()> {
1282-
let seq_table = resolver
1283-
.with_schema(ctx.database_id, |s| s.get_btree_table("sqlite_sequence"))
1284-
.ok_or_else(|| {
1285-
crate::error::LimboError::InternalError("sqlite_sequence table not found".to_string())
1286-
})?;
1317+
let seq_table = get_valid_sqlite_sequence_table(resolver, ctx.database_id)?;
12871318
let seq_cursor_id = program.alloc_cursor_id(CursorType::BTreeTable(seq_table.clone()));
12881319
program.emit_insn(Insn::OpenWrite {
12891320
cursor_id: seq_cursor_id,
@@ -2709,11 +2740,7 @@ fn ensure_sequence_initialized(
27092740
table: &schema::BTreeTable,
27102741
database_id: usize,
27112742
) -> Result<()> {
2712-
let seq_table = resolver
2713-
.with_schema(database_id, |s| s.get_btree_table("sqlite_sequence"))
2714-
.ok_or_else(|| {
2715-
crate::error::LimboError::InternalError("sqlite_sequence table not found".to_string())
2716-
})?;
2743+
let seq_table = get_valid_sqlite_sequence_table(resolver, database_id)?;
27172744

27182745
let seq_cursor_id = program.alloc_cursor_id(CursorType::BTreeTable(seq_table.clone()));
27192746

@@ -2801,7 +2828,7 @@ fn ensure_sequence_initialized(
28012828
key_reg: new_rowid_reg,
28022829
record_reg,
28032830
flag: InsertFlags::new(),
2804-
table_name: "sqlite_sequence".to_string(),
2831+
table_name: SQLITE_SEQUENCE_TABLE_NAME.to_string(),
28052832
});
28062833

28072834
program.preassign_label_to_next_insn(entry_exists_label);
@@ -3074,9 +3101,7 @@ fn emit_update_sqlite_sequence(
30743101
extra_amount: 0,
30753102
});
30763103

3077-
let seq_table = resolver
3078-
.with_schema(database_id, |s| s.get_btree_table("sqlite_sequence"))
3079-
.unwrap();
3104+
let seq_table = get_valid_sqlite_sequence_table(resolver, database_id)?;
30803105
let affinity_str = seq_table
30813106
.columns
30823107
.iter()
@@ -3107,7 +3132,7 @@ fn emit_update_sqlite_sequence(
31073132
key_reg: r_seq_rowid,
31083133
record_reg,
31093134
flag: InsertFlags::new(),
3110-
table_name: "sqlite_sequence".to_string(),
3135+
table_name: SQLITE_SEQUENCE_TABLE_NAME.to_string(),
31113136
});
31123137
program.emit_insn(Insn::Goto {
31133138
target_pc: end_update_label,
@@ -3119,7 +3144,7 @@ fn emit_update_sqlite_sequence(
31193144
key_reg: r_seq_rowid,
31203145
record_reg,
31213146
flag: InsertFlags(turso_parser::ast::ResolveType::Replace.bit_value() as u8),
3122-
table_name: "sqlite_sequence".to_string(),
3147+
table_name: SQLITE_SEQUENCE_TABLE_NAME.to_string(),
31233148
});
31243149

31253150
program.preassign_label_to_next_insn(end_update_label);

core/translate/mod.rs

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -379,6 +379,7 @@ pub fn translate_inner(
379379
mod tests {
380380
use super::*;
381381
use crate::io::MemoryIO;
382+
use crate::schema::{BTreeTable, Table, SQLITE_SEQUENCE_TABLE_NAME};
382383
use crate::Database;
383384

384385
/// Verify that REGEXP produces the correct error when no regexp function is registered.
@@ -414,4 +415,82 @@ mod tests {
414415
"expected 'no such function: regexp', got: {err}"
415416
);
416417
}
418+
419+
#[test]
420+
fn test_insert_autoincrement_with_malformed_sqlite_sequence_is_corrupt() {
421+
let io = Arc::new(MemoryIO::new());
422+
let db = Database::open_file(io, ":memory:").unwrap();
423+
let conn = db.connect().unwrap();
424+
conn.execute("CREATE TABLE t(id INTEGER PRIMARY KEY AUTOINCREMENT, v TEXT)")
425+
.unwrap();
426+
427+
let mut schema = db.schema.lock().as_ref().clone();
428+
let seq_root_page = schema
429+
.get_btree_table(SQLITE_SEQUENCE_TABLE_NAME)
430+
.expect("sqlite_sequence should exist after creating AUTOINCREMENT table")
431+
.root_page;
432+
let malformed_seq =
433+
BTreeTable::from_sql("CREATE TABLE sqlite_sequence(name)", seq_root_page)
434+
.expect("malformed sqlite_sequence SQL should parse");
435+
schema.tables.insert(
436+
SQLITE_SEQUENCE_TABLE_NAME.to_string(),
437+
Arc::new(Table::BTree(Arc::new(malformed_seq))),
438+
);
439+
440+
let pager = conn.pager.load().clone();
441+
let syms = SymbolTable::new();
442+
443+
let mut parser = turso_parser::parser::Parser::new(b"INSERT INTO t(v) VALUES('x')");
444+
let cmd = parser.next().unwrap().unwrap();
445+
let stmt = match cmd {
446+
ast::Cmd::Stmt(s) => s,
447+
_ => panic!("expected statement"),
448+
};
449+
450+
let err = translate(&schema, stmt, pager, conn, &syms, QueryMode::Normal, "")
451+
.expect_err("translation should fail with malformed sqlite_sequence");
452+
match err {
453+
crate::LimboError::Corrupt(msg) => {
454+
assert!(
455+
msg.contains("sqlite_sequence"),
456+
"expected sqlite_sequence corruption error, got: {msg}"
457+
);
458+
}
459+
other => panic!("expected LimboError::Corrupt, got: {other}"),
460+
}
461+
}
462+
463+
#[test]
464+
fn test_insert_autoincrement_with_missing_sqlite_sequence_is_corrupt() {
465+
let io = Arc::new(MemoryIO::new());
466+
let db = Database::open_file(io, ":memory:").unwrap();
467+
let conn = db.connect().unwrap();
468+
conn.execute("CREATE TABLE t(id INTEGER PRIMARY KEY AUTOINCREMENT, v TEXT)")
469+
.unwrap();
470+
471+
let mut schema = db.schema.lock().as_ref().clone();
472+
schema.tables.remove(SQLITE_SEQUENCE_TABLE_NAME);
473+
474+
let pager = conn.pager.load().clone();
475+
let syms = SymbolTable::new();
476+
477+
let mut parser = turso_parser::parser::Parser::new(b"INSERT INTO t(v) VALUES('x')");
478+
let cmd = parser.next().unwrap().unwrap();
479+
let stmt = match cmd {
480+
ast::Cmd::Stmt(s) => s,
481+
_ => panic!("expected statement"),
482+
};
483+
484+
let err = translate(&schema, stmt, pager, conn, &syms, QueryMode::Normal, "")
485+
.expect_err("translation should fail with missing sqlite_sequence");
486+
match err {
487+
crate::LimboError::Corrupt(msg) => {
488+
assert!(
489+
msg.contains("missing sqlite_sequence"),
490+
"expected missing sqlite_sequence error, got: {msg}"
491+
);
492+
}
493+
other => panic!("expected LimboError::Corrupt, got: {other}"),
494+
}
495+
}
417496
}

0 commit comments

Comments
 (0)