Skip to content

Commit a991029

Browse files
kamilkisielakamilkisiela
authored
fix(executor): fix escaped strings in subgraph responses (#436)
1 parent 9fb74b5 commit a991029

File tree

3 files changed

+106
-33
lines changed

3 files changed

+106
-33
lines changed

‎lib/executor/src/execution/rewrites.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ impl RewriteApplier for ValueSetter {
9191
path: &'a [FetchNodePathSegment],
9292
) {
9393
if path.is_empty() {
94-
*data = Value::String(self.set_value_to.as_str());
94+
*data = Value::String(self.set_value_to.as_str().into());
9595
return;
9696
}
9797

‎lib/executor/src/introspection/resolve.rs‎

Lines changed: 29 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
use std::borrow::Cow;
12
use std::collections::HashMap;
23

34
use graphql_parser::query::Value as QueryValue;
@@ -53,15 +54,16 @@ fn is_deprecated_enum<'exec, 'schema: 'exec>(enum_val: &'exec EnumValue<'schema,
5354

5455
fn kind_to_str<'exec, 'schema: 'exec>(
5556
type_def: &'exec TypeDefinition<'schema, String>,
56-
) -> &'exec str {
57-
match type_def {
57+
) -> Cow<'exec, str> {
58+
(match type_def {
5859
TypeDefinition::Scalar(_) => "SCALAR",
5960
TypeDefinition::Object(_) => "OBJECT",
6061
TypeDefinition::Interface(_) => "INTERFACE",
6162
TypeDefinition::Union(_) => "UNION",
6263
TypeDefinition::Enum(_) => "ENUM",
6364
TypeDefinition::InputObject(_) => "INPUT_OBJECT",
64-
}
65+
})
66+
.into()
6567
}
6668

6769
fn resolve_input_value<'exec, 'schema: 'exec>(
@@ -83,14 +85,14 @@ fn resolve_input_value_selections<'exec, 'schema: 'exec>(
8385
for item in selection_items {
8486
if let SelectionItem::Field(field) = item {
8587
let value = match field.name.as_str() {
86-
"name" => Value::String(&iv.name),
88+
"name" => Value::String(iv.name.as_str().into()),
8789
"description" => iv
8890
.description
8991
.as_ref()
90-
.map_or(Value::Null, |s| Value::String(s)),
92+
.map_or(Value::Null, |s| Value::String(s.into())),
9193
"type" => resolve_type(&iv.value_type, &field.selections, ctx),
9294
"defaultValue" => Value::Null, // TODO: support default values
93-
"__typename" => Value::String("__InputValue"),
95+
"__typename" => Value::String("__InputValue".into()),
9496
_ => Value::Null,
9597
};
9698
iv_data.push((field.selection_identifier(), value));
@@ -124,11 +126,11 @@ fn resolve_field_selections<'exec, 'schema: 'exec>(
124126
for item in selection_items {
125127
if let SelectionItem::Field(field) = item {
126128
let value = match field.name.as_str() {
127-
"name" => Value::String(&f.name),
129+
"name" => Value::String(f.name.as_str().into()),
128130
"description" => f
129131
.description
130132
.as_ref()
131-
.map_or(Value::Null, |s| Value::String(s)),
133+
.map_or(Value::Null, |s| Value::String(s.into())),
132134
"args" => {
133135
let args: Vec<_> = f
134136
.arguments
@@ -139,10 +141,9 @@ fn resolve_field_selections<'exec, 'schema: 'exec>(
139141
}
140142
"type" => resolve_type(&f.field_type, &field.selections, ctx),
141143
"isDeprecated" => Value::Bool(is_deprecated(&f.directives)),
142-
"deprecationReason" => {
143-
get_deprecation_reason(&f.directives).map_or(Value::Null, Value::String)
144-
}
145-
"__typename" => Value::String("__Field"),
144+
"deprecationReason" => get_deprecation_reason(&f.directives)
145+
.map_or(Value::Null, |s| Value::String(s.into())),
146+
"__typename" => Value::String("__Field".into()),
146147
_ => Value::Null,
147148
};
148149
field_data.push((field.selection_identifier(), value));
@@ -174,16 +175,15 @@ fn resolve_enum_value_selections<'exec, 'schema: 'exec>(
174175
for item in selection_items {
175176
if let SelectionItem::Field(field) = item {
176177
let value = match field.name.as_str() {
177-
"name" => Value::String(&ev.name),
178+
"name" => Value::String(ev.name.as_str().into()),
178179
"description" => ev
179180
.description
180181
.as_ref()
181-
.map_or(Value::Null, |s| Value::String(s)),
182+
.map_or(Value::Null, |s| Value::String(s.into())),
182183
"isDeprecated" => Value::Bool(is_deprecated_enum(ev)),
183-
"deprecationReason" => {
184-
get_deprecation_reason(&ev.directives).map_or(Value::Null, Value::String)
185-
}
186-
"__typename" => Value::String("__EnumValue"),
184+
"deprecationReason" => get_deprecation_reason(&ev.directives)
185+
.map_or(Value::Null, |s| Value::String(s.into())),
186+
"__typename" => Value::String("__EnumValue".into()),
187187
_ => Value::Null,
188188
};
189189
ev_data.push((field.selection_identifier(), value));
@@ -227,7 +227,7 @@ fn resolve_type_definition_selections<'exec, 'schema: 'exec>(
227227
TypeDefinition::Enum(e) => Some(&e.name),
228228
TypeDefinition::InputObject(io) => Some(&io.name),
229229
}
230-
.map(|s| Value::String(s))
230+
.map(|s| Value::String(s.into()))
231231
.unwrap_or(Value::Null),
232232
"description" => match type_def {
233233
TypeDefinition::Scalar(s) => s.description.as_ref(),
@@ -237,7 +237,7 @@ fn resolve_type_definition_selections<'exec, 'schema: 'exec>(
237237
TypeDefinition::Enum(e) => e.description.as_ref(),
238238
TypeDefinition::InputObject(io) => io.description.as_ref(),
239239
}
240-
.map_or(Value::Null, |s| Value::String(s)),
240+
.map_or(Value::Null, |s| Value::String(s.into())),
241241
"fields" => {
242242
let fields = match type_def {
243243
TypeDefinition::Object(o) => Some(&o.fields),
@@ -338,7 +338,7 @@ fn resolve_type_definition_selections<'exec, 'schema: 'exec>(
338338
_ => Value::Null,
339339
},
340340
"ofType" => Value::Null,
341-
"__typename" => Value::String("__Type"),
341+
"__typename" => Value::String("__Type".into()),
342342
_ => Value::Null,
343343
};
344344
type_data.push((field.selection_identifier(), value));
@@ -373,10 +373,10 @@ fn resolve_wrapper_type_selections<'exec, 'schema: 'exec>(
373373
for item in selection_items {
374374
if let SelectionItem::Field(field) = item {
375375
let value = match field.name.as_str() {
376-
"kind" => Value::String(kind),
376+
"kind" => Value::String(kind.into()),
377377
"name" => Value::Null,
378378
"ofType" => resolve_type(inner_type, &field.selections, ctx),
379-
"__typename" => Value::String("__Type"),
379+
"__typename" => Value::String("__Type".into()),
380380
_ => Value::Null,
381381
};
382382
type_data.push((field.selection_identifier(), value));
@@ -426,16 +426,16 @@ fn resolve_directive_selections<'exec, 'schema: 'exec>(
426426
for item in selection_items {
427427
if let SelectionItem::Field(field) = item {
428428
let value = match field.name.as_str() {
429-
"name" => Value::String(&d.name),
429+
"name" => Value::String(d.name.as_str().into()),
430430
"description" => d
431431
.description
432432
.as_ref()
433-
.map_or(Value::Null, |s| Value::String(s)),
433+
.map_or(Value::Null, |s| Value::String(s.into())),
434434
"locations" => {
435435
let locs: Vec<_> = d
436436
.locations
437437
.iter()
438-
.map(|l| Value::String(l.as_str()))
438+
.map(|l| Value::String(l.as_str().into()))
439439
.collect();
440440
Value::Array(locs)
441441
}
@@ -448,7 +448,7 @@ fn resolve_directive_selections<'exec, 'schema: 'exec>(
448448
Value::Array(args)
449449
}
450450
"isRepeatable" => Value::Bool(d.repeatable),
451-
"__typename" => Value::String("__Directive"),
451+
"__typename" => Value::String("__Directive".into()),
452452
_ => Value::Null,
453453
};
454454
directive_data.push((field.selection_identifier(), value));
@@ -526,7 +526,7 @@ fn resolve_schema_selections<'exec, 'schema: 'exec>(
526526
.collect();
527527
Value::Array(directives)
528528
}
529-
"__typename" => Value::String("__Schema"),
529+
"__typename" => Value::String("__Schema".into()),
530530
_ => Value::Null,
531531
};
532532
schema_data.push((inner_field.selection_identifier(), value));
@@ -590,7 +590,7 @@ fn resolve_root_introspection_selections<'exec, 'schema: 'exec>(
590590
Value::Null
591591
}
592592
}
593-
"__typename" => Value::String(root_type_name),
593+
"__typename" => Value::String(root_type_name.into()),
594594
_ => Value::Null,
595595
};
596596
data.push((field.selection_identifier(), value));

‎lib/executor/src/response/value.rs‎

Lines changed: 76 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ use serde::{
66
};
77
use sonic_rs::{JsonNumberTrait, ValueRef};
88
use std::{
9+
borrow::Cow,
910
fmt::Display,
1011
hash::{Hash, Hasher},
1112
};
@@ -20,7 +21,7 @@ pub enum Value<'a> {
2021
I64(i64),
2122
U64(u64),
2223
Bool(bool),
23-
String(&'a str),
24+
String(Cow<'a, str>),
2425
Array(Vec<Value<'a>>),
2526
Object(Vec<(&'a str, Value<'a>)>),
2627
}
@@ -139,7 +140,7 @@ impl<'a> Value<'a> {
139140
match json {
140141
ValueRef::Null => Value::Null,
141142
ValueRef::Bool(b) => Value::Bool(b),
142-
ValueRef::String(s) => Value::String(s),
143+
ValueRef::String(s) => Value::String(s.into()),
143144
ValueRef::Number(num) => {
144145
if let Some(num) = num.as_f64() {
145146
return Value::F64(num);
@@ -295,7 +296,21 @@ impl<'de> Visitor<'de> for ValueVisitor<'de> {
295296
where
296297
E: de::Error,
297298
{
298-
Ok(Value::String(value))
299+
Ok(Value::String(value.into()))
300+
}
301+
302+
fn visit_str<E>(self, v: &str) -> Result<Self::Value, E>
303+
where
304+
E: de::Error,
305+
{
306+
Ok(Value::String(v.to_owned().into()))
307+
}
308+
309+
fn visit_string<E>(self, value: String) -> Result<Self::Value, E>
310+
where
311+
E: de::Error,
312+
{
313+
Ok(Value::String(value.into()))
299314
}
300315

301316
fn visit_unit<E>(self) -> Result<Self::Value, E> {
@@ -358,3 +373,61 @@ impl serde::Serialize for Value<'_> {
358373
}
359374
}
360375
}
376+
377+
#[cfg(test)]
378+
mod tests {
379+
use super::Value;
380+
use serde::Deserialize;
381+
use std::borrow::Cow;
382+
383+
#[test]
384+
fn deserializes_escaped_string_as_owned() {
385+
let bytes = br#"{"message": "hello\nworld"}"#;
386+
let mut deserializer = sonic_rs::Deserializer::from_slice(bytes);
387+
388+
let value = Value::deserialize(&mut deserializer).unwrap();
389+
390+
let obj = match value {
391+
Value::Object(obj) => obj,
392+
_ => panic!("Expected Value::Object"),
393+
};
394+
395+
let message_value = &obj.iter().find(|(k, _)| *k == "message").unwrap().1;
396+
397+
match message_value {
398+
Value::String(value) => {
399+
assert_eq!(value, "hello\nworld");
400+
assert!(
401+
matches!(value, Cow::Owned(_)),
402+
"Expected Cow::Owned for escaped string"
403+
);
404+
}
405+
_ => panic!("Expected Value::String"),
406+
}
407+
}
408+
409+
#[test]
410+
fn deserializes_simple_string_as_borrowed() {
411+
let bytes = br#"{"message": "hello world"}"#;
412+
let mut deserializer = sonic_rs::Deserializer::from_slice(bytes);
413+
let value = Value::deserialize(&mut deserializer).unwrap();
414+
415+
let obj = match value {
416+
Value::Object(obj) => obj,
417+
_ => panic!("Expected Value::Object"),
418+
};
419+
420+
let message_value = &obj.iter().find(|(k, _)| *k == "message").unwrap().1;
421+
422+
match message_value {
423+
Value::String(value) => {
424+
assert_eq!(value, "hello world");
425+
assert!(
426+
matches!(value, Cow::Borrowed(_)),
427+
"Expected Cow::Borrowed for simple string"
428+
);
429+
}
430+
_ => panic!("Expected Value::String"),
431+
}
432+
}
433+
}

0 commit comments

Comments
 (0)