Restore HTTP payload size limit, make it configurable

.changesets/feat_limit_request_size.md

@@ -0,0 +1,21 @@
+### Restore HTTP payload size limit, make it configurable ([Issue #2000](https://github.com/apollographql/router/issues/2000))
+
+Early versions of Apollo Router used to rely on a part of the Axum web framework
+that imposed a 2 MB limit on the size of the HTTP request body.
+Version 1.7 changed to read the body directly, unintentionally removing this limit.
+
+The limit is now restored to help protect against unbounded memory usage, but is now configurable:
+
+```yaml
+preview_operation_limits:
+  experimental_http_max_request_bytes: 2000000 # Default value: 2 MB
+```
+
+This limit is checked while reading from the network, before JSON parsing.
+Both the GraphQL document and associated variables count toward it.
+
+Before increasing this limit significantly consider testing performance
+in an environment similar to your production, especially if some clients are untrusted.
+Many concurrent large requests could cause the Router to run out of memory.
+
+By [@SimonSapin](https://github.com/SimonSapin in https://github.com/apollographql/router/pull/3130

apollo-router/src/configuration/mod.rs

@@ -621,6 +621,10 @@ pub(crate) struct OperationLimits {
 
     /// Limit the number of tokens the GraphQL parser processes before aborting.
     pub(crate) parser_max_tokens: usize,
+
+    /// Limit the size of incoming HTTP requests read from the network,
+    /// to protect against running out of memory. Default: 2000000 (2 MB)
+    pub(crate) experimental_http_max_request_bytes: usize,
 }
 
 impl Default for OperationLimits {
@@ -632,12 +636,13 @@ impl Default for OperationLimits {
             max_root_fields: None,
             max_aliases: None,
             warn_only: false,
+            experimental_http_max_request_bytes: 2_000_000,
+            parser_max_tokens: 15_000,
 
             // This is `apollo-parser`’s default, which protects against stack overflow
             // but is still very high for "reasonable" queries.
             // https://docs.rs/apollo-parser/0.2.8/src/apollo_parser/parser/mod.rs.html#368
             parser_max_recursion: 4096,
-            parser_max_tokens: 15_000,
         }
     }
 }

apollo-router/src/configuration/snapshots/apollo_router__configuration__tests__schema_generation.snap

@@ -1129,10 +1129,18 @@ expression: "&schema"
         "max_aliases": null,
         "warn_only": false,
         "parser_max_recursion": 4096,
-        "parser_max_tokens": 15000
+        "parser_max_tokens": 15000,
+        "experimental_http_max_request_bytes": 2000000
       },
       "type": "object",
       "properties": {
+        "experimental_http_max_request_bytes": {
+          "description": "Limit the size of incoming HTTP requests read from the network, to protect against running out of memory. Default: 2000000 (2 MB)",
+          "default": 2000000,
+          "type": "integer",
+          "format": "uint",
+          "minimum": 0.0
+        },
         "max_aliases": {
           "description": "If set, requests with operations with more aliases than this maximum are rejected with a HTTP 400 Bad Request response and GraphQL error with `\"extensions\": {\"code\": \"MAX_ALIASES_LIMIT\"}`",
           "default": null,

apollo-router/src/services/router_service.rs

@@ -65,16 +65,22 @@ where
 {
     supergraph_creator: Arc<SF>,
     apq_layer: APQLayer,
+    experimental_http_max_request_bytes: usize,
 }
 
 impl<SF> RouterService<SF>
 where
     SF: ServiceFactory<supergraph::Request> + Clone + Send + Sync + 'static,
 {
-    pub(crate) fn new(supergraph_creator: Arc<SF>, apq_layer: APQLayer) -> Self {
+    pub(crate) fn new(
+        supergraph_creator: Arc<SF>,
+        apq_layer: APQLayer,
+        experimental_http_max_request_bytes: usize,
+    ) -> Self {
         RouterService {
             supergraph_creator,
             apq_layer,
+            experimental_http_max_request_bytes,
         }
     }
 }
@@ -176,9 +182,11 @@ where
 
         let supergraph_creator = self.supergraph_creator.clone();
         let apq = self.apq_layer.clone();
+        let experimental_http_max_request_bytes = self.experimental_http_max_request_bytes;
 
         let fut = async move {
-            let graphql_request: Result<graphql::Request, (&str, String)> = if parts.method
+            let graphql_request: Result<graphql::Request, (StatusCode, &str, String)> = if parts
+                .method
                 == Method::GET
             {
                 parts
@@ -187,32 +195,68 @@ where
                     .map(|q| {
                         graphql::Request::from_urlencoded_query(q.to_string()).map_err(|e| {
                             (
+                                StatusCode::BAD_REQUEST,
                                 "failed to decode a valid GraphQL request from path",
                                 format!("failed to decode a valid GraphQL request from path {e}"),
                             )
                         })
                     })
                     .unwrap_or_else(|| {
-                        Err(("There was no GraphQL operation to execute. Use the `query` parameter to send an operation, using either GET or POST.", "There was no GraphQL operation to execute. Use the `query` parameter to send an operation, using either GET or POST.".to_string()))
+                        Err((
+                            StatusCode::BAD_REQUEST,
+                            "There was no GraphQL operation to execute. Use the `query` parameter to send an operation, using either GET or POST.", 
+                            "There was no GraphQL operation to execute. Use the `query` parameter to send an operation, using either GET or POST.".to_string()
+                        ))
                     })
             } else {
-                hyper::body::to_bytes(body)
-                    .instrument(tracing::debug_span!("receive_body"))
-                    .await
-                    .map_err(|e| {
-                        (
-                            "failed to get the request body",
-                            format!("failed to get the request body: {e}"),
-                        )
-                    })
-                    .and_then(|bytes| {
-                        graphql::Request::deserialize_from_bytes(&bytes).map_err(|err| {
-                            (
-                                "failed to deserialize the request body into JSON",
-                                format!("failed to deserialize the request body into JSON: {err}"),
-                            )
+                // FIXME: use a try block when available: https://github.com/rust-lang/rust/issues/31436
+                let content_length = (|| {
+                    parts
+                        .headers
+                        .get(http::header::CONTENT_LENGTH)?
+                        .to_str()
+                        .ok()?
+                        .parse()
+                        .ok()
+                })();
+                if content_length.unwrap_or(0) > experimental_http_max_request_bytes {
+                    Err((
+                        StatusCode::PAYLOAD_TOO_LARGE,
+                        "payload too large for the `experimental_http_max_request_bytes` configuration",
+                        "payload too large".to_string(),
+                    ))
+                } else {
+                    let body = http_body::Limited::new(body, experimental_http_max_request_bytes);
+                    hyper::body::to_bytes(body)
+                        .instrument(tracing::debug_span!("receive_body"))
+                        .await
+                        .map_err(|e| {
+                            if e.is::<http_body::LengthLimitError>() {
+                                (
+                                    StatusCode::PAYLOAD_TOO_LARGE,
+                                    "payload too large for the `experimental_http_max_request_bytes` configuration",
+                                    "payload too large".to_string(),
+                                )
+                            } else {
+                                (
+                                    StatusCode::BAD_REQUEST,
+                                    "failed to get the request body",
+                                    format!("failed to get the request body: {e}"),
+                                )
+                            }
                         })
-                    })
+                        .and_then(|bytes| {
+                            graphql::Request::deserialize_from_bytes(&bytes).map_err(|err| {
+                                (
+                                    StatusCode::BAD_REQUEST,
+                                    "failed to deserialize the request body into JSON",
+                                    format!(
+                                        "failed to deserialize the request body into JSON: {err}"
+                                    ),
+                                )
+                            })
+                        })
+                }
             };
 
             match graphql_request {
@@ -377,11 +421,10 @@ where
                         }
                     }
                 }
-                Err((error, extension_details)) => {
-                    // BAD REQUEST
+                Err((status_code, error, extension_details)) => {
                     ::tracing::error!(
                         monotonic_counter.apollo_router_http_requests_total = 1u64,
-                        status = %400,
+                        status = %status_code.as_u16(),
                         error = %error,
                         %error
                     );
@@ -394,7 +437,7 @@ where
                                 .extension("details", extension_details)
                                 .build(),
                         )
-                        .status_code(StatusCode::BAD_REQUEST)
+                        .status_code(status_code)
                         .header(CONTENT_TYPE, APPLICATION_JSON.essence_str())
                         .context(context)
                         .build()
@@ -422,6 +465,7 @@ where
     supergraph_creator: Arc<SF>,
     static_page: StaticPageLayer,
     apq_layer: APQLayer,
+    experimental_http_max_request_bytes: usize,
 }
 
 impl<SF> ServiceFactory<router::Request> for RouterCreator<SF>
@@ -485,6 +529,9 @@ where
             supergraph_creator,
             static_page,
             apq_layer,
+            experimental_http_max_request_bytes: configuration
+                .preview_operation_limits
+                .experimental_http_max_request_bytes,
         }
     }
 
@@ -499,6 +546,7 @@ where
         let router_service = content_negociation::RouterLayer::default().layer(RouterService::new(
             self.supergraph_creator.clone(),
             self.apq_layer.clone(),
+            self.experimental_http_max_request_bytes,
         ));
 
         ServiceBuilder::new()
@@ -690,4 +738,50 @@ mod tests {
         assert_eq!(expected_error, actual_error);
         assert!(response.errors[0].extensions.contains_key("code"));
     }
+
+    #[tokio::test]
+    async fn test_experimental_http_max_request_bytes() {
+        /// Size of the JSON serialization of the request created by `fn canned_new`
+        /// in `apollo-router/src/services/supergraph.rs`
+        const CANNED_REQUEST_LEN: usize = 391;
+
+        async fn with_config(experimental_http_max_request_bytes: usize) -> router::Response {
+            let http_request = supergraph::Request::canned_builder()
+                .build()
+                .unwrap()
+                .supergraph_request
+                .map(|body| {
+                    let json_bytes = serde_json::to_vec(&body).unwrap();
+                    assert_eq!(
+                        json_bytes.len(),
+                        CANNED_REQUEST_LEN,
+                        "The request generated by `fn canned_new` \
+                         in `apollo-router/src/services/supergraph.rs` has changed. \
+                         Please update `CANNED_REQUEST_LEN` accordingly."
+                    );
+                    hyper::Body::from(json_bytes)
+                });
+            let config = serde_json::json!({
+                "preview_operation_limits": {
+                    "experimental_http_max_request_bytes": experimental_http_max_request_bytes
+                }
+            });
+            crate::TestHarness::builder()
+                .configuration_json(config)
+                .unwrap()
+                .build_router()
+                .await
+                .unwrap()
+                .oneshot(router::Request::from(http_request))
+                .await
+                .unwrap()
+        }
+        // Send a request just at (under) the limit
+        let response = with_config(CANNED_REQUEST_LEN).await.response;
+        assert_eq!(response.status(), http::StatusCode::OK);
+
+        // Send a request just over the limit
+        let response = with_config(CANNED_REQUEST_LEN - 1).await.response;
+        assert_eq!(response.status(), http::StatusCode::PAYLOAD_TOO_LARGE);
+    }
 }

docs/source/configuration/overview.mdx

@@ -513,15 +513,19 @@ tls:
 >
 > To set request limits, you must run v1.17 or later of the Apollo Router. [Download the latest version.](../quickstart#download-options)
 
-The Apollo Router supports enforcing two types of request limits for enhanced security:
+The Apollo Router supports enforcing three types of request limits for enhanced security:
 
+- Network-based limits
 - Lexical, parser-based limits
 - Semantic, operation-based limits (this is an [Enterprise feature](../enterprise-features/))
 
 The router rejects any request that violates at least one of these limits.
 
 ```yaml title="router.yaml"
 preview_operation_limits:
+  # Network-based limits
+  experimental_http_max_request_bytes: 2000000 # Default value: 2 MB
+
   # Parser-based limits
   parser_max_tokens: 15000 # Default value
   parser_max_recursion: 4096 # Default value
@@ -537,6 +541,23 @@ preview_operation_limits:
 
 See [this article](./operation-limits/).
 
+#### Network-based limits
+
+##### `http_max_request_bytes`
+
+> **This configuration is currently [experimental](/resources/product-launch-stages#experimental-features).**
+
+Limits the amount of data read from the network for the body of HTTP requests,
+to protect against unbounded memory consumption.
+This limit is checked before JSON parsing.
+Both the GraphQL document and associated variables count toward it.
+
+The default value is `2000000` bytes, 2 MB.
+
+Before increasing this limit significantly consider testing performance
+in an environment similar to your production, especially if some clients are untrusted.
+Many concurrent large requests could cause the Router to run out of memory.
+
 #### Parser-based limits
 
 ##### `parser_max_tokens`