Skip to content

Commit

Permalink
Subscriptions: Correct v1.28.x regression allowing panic via un-named…
Browse files Browse the repository at this point in the history
… 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]
  • Loading branch information
o0Ignition0o authored and abernix committed Sep 4, 2023
1 parent 0672961 commit b295c10
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 8 deletions.
9 changes: 9 additions & 0 deletions .changesets/fix_dragonfly_ship_win_folder.md
Original file line number Diff line number Diff line change
@@ -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
7 changes: 0 additions & 7 deletions apollo-router/src/context/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,13 +70,6 @@ impl Context {
}

impl Context {
pub(crate) fn operation_name(&self) -> Option<String> {
// 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<K>(&self, key: K) -> bool
where
Expand Down
8 changes: 7 additions & 1 deletion apollo-router/src/services/subgraph_service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -431,6 +431,13 @@ async fn call_websocket(
subgraph_cfg: &WebSocketConfiguration,
subscription_hash: String,
) -> Result<SubgraphResponse, BoxError> {
let operation_name = request
.subgraph_request
.body()
.operation_name
.clone()
.unwrap_or_default();

let SubgraphRequest {
subgraph_request,
subscription_stream,
Expand All @@ -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",
Expand Down

0 comments on commit b295c10

Please # to comment.