Skip to content

Commit 087b0cb

Browse files
authored
Merge pull request #75 from powersync-ja/fix-key-encoding
Add helper function to fix JS subkey encoding
2 parents 32384f7 + 5456d1c commit 087b0cb

File tree

7 files changed

+305
-75
lines changed

7 files changed

+305
-75
lines changed

crates/core/src/error.rs

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
1-
use alloc::string::{String, ToString};
1+
use alloc::{
2+
format,
3+
string::{String, ToString},
4+
};
25
use core::error::Error;
3-
use sqlite_nostd::{sqlite3, Connection, ResultCode};
6+
use sqlite_nostd::{context, sqlite3, Connection, Context, ResultCode};
47

58
#[derive(Debug)]
69
pub struct SQLiteError(pub ResultCode, pub Option<String>);
@@ -11,6 +14,24 @@ impl core::fmt::Display for SQLiteError {
1114
}
1215
}
1316

17+
impl SQLiteError {
18+
pub fn apply_to_ctx(self, description: &str, ctx: *mut context) {
19+
let SQLiteError(code, message) = self;
20+
21+
if message.is_some() {
22+
ctx.result_error(&format!("{:} {:}", description, message.unwrap()));
23+
} else {
24+
let error = ctx.db_handle().errmsg().unwrap();
25+
if error == "not an error" {
26+
ctx.result_error(&format!("{:}", description));
27+
} else {
28+
ctx.result_error(&format!("{:} {:}", description, error));
29+
}
30+
}
31+
ctx.result_error_code(code);
32+
}
33+
}
34+
1435
impl Error for SQLiteError {}
1536

1637
pub trait PSResult<T> {

crates/core/src/fix035.rs

Lines changed: 0 additions & 47 deletions
This file was deleted.

crates/core/src/fix_data.rs

Lines changed: 200 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,200 @@
1+
use core::ffi::c_int;
2+
3+
use alloc::format;
4+
use alloc::string::String;
5+
6+
use crate::create_sqlite_optional_text_fn;
7+
use crate::error::{PSResult, SQLiteError};
8+
use sqlite_nostd::{self as sqlite, ColumnType, Value};
9+
use sqlite_nostd::{Connection, Context, ResultCode};
10+
11+
use crate::ext::SafeManagedStmt;
12+
use crate::util::quote_identifier;
13+
14+
// Apply a data migration to fix any existing data affected by the issue
15+
// fixed in v0.3.5.
16+
//
17+
// The issue was that the `ps_updated_rows` table was not being populated
18+
// with remove operations in some cases. This causes the rows to be removed
19+
// from ps_oplog, but not from the ps_data__tables, resulting in dangling rows.
20+
//
21+
// The fix here is to find these dangling rows, and add them to ps_updated_rows.
22+
// The next time the sync_local operation is run, these rows will be removed.
23+
pub fn apply_v035_fix(db: *mut sqlite::sqlite3) -> Result<i64, SQLiteError> {
24+
// language=SQLite
25+
let statement = db
26+
.prepare_v2("SELECT name, powersync_external_table_name(name) FROM sqlite_master WHERE type='table' AND name GLOB 'ps_data__*'")
27+
.into_db_result(db)?;
28+
29+
while statement.step()? == ResultCode::ROW {
30+
let full_name = statement.column_text(0)?;
31+
let short_name = statement.column_text(1)?;
32+
let quoted = quote_identifier(full_name);
33+
34+
// language=SQLite
35+
let statement = db.prepare_v2(&format!(
36+
"
37+
INSERT OR IGNORE INTO ps_updated_rows(row_type, row_id)
38+
SELECT ?1, id FROM {}
39+
WHERE NOT EXISTS (
40+
SELECT 1 FROM ps_oplog
41+
WHERE row_type = ?1 AND row_id = {}.id
42+
);",
43+
quoted, quoted
44+
))?;
45+
statement.bind_text(1, short_name, sqlite::Destructor::STATIC)?;
46+
47+
statement.exec()?;
48+
}
49+
50+
Ok(1)
51+
}
52+
53+
/// Older versions of the JavaScript SDK for PowerSync used to encode the subkey in oplog data
54+
/// entries as JSON.
55+
///
56+
/// It wasn't supposed to do that, since the keys are regular strings already. To make databases
57+
/// created with those SDKs compatible with other SDKs or the sync client implemented in the core
58+
/// extensions, a migration is necessary. Since this migration is only relevant for the JS SDK, it
59+
/// is mostly implemented there. However, the helper function to remove the key encoding is
60+
/// implemented here because user-defined functions are expensive on JavaScript.
61+
fn remove_duplicate_key_encoding(key: &str) -> Option<String> {
62+
// Acceptable format: <type>/<id>/<subkey>
63+
// Inacceptable format: <type>/<id>/"<subkey>"
64+
// This is a bit of a tricky conversion because both type and id can contain slashes and quotes.
65+
// However, the subkey is either a UUID value or a `<table>/UUID` value - so we know it can't
66+
// end in a quote unless the improper encoding was used.
67+
if !key.ends_with('"') {
68+
return None;
69+
}
70+
71+
// Since the subkey is JSON-encoded, find the start quote by going backwards.
72+
let mut chars = key.char_indices();
73+
chars.next_back()?; // Skip the quote ending the string
74+
75+
enum FoundStartingQuote {
76+
HasQuote { index: usize },
77+
HasBackslachThenQuote { quote_index: usize },
78+
}
79+
let mut state: Option<FoundStartingQuote> = None;
80+
let found_starting_quote = loop {
81+
if let Some((i, char)) = chars.next_back() {
82+
state = match state {
83+
Some(FoundStartingQuote::HasQuote { index }) => {
84+
if char == '\\' {
85+
// We've seen a \" pattern, not the start of the string
86+
Some(FoundStartingQuote::HasBackslachThenQuote { quote_index: index })
87+
} else {
88+
break Some(index);
89+
}
90+
}
91+
Some(FoundStartingQuote::HasBackslachThenQuote { quote_index }) => {
92+
if char == '\\' {
93+
// \\" pattern, the quote is unescaped
94+
break Some(quote_index);
95+
} else {
96+
None
97+
}
98+
}
99+
None => {
100+
if char == '"' {
101+
Some(FoundStartingQuote::HasQuote { index: i })
102+
} else {
103+
None
104+
}
105+
}
106+
}
107+
} else {
108+
break None;
109+
}
110+
}?;
111+
112+
let before_json = &key[..found_starting_quote];
113+
let mut result: String = serde_json::from_str(&key[found_starting_quote..]).ok()?;
114+
115+
result.insert_str(0, before_json);
116+
Some(result)
117+
}
118+
119+
fn powersync_remove_duplicate_key_encoding_impl(
120+
ctx: *mut sqlite::context,
121+
args: &[*mut sqlite::value],
122+
) -> Result<Option<String>, SQLiteError> {
123+
let arg = args.get(0).ok_or(ResultCode::MISUSE)?;
124+
125+
if arg.value_type() != ColumnType::Text {
126+
return Err(ResultCode::MISMATCH.into());
127+
}
128+
129+
return Ok(remove_duplicate_key_encoding(arg.text()));
130+
}
131+
132+
create_sqlite_optional_text_fn!(
133+
powersync_remote_duplicate_key_encoding,
134+
powersync_remove_duplicate_key_encoding_impl,
135+
"powersync_remote_duplicate_key_encoding"
136+
);
137+
138+
pub fn register(db: *mut sqlite::sqlite3) -> Result<(), ResultCode> {
139+
db.create_function_v2(
140+
"powersync_remote_duplicate_key_encoding",
141+
1,
142+
sqlite::UTF8 | sqlite::DETERMINISTIC,
143+
None,
144+
Some(powersync_remote_duplicate_key_encoding),
145+
None,
146+
None,
147+
None,
148+
)?;
149+
Ok(())
150+
}
151+
152+
#[cfg(test)]
153+
mod test {
154+
use core::assert_matches::assert_matches;
155+
156+
use super::remove_duplicate_key_encoding;
157+
158+
fn assert_unaffected(source: &str) {
159+
assert_matches!(remove_duplicate_key_encoding(source), None);
160+
}
161+
162+
#[test]
163+
fn does_not_change_unaffected_keys() {
164+
assert_unaffected("object_type/object_id/subkey");
165+
assert_unaffected("object_type/object_id/null");
166+
167+
// Object type and ID could technically contain quotes and forward slashes
168+
assert_unaffected(r#""object"/"type"/subkey"#);
169+
assert_unaffected("object\"/type/object\"/id/subkey");
170+
171+
// Invalid key, but we shouldn't crash
172+
assert_unaffected("\"key\"");
173+
}
174+
175+
#[test]
176+
fn removes_quotes() {
177+
assert_eq!(
178+
remove_duplicate_key_encoding("foo/bar/\"baz\"").unwrap(),
179+
"foo/bar/baz",
180+
);
181+
182+
assert_eq!(
183+
remove_duplicate_key_encoding(r#"foo/bar/"nested/subkey""#).unwrap(),
184+
"foo/bar/nested/subkey"
185+
);
186+
187+
assert_eq!(
188+
remove_duplicate_key_encoding(r#"foo/bar/"escaped\"key""#).unwrap(),
189+
"foo/bar/escaped\"key"
190+
);
191+
assert_eq!(
192+
remove_duplicate_key_encoding(r#"foo/bar/"escaped\\key""#).unwrap(),
193+
"foo/bar/escaped\\key"
194+
);
195+
assert_eq!(
196+
remove_duplicate_key_encoding(r#"foo/bar/"/\\"subkey""#).unwrap(),
197+
"foo/bar/\"/\\\\subkey"
198+
);
199+
}
200+
}

crates/core/src/lib.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ mod crud_vtab;
1818
mod diff;
1919
mod error;
2020
mod ext;
21-
mod fix035;
21+
mod fix_data;
2222
mod json_merge;
2323
mod kv;
2424
mod macros;
@@ -57,6 +57,7 @@ fn init_extension(db: *mut sqlite::sqlite3) -> Result<(), ResultCode> {
5757
crate::views::register(db)?;
5858
crate::uuid::register(db)?;
5959
crate::diff::register(db)?;
60+
crate::fix_data::register(db)?;
6061
crate::json_merge::register(db)?;
6162
crate::view_admin::register(db)?;
6263
crate::checkpoint::register(db)?;

crates/core/src/macros.rs

Lines changed: 2 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -11,18 +11,7 @@ macro_rules! create_sqlite_text_fn {
1111
let result = $fn_impl_name(ctx, args);
1212

1313
if let Err(err) = result {
14-
let SQLiteError(code, message) = SQLiteError::from(err);
15-
if message.is_some() {
16-
ctx.result_error(&format!("{:} {:}", $description, message.unwrap()));
17-
} else {
18-
let error = ctx.db_handle().errmsg().unwrap();
19-
if error == "not an error" {
20-
ctx.result_error(&format!("{:}", $description));
21-
} else {
22-
ctx.result_error(&format!("{:} {:}", $description, error));
23-
}
24-
}
25-
ctx.result_error_code(code);
14+
SQLiteError::from(err).apply_to_ctx($description, ctx);
2615
} else if let Ok(r) = result {
2716
ctx.result_text_transient(&r);
2817
}
@@ -43,18 +32,7 @@ macro_rules! create_sqlite_optional_text_fn {
4332
let result = $fn_impl_name(ctx, args);
4433

4534
if let Err(err) = result {
46-
let SQLiteError(code, message) = SQLiteError::from(err);
47-
if message.is_some() {
48-
ctx.result_error(&format!("{:} {:}", $description, message.unwrap()));
49-
} else {
50-
let error = ctx.db_handle().errmsg().unwrap();
51-
if error == "not an error" {
52-
ctx.result_error(&format!("{:}", $description));
53-
} else {
54-
ctx.result_error(&format!("{:} {:}", $description, error));
55-
}
56-
}
57-
ctx.result_error_code(code);
35+
SQLiteError::from(err).apply_to_ctx($description, ctx);
5836
} else if let Ok(r) = result {
5937
if let Some(s) = r {
6038
ctx.result_text_transient(&s);

crates/core/src/migrations.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ use sqlite_nostd::{Connection, Context};
1010

1111
use crate::bucket_priority::BucketPriority;
1212
use crate::error::{PSResult, SQLiteError};
13-
use crate::fix035::apply_v035_fix;
13+
use crate::fix_data::apply_v035_fix;
1414

1515
pub const LATEST_VERSION: i32 = 9;
1616

0 commit comments

Comments
 (0)