Skip to content

Commit 1d951c8

Browse files
authored
Merge pull request #405 from stone-lyl/main
fix: Escape single quotes in JSON data for DuckDB storage
2 parents 0f7b36e + f040758 commit 1d951c8

File tree

4 files changed

+239
-21
lines changed

4 files changed

+239
-21
lines changed

packages/ds-ext/package.json

+1
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,7 @@
118118
"pretest": "yarn run compile-tests && yarn run build && yarn run lint",
119119
"lint": "eslint src",
120120
"test": "vscode-test",
121+
"test:unit": "yarn run -T vitest --passWithNoTests",
121122
"release": "release-it"
122123
},
123124
"release-it": {

packages/ds-ext/src/duckDBStorage.ts

+25-9
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { DiagramId, GetLinkItemsParams, ItemValue, LinkCount, LinkId, NodeId, ObserverStorage } from '@data-story/core';
22
import type { Database as DatabaseType } from 'duckdb-async';
33
import { createDataStoryDBPath } from './commands/createDataStoryDBPath';
4+
45
export class DuckDBStorage implements ObserverStorage {
56
private db: DatabaseType | null = null;
67
private insertSequence: bigint = BigInt(0);
@@ -70,7 +71,7 @@ export class DuckDBStorage implements ObserverStorage {
7071
if (!data || data.length === 0) {
7172
return undefined;
7273
}
73-
const result:ItemValue[] = data.map(row => JSON.parse(row.item));
74+
const result: ItemValue[] = data.map(row => JSON.parse(row.item));
7475
return result;
7576
}
7677

@@ -81,17 +82,32 @@ export class DuckDBStorage implements ObserverStorage {
8182
}
8283

8384
async appendLinkItems(linkId: LinkId, items: ItemValue[]): Promise<void> {
85+
if (items.length === 0) {
86+
return; // Nothing to insert
87+
}
88+
8489
const currentTime = new Date().toISOString();
85-
const values = items.map(item => {
86-
const data = JSON.stringify(item);
87-
return `('${linkId}', ${this.nextSequenceVal()}, '${data}', '${currentTime}', '${currentTime}')`;
88-
});
8990

90-
const sql = `INSERT INTO linkItems (linkId, sequenceNumber, item, createTime, updateTime) VALUES
91-
${values.join(', ')}`;
91+
// Create parameterized query with placeholders
92+
const placeholders = items.map(() => '(?, ?, ?, ?, ?)').join(', ');
93+
const sql = `INSERT INTO linkItems (linkId, sequenceNumber, item, createTime, updateTime)
94+
VALUES ${placeholders}`;
95+
96+
// Flatten the parameters into a single array
97+
const params: any[] = [];
98+
items.forEach(item => {
99+
const sequenceNumber = this.nextSequenceVal();
100+
params.push(
101+
linkId,
102+
sequenceNumber.toString(),
103+
JSON.stringify(item),
104+
currentTime, // createTime
105+
currentTime, // updateTime
106+
);
107+
});
92108

93-
// execute batch insert
94-
await this.db?.run(sql);
109+
// Execute parameterized query
110+
await this.db?.run(sql, ...params);
95111
}
96112

97113
async getNodeStatus(nodeId: NodeId): Promise<'BUSY' | 'COMPLETE' | undefined> {
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,201 @@
1+
/* eslint-disable @stylistic/quotes */
2+
import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest';
3+
import { DuckDBStorage } from '../duckDBStorage';
4+
import { LinkId, ItemValue } from '@data-story/core';
5+
import { Database } from 'duckdb-async';
6+
7+
// Mock only the createDataStoryDBPath function to use in-memory database
8+
vi.mock('../commands/createDataStoryDBPath', () => ({
9+
createDataStoryDBPath: vi.fn().mockReturnValue(':memory:'),
10+
}));
11+
12+
describe('DuckDBStorage', () => {
13+
let storage: DuckDBStorage;
14+
let db: Database;
15+
const mockDiagramId = 'test-diagram-id';
16+
const mockLinkId: LinkId = 'test-link-id';
17+
18+
beforeEach(async () => {
19+
// Create a new instance of DuckDBStorage
20+
storage = new DuckDBStorage(mockDiagramId);
21+
await storage.init();
22+
23+
// Reset the sequence counter
24+
storage.resetSequence();
25+
26+
// Get access to the actual database
27+
db = (storage as any).db;
28+
29+
// Verify the database was initialized properly
30+
expect(db).toBeDefined();
31+
});
32+
33+
afterEach(async () => {
34+
await storage.close();
35+
});
36+
37+
it('should properly escape single quotes in JSON data', async () => {
38+
// Create test items with single quotes that need escaping
39+
const testItems: ItemValue[] = [
40+
{ id: 1, name: 'Item with no quotes' },
41+
{ id: 2, name: 'Item with \'single\' quotes' },
42+
{ id: 3, name: `Item with ''multiple'' 'single' quotes` },
43+
{ id: 4, name: `\\nfo$oo$` },
44+
{ id: 5, name: `hello, "world"` },
45+
];
46+
47+
// Call the method under test
48+
await storage.appendLinkItems(mockLinkId, testItems);
49+
50+
// Query the database to verify the items were inserted correctly
51+
const result = await db.all('SELECT * FROM linkItems WHERE linkId = ? ORDER BY sequenceNumber ASC', mockLinkId);
52+
53+
// Verify the correct number of items were inserted
54+
expect(result.length).toBe(testItems.length);
55+
56+
// Verify each item was stored correctly
57+
result.forEach((row, index) => {
58+
const parsedItem = JSON.parse(row.item);
59+
expect(parsedItem.id).toBe(testItems[index].id);
60+
expect(parsedItem.name).toBe(testItems[index].name);
61+
expect(row.linkId).toBe(mockLinkId);
62+
63+
// Verify sequence numbers are sequential, but don't assume they start at 0
64+
expect(Number(row.sequenceNumber)).toBe(index);
65+
});
66+
});
67+
68+
it('should increment sequence numbers correctly', async () => {
69+
// Create test items
70+
const testItems: ItemValue[] = [
71+
{ id: 1, value: 'First item' },
72+
{ id: 2, value: 'Second item' },
73+
{ id: 3, value: 'Third item' },
74+
];
75+
76+
// Call the method under test
77+
await storage.appendLinkItems(mockLinkId, testItems);
78+
79+
// Query the database to verify sequence numbers
80+
const result = await db.all('SELECT sequenceNumber FROM linkItems WHERE linkId = ? ORDER BY sequenceNumber ASC', mockLinkId);
81+
82+
// Verify sequence numbers are incremented sequentially
83+
expect(result.length).toBe(3);
84+
85+
// Verify sequence numbers are sequential
86+
for (let i = 1; i < result.length; i++) {
87+
expect(Number(result[i].sequenceNumber)).toBe(Number(result[i - 1].sequenceNumber) + 1);
88+
}
89+
});
90+
91+
it('should handle empty items array', async () => {
92+
// Call with empty array
93+
await storage.appendLinkItems(mockLinkId, []);
94+
95+
// Query the database to verify no items were inserted
96+
const result = await db.all('SELECT * FROM linkItems WHERE linkId = ?', mockLinkId);
97+
98+
// Verify no items were inserted
99+
expect(result.length).toBe(0);
100+
});
101+
102+
it('should use the current time for timestamps', async () => {
103+
// Create a fixed timestamp for testing
104+
const mockTimestamp = '2023-01-01T00:00:00.000Z';
105+
const originalToISOString = Date.prototype.toISOString;
106+
Date.prototype.toISOString = vi.fn().mockReturnValue(mockTimestamp);
107+
108+
// Create a test item
109+
const testItems: ItemValue[] = [{ id: 1, value: 'Test item' }];
110+
111+
// Call the method under test
112+
await storage.appendLinkItems(mockLinkId, testItems);
113+
114+
// Query the database to verify timestamps
115+
const result = await db.all('SELECT createTime, updateTime FROM linkItems WHERE linkId = ?', mockLinkId);
116+
117+
// Verify timestamps are set correctly
118+
expect(result.length).toBe(1);
119+
expect(result[0].createTime.toISOString()).toBe(mockTimestamp);
120+
expect(result[0].updateTime.toISOString()).toBe(mockTimestamp);
121+
122+
// Restore the original toISOString method
123+
Date.prototype.toISOString = originalToISOString;
124+
});
125+
126+
it('should handle complex nested objects', async () => {
127+
// Create a complex nested object
128+
const complexItem: ItemValue = {
129+
id: 1,
130+
name: 'Complex item',
131+
nested: {
132+
array: [1, 2, 3],
133+
object: { key: 'value' },
134+
withQuotes: 'Item with \'quotes\'',
135+
},
136+
};
137+
138+
// Call the method under test
139+
await storage.appendLinkItems(mockLinkId, [complexItem]);
140+
141+
// Query the database to verify the complex object was stored correctly
142+
const result = await db.all('SELECT item FROM linkItems WHERE linkId = ?', mockLinkId);
143+
144+
// Verify the complex object was stored correctly
145+
expect(result.length).toBe(1);
146+
const parsedItem = JSON.parse(result[0].item);
147+
expect(parsedItem).toEqual(complexItem);
148+
expect(parsedItem.nested.array).toEqual([1, 2, 3]);
149+
expect(parsedItem.nested.object.key).toBe('value');
150+
expect(parsedItem.nested.withQuotes).toBe('Item with \'quotes\'');
151+
});
152+
153+
it('should properly handle SQL injection attempts', async () => {
154+
// Create test items with potential SQL injection payloads
155+
const sqlInjectionItems: ItemValue[] = [
156+
{
157+
id: 1,
158+
name: 'SQL Injection attempt',
159+
payload: '\'); DELETE FROM linkItems; --',
160+
},
161+
{
162+
id: 2,
163+
name: 'Another SQL Injection attempt',
164+
payload: '\' OR 1=1; --',
165+
},
166+
{
167+
id: 3,
168+
name: 'Complex SQL Injection',
169+
payload: '\'); DROP TABLE linkItems; INSERT INTO linkItems VALUES(\'hacked',
170+
},
171+
];
172+
173+
// Call the method under test
174+
await storage.appendLinkItems(mockLinkId, sqlInjectionItems);
175+
176+
// Query the database to verify the items were inserted correctly
177+
// and that no SQL injection occurred
178+
const result = await db.all('SELECT * FROM linkItems WHERE linkId = ? ORDER BY sequenceNumber ASC', mockLinkId);
179+
180+
// Verify the correct number of items were inserted
181+
expect(result.length).toBe(3);
182+
183+
// Verify each item was stored correctly with the SQL injection payload intact
184+
result.forEach((row, index) => {
185+
const parsedItem = JSON.parse(row.item);
186+
expect(parsedItem.id).toBe(sqlInjectionItems[index].id);
187+
expect(parsedItem.name).toBe(sqlInjectionItems[index].name);
188+
expect(parsedItem.payload).toBe(sqlInjectionItems[index].payload);
189+
expect(row.linkId).toBe(mockLinkId);
190+
});
191+
192+
// Verify that the table structure is still intact
193+
// If SQL injection had worked, this query might fail or return unexpected results
194+
const tableInfo = await db.all('PRAGMA table_info(linkItems)');
195+
expect(tableInfo.length).toBeGreaterThan(0);
196+
197+
// Verify we can still query the table normally
198+
const count = await db.all('SELECT COUNT(*) as count FROM linkItems');
199+
expect(Number(count[0].count)).toBe(3);
200+
});
201+
});
+12-12
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,15 @@
1-
import * as assert from 'assert';
1+
// import * as assert from 'assert';
22

3-
// You can import and use all API from the 'vscode' module
4-
// as well as import your extension to test it
5-
import * as vscode from 'vscode';
6-
// import * as myExtension from '../../extension';
3+
// // You can import and use all API from the 'vscode' module
4+
// // as well as import your extension to test it
5+
// import * as vscode from 'vscode';
6+
// // import * as myExtension from '../../extension';
77

8-
suite('Extension Test Suite', () => {
9-
vscode.window.showInformationMessage('Start all tests.');
8+
// suite('Extension Test Suite', () => {
9+
// vscode.window.showInformationMessage('Start all tests.');
1010

11-
test('Sample test', () => {
12-
assert.strictEqual(-1, [1, 2, 3].indexOf(5));
13-
assert.strictEqual(-1, [1, 2, 3].indexOf(0));
14-
});
15-
});
11+
// test('Sample test', () => {
12+
// assert.strictEqual(-1, [1, 2, 3].indexOf(5));
13+
// assert.strictEqual(-1, [1, 2, 3].indexOf(0));
14+
// });
15+
// });

0 commit comments

Comments
 (0)