From 5b0f608bd4ff613bdcc1a99e0c1a13f7735f0070 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9e=20Kooi?= Date: Wed, 13 Sep 2023 16:53:08 +0200 Subject: [PATCH 1/3] Preserve large numbers in query transformations In GraphQL source text, you can write an integer outside the int32 range if a float is expected. apollo-rs parses these numbers as `hir::Value::Int` because at that time, the expected type of the value is not known, especially because it's possible to parse queries without knowledge of the schema. In the defer label transformation, any apollo-rs `hir::Value::Int` was treated as an i32. This would cause an error if it's one such large integer. The fully proper way to convert `hir::Value` to another type (ie. encoder or serde_json, as is done elsewhere) would be to match on the actual GraphQL type expected by the schema. But the simpler thing to do is to just use a Float if the value is too big for an Int. This will round-trip correctly. --- apollo-router/src/query_planner/labeler.rs | 15 +++++++++++++++ ...abeler__tests__large_float_written_as_int.snap | 8 ++++++++ apollo-router/src/spec/query/transform.rs | 7 ++++--- 3 files changed, 27 insertions(+), 3 deletions(-) create mode 100644 apollo-router/src/query_planner/snapshots/apollo_router__query_planner__labeler__tests__large_float_written_as_int.snap diff --git a/apollo-router/src/query_planner/labeler.rs b/apollo-router/src/query_planner/labeler.rs index 7bbb095eb7..3ecdadc93f 100644 --- a/apollo-router/src/query_planner/labeler.rs +++ b/apollo-router/src/query_planner/labeler.rs @@ -116,3 +116,18 @@ pub(crate) fn directive( Ok(encoder_directive) } + +#[cfg(test)] +mod tests { + use super::add_defer_labels; + use apollo_compiler::ApolloCompiler; + + #[test] + fn large_float_written_as_int() { + let mut compiler = ApolloCompiler::new(); + compiler.add_type_system("type Query { field(id: Float): String! }", "schema.graphql"); + let file_id = compiler.add_executable(r#"{ field(id: 1234567890123) }"#, "query.graphql"); + let result = add_defer_labels(file_id, &compiler).unwrap(); + insta::assert_snapshot!(result); + } +} diff --git a/apollo-router/src/query_planner/snapshots/apollo_router__query_planner__labeler__tests__large_float_written_as_int.snap b/apollo-router/src/query_planner/snapshots/apollo_router__query_planner__labeler__tests__large_float_written_as_int.snap new file mode 100644 index 0000000000..8be08a97cc --- /dev/null +++ b/apollo-router/src/query_planner/snapshots/apollo_router__query_planner__labeler__tests__large_float_written_as_int.snap @@ -0,0 +1,8 @@ +--- +source: apollo-router/src/query_planner/labeler.rs +expression: result +--- +query { + field(id: 1234567890123) +} + diff --git a/apollo-router/src/spec/query/transform.rs b/apollo-router/src/spec/query/transform.rs index 76bfe0f9be..2a5571fc37 100644 --- a/apollo-router/src/spec/query/transform.rs +++ b/apollo-router/src/spec/query/transform.rs @@ -369,9 +369,10 @@ pub(crate) fn ty(hir: &hir::Type) -> apollo_encoder::Type_ { pub(crate) fn value(hir: &hir::Value) -> Result { Ok(match hir { hir::Value::Variable(val) => apollo_encoder::Value::Variable(val.name().into()), - hir::Value::Int { value, .. } => { - apollo_encoder::Value::Int(value.to_i32_checked().ok_or("Int value overflows i32")?) - } + hir::Value::Int { value, .. } => value + .to_i32_checked() + .map(apollo_encoder::Value::Int) + .unwrap_or_else(|| apollo_encoder::Value::Float(value.get())), hir::Value::Float { value, .. } => apollo_encoder::Value::Float(value.get()), hir::Value::String { value, .. } => apollo_encoder::Value::String(value.clone()), hir::Value::Boolean { value, .. } => apollo_encoder::Value::Boolean(*value), From b5daff5dadd39762a32069c634ed0b4fdb3e115e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9e=20Kooi?= Date: Wed, 13 Sep 2023 16:59:47 +0200 Subject: [PATCH 2/3] One day i will remember to run xtask lint before creating a PR --- apollo-router/src/query_planner/labeler.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/apollo-router/src/query_planner/labeler.rs b/apollo-router/src/query_planner/labeler.rs index 3ecdadc93f..45eac5215c 100644 --- a/apollo-router/src/query_planner/labeler.rs +++ b/apollo-router/src/query_planner/labeler.rs @@ -119,9 +119,10 @@ pub(crate) fn directive( #[cfg(test)] mod tests { - use super::add_defer_labels; use apollo_compiler::ApolloCompiler; + use super::add_defer_labels; + #[test] fn large_float_written_as_int() { let mut compiler = ApolloCompiler::new(); From ca954a2ef79d14aa7b7b3ec3be2f5ebe3641eabd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9e=20Kooi?= Date: Wed, 13 Sep 2023 17:12:50 +0200 Subject: [PATCH 3/3] add changeset --- .changesets/fix_renee_fix_large_ints.md | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) create mode 100644 .changesets/fix_renee_fix_large_ints.md diff --git a/.changesets/fix_renee_fix_large_ints.md b/.changesets/fix_renee_fix_large_ints.md new file mode 100644 index 0000000000..756249f92c --- /dev/null +++ b/.changesets/fix_renee_fix_large_ints.md @@ -0,0 +1,22 @@ +### Fix error response on large numbers in query transformations ([PR #3820](https://github.com/apollographql/router/pull/3820)) + +This bug caused the router to reject operations where a large hardcoded integer was used as input for a Float field: + +```graphql +# Schema +type Query { + field(argument: Float): Int! +} +# Operation +{ + field(argument: 123456789123) +} +``` + +Now the number is correctly interpreted as a Float. +This bug only affected hardcoded numbers, not numbers provided through variables. + + +--- + +By [@goto-bus-stop](https://github.com/goto-bus-stop) in https://github.com/apollographql/router/pull/3820 \ No newline at end of file