Skip to content

Commit 26b0ef6

Browse files
authored
fix: kv storage double encoding (#342)
* fix: kv storage double encoding * test: add regression
1 parent b270f10 commit 26b0ef6

File tree

1 file changed

+58
-2
lines changed

1 file changed

+58
-2
lines changed

backend/src/commands/blocks.rs

Lines changed: 58 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,9 +72,13 @@ impl LocalValueProvider for KvBlockLocalValueProvider {
7272
.await
7373
.map_err(|_| Box::new(std::io::Error::other("Failed to open KV database")))?;
7474
let key = format!("block.{block_id}.{property_name}");
75-
// KV stores JSON objects; serialize to string for the runtime to parse
75+
// KV stores JSON objects; extract string values directly, serialize others
7676
let value: Option<serde_json::Value> = kv::get(&db, &key).await?;
77-
Ok(value.map(|v| v.to_string()))
77+
match value {
78+
Some(serde_json::Value::String(s)) => Ok(Some(s)),
79+
Some(v) => Ok(Some(v.to_string())),
80+
None => Ok(None),
81+
}
7882
}
7983
}
8084

@@ -794,3 +798,55 @@ async fn execute_single_block(
794798
// Execute the block
795799
block.execute(context).await
796800
}
801+
802+
#[cfg(test)]
803+
mod tests {
804+
/// Regression test for the JSON string extraction bug (commit ad2a0dd4).
805+
///
806+
/// When retrieving values from the KV store, we must extract the inner string
807+
/// from `serde_json::Value::String` directly, NOT use `.to_string()` which
808+
/// JSON-encodes the value again (adding extra quotes).
809+
///
810+
/// Example of the bug:
811+
/// - KV stores: "/Users/test/path" (as JSON string)
812+
/// - Correct: extract inner string → "/Users/test/path"
813+
/// - Bug: v.to_string() → "\"/Users/test/path\"" (double-encoded)
814+
#[test]
815+
fn test_json_string_extraction_does_not_double_encode() {
816+
let json_string = serde_json::Value::String("/Users/test/path".to_string());
817+
818+
// This is the correct extraction (what our code does)
819+
let result = match json_string {
820+
serde_json::Value::String(s) => Some(s),
821+
v => Some(v.to_string()),
822+
};
823+
824+
assert_eq!(result, Some("/Users/test/path".to_string()));
825+
// Verify it doesn't have extra quotes that would break path resolution
826+
let value = result.unwrap();
827+
assert!(
828+
!value.starts_with('"'),
829+
"Path should not start with quote: {value}"
830+
);
831+
assert!(
832+
!value.ends_with('"'),
833+
"Path should not end with quote: {value}"
834+
);
835+
}
836+
837+
/// Documents why we can't use `.to_string()` directly on Value::String.
838+
#[test]
839+
fn test_to_string_on_json_string_adds_quotes() {
840+
let json_string = serde_json::Value::String("/Users/test/path".to_string());
841+
842+
// This is what the buggy code did - produces JSON output with quotes
843+
let wrong_result = json_string.to_string();
844+
845+
// to_string() on a Value::String produces JSON, which includes quotes
846+
assert_eq!(wrong_result, "\"/Users/test/path\"");
847+
assert!(
848+
wrong_result.starts_with('"'),
849+
"to_string() adds leading quote"
850+
);
851+
}
852+
}

0 commit comments

Comments
 (0)