Skip to content

Commit 7e5585d

Browse files
invisalvisal
andauthored
fix: escape column names and handle mismatched data types in D1 SQL dump (#9866)
* fix: escape column names and handle mismatched data types in D1 SQL dump * fix(d1): allow exporting blob in tests and importing without errors * fix(d1): correct test case by quoting table names with double quotes in SQL INSERT * fix(d1): Add patch note for escape identifiers and handle dynamic typing --------- Co-authored-by: visal <[email protected]>
1 parent ca00d74 commit 7e5585d

File tree

4 files changed

+193
-18
lines changed

4 files changed

+193
-18
lines changed

.changeset/mean-schools-see.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
---
2+
"miniflare": patch
3+
---
4+
5+
Fix D1 SQL dump generation: escape identifiers and handle SQLite's dynamic typing
6+
7+
Escape column and table names to prevent SQL syntax errors.
8+
Escape values based on their runtime type to support SQLite's flexible typing.

packages/miniflare/src/workers/d1/dumpSql.ts

Lines changed: 30 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -19,13 +19,14 @@ export function* dumpSql(
1919
// Taken from SQLite shell.c.in https://github.com/sqlite/sqlite/blob/105c20648e1b05839fd0638686b95f2e3998abcb/src/shell.c.in#L8463-L8469
2020
// @ts-ignore (SqlStorageStatement needs to be callable)
2121
const tables_cursor = db.prepare(`
22-
SELECT name, type, sql
23-
FROM sqlite_schema AS o
24-
WHERE (true) AND type=='table'
25-
AND sql NOT NULL
22+
SELECT name, type, sql
23+
FROM sqlite_schema AS o
24+
WHERE (true) AND type=='table'
25+
AND sql NOT NULL
2626
ORDER BY tbl_name='sqlite_sequence', rowid;
2727
`)();
28-
const tables: any[] = Array.from(tables_cursor);
28+
const tables: { name: string; type: string; sql: string }[] =
29+
Array.from(tables_cursor);
2930

3031
for (const { name: table, sql } of tables) {
3132
if (filterTables.size > 0 && !filterTables.has(table)) continue;
@@ -54,20 +55,28 @@ export function* dumpSql(
5455
}
5556

5657
if (noData) continue;
57-
const columns_cursor = db.exec(`PRAGMA table_info="${table}"`);
58-
const columns = Array.from(columns_cursor);
59-
const select = `SELECT ${columns.map((c) => c.name).join(", ")}
60-
FROM "${table}";`;
58+
const columns_cursor = db.exec(`PRAGMA table_info=${escapeId(table)}`);
59+
60+
const columns = Array.from(columns_cursor) as {
61+
cid: string;
62+
name: string;
63+
type: string;
64+
notnull: number;
65+
dflt_val: string | null;
66+
pk: number;
67+
}[];
68+
69+
const select = `SELECT ${columns.map((c) => escapeId(c.name)).join(", ")} FROM ${escapeId(table)};`;
6170
const rows_cursor = db.exec(select);
6271
for (const dataRow of rows_cursor.raw()) {
6372
const formattedCells = dataRow.map((cell: unknown, i: number) => {
6473
const colType = columns[i].type;
6574
const cellType = typeof cell;
6675
if (cell === null) {
6776
return "NULL";
68-
} else if (colType === "INTEGER" || cellType === "number") {
77+
} else if (cellType === "number") {
6978
return cell;
70-
} else if (colType === "TEXT" || cellType === "string") {
79+
} else if (cellType === "string") {
7180
return outputQuotedEscapedString(cell);
7281
} else if (cell instanceof ArrayBuffer) {
7382
return `X'${Array.prototype.map
@@ -79,7 +88,7 @@ export function* dumpSql(
7988
}
8089
});
8190

82-
yield `INSERT INTO ${sqliteQuote(table)} VALUES(${formattedCells.join(",")});`;
91+
yield `INSERT INTO ${escapeId(table)} VALUES(${formattedCells.join(",")});`;
8392
}
8493
}
8594

@@ -138,6 +147,15 @@ export function sqliteQuote(token: string) {
138147
: token;
139148
}
140149

150+
/**
151+
* Escape an identifier for use in SQL statements.
152+
* @param id - The identifier to escape.
153+
* @returns
154+
*/
155+
function escapeId(id: string) {
156+
return `"${id.replace(/"/g, '""')}"`;
157+
}
158+
141159
// List taken from `aKeywordTable` inhttps://github.com/sqlite/sqlite/blob/378bf82e2bc09734b8c5869f9b148efe37d29527/tool/mkkeywordhash.c#L172
142160
// prettier-ignore
143161
export const SQLITE_KEYWORDS = new Set([

packages/miniflare/test/plugins/d1/suite.ts

Lines changed: 149 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,11 @@
11
import assert from "assert";
22
import fs from "fs/promises";
3+
import { type D1Database } from "@cloudflare/workers-types/experimental";
4+
import { ExecutionContext } from "ava";
35
import { Miniflare, MiniflareOptions } from "miniflare";
46
import { useTmp, utf8Encode } from "../../test-shared";
57
import { binding, getDatabase, opts, test } from "./test";
8+
import type { Context } from "./test";
69

710
export const SCHEMA = (
811
tableColours: string,
@@ -580,3 +583,149 @@ test("it properly handles ROWS_AND_COLUMNS results format", async (t) => {
580583
}
581584
t.deepEqual(results, expectedResults);
582585
});
586+
587+
/**
588+
* Test that the `dumpSql` method returns a valid SQL dump of the database.
589+
* This test creates a new D1 database, fills it with dummy data, and then
590+
* exports the SQL dump using the `PRAGMA miniflare_d1_export` command.
591+
* It then executes the dump in a new D1 database and checks if both databases
592+
* are equal in terms of schema and data.
593+
*/
594+
test("dumpSql exports and imports complete database structure and content correctly", async (t) => {
595+
// Create a new Miniflare instance with D1 database
596+
const originalMF = new Miniflare({
597+
...opts,
598+
d1Databases: { test: "test" },
599+
});
600+
const mirrorMF = new Miniflare({
601+
...opts,
602+
d1Databases: { test: "test" },
603+
});
604+
605+
t.teardown(() => originalMF.dispose());
606+
t.teardown(() => mirrorMF.dispose());
607+
608+
const originalDb = await originalMF.getD1Database("test");
609+
const mirrorDb = await mirrorMF.getD1Database("test");
610+
611+
// Fill the original database with dummy data
612+
await fillDummyData(originalDb);
613+
614+
// Export the database schema and data
615+
const result = await originalDb
616+
.prepare("PRAGMA miniflare_d1_export(?,?,?);")
617+
.bind(0, 0)
618+
.raw();
619+
620+
const [dumpStatements] = result as [string[]];
621+
const dump = dumpStatements.join("\n");
622+
623+
await mirrorDb.exec(dump);
624+
625+
// Verify that the schema and data in both databases are equal
626+
await isDatabaseEqual(t, originalDb, mirrorDb);
627+
});
628+
629+
/**
630+
* Populates a D1 database with test data for schema export testing.
631+
* Creates tables with various schema features (foreign keys, special characters, etc.)
632+
* and inserts sample data including edge cases like NULL values and type mismatches.
633+
*/
634+
async function fillDummyData(db: D1Database) {
635+
// Create schema with various SQL features to test export compatibility
636+
// Each table must have an ID column as primary key so that we can use it for ordering in equality tests
637+
638+
const schemas = [
639+
// Create basic table with text primary key
640+
`CREATE TABLE "classrooms"(id TEXT PRIMARY KEY, capacity INTEGER, test_blob BLOB)`,
641+
642+
// Create table with foreign key constraint
643+
`CREATE TABLE "students" (id INTEGER PRIMARY KEY, name TEXT NOT NULL, classroom TEXT NOT NULL, FOREIGN KEY (classroom) REFERENCES "classrooms" (id) ON DELETE CASCADE)`,
644+
645+
// Create table with spaces in name to test quoting
646+
`CREATE TABLE "test space table" (id INTEGER PRIMARY KEY, name TEXT NOT NULL)`,
647+
648+
// Create table with escaped quotes and SQL reserved keywords
649+
`CREATE TABLE "test""name" (id INTEGER PRIMARY KEY, "escaped""column" TEXT, "order" INTEGER)`,
650+
];
651+
652+
await db.exec(schemas.join(";"));
653+
654+
// Prepare sample data
655+
const classroomData = [
656+
// Standard numeric data
657+
...Array.from({ length: 10 }, (_, i) => ({
658+
id: `classroom_${i + 1}`,
659+
capacity: (i + 1) * 10,
660+
test_blob: utf8Encode(`Blob data for classroom ${i + 1}`),
661+
})),
662+
663+
// Edge case: type mismatch (string where number expected)
664+
{ id: "different_type_classroom", capacity: "not_a_number" },
665+
666+
// Edge case: NULL value
667+
{ id: "null_classroom", capacity: null },
668+
];
669+
670+
// Insert classroom data
671+
const classroomStmt = db.prepare(
672+
`INSERT INTO classrooms (id, capacity) VALUES (?, ?)`
673+
);
674+
675+
for (const classroom of classroomData) {
676+
await classroomStmt.bind(classroom.id, classroom.capacity).run();
677+
}
678+
679+
// Generate and insert student data with classroom references
680+
const studentStmt = db.prepare(
681+
`INSERT INTO students (id, name, classroom) VALUES (?, ?, ?)`
682+
);
683+
684+
// Create 2 students for each classroom
685+
for (let i = 0; i < 10; i++) {
686+
for (let j = 1; j <= 2; j++) {
687+
const studentId = i * 2 + j;
688+
await studentStmt
689+
.bind(studentId, `student_${studentId}`, `classroom_${i + 1}`)
690+
.run();
691+
}
692+
}
693+
}
694+
695+
/**
696+
* Compares two D1 databases to check if they are equal in terms of schema and data.
697+
* It retrieves the schema of both databases, compares the tables, and then
698+
* checks if the data in each table is identical.
699+
*/
700+
async function isDatabaseEqual(
701+
t: ExecutionContext<Context>,
702+
db: D1Database,
703+
db2: D1Database
704+
) {
705+
// SQL to select schema excluding internal tables
706+
const selectSchemaSQL =
707+
"SELECT * FROM sqlite_master WHERE type = 'table' AND (name NOT LIKE 'sqlite_%' AND name NOT LIKE '_cf_%')";
708+
709+
// Check if schema (tables) in both databases is equal
710+
const tablesFromMirror = (await db2.prepare(selectSchemaSQL).all()).results;
711+
const tablesFromOriginal = (await db.prepare(selectSchemaSQL).all()).results;
712+
t.deepEqual(tablesFromMirror, tablesFromOriginal);
713+
714+
// Check if data in each table is equal
715+
// We will use a simple SELECT * FROM table ORDER BY id to ensure consistent ordering
716+
for (const table of tablesFromMirror) {
717+
const tableName = table.name as string;
718+
719+
// Escape and ORDER BY to ensure consistent ordering
720+
const selectTableSQL = `SELECT * FROM "${tableName.replace(/"/g, '""')}" ORDER BY id ASC`;
721+
722+
const originalData = (await db.prepare(selectTableSQL).all()).results;
723+
const mirrorData = (await db2.prepare(selectTableSQL).all()).results;
724+
725+
t.deepEqual(
726+
originalData,
727+
mirrorData,
728+
`Data mismatch in table: ${tableName}`
729+
);
730+
}
731+
}

packages/wrangler/src/__tests__/d1/export.test.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -59,14 +59,14 @@ describe("export", () => {
5959
const create_foo = "CREATE TABLE foo(id INTEGER PRIMARY KEY, value TEXT);";
6060
const create_bar = "CREATE TABLE bar(id INTEGER PRIMARY KEY, value TEXT);";
6161
const insert_foo = [
62-
"INSERT INTO foo VALUES(1,'xxx');",
63-
"INSERT INTO foo VALUES(2,'yyy');",
64-
"INSERT INTO foo VALUES(3,'zzz');",
62+
`INSERT INTO "foo" VALUES(1,'xxx');`,
63+
`INSERT INTO "foo" VALUES(2,'yyy');`,
64+
`INSERT INTO "foo" VALUES(3,'zzz');`,
6565
];
6666
const insert_bar = [
67-
"INSERT INTO bar VALUES(1,'aaa');",
68-
"INSERT INTO bar VALUES(2,'bbb');",
69-
"INSERT INTO bar VALUES(3,'ccc');",
67+
`INSERT INTO "bar" VALUES(1,'aaa');`,
68+
`INSERT INTO "bar" VALUES(2,'bbb');`,
69+
`INSERT INTO "bar" VALUES(3,'ccc');`,
7070
];
7171

7272
// Full export

0 commit comments

Comments
 (0)