Skip to content

Commit 397a47a

Browse files
nunogoncalves03nunogoncalves03
committed
fix(json): handle BLOB arguments correctly in JSON functions
1 parent 46740ee commit 397a47a

File tree

4 files changed

+221
-40
lines changed

4 files changed

+221
-40
lines changed

‎core/json/mod.rs‎

Lines changed: 103 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -49,10 +49,15 @@ pub fn get_json(json_value: &Value, indent: Option<&str>) -> crate::Result<Value
4949

5050
Ok(Value::Text(Text::json(json)))
5151
}
52-
Value::Blob(b) => {
53-
let jsonbin = Jsonb::new(b.len(), Some(b));
54-
jsonbin.element_type()?;
55-
Ok(Value::Text(Text::json(jsonbin.to_string())))
52+
Value::Blob(_) => {
53+
// Blobs can contain either text JSON or binary JSONB
54+
// Use convert_dbtype_to_jsonb to properly detect and parse both formats
55+
let json_val = convert_dbtype_to_jsonb(json_value, Conv::Strict)?;
56+
let json = match indent {
57+
Some(indent) => json_val.to_string_pretty(Some(indent))?,
58+
None => json_val.to_string(),
59+
};
60+
Ok(Value::Text(Text::json(json)))
5661
}
5762
Value::Null => Ok(Value::Null),
5863
_ => {
@@ -283,6 +288,10 @@ where
283288
let second = args.next().ok_or_else(|| {
284289
crate::LimboError::InternalError("args should have second element in loop".to_string())
285290
})?;
291+
let second_ref = second.as_value_ref();
292+
if matches!(second_ref, ValueRef::Blob(_)) {
293+
crate::bail_constraint_error!("JSON cannot hold BLOB values")
294+
}
286295
let value = convert_dbtype_to_jsonb(second, Conv::NotStrict)?;
287296
let mut op = SetOperation::new(value);
288297
if let Some(path) = path {
@@ -324,6 +333,10 @@ where
324333
let second = args.next().ok_or_else(|| {
325334
crate::LimboError::InternalError("args should have second element in loop".to_string())
326335
})?;
336+
let second_ref = second.as_value_ref();
337+
if matches!(second_ref, ValueRef::Blob(_)) {
338+
crate::bail_constraint_error!("JSON cannot hold BLOB values")
339+
}
327340
let value = convert_dbtype_to_jsonb(second, Conv::NotStrict)?;
328341
let mut op = SetOperation::new(value);
329342
if let Some(path) = path {
@@ -699,6 +712,10 @@ where
699712
"values should have second element in loop".to_string(),
700713
)
701714
})?;
715+
let second_ref = second.as_value_ref();
716+
if matches!(second_ref, ValueRef::Blob(_)) {
717+
crate::bail_constraint_error!("JSON cannot hold BLOB values")
718+
}
702719
let value = convert_dbtype_to_jsonb(second, Conv::NotStrict)?;
703720
json.append_jsonb_to_end(value.data());
704721
}
@@ -739,6 +756,10 @@ where
739756
"values should have second element in loop".to_string(),
740757
)
741758
})?;
759+
let second_ref = second.as_value_ref();
760+
if matches!(second_ref, ValueRef::Blob(_)) {
761+
crate::bail_constraint_error!("JSON cannot hold BLOB values")
762+
}
742763
let value = convert_dbtype_to_jsonb(second, Conv::NotStrict)?;
743764
json.append_jsonb_to_end(value.data());
744765
}
@@ -905,6 +926,54 @@ mod tests {
905926
}
906927
}
907928

929+
#[test]
930+
fn test_get_json_blob_text_json() {
931+
let true_blob = Value::Blob(b"true".to_vec());
932+
let result = get_json(&true_blob, None).unwrap();
933+
if let Value::Text(result_str) = result {
934+
assert_eq!(result_str.as_str(), "true");
935+
assert_eq!(result_str.subtype, TextSubtype::Json);
936+
} else {
937+
panic!("Expected Value::Text, got: {result:?}");
938+
}
939+
940+
let false_blob = Value::Blob(b"false".to_vec());
941+
let result = get_json(&false_blob, None).unwrap();
942+
if let Value::Text(result_str) = result {
943+
assert_eq!(result_str.as_str(), "false");
944+
assert_eq!(result_str.subtype, TextSubtype::Json);
945+
} else {
946+
panic!("Expected Value::Text, got: {result:?}");
947+
}
948+
949+
let null_blob = Value::Blob(b"null".to_vec());
950+
let result = get_json(&null_blob, None).unwrap();
951+
if let Value::Text(result_str) = result {
952+
assert_eq!(result_str.as_str(), "null");
953+
assert_eq!(result_str.subtype, TextSubtype::Json);
954+
} else {
955+
panic!("Expected Value::Text, got: {result:?}");
956+
}
957+
958+
let number_blob = Value::Blob(b"123".to_vec());
959+
let result = get_json(&number_blob, None).unwrap();
960+
if let Value::Text(result_str) = result {
961+
assert_eq!(result_str.as_str(), "123");
962+
assert_eq!(result_str.subtype, TextSubtype::Json);
963+
} else {
964+
panic!("Expected Value::Text, got: {result:?}");
965+
}
966+
967+
let string_blob = Value::Blob(b"\"hello\"".to_vec());
968+
let result = get_json(&string_blob, None).unwrap();
969+
if let Value::Text(result_str) = result {
970+
assert_eq!(result_str.as_str(), "\"hello\"");
971+
assert_eq!(result_str.subtype, TextSubtype::Json);
972+
} else {
973+
panic!("Expected Value::Text, got: {result:?}");
974+
}
975+
}
976+
908977
#[test]
909978
fn test_get_json_non_text() {
910979
let input = Value::Null;
@@ -958,6 +1027,36 @@ mod tests {
9581027
}
9591028
}
9601029

1030+
#[test]
1031+
fn test_json_object_blob_invalid() {
1032+
let key = Value::build_text("a");
1033+
let blob = Value::Blob("true".as_bytes().to_vec());
1034+
1035+
let input = [key, blob];
1036+
1037+
let result = json_object(&input);
1038+
1039+
match result {
1040+
Ok(_) => panic!("Expected error for blob value in json_object"),
1041+
Err(e) => assert!(e.to_string().contains("JSON cannot hold BLOB values")),
1042+
}
1043+
}
1044+
1045+
#[test]
1046+
fn test_json_set_blob_invalid() {
1047+
let json_cache = JsonCacheCell::new();
1048+
let blob = Value::Blob("test".as_bytes().to_vec());
1049+
let result = json_set(
1050+
&[Value::build_text("{}"), Value::build_text("$.field"), blob],
1051+
&json_cache,
1052+
);
1053+
1054+
match result {
1055+
Ok(_) => panic!("Expected error for blob value in json_set"),
1056+
Err(e) => assert!(e.to_string().contains("JSON cannot hold BLOB values")),
1057+
}
1058+
}
1059+
9611060
#[test]
9621061
fn test_json_array_length() {
9631062
let input = Value::build_text("[1,2,3,4]");

‎core/json/ops.rs‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,10 @@ where
145145
let second = args.next().ok_or_else(|| {
146146
crate::LimboError::InternalError("args should have second element in loop".to_string())
147147
})?;
148+
let second_ref = second.as_value_ref();
149+
if matches!(second_ref, ValueRef::Blob(_)) {
150+
crate::bail_constraint_error!("JSON cannot hold BLOB values")
151+
}
148152
let value = convert_dbtype_to_jsonb(&second, Conv::NotStrict)?;
149153
if let Some(path) = path {
150154
let mut op = ReplaceOperation::new(value);
@@ -186,6 +190,10 @@ where
186190
let second = args.next().ok_or_else(|| {
187191
crate::LimboError::InternalError("args should have second element in loop".to_string())
188192
})?;
193+
let second_ref = second.as_value_ref();
194+
if matches!(second_ref, ValueRef::Blob(_)) {
195+
crate::bail_constraint_error!("JSON cannot hold BLOB values")
196+
}
189197
let value = convert_dbtype_to_jsonb(&second, Conv::NotStrict)?;
190198
if let Some(path) = path {
191199
let mut op = ReplaceOperation::new(value);
@@ -228,6 +236,10 @@ where
228236
let second = args.next().ok_or_else(|| {
229237
crate::LimboError::InternalError("args should have second element in loop".to_string())
230238
})?;
239+
let second_ref = second.as_value_ref();
240+
if matches!(second_ref, ValueRef::Blob(_)) {
241+
crate::bail_constraint_error!("JSON cannot hold BLOB values")
242+
}
231243
let value = convert_dbtype_to_jsonb(&second, Conv::NotStrict)?;
232244
if let Some(path) = path {
233245
let mut op = InsertOperation::new(value);
@@ -270,6 +282,10 @@ where
270282
let second = args.next().ok_or_else(|| {
271283
crate::LimboError::InternalError("args should have second element in loop".to_string())
272284
})?;
285+
let second_ref = second.as_value_ref();
286+
if matches!(second_ref, ValueRef::Blob(_)) {
287+
crate::bail_constraint_error!("JSON cannot hold BLOB values")
288+
}
273289
let value = convert_dbtype_to_jsonb(&second, Conv::NotStrict)?;
274290
if let Some(path) = path {
275291
let mut op = InsertOperation::new(value);
@@ -462,6 +478,38 @@ mod tests {
462478
assert!(json_remove(&args, &json_cache).is_err());
463479
}
464480

481+
#[test]
482+
fn test_json_insert_blob_invalid() {
483+
let json_cache = JsonCacheCell::new();
484+
let blob = Value::Blob("test".as_bytes().to_vec());
485+
let args = [create_json("{}"), create_text("$.field"), blob];
486+
487+
let result = json_insert(&args, &json_cache);
488+
489+
match result {
490+
Ok(_) => panic!("Expected error for blob value in json_insert"),
491+
Err(e) => assert!(e.to_string().contains("JSON cannot hold BLOB values")),
492+
}
493+
}
494+
495+
#[test]
496+
fn test_json_replace_blob_invalid() {
497+
let json_cache = JsonCacheCell::new();
498+
let blob = Value::Blob("test".as_bytes().to_vec());
499+
let args = [
500+
create_json(r#"{"field":"value"}"#),
501+
create_text("$.field"),
502+
blob,
503+
];
504+
505+
let result = json_replace(&args, &json_cache);
506+
507+
match result {
508+
Ok(_) => panic!("Expected error for blob value in json_replace"),
509+
Err(e) => assert!(e.to_string().contains("JSON cannot hold BLOB values")),
510+
}
511+
}
512+
465513
#[test]
466514
fn test_json_remove_complex_case() {
467515
let args = [

‎core/vdbe/execute.rs‎

Lines changed: 18 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -4831,64 +4831,46 @@ pub fn op_function(
48314831
)?);
48324832
}
48334833
JsonFunc::JsonRemove => {
4834-
if let Ok(json) = json_remove(
4834+
let json = json_remove(
48354835
registers_to_ref_values(&state.registers[*start_reg..*start_reg + arg_count]),
48364836
&state.json_cache,
4837-
) {
4838-
state.registers[*dest] = Register::Value(json);
4839-
} else {
4840-
state.registers[*dest] = Register::Value(Value::Null);
4841-
}
4837+
)?;
4838+
state.registers[*dest] = Register::Value(json);
48424839
}
48434840
JsonFunc::JsonbRemove => {
4844-
if let Ok(json) = jsonb_remove(
4841+
let json = jsonb_remove(
48454842
registers_to_ref_values(&state.registers[*start_reg..*start_reg + arg_count]),
48464843
&state.json_cache,
4847-
) {
4848-
state.registers[*dest] = Register::Value(json);
4849-
} else {
4850-
state.registers[*dest] = Register::Value(Value::Null);
4851-
}
4844+
)?;
4845+
state.registers[*dest] = Register::Value(json);
48524846
}
48534847
JsonFunc::JsonReplace => {
4854-
if let Ok(json) = json_replace(
4848+
let json = json_replace(
48554849
registers_to_ref_values(&state.registers[*start_reg..*start_reg + arg_count]),
48564850
&state.json_cache,
4857-
) {
4858-
state.registers[*dest] = Register::Value(json);
4859-
} else {
4860-
state.registers[*dest] = Register::Value(Value::Null);
4861-
}
4851+
)?;
4852+
state.registers[*dest] = Register::Value(json);
48624853
}
48634854
JsonFunc::JsonbReplace => {
4864-
if let Ok(json) = jsonb_replace(
4855+
let json = jsonb_replace(
48654856
registers_to_ref_values(&state.registers[*start_reg..*start_reg + arg_count]),
48664857
&state.json_cache,
4867-
) {
4868-
state.registers[*dest] = Register::Value(json);
4869-
} else {
4870-
state.registers[*dest] = Register::Value(Value::Null);
4871-
}
4858+
)?;
4859+
state.registers[*dest] = Register::Value(json);
48724860
}
48734861
JsonFunc::JsonInsert => {
4874-
if let Ok(json) = json_insert(
4862+
let json = json_insert(
48754863
registers_to_ref_values(&state.registers[*start_reg..*start_reg + arg_count]),
48764864
&state.json_cache,
4877-
) {
4878-
state.registers[*dest] = Register::Value(json);
4879-
} else {
4880-
state.registers[*dest] = Register::Value(Value::Null);
4881-
}
4865+
)?;
4866+
state.registers[*dest] = Register::Value(json);
48824867
}
48834868
JsonFunc::JsonbInsert => {
4884-
if let Ok(json) = jsonb_insert(
4869+
let json = jsonb_insert(
48854870
registers_to_ref_values(&state.registers[*start_reg..*start_reg + arg_count]),
48864871
&state.json_cache,
4887-
) {
4888-
state.registers[*dest] = Register::Value(json);
4889-
} else {
4890-
state.registers[*dest] = Register::Value(Value::Null);
4891-
}
4872+
)?;
4873+
state.registers[*dest] = Register::Value(json);
48924874
}
48934875
JsonFunc::JsonPretty => {
48944876
let json_value = &state.registers[*start_reg];

‎testing/json.test‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,34 @@ do_execsql_test json5-multi-comment-pretty {
148148
"aaa": 123
149149
}}}
150150

151+
do_execsql_test json-blob-text-true {
152+
SELECT json(x'74727565');
153+
} {true}
154+
155+
do_execsql_test json-blob-text-false {
156+
SELECT json(x'66616c7365');
157+
} {false}
158+
159+
do_execsql_test json-blob-text-null {
160+
SELECT json(x'6e756c6c');
161+
} {null}
162+
163+
do_execsql_test json-blob-text-number {
164+
SELECT json(x'313233');
165+
} {123}
166+
167+
do_execsql_test json-blob-text-string {
168+
SELECT json(x'2268656c6c6f22');
169+
} {{"hello"}}
170+
171+
do_execsql_test json-blob-text-array {
172+
SELECT json(x'5b312c322c335d');
173+
} {{[1,2,3]}}
174+
175+
do_execsql_test json-blob-text-object {
176+
SELECT json(x'7b226b6579223a2276616c7565227d');
177+
} {{{"key":"value"}}}
178+
151179
do_execsql_test json-pretty-ident-1 {
152180
SELECT json_pretty('{x: 1}', '');
153181
} {{{
@@ -698,6 +726,30 @@ do_execsql_test json_object_empty {
698726
SELECT json_object();
699727
} {{{}}}
700728

729+
do_execsql_test_in_memory_error_content json_object_blob_error {
730+
CREATE TABLE t(a);
731+
INSERT INTO t VALUES (x'74727565'); -- ASCII "true";
732+
SELECT json_object('a', a) FROM t;
733+
} {JSON cannot hold BLOB values}
734+
735+
do_execsql_test_in_memory_error_content json_set_blob_error {
736+
CREATE TABLE t(a);
737+
INSERT INTO t VALUES (x'74727565'); -- ASCII "true";
738+
SELECT json_set('{}', '$.field', a) FROM t;
739+
} {JSON cannot hold BLOB values}
740+
741+
do_execsql_test_in_memory_error_content json_insert_blob_error {
742+
CREATE TABLE t(a);
743+
INSERT INTO t VALUES (x'74727565'); -- ASCII "true";
744+
SELECT json_insert('{}', '$.field', a) FROM t;
745+
} {JSON cannot hold BLOB values}
746+
747+
do_execsql_test_in_memory_error_content json_replace_blob_error {
748+
CREATE TABLE t(a);
749+
INSERT INTO t VALUES (x'74727565'); -- ASCII "true";
750+
SELECT json_replace('{"field":1}', '$.field', a) FROM t;
751+
} {JSON cannot hold BLOB values}
752+
701753
do_execsql_test json_object_json_array {
702754
SELECT json_object('ex',json('[52,3]'));
703755
} {{{"ex":[52,3]}}}

0 commit comments

Comments
 (0)