chore: add DecoderConfig when decoding candid in BN code

dfinity · Mar 13, 2024 · 7625fb2 · 7625fb2
1 parent bb0ac2e
commit 7625fb2
Show file tree

Hide file tree

Showing 8 changed files with 68 additions and 22 deletions.
diff --git a/rs/boundary_node/certificate_issuance/certificate_issuer/src/certificate.rs b/rs/boundary_node/certificate_issuance/certificate_issuer/src/certificate.rs
@@ -10,6 +10,7 @@ use mockall::automock;
 use serde::Serialize;
 
 use crate::{
+    decoder_config,
     encode::{Decode, Encode},
     verification::Verify,
 };
@@ -102,7 +103,8 @@ impl GetCert for CanisterCertGetter {
             .await
             .context("failed to query canister")?;
 
-        let resp = Decode!(&resp, Response).context("failed to decode canister response")?;
+        let resp = Decode!([decoder_config()]; &resp, Response)
+            .context("failed to decode canister response")?;
 
         match resp {
             Response::Ok(enc_pair) => Ok(Pair(
@@ -154,7 +156,8 @@ impl Upload for CanisterUploader {
             .await
             .context("failed to query canister")?;
 
-        let resp = Decode!(&resp, Response).context("failed to decode canister response")?;
+        let resp = Decode!([decoder_config()]; &resp, Response)
+            .context("failed to decode canister response")?;
 
         match resp {
             Response::Ok(()) => Ok(()),
@@ -199,7 +202,8 @@ impl Export for CanisterExporter {
             .await
             .context("failed to query canister")?;
 
-        let resp = Decode!(&resp, Response).context("failed to decode canister response")?;
+        let resp = Decode!([decoder_config()]; &resp, Response)
+            .context("failed to decode canister response")?;
 
         match resp {
             Response::Ok((pkgs, iccert)) => Ok((

diff --git a/rs/boundary_node/certificate_issuance/certificate_issuer/src/main.rs b/rs/boundary_node/certificate_issuance/certificate_issuer/src/main.rs
@@ -20,7 +20,7 @@ use axum::{
     routing::{delete, get, post, put},
     Extension, Router, Server,
 };
-use candid::Principal;
+use candid::{DecoderConfig, Principal};
 use chacha20poly1305::{KeyInit, XChaCha20Poly1305};
 use clap::Parser;
 use futures::future::TryFutureExt;
@@ -82,6 +82,17 @@ const SERVICE_NAME: &str = "certificate-issuer";
 pub(crate) static TASK_DELAY_SEC: AtomicU64 = AtomicU64::new(60);
 pub(crate) static TASK_ERROR_DELAY_SEC: AtomicU64 = AtomicU64::new(10 * 60);
 
+/// Limit the amount of work for skipping unneeded data on the wire when parsing Candid.
+/// The value of 10_000 follows the Candid recommendation.
+const DEFAULT_SKIPPING_QUOTA: usize = 10_000;
+
+pub(crate) fn decoder_config() -> DecoderConfig {
+    let mut config = DecoderConfig::new();
+    config.set_skipping_quota(DEFAULT_SKIPPING_QUOTA);
+    config.set_full_error_message(false);
+    config
+}
+
 #[derive(Parser)]
 #[command(name = SERVICE_NAME)]
 struct Cli {

diff --git a/rs/boundary_node/certificate_issuance/certificate_issuer/src/registration.rs b/rs/boundary_node/certificate_issuance/certificate_issuer/src/registration.rs
@@ -8,6 +8,7 @@ use ic_agent::Agent;
 use mockall::automock;
 use serde::{Deserialize, Serialize};
 
+use crate::decoder_config;
 use crate::work::ProcessError;
 
 pub type Id = String;
@@ -169,7 +170,8 @@ impl Get for CanisterGetter {
             .await
             .context("failed to query canister")?;
 
-        let resp = Decode!(&resp, Response).context("failed to decode canister response")?;
+        let resp = Decode!([decoder_config()]; &resp, Response)
+            .context("failed to decode canister response")?;
 
         match resp {
             Response::Ok(reg) => Ok(reg.into()),
@@ -199,7 +201,8 @@ impl Create for CanisterCreator {
             .await
             .context("failed to query canister")?;
 
-        let resp = Decode!(&resp, Response).context("failed to decode canister response")?;
+        let resp = Decode!([decoder_config()]; &resp, Response)
+            .context("failed to decode canister response")?;
 
         match resp {
             Response::Ok(id) => Ok(id),
@@ -232,7 +235,8 @@ impl Update for CanisterUpdater {
             .await
             .context("failed to query canister")?;
 
-        let resp = Decode!(&resp, Response).context("failed to decode canister response")?;
+        let resp = Decode!([decoder_config()]; &resp, Response)
+            .context("failed to decode canister response")?;
 
         match resp {
             Response::Ok(()) => Ok(()),
@@ -262,7 +266,8 @@ impl Remove for CanisterRemover {
             .await
             .context("failed to query canister")?;
 
-        let resp = Decode!(&resp, Response).context("failed to decode canister response")?;
+        let resp = Decode!([decoder_config()]; &resp, Response)
+            .context("failed to decode canister response")?;
 
         match resp {
             Response::Ok(()) => Ok(()),

diff --git a/rs/boundary_node/certificate_issuance/certificate_issuer/src/work.rs b/rs/boundary_node/certificate_issuance/certificate_issuer/src/work.rs
@@ -15,6 +15,7 @@ use crate::{
     acme::{self, FinalizeError},
     certificate::{self, GetCert, GetCertError, Pair},
     check::Check,
+    decoder_config,
     dns::{self, Resolve},
     registration::{Id, Registration, State},
     TASK_DELAY_SEC, TASK_ERROR_DELAY_SEC,
@@ -152,7 +153,8 @@ impl Queue for CanisterQueuer {
             .await
             .context("failed to query canister")?;
 
-        let resp = Decode!(&resp, Response).context("failed to decode canister response")?;
+        let resp = Decode!([decoder_config()]; &resp, Response)
+            .context("failed to decode canister response")?;
 
         match resp {
             Response::Ok(()) => Ok(()),
@@ -182,7 +184,8 @@ impl Peek for CanisterPeeker {
             .await
             .context("failed to query canister")?;
 
-        let resp = Decode!(&resp, Response).context("failed to decode canister response")?;
+        let resp = Decode!([decoder_config()]; &resp, Response)
+            .context("failed to decode canister response")?;
 
         match resp {
             Response::Ok(id) => Ok(id),
@@ -213,7 +216,8 @@ impl Dispense for CanisterDispenser {
                 .await
                 .context("failed to query canister")?;
 
-            let resp = Decode!(&resp, Response).context("failed to decode canister response")?;
+            let resp = Decode!([decoder_config()]; &resp, Response)
+                .context("failed to decode canister response")?;
 
             match resp {
                 Response::Ok(id) => Ok(id),
@@ -238,7 +242,8 @@ impl Dispense for CanisterDispenser {
                 .await
                 .context("failed to query canister")?;
 
-            let resp = Decode!(&resp, Response).context("failed to decode canister response")?;
+            let resp = Decode!([decoder_config()]; &resp, Response)
+                .context("failed to decode canister response")?;
 
             match resp {
                 Response::Ok(reg) => Ok(reg.into()),

diff --git a/rs/boundary_node/ic_balance_exporter/src/main.rs b/rs/boundary_node/ic_balance_exporter/src/main.rs
@@ -9,7 +9,7 @@ use std::{
 use anyhow::{anyhow, Context, Error};
 use async_trait::async_trait;
 use axum::{handler::Handler, routing::get, Extension, Router};
-use candid::{CandidType, Decode, Encode, Principal};
+use candid::{CandidType, Decode, DecoderConfig, Encode, Principal};
 use clap::Parser;
 use dashmap::DashMap;
 use futures::{future::TryFutureExt, stream::FuturesUnordered};
@@ -232,8 +232,15 @@ impl Scrape for Scraper {
             .await
             .context("failed to query canister")?;
 
+        // Limit the amount of work for skipping unneeded data on the wire when parsing Candid.
+        // The value of 10_000 follows the Candid recommendation.
+        const DEFAULT_SKIPPING_QUOTA: usize = 10_000;
+        let mut config = DecoderConfig::new();
+        config.set_skipping_quota(DEFAULT_SKIPPING_QUOTA);
+        config.set_full_error_message(false);
+
         let Amount { amount } =
-            candid::Decode!(&result, Amount).context("failed to decode result")?;
+            candid::Decode!([config]; &result, Amount).context("failed to decode result")?;
 
         Ok(amount)
     }

diff --git a/rs/boundary_node/ic_boundary/src/core.rs b/rs/boundary_node/ic_boundary/src/core.rs
@@ -19,6 +19,7 @@ use axum::{
 };
 use axum_extra::middleware::option_layer;
 use axum_server::{accept::DefaultAcceptor, Server};
+use candid::DecoderConfig;
 use futures::TryFutureExt;
 use ic_interfaces_registry::ZERO_REGISTRY_VERSION;
 use ic_registry_client::client::RegistryClientImpl;
@@ -75,6 +76,17 @@ const METRICS_CACHE_CAPACITY: usize = 15 * MB;
 
 pub const MANAGEMENT_CANISTER_ID_PRINCIPAL: CanisterId = CanisterId::ic_00();
 
+/// Limit the amount of work for skipping unneeded data on the wire when parsing Candid.
+/// The value of 10_000 follows the Candid recommendation.
+const DEFAULT_SKIPPING_QUOTA: usize = 10_000;
+
+pub fn decoder_config() -> DecoderConfig {
+    let mut config = DecoderConfig::new();
+    config.set_skipping_quota(DEFAULT_SKIPPING_QUOTA);
+    config.set_full_error_message(false);
+    config
+}
+
 pub async fn main(cli: Cli) -> Result<(), Error> {
     if cli.listen.http_timeout_connect > cli.health.check_timeout {
         panic!("--check-timeout should be longer than --http-timeout-connect");

diff --git a/rs/boundary_node/ic_boundary/src/management.rs b/rs/boundary_node/ic_boundary/src/management.rs
@@ -13,7 +13,7 @@ use ratelimit::Ratelimiter;
 use serde::Deserialize;
 
 use crate::{
-    core::MANAGEMENT_CANISTER_ID_PRINCIPAL,
+    core::{decoder_config, MANAGEMENT_CANISTER_ID_PRINCIPAL},
     routes::{ApiError, ErrorCause, RateLimitCause, RequestContext, RequestType},
 };
 
@@ -88,7 +88,8 @@ struct BitcoinNetworkRecord {
 
 fn extract_btc_network(ctx: &RequestContext) -> Result<BitcoinNetwork, Error> {
     let arg = ctx.arg.as_ref().ok_or_else(|| anyhow!("missing arg"))?;
-    let r = Decode!(arg, BitcoinNetworkRecord).context("failed to decode arg")?;
+    let r =
+        Decode!([decoder_config()]; arg, BitcoinNetworkRecord).context("failed to decode arg")?;
 
     Ok(r.network.into())
 }

diff --git a/rs/boundary_node/ic_boundary/src/routes.rs b/rs/boundary_node/ic_boundary/src/routes.rs
@@ -36,7 +36,7 @@ use url::Url;
 
 use crate::{
     cache::CacheStatus,
-    core::MAX_REQUEST_BODY_SIZE,
+    core::{decoder_config, MAX_REQUEST_BODY_SIZE},
     http::{read_streaming_body, reqwest_error_infer, HttpClient},
     persist::{RouteSubnet, Routes},
     retry::RetryResult,
@@ -579,11 +579,12 @@ pub async fn preprocess_request(
     let (arg, http_request) = match (&content.method_name, content.arg) {
         (Some(method), Some(arg)) => {
             if request_type == RequestType::Query && method == METHOD_HTTP {
-                let mut req: HttpRequest = Decode!(&arg.0, HttpRequest).map_err(|err| {
-                    ErrorCause::UnableToParseHTTPArg(format!(
-                        "unable to decode arg as HttpRequest: {err}"
-                    ))
-                })?;
+                let mut req: HttpRequest = Decode!([decoder_config()]; &arg.0, HttpRequest)
+                    .map_err(|err| {
+                        ErrorCause::UnableToParseHTTPArg(format!(
+                            "unable to decode arg as HttpRequest: {err}"
+                        ))
+                    })?;
 
                 // Remove specific headers
                 req.headers