From ae02e0487759ae99e24656ff301af9df17997f75 Mon Sep 17 00:00:00 2001 From: Dyslex7c Date: Wed, 21 Jan 2026 15:27:01 +0530 Subject: [PATCH 1/6] feat(rpc): implement duplicate key detection for json-rpc requests --- src/lib.rs | 2 +- src/rpc/json_validator.rs | 183 +++++++++++++++++++++++++++++++ src/rpc/mod.rs | 3 + src/rpc/validation_layer.rs | 80 ++++++++++++++ tests/rpc_duplicate_keys_test.rs | 58 ++++++++++ 5 files changed, 325 insertions(+), 1 deletion(-) create mode 100644 src/rpc/json_validator.rs create mode 100644 src/rpc/validation_layer.rs create mode 100644 tests/rpc_duplicate_keys_test.rs diff --git a/src/lib.rs b/src/lib.rs index 2cb0ca6e71a..518bbfdc382 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -64,7 +64,7 @@ mod message; mod message_pool; mod metrics; mod networks; -mod rpc; +pub mod rpc; mod shim; mod state_manager; mod state_migration; diff --git a/src/rpc/json_validator.rs b/src/rpc/json_validator.rs new file mode 100644 index 00000000000..5886b8df5e4 --- /dev/null +++ b/src/rpc/json_validator.rs @@ -0,0 +1,183 @@ +// Copyright 2019-2026 ChainSafe Systems +// SPDX-License-Identifier: Apache-2.0, MIT + +//! JSON validation utilities for detecting duplicate keys before serde_json processing. +//! +//! serde_json automatically deduplicates keys at parse time using a "last-write-wins" strategy +//! This means JSON like `{"/":"cid1", "/":"cid2"}` will keep only the last value, which can lead to unexpected behaviour in RPC calls. + +use ahash::HashSet; + +#[cfg(not(test))] +use std::sync::LazyLock; + +pub const STRICT_JSON_ENV: &str = "FOREST_STRICT_JSON"; + +#[inline] +pub fn is_strict_mode() -> bool { + #[cfg(test)] + { + std::env::var(STRICT_JSON_ENV) + .map(|v| v == "1" || v.eq_ignore_ascii_case("true")) + .unwrap_or(false) + } + + #[cfg(not(test))] + { + static STRICT_MODE: LazyLock = LazyLock::new(|| { + std::env::var(STRICT_JSON_ENV) + .map(|v| v == "1" || v.eq_ignore_ascii_case("true")) + .unwrap_or(false) + }); + *STRICT_MODE + } +} + +/// validates JSON for duplicate keys by parsing at the token level. +pub fn validate_json_for_duplicates(json_str: &str) -> Result<(), String> { + if !is_strict_mode() { + return Ok(()); + } + + let mut object_stack: Vec> = Vec::new(); + + let mut chars = json_str.chars(); + let mut current_key = String::new(); + let mut in_string = false; + let mut escape_next = false; + let mut expecting_key = false; + + while let Some(ch) = chars.next() { + if in_string { + if escape_next { + escape_next = false; + if expecting_key { + current_key.push('\\'); + current_key.push(ch); + } + continue; + } + if ch == '\\' { + escape_next = true; + continue; + } + if ch == '"' { + in_string = false; + if expecting_key { + if let Some(keys) = object_stack.last_mut() { + if !keys.insert(current_key.clone()) { + return Err(format!( + "duplicate key '{}' in JSON object - this likely indicates malformed input. \ + Set {}=0 to disable this check", + current_key, STRICT_JSON_ENV + )); + } + } + current_key.clear(); + expecting_key = false; + } + continue; + } + if expecting_key { + current_key.push(ch); + } + continue; + } + + match ch { + '"' => { + in_string = true; + } + '{' => { + object_stack.push(HashSet::default()); + expecting_key = true; + } + '}' => { + if !object_stack.is_empty() { + object_stack.pop(); + } + expecting_key = false; + } + '[' => { + expecting_key = false; + } + ']' => { + expecting_key = false; + } + ':' => {} + ',' => { + if !object_stack.is_empty() { + expecting_key = true; + } + } + c if c.is_whitespace() => {} + _ => {} + } + } + + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + + fn with_strict_mode(enabled: bool, f: F) + where + F: FnOnce(), + { + if enabled { + unsafe { std::env::set_var(STRICT_JSON_ENV, "1"); } + } else { + unsafe { std::env::remove_var(STRICT_JSON_ENV); } + } + f(); + unsafe { std::env::remove_var(STRICT_JSON_ENV); } + } + + #[test] + fn test_no_duplicates() { + with_strict_mode(true, || { + let json = r#"{"a": 1, "b": 2, "c": 3}"#; + assert!(validate_json_for_duplicates(json).is_ok()); + }); + } + + #[test] + fn test_duplicate_keys_detected() { + with_strict_mode(true, || { + let json = r#"{"/":"cid1", "/":"cid2"}"#; + let result = validate_json_for_duplicates(json); + assert!(result.is_err(), "Should have detected duplicate key"); + assert!(result.unwrap_err().contains("duplicate key")); + }); + } + + #[test] + fn test_strict_mode_disabled() { + with_strict_mode(false, || { // should pass with strict mode disabled + let json = r#"{"/":"cid1", "/":"cid2"}"#; + assert!(validate_json_for_duplicates(json).is_ok()); + }); + } + + #[test] + fn test_original_issue_case() { + with_strict_mode(true, || { + let json = r#"{ + "jsonrpc": "2.0", + "id": 1, + "method": "Filecoin.ChainGetMessagesInTipset", + "params": [[{ + "/":"bafy2bzacea43254b5x6c4l22ynpjfoct5qvabbbk2abcfspfcjkiltivrlyqi", + "/":"bafy2bzacea4viqyaozpfk57lnemwufryb76llxzmebxc7it2rnssqz2ljdl6a", + "/":"bafy2bzaceav6j67epppz5ib55v5ty26dhkq4jinbsizq2olb3azbzxvfmc73o" + }]] + }"#; + + let result = validate_json_for_duplicates(json); + assert!(result.is_err()); + assert!(result.unwrap_err().contains("duplicate key '/'")); + }); + } +} diff --git a/src/rpc/mod.rs b/src/rpc/mod.rs index 7244acf2f65..f75f6b611b8 100644 --- a/src/rpc/mod.rs +++ b/src/rpc/mod.rs @@ -12,6 +12,8 @@ mod metrics_layer; mod request; mod segregation_layer; mod set_extension_layer; +pub mod json_validator; +mod validation_layer; use crate::rpc::eth::types::RandomHexStringIdProvider; use crate::shim::clock::ChainEpoch; @@ -618,6 +620,7 @@ where .layer(SetExtensionLayer { path }) .layer(SegregationLayer) .layer(FilterLayer::new(filter_list.clone())) + .layer(validation_layer::ValidationLayer) .layer(AuthLayer { headers, keystore: keystore.clone(), diff --git a/src/rpc/validation_layer.rs b/src/rpc/validation_layer.rs new file mode 100644 index 00000000000..0dadb2dd03e --- /dev/null +++ b/src/rpc/validation_layer.rs @@ -0,0 +1,80 @@ +// Copyright 2019-2026 ChainSafe Systems +// SPDX-License-Identifier: Apache-2.0, MIT + +use futures::future::Either; +use jsonrpsee::server::middleware::rpc::RpcServiceT; +use jsonrpsee::types::ErrorObject; +use jsonrpsee::MethodResponse; +use tower::Layer; + +use super::json_validator; + +/// stateless jsonrpsee layer for validating JSON-RPC requests +#[derive(Clone, Default)] +pub(super) struct ValidationLayer; + +impl Layer for ValidationLayer { + type Service = Validation; + + fn layer(&self, service: S) -> Self::Service { + Validation { service } + } +} + +#[derive(Clone)] +pub(super) struct Validation { + service: S, +} + +impl Validation { + fn validation_enabled() -> bool { + json_validator::is_strict_mode() + } +} + +impl RpcServiceT for Validation +where + S: RpcServiceT + + Send + + Sync + + Clone + + 'static, +{ + type MethodResponse = S::MethodResponse; + type NotificationResponse = S::NotificationResponse; + type BatchResponse = S::BatchResponse; + + fn call<'a>( + &self, + req: jsonrpsee::types::Request<'a>, + ) -> impl Future + Send + 'a { + if !Self::validation_enabled() { + return Either::Left(self.service.call(req)); + } + + if let Err(e) = json_validator::validate_json_for_duplicates(req.params().as_str().unwrap_or("[]")) { + let err = ErrorObject::owned( + -32600, + e, + None::<()>, + ); + return Either::Right(async move { MethodResponse::error(req.id(), err) }); + } + + Either::Left(self.service.call(req)) + } + + fn batch<'a>( + &self, + batch: jsonrpsee::core::middleware::Batch<'a>, + ) -> impl Future + Send + 'a { + self.service.batch(batch) + } + + fn notification<'a>( + &self, + n: jsonrpsee::core::middleware::Notification<'a>, + ) -> impl Future + Send + 'a { + self.service.notification(n) + } +} diff --git a/tests/rpc_duplicate_keys_test.rs b/tests/rpc_duplicate_keys_test.rs new file mode 100644 index 00000000000..0197a40370b --- /dev/null +++ b/tests/rpc_duplicate_keys_test.rs @@ -0,0 +1,58 @@ +// Copyright 2019-2026 ChainSafe Systems +// SPDX-License-Identifier: Apache-2.0, MIT + +use forest::rpc::json_validator::{validate_json_for_duplicates, STRICT_JSON_ENV}; + +// https://github.com/ChainSafe/forest/issues/6424 +#[test] +fn test_issue_duplicate_cids() { + unsafe { + std::env::set_var(STRICT_JSON_ENV, "1"); + } + + let json = r#"{ + "jsonrpc": "2.0", + "id": 1, + "method": "Filecoin.ChainGetMessagesInTipset", + "params": [[{ + "/":"bafy2bzacea43254b5x6c4l22ynpjfoct5qvabbbk2abcfspfcjkiltivrlyqi", + "/":"bafy2bzacea4viqyaozpfk57lnemwufryb76llxzmebxc7it2rnssqz2ljdl6a", + "/":"bafy2bzaceav6j67epppz5ib55v5ty26dhkq4jinbsizq2olb3azbzxvfmc73o" + }]] + }"#; + + let result = validate_json_for_duplicates(json); + assert!(result.is_err(), "Should detect duplicate '/' keys"); + let error = result.unwrap_err(); + assert!(error.contains("duplicate key")); + assert!(error.contains("/")); + + unsafe { + std::env::remove_var(STRICT_JSON_ENV); + } +} + +#[test] +fn test_correct_format_passes() { + unsafe { + std::env::set_var(STRICT_JSON_ENV, "1"); + } + + let json = r#"{ + "jsonrpc": "2.0", + "id": 1, + "method": "Filecoin.ChainGetMessagesInTipset", + "params": [[ + {"/":"bafy2bzacea43254b5x6c4l22ynpjfoct5qvabbbk2abcfspfcjkiltivrlyqi"}, + {"/":"bafy2bzacea4viqyaozpfk57lnemwufryb76llxzmebxc7it2rnssqz2ljdl6a"}, + {"/":"bafy2bzaceav6j67epppz5ib55v5ty26dhkq4jinbsizq2olb3azbzxvfmc73o"} + ]] + }"#; + + let result = validate_json_for_duplicates(json); + assert!(result.is_ok(), "Correct format should pass validation"); + + unsafe { + std::env::remove_var(STRICT_JSON_ENV); + } +} From 0e772900977a3b17d4aa48b615bb2fb1002be105 Mon Sep 17 00:00:00 2001 From: Dyslex7c Date: Wed, 21 Jan 2026 18:35:09 +0530 Subject: [PATCH 2/6] use justjson and update env_variables.md --- Cargo.lock | 7 ++ Cargo.toml | 1 + docs/docs/users/reference/env_variables.md | 2 +- src/rpc/json_validator.rs | 116 +++++---------------- src/rpc/mod.rs | 2 +- src/rpc/validation_layer.rs | 63 ++++++++++- 6 files changed, 95 insertions(+), 96 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index e2a86ab880a..2e380b66ddb 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3219,6 +3219,7 @@ dependencies = [ "itertools 0.14.0", "jsonrpsee", "jsonwebtoken", + "justjson", "keccak-hash", "kubert-prometheus-process", "lazy-regex", @@ -5105,6 +5106,12 @@ dependencies = [ "simple_asn1", ] +[[package]] +name = "justjson" +version = "0.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d7e253b574775d0ebd7975c471fc18f72f0775a4d42b563b5fbc3c4068aa1075" + [[package]] name = "k256" version = "0.13.4" diff --git a/Cargo.toml b/Cargo.toml index b7615232496..61f1d329c14 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -116,6 +116,7 @@ is-terminal = "0.4" itertools = "0.14" jsonrpsee = { version = "0.26", features = ["server", "ws-client", "http-client", "macros"] } jsonwebtoken = { version = "10", features = ["aws_lc_rs"] } +justjson = "0.3" keccak-hash = "0.12" kubert-prometheus-process = "0.2" lazy-regex = "3" diff --git a/docs/docs/users/reference/env_variables.md b/docs/docs/users/reference/env_variables.md index c488020f2c4..d905edaa8d6 100644 --- a/docs/docs/users/reference/env_variables.md +++ b/docs/docs/users/reference/env_variables.md @@ -54,7 +54,7 @@ process. | `FOREST_JWT_DISABLE_EXP_VALIDATION` | 1 or true | empty | 1 | Whether or not to disable JWT expiration validation | | `FOREST_ETH_BLOCK_CACHE_SIZE` | positive integer | 500 | 1 | The size of Eth block cache | | `FOREST_RPC_BACKFILL_FULL_TIPSET_FROM_NETWORK` | 1 or true | false | 1 | Whether or not to backfill full tipsets from the p2p network | - +| `FOREST_STRICT_JSON` | 1 or true | false | 1 | Enable strict JSON validation to detect duplicate keys in RPC requests ### `FOREST_F3_SIDECAR_FFI_BUILD_OPT_OUT` This is an environment variable that allows users to opt out of building the f3-sidecar. It's only useful when building diff --git a/src/rpc/json_validator.rs b/src/rpc/json_validator.rs index 5886b8df5e4..ba57b470aec 100644 --- a/src/rpc/json_validator.rs +++ b/src/rpc/json_validator.rs @@ -7,30 +7,13 @@ //! This means JSON like `{"/":"cid1", "/":"cid2"}` will keep only the last value, which can lead to unexpected behaviour in RPC calls. use ahash::HashSet; - -#[cfg(not(test))] -use std::sync::LazyLock; +use justjson::Value; pub const STRICT_JSON_ENV: &str = "FOREST_STRICT_JSON"; #[inline] pub fn is_strict_mode() -> bool { - #[cfg(test)] - { - std::env::var(STRICT_JSON_ENV) - .map(|v| v == "1" || v.eq_ignore_ascii_case("true")) - .unwrap_or(false) - } - - #[cfg(not(test))] - { - static STRICT_MODE: LazyLock = LazyLock::new(|| { - std::env::var(STRICT_JSON_ENV) - .map(|v| v == "1" || v.eq_ignore_ascii_case("true")) - .unwrap_or(false) - }); - *STRICT_MODE - } + crate::utils::misc::env::is_env_truthy(STRICT_JSON_ENV) } /// validates JSON for duplicate keys by parsing at the token level. @@ -39,83 +22,38 @@ pub fn validate_json_for_duplicates(json_str: &str) -> Result<(), String> { return Ok(()); } - let mut object_stack: Vec> = Vec::new(); - - let mut chars = json_str.chars(); - let mut current_key = String::new(); - let mut in_string = false; - let mut escape_next = false; - let mut expecting_key = false; - - while let Some(ch) = chars.next() { - if in_string { - if escape_next { - escape_next = false; - if expecting_key { - current_key.push('\\'); - current_key.push(ch); - } - continue; - } - if ch == '\\' { - escape_next = true; - continue; - } - if ch == '"' { - in_string = false; - if expecting_key { - if let Some(keys) = object_stack.last_mut() { - if !keys.insert(current_key.clone()) { - return Err(format!( - "duplicate key '{}' in JSON object - this likely indicates malformed input. \ - Set {}=0 to disable this check", - current_key, STRICT_JSON_ENV - )); - } + fn check_value(value: &Value) -> Result<(), String> { + match value { + Value::Object(obj) => { + let mut seen = HashSet::default(); + for entry in obj.iter() { + let key = entry.key.as_str().ok_or_else(|| { + "Invalid JSON key".to_string() + })?; + + if !seen.insert(key) { + return Err(format!( + "duplicate key '{}' in JSON object - this likely indicates malformed input. \ + Set {}=0 to disable this check", + key, STRICT_JSON_ENV + )); } - current_key.clear(); - expecting_key = false; + check_value(&entry.value)?; } - continue; - } - if expecting_key { - current_key.push(ch); - } - continue; - } - - match ch { - '"' => { - in_string = true; - } - '{' => { - object_stack.push(HashSet::default()); - expecting_key = true; - } - '}' => { - if !object_stack.is_empty() { - object_stack.pop(); - } - expecting_key = false; - } - '[' => { - expecting_key = false; - } - ']' => { - expecting_key = false; + Ok(()) } - ':' => {} - ',' => { - if !object_stack.is_empty() { - expecting_key = true; + Value::Array(arr) => { + for item in arr.iter() { + check_value(item)?; } + Ok(()) } - c if c.is_whitespace() => {} - _ => {} + _ => Ok(()) } } - - Ok(()) + let value = Value::from_json(json_str) + .map_err(|e| format!("Invalid JSON: {}", e))?; + check_value(&value) } #[cfg(test)] diff --git a/src/rpc/mod.rs b/src/rpc/mod.rs index f75f6b611b8..e49d6bb6cdd 100644 --- a/src/rpc/mod.rs +++ b/src/rpc/mod.rs @@ -620,7 +620,7 @@ where .layer(SetExtensionLayer { path }) .layer(SegregationLayer) .layer(FilterLayer::new(filter_list.clone())) - .layer(validation_layer::ValidationLayer) + .layer(validation_layer::JsonValidationLayer) .layer(AuthLayer { headers, keystore: keystore.clone(), diff --git a/src/rpc/validation_layer.rs b/src/rpc/validation_layer.rs index 0dadb2dd03e..bb160fcb7e7 100644 --- a/src/rpc/validation_layer.rs +++ b/src/rpc/validation_layer.rs @@ -11,9 +11,9 @@ use super::json_validator; /// stateless jsonrpsee layer for validating JSON-RPC requests #[derive(Clone, Default)] -pub(super) struct ValidationLayer; +pub(super) struct JsonValidationLayer; -impl Layer for ValidationLayer { +impl Layer for JsonValidationLayer { type Service = Validation; fn layer(&self, service: S) -> Self::Service { @@ -30,6 +30,10 @@ impl Validation { fn validation_enabled() -> bool { json_validator::is_strict_mode() } + + fn validate_params(params_str: &str) -> Result<(), String> { + json_validator::validate_json_for_duplicates(params_str) + } } impl RpcServiceT for Validation @@ -66,15 +70,64 @@ where fn batch<'a>( &self, - batch: jsonrpsee::core::middleware::Batch<'a>, + mut batch: jsonrpsee::core::middleware::Batch<'a>, ) -> impl Future + Send + 'a { - self.service.batch(batch) + let service = self.service.clone(); + + async move { + if !Self::validation_enabled() { + return service.batch(batch).await; + } + + for entry in batch.iter_mut() { + if let Ok(batch_entry) = entry { + let params_str = match batch_entry.params() { + Some(p) => p.as_ref().get(), + None => "[]", + }; + + if let Err(e) = Self::validate_params(params_str) { + match batch_entry { + jsonrpsee::core::middleware::BatchEntry::Call(req) => { + let err = ErrorObject::owned(-32600, e, None::<()>); + let err_entry = jsonrpsee::core::middleware::BatchEntryErr::new( + req.id().clone(), + err, + ); + *entry = Err(err_entry); + } + jsonrpsee::core::middleware::BatchEntry::Notification(_) => {} + } + } + } + } + + service.batch(batch).await + } } fn notification<'a>( &self, n: jsonrpsee::core::middleware::Notification<'a>, ) -> impl Future + Send + 'a { - self.service.notification(n) + let service = self.service.clone(); + + async move { + if !Self::validation_enabled() { + return service.notification(n).await; + } + + let params_str = match n.params() { + Some(p) => p.as_ref().get(), + None => "[]", + }; + + if let Err(e) = Self::validate_params(params_str) { + let err = ErrorObject::owned(-32600, e, None::<()>); + return MethodResponse::error(jsonrpsee::types::Id::Null, err); + } + + service.notification(n).await + } } } From 39491a8c2b8219615de039ed4ab43ac0e262bbe6 Mon Sep 17 00:00:00 2001 From: Dyslex7c Date: Wed, 21 Jan 2026 18:39:20 +0530 Subject: [PATCH 3/6] revert rpc module to private --- src/lib.rs | 2 +- tests/rpc_duplicate_keys_test.rs | 58 -------------------------------- 2 files changed, 1 insertion(+), 59 deletions(-) delete mode 100644 tests/rpc_duplicate_keys_test.rs diff --git a/src/lib.rs b/src/lib.rs index 518bbfdc382..2cb0ca6e71a 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -64,7 +64,7 @@ mod message; mod message_pool; mod metrics; mod networks; -pub mod rpc; +mod rpc; mod shim; mod state_manager; mod state_migration; diff --git a/tests/rpc_duplicate_keys_test.rs b/tests/rpc_duplicate_keys_test.rs deleted file mode 100644 index 0197a40370b..00000000000 --- a/tests/rpc_duplicate_keys_test.rs +++ /dev/null @@ -1,58 +0,0 @@ -// Copyright 2019-2026 ChainSafe Systems -// SPDX-License-Identifier: Apache-2.0, MIT - -use forest::rpc::json_validator::{validate_json_for_duplicates, STRICT_JSON_ENV}; - -// https://github.com/ChainSafe/forest/issues/6424 -#[test] -fn test_issue_duplicate_cids() { - unsafe { - std::env::set_var(STRICT_JSON_ENV, "1"); - } - - let json = r#"{ - "jsonrpc": "2.0", - "id": 1, - "method": "Filecoin.ChainGetMessagesInTipset", - "params": [[{ - "/":"bafy2bzacea43254b5x6c4l22ynpjfoct5qvabbbk2abcfspfcjkiltivrlyqi", - "/":"bafy2bzacea4viqyaozpfk57lnemwufryb76llxzmebxc7it2rnssqz2ljdl6a", - "/":"bafy2bzaceav6j67epppz5ib55v5ty26dhkq4jinbsizq2olb3azbzxvfmc73o" - }]] - }"#; - - let result = validate_json_for_duplicates(json); - assert!(result.is_err(), "Should detect duplicate '/' keys"); - let error = result.unwrap_err(); - assert!(error.contains("duplicate key")); - assert!(error.contains("/")); - - unsafe { - std::env::remove_var(STRICT_JSON_ENV); - } -} - -#[test] -fn test_correct_format_passes() { - unsafe { - std::env::set_var(STRICT_JSON_ENV, "1"); - } - - let json = r#"{ - "jsonrpc": "2.0", - "id": 1, - "method": "Filecoin.ChainGetMessagesInTipset", - "params": [[ - {"/":"bafy2bzacea43254b5x6c4l22ynpjfoct5qvabbbk2abcfspfcjkiltivrlyqi"}, - {"/":"bafy2bzacea4viqyaozpfk57lnemwufryb76llxzmebxc7it2rnssqz2ljdl6a"}, - {"/":"bafy2bzaceav6j67epppz5ib55v5ty26dhkq4jinbsizq2olb3azbzxvfmc73o"} - ]] - }"#; - - let result = validate_json_for_duplicates(json); - assert!(result.is_ok(), "Correct format should pass validation"); - - unsafe { - std::env::remove_var(STRICT_JSON_ENV); - } -} From c3ef87d192e1ba2e4cb0fd6de7ccb4a0e542c726 Mon Sep 17 00:00:00 2001 From: Dyslex7c Date: Wed, 21 Jan 2026 20:12:34 +0530 Subject: [PATCH 4/6] skip validation for None branch and re-add lazylock --- docs/docs/users/reference/env_variables.md | 3 +- src/rpc/json_validator.rs | 54 +++++++++++++++++----- src/rpc/mod.rs | 2 +- src/rpc/validation_layer.rs | 31 +++++++------ 4 files changed, 63 insertions(+), 27 deletions(-) diff --git a/docs/docs/users/reference/env_variables.md b/docs/docs/users/reference/env_variables.md index d905edaa8d6..286f3e644aa 100644 --- a/docs/docs/users/reference/env_variables.md +++ b/docs/docs/users/reference/env_variables.md @@ -54,7 +54,8 @@ process. | `FOREST_JWT_DISABLE_EXP_VALIDATION` | 1 or true | empty | 1 | Whether or not to disable JWT expiration validation | | `FOREST_ETH_BLOCK_CACHE_SIZE` | positive integer | 500 | 1 | The size of Eth block cache | | `FOREST_RPC_BACKFILL_FULL_TIPSET_FROM_NETWORK` | 1 or true | false | 1 | Whether or not to backfill full tipsets from the p2p network | -| `FOREST_STRICT_JSON` | 1 or true | false | 1 | Enable strict JSON validation to detect duplicate keys in RPC requests +| `FOREST_STRICT_JSON` | 1 or true | false | 1 | Enable strict JSON validation to detect duplicate keys in RPC requests | + ### `FOREST_F3_SIDECAR_FFI_BUILD_OPT_OUT` This is an environment variable that allows users to opt out of building the f3-sidecar. It's only useful when building diff --git a/src/rpc/json_validator.rs b/src/rpc/json_validator.rs index ba57b470aec..3d658a585fb 100644 --- a/src/rpc/json_validator.rs +++ b/src/rpc/json_validator.rs @@ -4,16 +4,30 @@ //! JSON validation utilities for detecting duplicate keys before serde_json processing. //! //! serde_json automatically deduplicates keys at parse time using a "last-write-wins" strategy -//! This means JSON like `{"/":"cid1", "/":"cid2"}` will keep only the last value, which can lead to unexpected behaviour in RPC calls. +//! This means JSON like `{"/":"cid1", "/":"cid2"}` will keep only the last value, which can lead to unexpected behaviour in RPC calls. use ahash::HashSet; use justjson::Value; pub const STRICT_JSON_ENV: &str = "FOREST_STRICT_JSON"; +#[cfg(not(test))] +use std::sync::LazyLock; + +#[cfg(not(test))] +static STRICT_MODE: LazyLock = + LazyLock::new(|| crate::utils::misc::env::is_env_truthy(STRICT_JSON_ENV)); + #[inline] pub fn is_strict_mode() -> bool { - crate::utils::misc::env::is_env_truthy(STRICT_JSON_ENV) + #[cfg(test)] + { + crate::utils::misc::env::is_env_truthy(STRICT_JSON_ENV) + } + #[cfg(not(test))] + { + *STRICT_MODE + } } /// validates JSON for duplicate keys by parsing at the token level. @@ -27,10 +41,11 @@ pub fn validate_json_for_duplicates(json_str: &str) -> Result<(), String> { Value::Object(obj) => { let mut seen = HashSet::default(); for entry in obj.iter() { - let key = entry.key.as_str().ok_or_else(|| { - "Invalid JSON key".to_string() - })?; - + let key = entry + .key + .as_str() + .ok_or_else(|| "Invalid JSON key".to_string())?; + if !seen.insert(key) { return Err(format!( "duplicate key '{}' in JSON object - this likely indicates malformed input. \ @@ -48,11 +63,13 @@ pub fn validate_json_for_duplicates(json_str: &str) -> Result<(), String> { } Ok(()) } - _ => Ok(()) + _ => Ok(()), } } - let value = Value::from_json(json_str) - .map_err(|e| format!("Invalid JSON: {}", e))?; + let value = match Value::from_json(json_str) { + Ok(v) => v, + Err(_) => return Ok(()), + }; check_value(&value) } @@ -64,13 +81,26 @@ mod tests { where F: FnOnce(), { + let original = std::env::var(STRICT_JSON_ENV).ok(); + if enabled { - unsafe { std::env::set_var(STRICT_JSON_ENV, "1"); } + unsafe { + std::env::set_var(STRICT_JSON_ENV, "1"); + } } else { - unsafe { std::env::remove_var(STRICT_JSON_ENV); } + unsafe { + std::env::remove_var(STRICT_JSON_ENV); + } } + f(); - unsafe { std::env::remove_var(STRICT_JSON_ENV); } + + unsafe { + match original { + Some(val) => std::env::set_var(STRICT_JSON_ENV, val), + None => std::env::remove_var(STRICT_JSON_ENV), + } + } } #[test] diff --git a/src/rpc/mod.rs b/src/rpc/mod.rs index e49d6bb6cdd..4e2a4a7a4e4 100644 --- a/src/rpc/mod.rs +++ b/src/rpc/mod.rs @@ -7,12 +7,12 @@ mod channel; mod client; mod filter_layer; mod filter_list; +pub mod json_validator; mod log_layer; mod metrics_layer; mod request; mod segregation_layer; mod set_extension_layer; -pub mod json_validator; mod validation_layer; use crate::rpc::eth::types::RandomHexStringIdProvider; diff --git a/src/rpc/validation_layer.rs b/src/rpc/validation_layer.rs index bb160fcb7e7..4c9688e2a0a 100644 --- a/src/rpc/validation_layer.rs +++ b/src/rpc/validation_layer.rs @@ -2,9 +2,9 @@ // SPDX-License-Identifier: Apache-2.0, MIT use futures::future::Either; +use jsonrpsee::MethodResponse; use jsonrpsee::server::middleware::rpc::RpcServiceT; use jsonrpsee::types::ErrorObject; -use jsonrpsee::MethodResponse; use tower::Layer; use super::json_validator; @@ -56,12 +56,10 @@ where return Either::Left(self.service.call(req)); } - if let Err(e) = json_validator::validate_json_for_duplicates(req.params().as_str().unwrap_or("[]")) { - let err = ErrorObject::owned( - -32600, - e, - None::<()>, - ); + if let Err(e) = + json_validator::validate_json_for_duplicates(req.params().as_str().unwrap_or("[]")) + { + let err = ErrorObject::owned(-32600, e, None::<()>); return Either::Right(async move { MethodResponse::error(req.id(), err) }); } @@ -73,7 +71,7 @@ where mut batch: jsonrpsee::core::middleware::Batch<'a>, ) -> impl Future + Send + 'a { let service = self.service.clone(); - + async move { if !Self::validation_enabled() { return service.batch(batch).await; @@ -83,7 +81,7 @@ where if let Ok(batch_entry) = entry { let params_str = match batch_entry.params() { Some(p) => p.as_ref().get(), - None => "[]", + None => continue, }; if let Err(e) = Self::validate_params(params_str) { @@ -96,7 +94,14 @@ where ); *entry = Err(err_entry); } - jsonrpsee::core::middleware::BatchEntry::Notification(_) => {} + jsonrpsee::core::middleware::BatchEntry::Notification(_notif) => { + let err = ErrorObject::owned(-32600, e, None::<()>); + let err_entry = jsonrpsee::core::middleware::BatchEntryErr::new( + jsonrpsee::types::Id::Null, + err, + ); + *entry = Err(err_entry); + } } } } @@ -111,7 +116,7 @@ where n: jsonrpsee::core::middleware::Notification<'a>, ) -> impl Future + Send + 'a { let service = self.service.clone(); - + async move { if !Self::validation_enabled() { return service.notification(n).await; @@ -119,9 +124,9 @@ where let params_str = match n.params() { Some(p) => p.as_ref().get(), - None => "[]", + None => return service.notification(n).await, }; - + if let Err(e) = Self::validate_params(params_str) { let err = ErrorObject::owned(-32600, e, None::<()>); return MethodResponse::error(jsonrpsee::types::Id::Null, err); From a2fe9a11618292fda746a4949cde8084a5be667b Mon Sep 17 00:00:00 2001 From: Dyslex7c Date: Thu, 22 Jan 2026 21:06:12 +0530 Subject: [PATCH 5/6] serialize tests and add comments --- src/rpc/json_validator.rs | 8 +++++++- src/rpc/validation_layer.rs | 25 +++++++++++++++---------- 2 files changed, 22 insertions(+), 11 deletions(-) diff --git a/src/rpc/json_validator.rs b/src/rpc/json_validator.rs index 3d658a585fb..212738beb2f 100644 --- a/src/rpc/json_validator.rs +++ b/src/rpc/json_validator.rs @@ -66,6 +66,7 @@ pub fn validate_json_for_duplicates(json_str: &str) -> Result<(), String> { _ => Ok(()), } } + // defer to serde_json for invalid JSON let value = match Value::from_json(json_str) { Ok(v) => v, Err(_) => return Ok(()), @@ -76,6 +77,7 @@ pub fn validate_json_for_duplicates(json_str: &str) -> Result<(), String> { #[cfg(test)] mod tests { use super::*; + use serial_test::serial; fn with_strict_mode(enabled: bool, f: F) where @@ -104,6 +106,7 @@ mod tests { } #[test] + #[serial] fn test_no_duplicates() { with_strict_mode(true, || { let json = r#"{"a": 1, "b": 2, "c": 3}"#; @@ -112,6 +115,7 @@ mod tests { } #[test] + #[serial] fn test_duplicate_keys_detected() { with_strict_mode(true, || { let json = r#"{"/":"cid1", "/":"cid2"}"#; @@ -122,6 +126,7 @@ mod tests { } #[test] + #[serial] fn test_strict_mode_disabled() { with_strict_mode(false, || { // should pass with strict mode disabled let json = r#"{"/":"cid1", "/":"cid2"}"#; @@ -130,7 +135,8 @@ mod tests { } #[test] - fn test_original_issue_case() { + #[serial] + fn test_duplicate_cid_keys() { with_strict_mode(true, || { let json = r#"{ "jsonrpc": "2.0", diff --git a/src/rpc/validation_layer.rs b/src/rpc/validation_layer.rs index 4c9688e2a0a..82a74a6a7c0 100644 --- a/src/rpc/validation_layer.rs +++ b/src/rpc/validation_layer.rs @@ -4,11 +4,16 @@ use futures::future::Either; use jsonrpsee::MethodResponse; use jsonrpsee::server::middleware::rpc::RpcServiceT; +use jsonrpsee::core::middleware::{Batch, BatchEntry, BatchEntryErr, Notification}; use jsonrpsee::types::ErrorObject; use tower::Layer; use super::json_validator; +/// JSON-RPC error code for invalid request +/// See: https://www.jsonrpc.org/specification#error_object +const INVALID_REQUEST: i32 = -32600; + /// stateless jsonrpsee layer for validating JSON-RPC requests #[derive(Clone, Default)] pub(super) struct JsonValidationLayer; @@ -59,7 +64,7 @@ where if let Err(e) = json_validator::validate_json_for_duplicates(req.params().as_str().unwrap_or("[]")) { - let err = ErrorObject::owned(-32600, e, None::<()>); + let err = ErrorObject::owned(INVALID_REQUEST, e, None::<()>); return Either::Right(async move { MethodResponse::error(req.id(), err) }); } @@ -68,7 +73,7 @@ where fn batch<'a>( &self, - mut batch: jsonrpsee::core::middleware::Batch<'a>, + mut batch: Batch<'a>, ) -> impl Future + Send + 'a { let service = self.service.clone(); @@ -86,17 +91,17 @@ where if let Err(e) = Self::validate_params(params_str) { match batch_entry { - jsonrpsee::core::middleware::BatchEntry::Call(req) => { - let err = ErrorObject::owned(-32600, e, None::<()>); - let err_entry = jsonrpsee::core::middleware::BatchEntryErr::new( + BatchEntry::Call(req) => { + let err = ErrorObject::owned(INVALID_REQUEST, e, None::<()>); + let err_entry = BatchEntryErr::new( req.id().clone(), err, ); *entry = Err(err_entry); } - jsonrpsee::core::middleware::BatchEntry::Notification(_notif) => { - let err = ErrorObject::owned(-32600, e, None::<()>); - let err_entry = jsonrpsee::core::middleware::BatchEntryErr::new( + BatchEntry::Notification(_notif) => { + let err = ErrorObject::owned(INVALID_REQUEST, e, None::<()>); + let err_entry = BatchEntryErr::new( jsonrpsee::types::Id::Null, err, ); @@ -113,7 +118,7 @@ where fn notification<'a>( &self, - n: jsonrpsee::core::middleware::Notification<'a>, + n: Notification<'a>, ) -> impl Future + Send + 'a { let service = self.service.clone(); @@ -128,7 +133,7 @@ where }; if let Err(e) = Self::validate_params(params_str) { - let err = ErrorObject::owned(-32600, e, None::<()>); + let err = ErrorObject::owned(INVALID_REQUEST, e, None::<()>); return MethodResponse::error(jsonrpsee::types::Id::Null, err); } From c2d8ac7080f7512c1724597633e87a805d36e052 Mon Sep 17 00:00:00 2001 From: Dyslex7c Date: Sat, 24 Jan 2026 20:45:32 +0530 Subject: [PATCH 6/6] fix lint --- .config/forest.dic | 4 ++++ src/rpc/json_validator.rs | 7 ++++--- src/rpc/validation_layer.rs | 13 +++---------- 3 files changed, 11 insertions(+), 13 deletions(-) diff --git a/.config/forest.dic b/.config/forest.dic index e503ebf36b9..2dd350500dd 100644 --- a/.config/forest.dic +++ b/.config/forest.dic @@ -39,6 +39,7 @@ daemonize Datacap devnet DB/S +deduplicates deserialize/D destructuring DNS @@ -70,6 +71,8 @@ IPFS ip IPLD JSON +jsonrpc +jsonrpsee JWT Kademlia Kubernetes @@ -110,6 +113,7 @@ SECP256k1 seekable serializable serializer/SM +serde_json skippable statediff stateful diff --git a/src/rpc/json_validator.rs b/src/rpc/json_validator.rs index 212738beb2f..cfb5e1e31b6 100644 --- a/src/rpc/json_validator.rs +++ b/src/rpc/json_validator.rs @@ -4,7 +4,7 @@ //! JSON validation utilities for detecting duplicate keys before serde_json processing. //! //! serde_json automatically deduplicates keys at parse time using a "last-write-wins" strategy -//! This means JSON like `{"/":"cid1", "/":"cid2"}` will keep only the last value, which can lead to unexpected behaviour in RPC calls. +//! This means JSON like `{"/":"cid1", "/":"cid2"}` will keep only the last value, which can lead to unexpected behavior in RPC calls. use ahash::HashSet; use justjson::Value; @@ -128,7 +128,8 @@ mod tests { #[test] #[serial] fn test_strict_mode_disabled() { - with_strict_mode(false, || { // should pass with strict mode disabled + with_strict_mode(false, || { + // should pass with strict mode disabled let json = r#"{"/":"cid1", "/":"cid2"}"#; assert!(validate_json_for_duplicates(json).is_ok()); }); @@ -148,7 +149,7 @@ mod tests { "/":"bafy2bzaceav6j67epppz5ib55v5ty26dhkq4jinbsizq2olb3azbzxvfmc73o" }]] }"#; - + let result = validate_json_for_duplicates(json); assert!(result.is_err()); assert!(result.unwrap_err().contains("duplicate key '/'")); diff --git a/src/rpc/validation_layer.rs b/src/rpc/validation_layer.rs index 82a74a6a7c0..ee0cd810368 100644 --- a/src/rpc/validation_layer.rs +++ b/src/rpc/validation_layer.rs @@ -3,15 +3,14 @@ use futures::future::Either; use jsonrpsee::MethodResponse; -use jsonrpsee::server::middleware::rpc::RpcServiceT; use jsonrpsee::core::middleware::{Batch, BatchEntry, BatchEntryErr, Notification}; +use jsonrpsee::server::middleware::rpc::RpcServiceT; use jsonrpsee::types::ErrorObject; use tower::Layer; use super::json_validator; /// JSON-RPC error code for invalid request -/// See: https://www.jsonrpc.org/specification#error_object const INVALID_REQUEST: i32 = -32600; /// stateless jsonrpsee layer for validating JSON-RPC requests @@ -93,18 +92,12 @@ where match batch_entry { BatchEntry::Call(req) => { let err = ErrorObject::owned(INVALID_REQUEST, e, None::<()>); - let err_entry = BatchEntryErr::new( - req.id().clone(), - err, - ); + let err_entry = BatchEntryErr::new(req.id().clone(), err); *entry = Err(err_entry); } BatchEntry::Notification(_notif) => { let err = ErrorObject::owned(INVALID_REQUEST, e, None::<()>); - let err_entry = BatchEntryErr::new( - jsonrpsee::types::Id::Null, - err, - ); + let err_entry = BatchEntryErr::new(jsonrpsee::types::Id::Null, err); *entry = Err(err_entry); } }