[api] add request content length limit to routes that accept request …

…body
sherry-x · Nov 13, 2021 · 50ac1d6 · 50ac1d6
1 parent a720f2e
commit 50ac1d6
Show file tree

Hide file tree

Showing 7 changed files with 104 additions and 7 deletions.
diff --git a/api/src/context.rs b/api/src/context.rs
@@ -2,7 +2,7 @@
 // SPDX-License-Identifier: Apache-2.0
 
 use diem_api_types::{Error, LedgerInfo, MoveConverter, TransactionOnChainData};
-use diem_config::config::{JsonRpcConfig, RoleType};
+use diem_config::config::{ApiConfig, JsonRpcConfig, RoleType};
 use diem_crypto::HashValue;
 use diem_mempool::{MempoolClientRequest, MempoolClientSender, SubmissionStatus};
 use diem_types::{
@@ -35,6 +35,7 @@ pub struct Context {
     mp_sender: MempoolClientSender,
     role: RoleType,
     jsonrpc_config: JsonRpcConfig,
+    api_config: ApiConfig,
 }
 
 impl Context {
@@ -44,13 +45,15 @@ impl Context {
         mp_sender: MempoolClientSender,
         role: RoleType,
         jsonrpc_config: JsonRpcConfig,
+        api_config: ApiConfig,
     ) -> Self {
         Self {
             chain_id,
             db,
             mp_sender,
             role,
             jsonrpc_config,
+            api_config,
         }
     }
 
@@ -62,6 +65,10 @@ impl Context {
         self.chain_id
     }
 
+    pub fn content_length_limit(&self) -> u64 {
+        self.api_config.content_length_limit()
+    }
+
     pub fn filter(self) -> impl Filter<Extract = (Context,), Error = Infallible> + Clone {
         warp::any().map(move || self.clone())
     }

diff --git a/api/src/index.rs b/api/src/index.rs
@@ -12,8 +12,10 @@ use diem_api_types::{Error, Response};
 
 use std::convert::Infallible;
 use warp::{
-    cors::CorsForbidden, http::StatusCode, reject::MethodNotAllowed, reply, Filter, Rejection,
-    Reply,
+    cors::CorsForbidden,
+    http::StatusCode,
+    reject::{MethodNotAllowed, PayloadTooLarge},
+    reply, Filter, Rejection, Reply,
 };
 
 const OPEN_API_HTML: &str = include_str!(concat!(env!("OUT_DIR"), "/spec.html"));
@@ -85,6 +87,9 @@ async fn handle_rejection(err: Rejection) -> Result<impl Reply, Infallible> {
     } else if let Some(cause) = err.find::<CorsForbidden>() {
         code = StatusCode::FORBIDDEN;
         body = reply::json(&Error::new(code, cause.to_string()));
+    } else if let Some(cause) = err.find::<PayloadTooLarge>() {
+        code = StatusCode::PAYLOAD_TOO_LARGE;
+        body = reply::json(&Error::new(code, cause.to_string()));
     } else if err.find::<MethodNotAllowed>().is_some() {
         code = StatusCode::BAD_REQUEST;
         body = reply::json(&Error::new(

diff --git a/api/src/runtime.rs b/api/src/runtime.rs
@@ -34,13 +34,13 @@ pub fn bootstrap(
     let role = config.base.role;
     let json_rpc_config = config.json_rpc.clone();
     let api_config = config.api.clone();
-    let api = WebServer::from(api_config);
+    let api = WebServer::from(api_config.clone());
     let jsonrpc = WebServer::from(json_rpc_config.clone());
     if api.port() == jsonrpc.port() && api != jsonrpc {
         bail!("API and JSON-RPC should have same configuration when they are configured to use same port. api: {:?}, jsonrpc: {:?}", api, jsonrpc);
     }
     runtime.spawn(async move {
-        let context = Context::new(chain_id, db, mp_sender, role, json_rpc_config);
+        let context = Context::new(chain_id, db, mp_sender, role, json_rpc_config, api_config);
         let routes = index::routes(context);
         if api.port() == jsonrpc.port() {
             api.serve(routes).await;

diff --git a/api/src/tests/test_context.rs b/api/src/tests/test_context.rs
@@ -7,7 +7,7 @@ use diem_api_types::{
     mime_types, TransactionOnChainData, X_DIEM_CHAIN_ID, X_DIEM_LEDGER_TIMESTAMP,
     X_DIEM_LEDGER_VERSION,
 };
-use diem_config::config::{JsonRpcConfig, RoleType};
+use diem_config::config::{ApiConfig, JsonRpcConfig, RoleType};
 use diem_crypto::hash::HashValue;
 use diem_genesis_tool::validator_builder::{RootKeys, ValidatorBuilder};
 use diem_global_constants::OWNER_ACCOUNT;
@@ -75,6 +75,7 @@ pub fn new_test_context() -> TestContext {
             mempool.ac_client.clone(),
             RoleType::Validator,
             JsonRpcConfig::default(),
+            ApiConfig::default(),
         ),
         rng,
         root_keys,

diff --git a/api/src/tests/transactions_test.rs b/api/src/tests/transactions_test.rs
@@ -25,6 +25,7 @@ use move_core_types::{
     identifier::Identifier,
     language_storage::{ModuleId, StructTag, TypeTag, CORE_CODE_ADDRESS},
 };
+use rand::{distributions::Alphanumeric, thread_rng, Rng};
 use serde_json::json;
 
 #[tokio::test]
@@ -1485,3 +1486,76 @@ async fn test_transaction_vm_status(
     );
     assert_eq!(resp["vm_status"].as_str().unwrap(), vm_status);
 }
+
+#[tokio::test]
+async fn test_submit_transaction_rejects_payload_too_large_bcs_txn_body() {
+    let context = new_test_context();
+
+    let resp = context
+        .expect_status_code(413)
+        .post_bcs_txn(
+            "/transactions",
+            gen_string(context.context.content_length_limit() + 1).as_bytes(),
+        )
+        .await;
+    assert_json(
+        resp,
+        json!({
+          "code": 413,
+          "message": "The request payload is too large"
+        }),
+    );
+}
+
+#[tokio::test]
+async fn test_submit_transaction_rejects_payload_too_large_json_body() {
+    let context = new_test_context();
+
+    let resp = context
+        .expect_status_code(413)
+        .post(
+            "/transactions",
+            json!({
+                "data": gen_string(context.context.content_length_limit()+1).as_bytes(),
+            }),
+        )
+        .await;
+    assert_json(
+        resp,
+        json!({
+          "code": 413,
+          "message": "The request payload is too large"
+        }),
+    );
+}
+
+#[tokio::test]
+async fn test_create_signing_message_rejects_payload_too_large_json_body() {
+    let context = new_test_context();
+
+    let resp = context
+        .expect_status_code(413)
+        .post(
+            "/transactions/signing_message",
+            json!({
+                "data": gen_string(context.context.content_length_limit()+1).as_bytes(),
+            }),
+        )
+        .await;
+    assert_json(
+        resp,
+        json!({
+          "code": 413,
+          "message": "The request payload is too large"
+        }),
+    );
+}
+
+fn gen_string(len: u64) -> String {
+    let mut rng = thread_rng();
+    std::iter::repeat(())
+        .map(|()| rng.sample(Alphanumeric))
+        .take(len as usize)
+        .map(char::from)
+        .collect()
+}
diff --git a/api/src/transactions.rs b/api/src/transactions.rs
@@ -94,6 +94,9 @@ pub fn post_transactions(
     warp::path!("transactions")
         .and(warp::post())
         .and(warp::header::<String>(CONTENT_TYPE.as_str()))
+        .and(warp::body::content_length_limit(
+            context.content_length_limit(),
+        ))
         .and(warp::body::bytes())
         .and(context.filter())
         .and_then(handle_post_transactions)
@@ -137,6 +140,9 @@ pub fn create_signing_message(
     warp::path!("transactions" / "signing_message")
         .and(warp::post())
         .and(warp::header::exact(CONTENT_TYPE.as_str(), mime_types::JSON))
+        .and(warp::body::content_length_limit(
+            context.content_length_limit(),
+        ))
         .and(warp::body::bytes())
         .and(context.filter())
         .and_then(handle_create_signing_message)

diff --git a/config/src/config/api_config.rs b/config/src/config/api_config.rs
@@ -19,6 +19,7 @@ pub struct ApiConfig {
 
 pub const DEFAULT_ADDRESS: &str = "127.0.0.1";
 pub const DEFAULT_PORT: u16 = 8080;
+pub const DEFAULT_REQUEST_CONTENT_LENGTH_LIMIT: u64 = 4 * 1024 * 1024; // 4mb
 
 fn default_enabled() -> bool {
     true
@@ -27,7 +28,6 @@ fn default_enabled() -> bool {
 impl Default for ApiConfig {
     fn default() -> ApiConfig {
         ApiConfig {
-            // disable by default until the API is ready for production
             enabled: false,
             address: format!("{}:{}", DEFAULT_ADDRESS, DEFAULT_PORT)
                 .parse()
@@ -42,4 +42,8 @@ impl ApiConfig {
     pub fn randomize_ports(&mut self) {
         self.address.set_port(utils::get_available_port());
     }
+
+    pub fn content_length_limit(&self) -> u64 {
+        DEFAULT_REQUEST_CONTENT_LENGTH_LIMIT
+    }
 }