From dc425562b45efd53954363087ee8a2da93ddd410 Mon Sep 17 00:00:00 2001 From: Geoffroy Couprie Date: Thu, 25 Aug 2022 15:22:10 +0200 Subject: [PATCH 1/9] queries with defer should have the Accept: multipart/mixed header Since deferred responses can come back as multipart responses, we must check that the client supports that content type. This will allow older clients to show a meaningful error message instead of a parsing error if the `@defer` directive is used but they don't support it --- .../src/services/supergraph_service.rs | 26 +++++++++++ apollo-router/tests/integration_tests.rs | 43 +++++++++++++++++++ 2 files changed, 69 insertions(+) diff --git a/apollo-router/src/services/supergraph_service.rs b/apollo-router/src/services/supergraph_service.rs index 8fdc848717..da23a51c3c 100644 --- a/apollo-router/src/services/supergraph_service.rs +++ b/apollo-router/src/services/supergraph_service.rs @@ -10,6 +10,7 @@ use futures::stream::BoxStream; use futures::stream::StreamExt; use futures::TryFutureExt; use http::StatusCode; +use http::header::ACCEPT; use indexmap::IndexMap; use lazy_static::__Deref; use opentelemetry::trace::SpanKind; @@ -152,6 +153,30 @@ where } QueryPlannerContent::Plan { query, plan } => { let can_be_deferred = plan.root.contains_defer(); + if can_be_deferred && !req.originating_request.headers().get_all(ACCEPT).iter().filter_map(|value| value.to_str().ok()).flat_map(|value| value.split(',') + .map(|a| a.trim())).any(|value| { + println!("testing value {:?}", value); + value == "multipart/mixed"}) { + tracing::error!("tried to send a defer request without accept: multipart/mixed"); + let mut resp = http::Response::new( + once(ready( + graphql::Response::builder() + .errors(vec![crate::error::Error::builder() + .message(String::from("the router received a query with the @defer directive but the client does not accept multipart/mixed HTTP responses")) + .build()]) + .build(), + )) + .boxed(), + ); + *resp.status_mut() = StatusCode::BAD_REQUEST; + + Ok(SupergraphResponse { + response: resp, + context, + }) + + + } else{ if let Some(err) = query.validate_variables(body, &schema).err() { let mut res = SupergraphResponse::new_from_graphql_response(err, context); @@ -250,6 +275,7 @@ where }) } } + } } } .or_else(|error: BoxError| async move { diff --git a/apollo-router/tests/integration_tests.rs b/apollo-router/tests/integration_tests.rs index 547c736d3f..2f22efc180 100644 --- a/apollo-router/tests/integration_tests.rs +++ b/apollo-router/tests/integration_tests.rs @@ -13,6 +13,7 @@ use apollo_router::plugin::Plugin; use apollo_router::plugin::PluginInit; use apollo_router::services::subgraph; use apollo_router::services::supergraph; +use http::header::ACCEPT; use http::Method; use http::StatusCode; use maplit::hashmap; @@ -547,6 +548,7 @@ async fn defer_path() { } }"#, ) + .header(ACCEPT, "multipart/mixed") .build() .expect("expecting valid request"); @@ -589,6 +591,7 @@ async fn defer_path_in_array() { } }"#, ) + .header(ACCEPT, "multipart/mixed") .build() .expect("expecting valid request"); @@ -603,6 +606,46 @@ async fn defer_path_in_array() { insta::assert_json_snapshot!(second); } +#[tokio::test(flavor = "multi_thread")] +async fn defer_query_without_accept() { + let config = serde_json::json!({ + "server": { + "experimental_defer_support": true + }, + "plugins": { + "experimental.include_subgraph_errors": { + "all": true + } + } + }); + let request = supergraph::Request::fake_builder() + .query( + r#"{ + me { + reviews { + id + author { + id + ... @defer(label: "author name") { + name + } + } + } + } + }"#, + ) + .header(ACCEPT, "application/json") + .build() + .expect("expecting valid request"); + + let (router, _) = setup_router_and_registry(config).await; + + let mut stream = router.oneshot(request).await.unwrap(); + + let first = stream.next_response().await.unwrap(); + insta::assert_json_snapshot!(first); +} + async fn query_node( request: &supergraph::Request, ) -> Result { From 7a225071d6dac153b466cf110f87e01f40515dae Mon Sep 17 00:00:00 2001 From: Geoffroy Couprie Date: Thu, 25 Aug 2022 15:29:09 +0200 Subject: [PATCH 2/9] fmt --- .../src/services/supergraph_service.rs | 191 +++++++++--------- 1 file changed, 94 insertions(+), 97 deletions(-) diff --git a/apollo-router/src/services/supergraph_service.rs b/apollo-router/src/services/supergraph_service.rs index da23a51c3c..93b03fd155 100644 --- a/apollo-router/src/services/supergraph_service.rs +++ b/apollo-router/src/services/supergraph_service.rs @@ -9,8 +9,8 @@ use futures::stream::once; use futures::stream::BoxStream; use futures::stream::StreamExt; use futures::TryFutureExt; -use http::StatusCode; use http::header::ACCEPT; +use http::StatusCode; use indexmap::IndexMap; use lazy_static::__Deref; use opentelemetry::trace::SpanKind; @@ -153,6 +153,7 @@ where } QueryPlannerContent::Plan { query, plan } => { let can_be_deferred = plan.root.contains_defer(); + if can_be_deferred && !req.originating_request.headers().get_all(ACCEPT).iter().filter_map(|value| value.to_str().ok()).flat_map(|value| value.split(',') .map(|a| a.trim())).any(|value| { println!("testing value {:?}", value); @@ -169,113 +170,109 @@ where .boxed(), ); *resp.status_mut() = StatusCode::BAD_REQUEST; - Ok(SupergraphResponse { response: resp, context, }) - - - } else{ - - if let Some(err) = query.validate_variables(body, &schema).err() { - let mut res = SupergraphResponse::new_from_graphql_response(err, context); - *res.response.status_mut() = StatusCode::BAD_REQUEST; - Ok(res) } else { - let operation_name = body.operation_name.clone(); - - let ExecutionResponse { response, context } = execution - .oneshot( - ExecutionRequest::builder() - .originating_request(req.originating_request) - .query_plan(plan) - .context(context) - .build(), - ) - .await?; - - let (parts, response_stream) = response.into_parts(); - - let stream = response_stream - .map(move |mut response: Response| { - tracing::debug_span!("format_response").in_scope(|| { - query.format_response( - &mut response, - operation_name.as_deref(), - variables.clone(), - schema.api_schema(), + if let Some(err) = query.validate_variables(body, &schema).err() { + let mut res = SupergraphResponse::new_from_graphql_response(err, context); + *res.response.status_mut() = StatusCode::BAD_REQUEST; + Ok(res) + } else { + let operation_name = body.operation_name.clone(); + + let ExecutionResponse { response, context } = execution + .oneshot( + ExecutionRequest::builder() + .originating_request(req.originating_request) + .query_plan(plan) + .context(context) + .build(), ) + .await?; + + let (parts, response_stream) = response.into_parts(); + + let stream = response_stream + .map(move |mut response: Response| { + tracing::debug_span!("format_response").in_scope(|| { + query.format_response( + &mut response, + operation_name.as_deref(), + variables.clone(), + schema.api_schema(), + ) + }); + + match (response.path.as_ref(), response.data.as_ref()) { + (None, _) | (_, None) => { + if can_be_deferred { + response.has_next = Some(true); + } + + response + } + // if the deferred response specified a path, we must extract the + //values matched by that path and create a separate response for + //each of them. + // While { "data": { "a": { "b": 1 } } } and { "data": { "b": 1 }, "path: ["a"] } + // would merge in the same ways, some clients will generate code + // that checks the specific type of the deferred response at that + // path, instead of starting from the root object, so to support + // this, we extract the value at that path. + // In particular, that means that a deferred fragment in an object + // under an array would generate one response par array element + (Some(response_path), Some(response_data)) => { + let mut sub_responses = Vec::new(); + response_data.select_values_and_paths( + response_path, + |path, value| { + sub_responses + .push((path.clone(), value.clone())); + }, + ); + + Response::builder() + .has_next(true) + .incremental( + sub_responses + .into_iter() + .map(move |(path, data)| { + IncrementalResponse::builder() + .and_label( + response.label.clone(), + ) + .data(data) + .path(path) + .errors(response.errors.clone()) + .extensions( + response.extensions.clone(), + ) + .build() + }) + .collect(), + ) + .build() + } + } }); - match (response.path.as_ref(), response.data.as_ref()) { - (None, _) | (_, None) => { + Ok(SupergraphResponse { + context, + response: http::Response::from_parts( + parts, if can_be_deferred { - response.has_next = Some(true); - } - - response - } - // if the deferred response specified a path, we must extract the - //values matched by that path and create a separate response for - //each of them. - // While { "data": { "a": { "b": 1 } } } and { "data": { "b": 1 }, "path: ["a"] } - // would merge in the same ways, some clients will generate code - // that checks the specific type of the deferred response at that - // path, instead of starting from the root object, so to support - // this, we extract the value at that path. - // In particular, that means that a deferred fragment in an object - // under an array would generate one response par array element - (Some(response_path), Some(response_data)) => { - let mut sub_responses = Vec::new(); - response_data.select_values_and_paths( - response_path, - |path, value| { - sub_responses - .push((path.clone(), value.clone())); - }, - ); - - Response::builder() - .has_next(true) - .incremental( - sub_responses - .into_iter() - .map(move |(path, data)| { - IncrementalResponse::builder() - .and_label( - response.label.clone(), - ) - .data(data) - .path(path) - .errors(response.errors.clone()) - .extensions( - response.extensions.clone(), - ) - .build() - }) - .collect(), - ) - .build() - } - } - }); - - Ok(SupergraphResponse { - context, - response: http::Response::from_parts( - parts, - if can_be_deferred { - stream.chain(once(ready(Response::builder().has_next(false).build()))).left_stream() - } else { - stream.right_stream() - }.in_current_span() - .boxed(), - ) - }) + stream.chain(once(ready(Response::builder().has_next(false).build()))).left_stream() + } else { + stream.right_stream() + }.in_current_span() + .boxed(), + ) + }) + } } } - } } } .or_else(|error: BoxError| async move { From 8ceae986e297718e370956fa5b1685ba5de795e2 Mon Sep 17 00:00:00 2001 From: Geoffroy Couprie Date: Thu, 25 Aug 2022 15:36:37 +0200 Subject: [PATCH 3/9] fmt --- .../src/services/supergraph_service.rs | 187 +++++++++--------- 1 file changed, 92 insertions(+), 95 deletions(-) diff --git a/apollo-router/src/services/supergraph_service.rs b/apollo-router/src/services/supergraph_service.rs index 93b03fd155..db0fc692e6 100644 --- a/apollo-router/src/services/supergraph_service.rs +++ b/apollo-router/src/services/supergraph_service.rs @@ -155,9 +155,8 @@ where let can_be_deferred = plan.root.contains_defer(); if can_be_deferred && !req.originating_request.headers().get_all(ACCEPT).iter().filter_map(|value| value.to_str().ok()).flat_map(|value| value.split(',') - .map(|a| a.trim())).any(|value| { - println!("testing value {:?}", value); - value == "multipart/mixed"}) { + .map(|a| a.trim())).any(|value| + value == "multipart/mixed") { tracing::error!("tried to send a defer request without accept: multipart/mixed"); let mut resp = http::Response::new( once(ready( @@ -174,103 +173,101 @@ where response: resp, context, }) + } else if let Some(err) = query.validate_variables(body, &schema).err() { + let mut res = SupergraphResponse::new_from_graphql_response(err, context); + *res.response.status_mut() = StatusCode::BAD_REQUEST; + Ok(res) } else { - if let Some(err) = query.validate_variables(body, &schema).err() { - let mut res = SupergraphResponse::new_from_graphql_response(err, context); - *res.response.status_mut() = StatusCode::BAD_REQUEST; - Ok(res) - } else { - let operation_name = body.operation_name.clone(); - - let ExecutionResponse { response, context } = execution - .oneshot( - ExecutionRequest::builder() - .originating_request(req.originating_request) - .query_plan(plan) - .context(context) - .build(), + let operation_name = body.operation_name.clone(); + + let ExecutionResponse { response, context } = execution + .oneshot( + ExecutionRequest::builder() + .originating_request(req.originating_request) + .query_plan(plan) + .context(context) + .build(), + ) + .await?; + + let (parts, response_stream) = response.into_parts(); + + let stream = response_stream + .map(move |mut response: Response| { + tracing::debug_span!("format_response").in_scope(|| { + query.format_response( + &mut response, + operation_name.as_deref(), + variables.clone(), + schema.api_schema(), ) - .await?; - - let (parts, response_stream) = response.into_parts(); - - let stream = response_stream - .map(move |mut response: Response| { - tracing::debug_span!("format_response").in_scope(|| { - query.format_response( - &mut response, - operation_name.as_deref(), - variables.clone(), - schema.api_schema(), - ) - }); - - match (response.path.as_ref(), response.data.as_ref()) { - (None, _) | (_, None) => { - if can_be_deferred { - response.has_next = Some(true); - } - - response - } - // if the deferred response specified a path, we must extract the - //values matched by that path and create a separate response for - //each of them. - // While { "data": { "a": { "b": 1 } } } and { "data": { "b": 1 }, "path: ["a"] } - // would merge in the same ways, some clients will generate code - // that checks the specific type of the deferred response at that - // path, instead of starting from the root object, so to support - // this, we extract the value at that path. - // In particular, that means that a deferred fragment in an object - // under an array would generate one response par array element - (Some(response_path), Some(response_data)) => { - let mut sub_responses = Vec::new(); - response_data.select_values_and_paths( - response_path, - |path, value| { - sub_responses - .push((path.clone(), value.clone())); - }, - ); - - Response::builder() - .has_next(true) - .incremental( - sub_responses - .into_iter() - .map(move |(path, data)| { - IncrementalResponse::builder() - .and_label( - response.label.clone(), - ) - .data(data) - .path(path) - .errors(response.errors.clone()) - .extensions( - response.extensions.clone(), - ) - .build() - }) - .collect(), - ) - .build() - } - } }); - Ok(SupergraphResponse { - context, - response: http::Response::from_parts( - parts, + match (response.path.as_ref(), response.data.as_ref()) { + (None, _) | (_, None) => { if can_be_deferred { - stream.chain(once(ready(Response::builder().has_next(false).build()))).left_stream() - } else { - stream.right_stream() - }.in_current_span() - .boxed(), - ) - }) - } + response.has_next = Some(true); + } + + response + } + // if the deferred response specified a path, we must extract the + //values matched by that path and create a separate response for + //each of them. + // While { "data": { "a": { "b": 1 } } } and { "data": { "b": 1 }, "path: ["a"] } + // would merge in the same ways, some clients will generate code + // that checks the specific type of the deferred response at that + // path, instead of starting from the root object, so to support + // this, we extract the value at that path. + // In particular, that means that a deferred fragment in an object + // under an array would generate one response par array element + (Some(response_path), Some(response_data)) => { + let mut sub_responses = Vec::new(); + response_data.select_values_and_paths( + response_path, + |path, value| { + sub_responses + .push((path.clone(), value.clone())); + }, + ); + + Response::builder() + .has_next(true) + .incremental( + sub_responses + .into_iter() + .map(move |(path, data)| { + IncrementalResponse::builder() + .and_label( + response.label.clone(), + ) + .data(data) + .path(path) + .errors(response.errors.clone()) + .extensions( + response.extensions.clone(), + ) + .build() + }) + .collect(), + ) + .build() + } + } + }); + + Ok(SupergraphResponse { + context, + response: http::Response::from_parts( + parts, + if can_be_deferred { + stream.chain(once(ready(Response::builder().has_next(false).build()))).left_stream() + } else { + stream.right_stream() + }.in_current_span() + .boxed(), + ) + }) } } } From cd544e77753458afd2f597a4b698be7f0f8a727f Mon Sep 17 00:00:00 2001 From: Geoffroy Couprie Date: Thu, 25 Aug 2022 15:39:09 +0200 Subject: [PATCH 4/9] changelog --- NEXT_CHANGELOG.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/NEXT_CHANGELOG.md b/NEXT_CHANGELOG.md index fb92fb5243..bb2ab5e104 100644 --- a/NEXT_CHANGELOG.md +++ b/NEXT_CHANGELOG.md @@ -93,6 +93,14 @@ next chunk to see the delimiter. By [@Geal](https://github.com/Geal) in https://github.com/apollographql/router/pull/1596 +### Queries with defer must have the Accept: multipart/mixed header ([PR #1610](https://github.com/apollographql/router/issues/1610)) + +Since deferred responses can come back as multipart responses, we must check that the client supports that content type. +This will allow older clients to show a meaningful error message instead of a parsing error if the `@defer` directive is +used but they don't support it. + +By [@Geal](https://github.com/Geal) in https://github.com/apollographql/router/pull/1610 + ## 🛠 Maintenance From 365322681207b8f6764bbd524d02aa10050fc5c5 Mon Sep 17 00:00:00 2001 From: Geoffroy Couprie Date: Thu, 25 Aug 2022 17:48:24 +0200 Subject: [PATCH 5/9] use the semicolon as separator --- apollo-router/src/services/supergraph_service.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apollo-router/src/services/supergraph_service.rs b/apollo-router/src/services/supergraph_service.rs index db0fc692e6..81beebac00 100644 --- a/apollo-router/src/services/supergraph_service.rs +++ b/apollo-router/src/services/supergraph_service.rs @@ -154,7 +154,7 @@ where QueryPlannerContent::Plan { query, plan } => { let can_be_deferred = plan.root.contains_defer(); - if can_be_deferred && !req.originating_request.headers().get_all(ACCEPT).iter().filter_map(|value| value.to_str().ok()).flat_map(|value| value.split(',') + if can_be_deferred && !req.originating_request.headers().get_all(ACCEPT).iter().filter_map(|value| value.to_str().ok()).flat_map(|value| value.split(';') .map(|a| a.trim())).any(|value| value == "multipart/mixed") { tracing::error!("tried to send a defer request without accept: multipart/mixed"); From 8ee99308379ed7cb5d57ff08ed7c37d327395513 Mon Sep 17 00:00:00 2001 From: Geoffroy Couprie Date: Thu, 25 Aug 2022 18:04:42 +0200 Subject: [PATCH 6/9] put back the comma --- apollo-router/src/services/supergraph_service.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apollo-router/src/services/supergraph_service.rs b/apollo-router/src/services/supergraph_service.rs index 81beebac00..db0fc692e6 100644 --- a/apollo-router/src/services/supergraph_service.rs +++ b/apollo-router/src/services/supergraph_service.rs @@ -154,7 +154,7 @@ where QueryPlannerContent::Plan { query, plan } => { let can_be_deferred = plan.root.contains_defer(); - if can_be_deferred && !req.originating_request.headers().get_all(ACCEPT).iter().filter_map(|value| value.to_str().ok()).flat_map(|value| value.split(';') + if can_be_deferred && !req.originating_request.headers().get_all(ACCEPT).iter().filter_map(|value| value.to_str().ok()).flat_map(|value| value.split(',') .map(|a| a.trim())).any(|value| value == "multipart/mixed") { tracing::error!("tried to send a defer request without accept: multipart/mixed"); From 934cf4170330444d0a1da2aa72b7c02fb71e1285 Mon Sep 17 00:00:00 2001 From: Geoffroy Couprie Date: Thu, 25 Aug 2022 18:05:59 +0200 Subject: [PATCH 7/9] missing snapshot --- ...egration_tests__defer_query_without_accept.snap | 14 ++++++++++++++ 1 file changed, 14 insertions(+) create mode 100644 apollo-router/tests/snapshots/integration_tests__defer_query_without_accept.snap diff --git a/apollo-router/tests/snapshots/integration_tests__defer_query_without_accept.snap b/apollo-router/tests/snapshots/integration_tests__defer_query_without_accept.snap new file mode 100644 index 0000000000..e7d0daf9ad --- /dev/null +++ b/apollo-router/tests/snapshots/integration_tests__defer_query_without_accept.snap @@ -0,0 +1,14 @@ +--- +source: apollo-router/tests/integration_tests.rs +assertion_line: 646 +expression: first +--- +{ + "errors": [ + { + "message": "the router received a query with the @defer directive but the client does not accept multipart/mixed HTTP responses", + "locations": [], + "path": null + } + ] +} From c613ca8504379b182d55343e8dca807c4ab30817 Mon Sep 17 00:00:00 2001 From: Geoffroy Couprie Date: Fri, 26 Aug 2022 11:37:26 +0200 Subject: [PATCH 8/9] use the mediatype crate to correctly parse the Accept header --- Cargo.lock | 7 ++++ apollo-router/Cargo.toml | 1 + apollo-router/src/axum_http_server_factory.rs | 35 ++++++++++--------- .../src/services/supergraph_service.rs | 24 +++++++++++-- 4 files changed, 47 insertions(+), 20 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 8f0be2b916..0b002fb5c9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -177,6 +177,7 @@ dependencies = [ "libc", "lru", "maplit", + "mediatype", "miette 5.3.0", "mime", "mockall", @@ -3016,6 +3017,12 @@ version = "0.7.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "490cc448043f947bae3cbee9c203358d62dbee0db12107a74be5c30ccfd09771" +[[package]] +name = "mediatype" +version = "0.19.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ac0db9ec602cd88b078efab731831a15461f90eb100c4a7da457d8cd6a917d2f" + [[package]] name = "memchr" version = "2.4.1" diff --git a/apollo-router/Cargo.toml b/apollo-router/Cargo.toml index 795c82caf7..8daddb2c02 100644 --- a/apollo-router/Cargo.toml +++ b/apollo-router/Cargo.toml @@ -167,6 +167,7 @@ url = { version = "2.2.2", features = ["serde"] } urlencoding = "2.1.0" yaml-rust = "0.4.5" pin-project-lite = "0.2.9" +mediatype = "0.19.7" [target.'cfg(macos)'.dependencies] uname = "0.1.1" diff --git a/apollo-router/src/axum_http_server_factory.rs b/apollo-router/src/axum_http_server_factory.rs index cb1ada2f91..3199592a75 100644 --- a/apollo-router/src/axum_http_server_factory.rs +++ b/apollo-router/src/axum_http_server_factory.rs @@ -33,6 +33,10 @@ use http::Request; use http::Uri; use hyper::server::conn::Http; use hyper::Body; +use mediatype::names::HTML; +use mediatype::names::TEXT; +use mediatype::MediaType; +use mediatype::MediaTypeList; use opentelemetry::global; use opentelemetry::trace::SpanKind; use opentelemetry::trace::TraceContextExt; @@ -424,13 +428,7 @@ async fn handle_get( http_request: Request, display_landing_page: bool, ) -> impl IntoResponse { - if http_request - .headers() - .get(&http::header::ACCEPT) - .map(prefers_html) - .unwrap_or_default() - && display_landing_page - { + if prefers_html(&http_request.headers()) && display_landing_page { return display_home_page().into_response(); } @@ -593,16 +591,19 @@ where } } -fn prefers_html(accept_header: &HeaderValue) -> bool { - accept_header - .to_str() - .map(|accept_str| { - accept_str - .split(',') - .map(|a| a.trim()) - .any(|a| a == "text/html") - }) - .unwrap_or_default() +fn prefers_html(headers: &HeaderMap) -> bool { + let text_html = MediaType::new(TEXT, HTML); + + headers.get_all(&http::header::ACCEPT).iter().any(|value| { + value + .to_str() + .map(|accept_str| { + let mut list = MediaTypeList::new(accept_str); + + list.any(|mime| mime.as_ref() == Ok(&text_html)) + }) + .unwrap_or(false) + }) } async fn decompress_request_body( diff --git a/apollo-router/src/services/supergraph_service.rs b/apollo-router/src/services/supergraph_service.rs index db0fc692e6..12de60dd35 100644 --- a/apollo-router/src/services/supergraph_service.rs +++ b/apollo-router/src/services/supergraph_service.rs @@ -10,9 +10,14 @@ use futures::stream::BoxStream; use futures::stream::StreamExt; use futures::TryFutureExt; use http::header::ACCEPT; +use http::HeaderMap; use http::StatusCode; use indexmap::IndexMap; use lazy_static::__Deref; +use mediatype::names::MIXED; +use mediatype::names::MULTIPART; +use mediatype::MediaType; +use mediatype::MediaTypeList; use opentelemetry::trace::SpanKind; use tower::util::BoxService; use tower::BoxError; @@ -154,9 +159,7 @@ where QueryPlannerContent::Plan { query, plan } => { let can_be_deferred = plan.root.contains_defer(); - if can_be_deferred && !req.originating_request.headers().get_all(ACCEPT).iter().filter_map(|value| value.to_str().ok()).flat_map(|value| value.split(',') - .map(|a| a.trim())).any(|value| - value == "multipart/mixed") { + if can_be_deferred && !accepts_multipart(req.originating_request.headers()) { tracing::error!("tried to send a defer request without accept: multipart/mixed"); let mut resp = http::Response::new( once(ready( @@ -302,6 +305,21 @@ where } } +fn accepts_multipart(headers: &HeaderMap) -> bool { + let multipart_mixed = MediaType::new(MULTIPART, MIXED); + + headers.get_all(ACCEPT).iter().any(|value| { + value + .to_str() + .map(|accept_str| { + let mut list = MediaTypeList::new(accept_str); + + list.any(|mime| mime.as_ref() == Ok(&multipart_mixed)) + }) + .unwrap_or(false) + }) +} + /// Builder which generates a plugin pipeline. /// /// This is at the heart of the delegation of responsibility model for the router. A schema, From 3df3d455eba85badd1a0c87fcf8f2d7617758652 Mon Sep 17 00:00:00 2001 From: Geoffroy Couprie Date: Fri, 26 Aug 2022 11:46:34 +0200 Subject: [PATCH 9/9] lint --- apollo-router/src/axum_http_server_factory.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apollo-router/src/axum_http_server_factory.rs b/apollo-router/src/axum_http_server_factory.rs index 9272ee1468..d1f1d3428c 100644 --- a/apollo-router/src/axum_http_server_factory.rs +++ b/apollo-router/src/axum_http_server_factory.rs @@ -434,7 +434,7 @@ async fn handle_get( http_request: Request, display_landing_page: bool, ) -> impl IntoResponse { - if prefers_html(&http_request.headers()) && display_landing_page { + if prefers_html(http_request.headers()) && display_landing_page { return display_home_page().into_response(); }