From 02772959771a064125ed72c89810eb77a78b15d0 Mon Sep 17 00:00:00 2001 From: Gabbi Fisher Date: Thu, 22 Aug 2019 13:51:35 -0700 Subject: [PATCH 1/6] check bulk upload api for excessive key sizes, value sizes, and pair count --- src/commands/kv/write_bulk.rs | 36 +++++++++++++++++++++++++++++++++-- 1 file changed, 34 insertions(+), 2 deletions(-) diff --git a/src/commands/kv/write_bulk.rs b/src/commands/kv/write_bulk.rs index 41f209b07..1894c36a0 100644 --- a/src/commands/kv/write_bulk.rs +++ b/src/commands/kv/write_bulk.rs @@ -13,6 +13,10 @@ use failure::bail; use crate::terminal::message; +const MAX_PAYLOAD_SIZE: usize = 100 * 1024 * 1024; // 100MB +const MAX_KEY_SIZE: usize = 512; +const MAX_VALUE_SIZE: usize = 2 * 1024 * 1024; // 2 MB + pub fn write_bulk(namespace_id: &str, filename: &Path) -> Result<(), failure::Error> { let client = super::api_client()?; let account_id = super::account_id()?; @@ -38,10 +42,38 @@ pub fn write_bulk(namespace_id: &str, filename: &Path) -> Result<(), failure::Er Err(e) => bail!(e), }; + // Validate that bulk upload is within size constraints + let pairs = pairs?; + // First check number of pairs is under limit + if pairs.len() > MAX_PAIRS { + bail!( + "Number of key-value pairs to upload {} exceeds max of {}", + pairs.len(), + MAX_PAIRS + ); + } + // Next, iterate over keys and values and make sure each is under limit + for pair in pairs.clone() { + if pair.key.len() > MAX_KEY_SIZE { + bail!( + "key {} is too large; it is over {} bytes", + pair.key, + MAX_KEY_SIZE + ); + } + if pair.value.len() > MAX_VALUE_SIZE { + bail!( + "value for key {} is too large; it is over {} bytes", + pair.key, + MAX_VALUE_SIZE + ); + } + } + let response = client.request(&WriteBulk { account_identifier: &account_id, namespace_identifier: namespace_id, - bulk_key_value_pairs: pairs?, + bulk_key_value_pairs: pairs, }); match response { @@ -64,7 +96,7 @@ fn parse_directory(directory: &Path) -> Result, failure::Error // Need to base64 encode value let b64_value = base64::encode(&value); - message::working(&format!("Uploading {}...", key.clone())); + message::working(&format!("Parsing {}...", key.clone())); upload_vec.push(KeyValuePair { key: key, value: b64_value, From 55583727855d80fc0674dcea710666f29380a8d6 Mon Sep 17 00:00:00 2001 From: Gabbi Fisher Date: Thu, 22 Aug 2019 13:53:28 -0700 Subject: [PATCH 2/6] add bulk delete validation logic --- src/commands/kv/delete_bulk.rs | 15 ++++++++++++++- src/commands/kv/write_bulk.rs | 4 ++-- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/src/commands/kv/delete_bulk.rs b/src/commands/kv/delete_bulk.rs index 3fbf6a112..6c37398e2 100644 --- a/src/commands/kv/delete_bulk.rs +++ b/src/commands/kv/delete_bulk.rs @@ -12,6 +12,8 @@ use failure::bail; use crate::terminal::message; +const MAX_PAIRS: usize = 10000; + pub fn delete_bulk(namespace_id: &str, filename: &Path) -> Result<(), failure::Error> { let client = super::api_client()?; let account_id = super::account_id()?; @@ -36,10 +38,21 @@ pub fn delete_bulk(namespace_id: &str, filename: &Path) -> Result<(), failure::E Err(e) => bail!(e), }; + // Validate that bulk delete is within API constraints + let keys = keys?; + // Check number of pairs is under limit + if keys.len() > MAX_PAIRS { + bail!( + "Number of keys to delete ({}) exceeds max of {}", + keys.len(), + MAX_PAIRS + ); + } + let response = client.request(&DeleteBulk { account_identifier: &account_id, namespace_identifier: namespace_id, - bulk_keys: keys?, + bulk_keys: keys, }); match response { diff --git a/src/commands/kv/write_bulk.rs b/src/commands/kv/write_bulk.rs index 1894c36a0..dc8ece6fb 100644 --- a/src/commands/kv/write_bulk.rs +++ b/src/commands/kv/write_bulk.rs @@ -13,7 +13,7 @@ use failure::bail; use crate::terminal::message; -const MAX_PAYLOAD_SIZE: usize = 100 * 1024 * 1024; // 100MB +const MAX_PAIRS: usize = 10000; const MAX_KEY_SIZE: usize = 512; const MAX_VALUE_SIZE: usize = 2 * 1024 * 1024; // 2 MB @@ -47,7 +47,7 @@ pub fn write_bulk(namespace_id: &str, filename: &Path) -> Result<(), failure::Er // First check number of pairs is under limit if pairs.len() > MAX_PAIRS { bail!( - "Number of key-value pairs to upload {} exceeds max of {}", + "Number of key-value pairs to upload ({}) exceeds max of {}", pairs.len(), MAX_PAIRS ); From 542fc58e505372c2ed9242e034aba2ef9255be9d Mon Sep 17 00:00:00 2001 From: Gabbi Fisher Date: Thu, 22 Aug 2019 13:58:56 -0700 Subject: [PATCH 3/6] make wording more accurate --- src/commands/kv/write_bulk.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/commands/kv/write_bulk.rs b/src/commands/kv/write_bulk.rs index dc8ece6fb..a3be3247f 100644 --- a/src/commands/kv/write_bulk.rs +++ b/src/commands/kv/write_bulk.rs @@ -70,6 +70,8 @@ pub fn write_bulk(namespace_id: &str, filename: &Path) -> Result<(), failure::Er } } + message::working("Parsing successful. Uploading all files above"); + let response = client.request(&WriteBulk { account_identifier: &account_id, namespace_identifier: namespace_id, From c5428f74efe8ee9e2d6669ba51c30dbf9ed6f198 Mon Sep 17 00:00:00 2001 From: Gabbi Fisher Date: Thu, 22 Aug 2019 14:21:10 -0700 Subject: [PATCH 4/6] Add better error messaging for cases where only HTTP status code exposes error --- Cargo.lock | 1 + Cargo.toml | 1 + src/commands/kv/mod.rs | 13 ++++++++++++- 3 files changed, 14 insertions(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index f5cba5341..88b38d469 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2396,6 +2396,7 @@ dependencies = [ "flate2 1.0.11 (registry+https://github.com/rust-lang/crates.io-index)", "fs2 0.4.3 (registry+https://github.com/rust-lang/crates.io-index)", "fs_extra 1.1.0 (registry+https://github.com/rust-lang/crates.io-index)", + "http 0.1.18 (registry+https://github.com/rust-lang/crates.io-index)", "lazy_static 1.3.0 (registry+https://github.com/rust-lang/crates.io-index)", "log 0.4.8 (registry+https://github.com/rust-lang/crates.io-index)", "notify 4.0.12 (registry+https://github.com/rust-lang/crates.io-index)", diff --git a/Cargo.toml b/Cargo.toml index 9176404e2..613fc8549 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -42,6 +42,7 @@ ws = "0.9.0" url = "2.1.0" walkdir = "2.2.9" percent-encoding = "1.0.1" +http = "0.1.1" [dev-dependencies] assert_cmd = "0.11.1" diff --git a/src/commands/kv/mod.rs b/src/commands/kv/mod.rs index 2363fea4c..304602d0f 100644 --- a/src/commands/kv/mod.rs +++ b/src/commands/kv/mod.rs @@ -4,6 +4,7 @@ use std::path::Path; use cloudflare::framework::auth::Credentials; use cloudflare::framework::response::ApiFailure; use cloudflare::framework::HttpApiClient; +use http::status::StatusCode; use crate::settings; use crate::terminal::message; @@ -47,7 +48,8 @@ fn account_id() -> Result { fn print_error(e: ApiFailure) { match e { - ApiFailure::Error(_status, api_errors) => { + ApiFailure::Error(status, api_errors) => { + give_status_code_context(status); for error in api_errors.errors { message::warn(&format!("Error {}: {}", error.code, error.message)); @@ -61,6 +63,15 @@ fn print_error(e: ApiFailure) { } } +// For handling cases where the API gateway returns errors via HTTP status codes +// (no KV error code is given). +fn give_status_code_context(status_code: StatusCode) { + match status_code { + StatusCode::PAYLOAD_TOO_LARGE => message::warn("Returned status code 413, Payload Too Large. Make sure your upload is less than 100MB in size"), + _ => (), + } +} + fn help(error_code: u16) -> &'static str { // https://api.cloudflare.com/#workers-kv-namespace-errors match error_code { From f3d9965d21d9dc3bed4e5242315e6b96fde8e03b Mon Sep 17 00:00:00 2001 From: Gabbi Fisher Date: Thu, 22 Aug 2019 16:50:10 -0700 Subject: [PATCH 5/6] replace clone with borrow --- src/commands/kv/write_bulk.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/commands/kv/write_bulk.rs b/src/commands/kv/write_bulk.rs index a3be3247f..8f0072418 100644 --- a/src/commands/kv/write_bulk.rs +++ b/src/commands/kv/write_bulk.rs @@ -53,7 +53,7 @@ pub fn write_bulk(namespace_id: &str, filename: &Path) -> Result<(), failure::Er ); } // Next, iterate over keys and values and make sure each is under limit - for pair in pairs.clone() { + for pair in &pairs { if pair.key.len() > MAX_KEY_SIZE { bail!( "key {} is too large; it is over {} bytes", From 7fa6174cbb31dcc5fb291a70ab422142b0dc7edc Mon Sep 17 00:00:00 2001 From: Gabbi Fisher Date: Fri, 23 Aug 2019 11:58:11 -0700 Subject: [PATCH 6/6] remove errors now h andled / clarified by workers kv api --- src/commands/kv/write_bulk.rs | 20 -------------------- 1 file changed, 20 deletions(-) diff --git a/src/commands/kv/write_bulk.rs b/src/commands/kv/write_bulk.rs index 8f0072418..072b63f3a 100644 --- a/src/commands/kv/write_bulk.rs +++ b/src/commands/kv/write_bulk.rs @@ -14,8 +14,6 @@ use failure::bail; use crate::terminal::message; const MAX_PAIRS: usize = 10000; -const MAX_KEY_SIZE: usize = 512; -const MAX_VALUE_SIZE: usize = 2 * 1024 * 1024; // 2 MB pub fn write_bulk(namespace_id: &str, filename: &Path) -> Result<(), failure::Error> { let client = super::api_client()?; @@ -44,7 +42,6 @@ pub fn write_bulk(namespace_id: &str, filename: &Path) -> Result<(), failure::Er // Validate that bulk upload is within size constraints let pairs = pairs?; - // First check number of pairs is under limit if pairs.len() > MAX_PAIRS { bail!( "Number of key-value pairs to upload ({}) exceeds max of {}", @@ -52,23 +49,6 @@ pub fn write_bulk(namespace_id: &str, filename: &Path) -> Result<(), failure::Er MAX_PAIRS ); } - // Next, iterate over keys and values and make sure each is under limit - for pair in &pairs { - if pair.key.len() > MAX_KEY_SIZE { - bail!( - "key {} is too large; it is over {} bytes", - pair.key, - MAX_KEY_SIZE - ); - } - if pair.value.len() > MAX_VALUE_SIZE { - bail!( - "value for key {} is too large; it is over {} bytes", - pair.key, - MAX_VALUE_SIZE - ); - } - } message::working("Parsing successful. Uploading all files above");