From b295c103dd86c57c848397d32e8094edfa8502aa Mon Sep 17 00:00:00 2001 From: Jeremy Lempereur Date: Mon, 4 Sep 2023 18:20:45 +0200 Subject: [PATCH] Subscriptions: Correct v1.28.x regression allowing panic via un-named subscription operation (#3738) ### Subscriptions: Correct v1.28.x regression allowing panic via un-named subscription operation Correct a regression that was introduced in Router v1.28.0 which made a Router **panic** possible when the following _three_ conditions are _all_ met: 1. When sending an un-named (i.e., "anonymous") `subscription` operation (e.g., `subscription { ... }`); **and**; 2. The Router has a `subscription` type defined in the Supergraph schema; **and** 3. Have subscriptions enabled (they are disabled by default) in the Router's YAML configuration, either by setting `enabled: true` _or_ by setting a `mode` within the `subscriptions` object (as seen in [the subscriptions documentation] --- .changesets/fix_dragonfly_ship_win_folder.md | 9 +++++++++ apollo-router/src/context/mod.rs | 7 ------- apollo-router/src/services/subgraph_service.rs | 8 +++++++- 3 files changed, 16 insertions(+), 8 deletions(-) create mode 100644 .changesets/fix_dragonfly_ship_win_folder.md diff --git a/.changesets/fix_dragonfly_ship_win_folder.md b/.changesets/fix_dragonfly_ship_win_folder.md new file mode 100644 index 0000000000..892df67579 --- /dev/null +++ b/.changesets/fix_dragonfly_ship_win_folder.md @@ -0,0 +1,9 @@ +### Subscriptions: Correct v1.28.x regression allowing panic via un-named subscription operation + +Correct a regression that was introduced in Router v1.28.0 which made a Router **panic** possible when the following _three_ conditions are _all_ met: + +1. When sending an un-named (i.e., "anonymous") `subscription` operation (e.g., `subscription { ... }`); **and**; +2. The Router has a `subscription` type defined in the Supergraph schema; **and** +3. Have subscriptions enabled (they are disabled by default) in the Router's YAML configuration, either by setting `enabled: true` _or_ by setting a `mode` within the `subscriptions` object (as seen in [the subscriptions documentation](https://www.apollographql.com/docs/router/executing-operations/subscription-support/#router-setup). + +By [@o0Ignition0o](https://github.com/o0Ignition0o) in https://github.com/apollographql/router/pull/3738 diff --git a/apollo-router/src/context/mod.rs b/apollo-router/src/context/mod.rs index 07042e08a8..83bdb2bc80 100644 --- a/apollo-router/src/context/mod.rs +++ b/apollo-router/src/context/mod.rs @@ -70,13 +70,6 @@ impl Context { } impl Context { - pub(crate) fn operation_name(&self) -> Option { - // This method should be removed once we have a proper way to get the operation name. - self.entries - .get(OPERATION_NAME) - .map(|v| v.value().as_str().unwrap().to_string()) - } - /// Returns true if the context contains a value for the specified key. pub fn contains_key(&self, key: K) -> bool where diff --git a/apollo-router/src/services/subgraph_service.rs b/apollo-router/src/services/subgraph_service.rs index e137177a7c..c99e482786 100644 --- a/apollo-router/src/services/subgraph_service.rs +++ b/apollo-router/src/services/subgraph_service.rs @@ -431,6 +431,13 @@ async fn call_websocket( subgraph_cfg: &WebSocketConfiguration, subscription_hash: String, ) -> Result { + let operation_name = request + .subgraph_request + .body() + .operation_name + .clone() + .unwrap_or_default(); + let SubgraphRequest { subgraph_request, subscription_stream, @@ -445,7 +452,6 @@ async fn call_websocket( let (handle, created) = notify .create_or_subscribe(subscription_hash.clone(), false) .await?; - let operation_name = context.operation_name().unwrap_or_default(); tracing::info!( monotonic_counter.apollo.router.operations.subscriptions = 1u64, subscriptions.mode = %"passthrough",