From 98dae683af8d94ef880736429ec874e0f3f0bbc6 Mon Sep 17 00:00:00 2001 From: mack42 Date: Wed, 13 May 2026 06:52:37 -0400 Subject: [PATCH 1/5] Batch 1: CONFIG SET honesty, CI hygiene, low-impact perf cleanup Closes #31 #35 #36. #31 CONFIG SET - Stop returning OK without applying changes. Return ERR explaining the parameter must be set via the matching CLI flag at startup. Silent no-op was actively misleading. #35 CI hygiene - Trim tokio features from "full" to the eight actually used (rt-multi-thread, net, io-util, sync, time, macros, signal, fs) - Trim hyper to ["server", "http1"]; hyper-util to ["tokio"] - Tag legitimate-but-unused API surface with #[allow(dead_code)] so #![warn(dead_code)] at the crate root catches accidental dead code from now on - Add .github/workflows/ci.yml running cargo check + clippy (-D warnings) + test + fmt on every PR and push to main #36 Low-impact perf cleanup - maxmemory_policy: borrow instead of cloning the String per write - ZRANGEBYSCORE: fold take_while + filter into a single filter_map - GETRANGE: zero-copy via Bytes::slice instead of slice.to_vec() - MGET: iterate args by index instead of building an intermediate Vec Bump version 0.11.0 -> 0.11.1. --- .github/workflows/ci.yml | 44 +++++++++ Cargo.lock | 167 ----------------------------------- Cargo.toml | 8 +- src/command/probabilistic.rs | 2 + src/command/registry.rs | 2 + src/command/server_cmds.rs | 7 +- src/command/strings.rs | 16 ++-- src/main.rs | 2 + src/server/connection.rs | 18 ++-- src/server/mod.rs | 4 + src/storage/types.rs | 6 +- 11 files changed, 89 insertions(+), 187 deletions(-) create mode 100644 .github/workflows/ci.yml diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..e5d03f1 --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,44 @@ +name: CI + +on: + push: + branches: [main] + pull_request: + branches: [main] + +env: + CARGO_TERM_COLOR: always + RUSTFLAGS: "-D warnings" + +jobs: + check: + name: cargo check + clippy + test + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + + - name: Install Rust toolchain + run: | + rustup show + rustup component add clippy rustfmt + + - name: Cache cargo registry + uses: actions/cache@v4 + with: + path: | + ~/.cargo/registry + ~/.cargo/git + target + key: ${{ runner.os }}-cargo-${{ hashFiles('**/Cargo.lock') }} + + - name: cargo check + run: cargo check --all-targets + + - name: cargo clippy + run: cargo clippy --all-targets -- -D warnings + + - name: cargo test + run: cargo test --all-targets + + - name: cargo fmt + run: cargo fmt --all -- --check diff --git a/Cargo.lock b/Cargo.lock index 777ab14..62b9040 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -65,12 +65,6 @@ dependencies = [ "fs_extra", ] -[[package]] -name = "base64" -version = "0.22.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "72b3254f16251a8381aa12e40e3c4d2f0199f8c6508fbecb9d91f575e0fbb8c6" - [[package]] name = "bitflags" version = "2.11.0" @@ -143,22 +137,6 @@ dependencies = [ "tokio-util", ] -[[package]] -name = "core-foundation" -version = "0.9.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "91e195e091a93c46f7102ec7818a2aa394e1e1771c3ab4825963fa03e45afb8f" -dependencies = [ - "core-foundation-sys", - "libc", -] - -[[package]] -name = "core-foundation-sys" -version = "0.8.7" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "773648b94d0e5d620f64f280777445740e61fe701025087ec8b57f45c791888b" - [[package]] name = "cpufeatures" version = "0.2.17" @@ -223,12 +201,6 @@ version = "0.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c7f84e12ccf0a7ddc17a6c41c93326024c42920d7ee630d04950e6926645c0fe" -[[package]] -name = "equivalent" -version = "1.0.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "877a4ace8713b0bcf2a4e7eec82529c029f1d0619886d18145fea96c3ffe5c0f" - [[package]] name = "errno" version = "0.3.14" @@ -245,12 +217,6 @@ version = "0.1.9" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "5baebc0774151f905a1a2cc41989300b1e6fbb29aff0ceffa1064fdd3088d582" -[[package]] -name = "fnv" -version = "1.0.7" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3f9eec918d3f24069decb9af1554cad7c880e2da24a9afd88aca000531ab82c1" - [[package]] name = "form_urlencoded" version = "1.2.2" @@ -339,31 +305,6 @@ dependencies = [ "wasip2", ] -[[package]] -name = "h2" -version = "0.4.13" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2f44da3a8150a6703ed5d34e164b875fd14c2cdab9af1252a9a1020bde2bdc54" -dependencies = [ - "atomic-waker", - "bytes", - "fnv", - "futures-core", - "futures-sink", - "http", - "indexmap", - "slab", - "tokio", - "tokio-util", - "tracing", -] - -[[package]] -name = "hashbrown" -version = "0.16.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "841d1cc9bed7f9236f321df977030373f4a4163ae1a7dbfe1a51a2c1a51d9100" - [[package]] name = "http" version = "1.4.0" @@ -419,7 +360,6 @@ dependencies = [ "bytes", "futures-channel", "futures-core", - "h2", "http", "http-body", "httparse", @@ -428,7 +368,6 @@ dependencies = [ "pin-project-lite", "smallvec", "tokio", - "want", ] [[package]] @@ -437,24 +376,12 @@ version = "0.1.20" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "96547c2556ec9d12fb1578c4eaf448b04993e7fb79cbaad930a656880a6bdfa0" dependencies = [ - "base64", "bytes", - "futures-channel", - "futures-util", "http", "http-body", "hyper", - "ipnet", - "libc", - "percent-encoding", "pin-project-lite", - "socket2 0.6.3", - "system-configuration", "tokio", - "tower-layer", - "tower-service", - "tracing", - "windows-registry", ] [[package]] @@ -560,22 +487,6 @@ dependencies = [ "icu_properties", ] -[[package]] -name = "indexmap" -version = "2.13.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "45a8a2b9cb3e0b0c1803dbb0758ffac5de2f425b23c28f518faabd9d805342ff" -dependencies = [ - "equivalent", - "hashbrown", -] - -[[package]] -name = "ipnet" -version = "2.12.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d98f6fed1fde3f8c21bc40a1abb88dd75e67924f9cffc3ef95607bad8017f8e2" - [[package]] name = "itertools" version = "0.13.0" @@ -1210,27 +1121,6 @@ dependencies = [ "syn", ] -[[package]] -name = "system-configuration" -version = "0.7.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a13f3d0daba03132c0aa9767f98351b3488edc2c100cda2d2ec2b04f3d8d3c8b" -dependencies = [ - "bitflags", - "core-foundation", - "system-configuration-sys", -] - -[[package]] -name = "system-configuration-sys" -version = "0.6.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8e1d1b10ced5ca923a1fcb8d03e96b8d3268065d724548c0211415ff6ac6bac4" -dependencies = [ - "core-foundation-sys", - "libc", -] - [[package]] name = "thiserror" version = "2.0.18" @@ -1279,7 +1169,6 @@ dependencies = [ "bytes", "libc", "mio", - "parking_lot", "pin-project-lite", "signal-hook-registry", "socket2 0.6.3", @@ -1321,18 +1210,6 @@ dependencies = [ "tokio", ] -[[package]] -name = "tower-layer" -version = "0.3.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "121c2a6cda46980bb0fcd1647ffaf6cd3fc79a013de288782836f6df9c48780e" - -[[package]] -name = "tower-service" -version = "0.3.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8df9b6e13f2d32c91b9bd719c00d1958837bc7dec474d94952798cc8e69eeec3" - [[package]] name = "tracing" version = "0.1.44" @@ -1394,12 +1271,6 @@ dependencies = [ "tracing-log", ] -[[package]] -name = "try-lock" -version = "0.2.5" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e421abadd41a4225275504ea4d6566923418b7f05506fbc9c0fe86ba7396114b" - [[package]] name = "twox-hash" version = "2.1.2" @@ -1454,15 +1325,6 @@ version = "0.9.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0b928f33d975fc6ad9f86c8f283853ad26bdd5b10b7f1542aa2fa15e2289105a" -[[package]] -name = "want" -version = "0.3.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "bfa7760aed19e106de2c7c0b581b509f2f25d3dacaf737cb82ac61bc6d760b0e" -dependencies = [ - "try-lock", -] - [[package]] name = "wasi" version = "0.11.1+wasi-snapshot-preview1" @@ -1496,35 +1358,6 @@ version = "0.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f0805222e57f7521d6a62e36fa9163bc891acd422f971defe97d64e70d0a4fe5" -[[package]] -name = "windows-registry" -version = "0.6.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "02752bf7fbdcce7f2a27a742f798510f3e5ad88dbe84871e5168e2120c3d5720" -dependencies = [ - "windows-link", - "windows-result", - "windows-strings", -] - -[[package]] -name = "windows-result" -version = "0.4.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7781fa89eaf60850ac3d2da7af8e5242a5ea78d1a11c49bf2910bb5a73853eb5" -dependencies = [ - "windows-link", -] - -[[package]] -name = "windows-strings" -version = "0.5.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7837d08f69c77cf6b07689544538e017c1bfcf57e34b4c0ff58e6c2cd3b37091" -dependencies = [ - "windows-link", -] - [[package]] name = "windows-sys" version = "0.52.0" diff --git a/Cargo.toml b/Cargo.toml index af7a140..a4b5f42 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,18 +1,18 @@ [package] name = "rcache" -version = "0.11.0" +version = "0.11.1" edition = "2024" [dependencies] -tokio = { version = "1", features = ["full"] } +tokio = { version = "1", features = ["rt-multi-thread", "net", "io-util", "sync", "time", "macros", "signal", "fs"] } bytes = "1" thiserror = "2" tracing = "0.1" tracing-subscriber = { version = "0.3", features = ["env-filter"] } rand = "0.8" crc16 = "0.4" -hyper = { version = "1", features = ["full"] } -hyper-util = { version = "0.1", features = ["tokio", "full"] } +hyper = { version = "1", features = ["server", "http1"] } +hyper-util = { version = "0.1", features = ["tokio"] } http-body-util = "0.1" serde_json = "1" tokio-rustls = "0.26" diff --git a/src/command/probabilistic.rs b/src/command/probabilistic.rs index f0bcaab..57ef76a 100644 --- a/src/command/probabilistic.rs +++ b/src/command/probabilistic.rs @@ -86,6 +86,7 @@ fn bloom_k(data: &[u8]) -> u32 { u32::from_le_bytes(data[16..20].try_into().unwrap()) } +#[allow(dead_code)] // exposed for future BF.INFO ERROR_RATE fn bloom_error_rate(data: &[u8]) -> f64 { f64::from_le_bytes(data[20..28].try_into().unwrap()) } @@ -748,6 +749,7 @@ fn topk_num_items(data: &[u8]) -> u32 { u32::from_le_bytes(data[24..28].try_into().unwrap()) } +#[allow(dead_code)] // mutator paired with topk_num_items getter for future use fn set_topk_num_items(data: &mut [u8], count: u32) { data[24..28].copy_from_slice(&count.to_le_bytes()); } diff --git a/src/command/registry.rs b/src/command/registry.rs index b6ce743..9e78f31 100644 --- a/src/command/registry.rs +++ b/src/command/registry.rs @@ -69,10 +69,12 @@ impl CommandRegistry { (entry.handler)(ctx) } + #[allow(dead_code)] // public registry introspection API pub fn command_count(&self) -> usize { self.commands.len() } + #[allow(dead_code)] // public registry introspection API pub fn command_names(&self) -> Vec { self.commands.keys().cloned().collect() } diff --git a/src/command/server_cmds.rs b/src/command/server_cmds.rs index 2a7376d..89af68b 100644 --- a/src/command/server_cmds.rs +++ b/src/command/server_cmds.rs @@ -200,7 +200,12 @@ pub fn cmd_config(ctx: &mut CommandContext) -> RespValue { if ctx.args.len() < 4 { return RespValue::wrong_arity("config|set"); } - RespValue::ok() + // Returning OK without applying changes misled operators who thought + // their parameter took effect. Be honest until SET is wired into a + // mutable runtime config. Restart with CLI flags to change values. + RespValue::error( + "ERR CONFIG SET is not supported in rCache; restart with the matching CLI flag", + ) } "RESETSTAT" => RespValue::ok(), "REWRITE" => RespValue::ok(), diff --git a/src/command/strings.rs b/src/command/strings.rs index 0d53889..6c9e9b2 100644 --- a/src/command/strings.rs +++ b/src/command/strings.rs @@ -213,11 +213,14 @@ pub fn cmd_psetex(ctx: &mut CommandContext) -> RespValue { } pub fn cmd_mget(ctx: &mut CommandContext) -> RespValue { - let keys: Vec = ctx.args[1..].to_vec(); - let mut results = Vec::with_capacity(keys.len()); - for key in &keys { + let n = ctx.args.len(); + let mut results = Vec::with_capacity(n.saturating_sub(1)); + let mut idx = 1; + while idx < n { + let key = ctx.args[idx].clone(); + idx += 1; let db = ctx.db(); - match db.get(key) { + match db.get(&key) { Some(RedisObject::String(b)) => results.push(RespValue::bulk_string(b.clone())), _ => results.push(RespValue::Null), } @@ -404,8 +407,9 @@ pub fn cmd_getrange(ctx: &mut CommandContext) -> RespValue { if s > e || s >= b.len() { return RespValue::bulk_string(Bytes::new()); } - let slice = &b[s..=e.min(b.len() - 1)]; - RespValue::bulk_string(Bytes::from(slice.to_vec())) + // Zero-copy slice — Bytes::slice shares the underlying buffer via + // refcount instead of allocating a new Vec. + RespValue::bulk_string(b.slice(s..=e.min(b.len() - 1))) } Some(_) => RespValue::wrong_type(), None => RespValue::bulk_string(Bytes::new()), diff --git a/src/main.rs b/src/main.rs index c46911e..88e0207 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1,3 +1,5 @@ +#![warn(dead_code)] + mod protocol; mod server; mod storage; diff --git a/src/server/connection.rs b/src/server/connection.rs index 6951fb5..c365007 100644 --- a/src/server/connection.rs +++ b/src/server/connection.rs @@ -84,6 +84,7 @@ pub struct Connection { tracking_enabled: bool, tracked_keys: HashSet, tracking_tx: Option>, + #[allow(dead_code)] // receiver kept on Connection for future tracking-channel reads tracking_rx: Option>, // Multi-tenancy namespace namespace: Option, @@ -546,12 +547,17 @@ impl Connection { // Check memory eviction before write commands if is_write { - let maxmemory = self.state.config.maxmemory; - let policy = self.state.config.maxmemory_policy.clone(); - let samples = self.state.config.maxmemory_samples; - let lfu_log_factor = self.state.config.lfu_log_factor; - let lfu_decay_time = self.state.config.lfu_decay_time; - if store.check_memory_limit(maxmemory, &policy, samples, lfu_log_factor, lfu_decay_time).is_err() { + let cfg = &self.state.config; + if store + .check_memory_limit( + cfg.maxmemory, + &cfg.maxmemory_policy, + cfg.maxmemory_samples, + cfg.lfu_log_factor, + cfg.lfu_decay_time, + ) + .is_err() + { return RespValue::error("OOM command not allowed when used memory > 'maxmemory'."); } } diff --git a/src/server/mod.rs b/src/server/mod.rs index fb8a1c1..0bb1050 100644 --- a/src/server/mod.rs +++ b/src/server/mod.rs @@ -130,6 +130,7 @@ impl PubSubManager { /// ACL user definition. #[derive(Debug, Clone)] +#[allow(dead_code)] pub struct AclUser { pub enabled: bool, /// SHA-256 hashed passwords. @@ -198,6 +199,7 @@ pub struct LatencyStats { /// A slow log entry. #[derive(Debug, Clone)] +#[allow(dead_code)] pub struct SlowLogEntry { pub id: u64, pub timestamp: u64, @@ -209,11 +211,13 @@ pub struct SlowLogEntry { /// A named namespace containing its own store. #[derive(Debug)] +#[allow(dead_code)] pub struct NamespaceInfo { pub name: String, pub store: Store, } +#[allow(dead_code)] pub struct SharedState { pub store: Mutex, pub config: Config, diff --git a/src/storage/types.rs b/src/storage/types.rs index e523f94..71051d3 100644 --- a/src/storage/types.rs +++ b/src/storage/types.rs @@ -257,10 +257,10 @@ impl SortedSetData { .take_while(|(k, _)| { if max_inclusive { k.score <= max } else { k.score < max } }) - .filter(|(k, _)| { - if min_inclusive { k.score >= min } else { k.score > min } + .filter_map(|(k, _)| { + let in_min = if min_inclusive { k.score >= min } else { k.score > min }; + if in_min { Some((k.member.clone(), k.score)) } else { None } }) - .map(|(k, _)| (k.member.clone(), k.score)) .collect() } From 1e611b1d2ddd35d43377ae07b7eb3953bd2e64aa Mon Sep 17 00:00:00 2001 From: mack42 Date: Wed, 13 May 2026 07:02:11 -0400 Subject: [PATCH 2/5] Batch 2: AUTH backoff, latency-stat gating, Pub/Sub cleanup Closes #30 #32 #33. #30 AUTH brute-force protection - Per-connection consecutive-failure counter, reset on success - After 5 failures, exponential backoff (100ms doubling, capped 5s) applied before returning WRONGPASS - handle_auth / handle_auth_hello are now async and consult ACL via proper .await locking instead of try_lock (the old try_lock could spuriously fail under contention and report WRONGPASS) - Extract AclMatch enum to clean up the auth flow #32 Information-disclosure cleanup - Skip recording latency stats and command-name entries when the response is NOAUTH or WRONGPASS; unauthenticated probes can no longer populate the histograms visible via INFO commandstats / LATENCY HISTORY #33 Pub/Sub dead-sender cleanup - PubSubManager::publish now takes &mut self and prunes any client whose Sender returns an error from send() (channel closed). This removes the slow leak on abrupt-disconnect paths where the normal cleanup_pubsub didn't run Bump version 0.11.1 -> 0.11.2. --- Cargo.lock | 2 +- Cargo.toml | 2 +- src/server/connection.rs | 152 +++++++++++++++++++++++++-------------- src/server/mod.rs | 16 ++++- 4 files changed, 114 insertions(+), 58 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 62b9040..a105975 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -791,7 +791,7 @@ dependencies = [ [[package]] name = "rcache" -version = "0.11.0" +version = "0.11.1" dependencies = [ "bytes", "crc16", diff --git a/Cargo.toml b/Cargo.toml index a4b5f42..f37d81c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "rcache" -version = "0.11.1" +version = "0.11.2" edition = "2024" [dependencies] diff --git a/src/server/connection.rs b/src/server/connection.rs index c365007..7569e22 100644 --- a/src/server/connection.rs +++ b/src/server/connection.rs @@ -59,6 +59,12 @@ impl AsyncWrite for MaybeTls { } } +enum AclMatch { + Ok, + WrongPass, + Disabled, +} + pub struct Connection { stream: MaybeTls, state: Arc, @@ -88,6 +94,8 @@ pub struct Connection { tracking_rx: Option>, // Multi-tenancy namespace namespace: Option, + // AUTH brute-force protection: count consecutive failures, reset on success. + auth_failures: u32, } impl Connection { @@ -117,6 +125,7 @@ impl Connection { tracked_keys: HashSet::new(), tracking_tx: Some(tracking_tx), tracking_rx: Some(tracking_rx), + auth_failures: 0, namespace: None, } } @@ -209,8 +218,14 @@ impl Connection { let elapsed_us = start.elapsed().as_micros() as u64; let response = self.convert_to_resp3(response); - // Record latency stats - { + // Record latency stats. Skip on auth-related rejections so an + // unauthenticated probe can't populate command-name histograms + // visible via INFO commandstats / LATENCY HISTORY. + let auth_rejected = matches!( + &response, + RespValue::Error(e) if e.starts_with("NOAUTH") || e.starts_with("WRONGPASS") + ); + if !auth_rejected { let mut stats = self.state.latency_stats.lock().await; let entry = stats.entry(cmd_name.to_string()).or_default(); entry.count += 1; @@ -302,7 +317,7 @@ impl Connection { // Handle AUTH if cmd_name == "AUTH" { - return self.handle_auth(&args); + return self.handle_auth(&args).await; } // Check authentication @@ -483,7 +498,7 @@ impl Connection { if i + 2 >= args.len() { return RespValue::error("ERR syntax error"); } - let auth_result = self.handle_auth_hello(&args[i + 1], &args[i + 2]); + let auth_result = self.handle_auth_hello(&args[i + 1], &args[i + 2]).await; if let Some(err) = auth_result { return err; } @@ -769,7 +784,30 @@ impl Connection { } } - fn handle_auth(&mut self, args: &[Bytes]) -> RespValue { + /// After `BACKOFF_AFTER` consecutive failed AUTH/HELLO attempts on this + /// connection, sleep before returning the failure response. Doubles each + /// time, capped at 5 s. Resets to zero on a successful auth. + async fn auth_backoff(&mut self) { + const BACKOFF_AFTER: u32 = 5; + const MAX_BACKOFF: std::time::Duration = std::time::Duration::from_secs(5); + + self.auth_failures = self.auth_failures.saturating_add(1); + if self.auth_failures <= BACKOFF_AFTER { + return; + } + let over = self.auth_failures - BACKOFF_AFTER; + let ms: u64 = 100u64.saturating_mul(1u64 << over.min(10)); + let delay = std::time::Duration::from_millis(ms).min(MAX_BACKOFF); + tokio::time::sleep(delay).await; + } + + fn auth_success(&mut self, username: String) { + self.authenticated = true; + self.auth_username = username; + self.auth_failures = 0; + } + + async fn handle_auth(&mut self, args: &[Bytes]) -> RespValue { if args.len() < 2 { return RespValue::wrong_arity("auth"); } @@ -783,14 +821,10 @@ impl Connection { ("default".to_string(), String::from_utf8_lossy(&args[1]).to_string()) }; - // Try ACL-based authentication first - // We can't await here (sync fn), so check against config requirepass as fallback - // For the default user with requirepass, check directly if username == "default" { if let Some(ref req_pass) = self.state.config.requirepass { if password == *req_pass { - self.authenticated = true; - self.auth_username = username; + self.auth_success(username); return RespValue::ok(); } } else { @@ -798,67 +832,79 @@ impl Connection { } } - // For ACL users, check hashed password - // Note: we need to use try_lock since this is a sync function - if let Ok(acl_users) = self.state.acl_users.try_lock() { - if let Some(user) = acl_users.get(&username) { - if !user.enabled { - return RespValue::error("WRONGPASS invalid username-password pair or user is disabled."); - } - if user.no_pass { - self.authenticated = true; - self.auth_username = username; - return RespValue::ok(); - } - // Hash the provided password and compare - use sha2::{Sha256, Digest}; - let hash = format!("{:x}", Sha256::digest(password.as_bytes())); - if user.passwords.contains(&hash) { - self.authenticated = true; - self.auth_username = username; - return RespValue::ok(); + let acl_match = { + let users = self.state.acl_users.lock().await; + users.get(&username).map(|u| { + if !u.enabled { + AclMatch::Disabled + } else if u.no_pass { + AclMatch::Ok + } else { + use sha2::{Digest, Sha256}; + let hash = format!("{:x}", Sha256::digest(password.as_bytes())); + if u.passwords.contains(&hash) { + AclMatch::Ok + } else { + AclMatch::WrongPass + } } + }) + }; + + match acl_match { + Some(AclMatch::Ok) => { + self.auth_success(username); + RespValue::ok() + } + _ => { + self.auth_backoff().await; + RespValue::error("WRONGPASS invalid username-password pair or user is disabled.") } } - - RespValue::error("WRONGPASS invalid username-password pair or user is disabled.") } - fn handle_auth_hello(&mut self, username: &Bytes, password: &Bytes) -> Option { + async fn handle_auth_hello(&mut self, username: &Bytes, password: &Bytes) -> Option { let username = String::from_utf8_lossy(username).to_string(); let password = String::from_utf8_lossy(password).to_string(); if username == "default" { if let Some(ref req_pass) = self.state.config.requirepass { if password == *req_pass { - self.authenticated = true; - self.auth_username = username; + self.auth_success(username); return None; } } } - if let Ok(acl_users) = self.state.acl_users.try_lock() { - if let Some(user) = acl_users.get(&username) { - if !user.enabled { - return Some(RespValue::error("WRONGPASS invalid username-password pair or user is disabled.")); - } - if user.no_pass { - self.authenticated = true; - self.auth_username = username; - return None; - } - use sha2::{Sha256, Digest}; - let hash = format!("{:x}", Sha256::digest(password.as_bytes())); - if user.passwords.contains(&hash) { - self.authenticated = true; - self.auth_username = username; - return None; + let acl_match = { + let users = self.state.acl_users.lock().await; + users.get(&username).map(|u| { + if !u.enabled { + AclMatch::Disabled + } else if u.no_pass { + AclMatch::Ok + } else { + use sha2::{Digest, Sha256}; + let hash = format!("{:x}", Sha256::digest(password.as_bytes())); + if u.passwords.contains(&hash) { + AclMatch::Ok + } else { + AclMatch::WrongPass + } } + }) + }; + + match acl_match { + Some(AclMatch::Ok) => { + self.auth_success(username); + None + } + _ => { + self.auth_backoff().await; + Some(RespValue::error("WRONGPASS invalid username-password pair or user is disabled.")) } } - - Some(RespValue::error("WRONGPASS invalid username-password pair or user is disabled.")) } // === Pub/Sub handlers === @@ -1012,7 +1058,7 @@ impl Connection { } let channel = args[1].clone(); let message = args[2].clone(); - let pubsub = self.state.pubsub.lock().await; + let mut pubsub = self.state.pubsub.lock().await; let count = pubsub.publish(&channel, &message); RespValue::integer(count) } diff --git a/src/server/mod.rs b/src/server/mod.rs index 0bb1050..2831024 100644 --- a/src/server/mod.rs +++ b/src/server/mod.rs @@ -64,10 +64,13 @@ impl PubSubManager { } } - pub fn publish(&self, channel: &Bytes, message: &Bytes) -> i64 { + /// Publish to channel + matching patterns. Returns the number of clients + /// that received the message. Also opportunistically prunes subscribers + /// whose receiver has been dropped (abrupt disconnect path). + pub fn publish(&mut self, channel: &Bytes, message: &Bytes) -> i64 { let mut count = 0i64; + let mut dead: Vec = Vec::new(); - // Exact channel subscribers if let Some(subs) = self.channels.get(channel) { for client_id in subs { if let Some(sender) = self.subscribers.get(client_id) { @@ -78,12 +81,13 @@ impl PubSubManager { ]); if sender.send(msg).is_ok() { count += 1; + } else { + dead.push(*client_id); } } } } - // Pattern subscribers let channel_str = String::from_utf8_lossy(channel); for (pattern, subs) in &self.patterns { let pattern_str = String::from_utf8_lossy(pattern); @@ -98,12 +102,18 @@ impl PubSubManager { ]); if sender.send(msg).is_ok() { count += 1; + } else { + dead.push(*client_id); } } } } } + for client_id in dead { + self.remove_client(client_id); + } + count } From f0b81b3c63dc0980ebb6018c9dfb297e6a2dc4ce Mon Sep 17 00:00:00 2001 From: mack42 Date: Wed, 13 May 2026 07:19:57 -0400 Subject: [PATCH 3/5] Sweep from_utf8_lossy().parse() out of command arg paths MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #27 (with follow-up note below). Add src/command/parse.rs with strict numeric helpers (int, u64_, usize_, float) that use std::str::from_utf8 instead of from_utf8_lossy. The old chain allocated a Cow + an owned String per call and silently accepted malformed UTF-8 in numeric arguments. 96 sites converted across: - sorted_set.rs (12), list.rs (14), geo.rs (14) - strings.rs (13), bitmap.rs (8) - stream.rs (9), keys.rs (5), set.rs (3) - server_cmds.rs (3), hash.rs (3), cluster.rs (3) - scripting.rs (3), scan.rs (4), advanced.rs (1), json.rs (1) Sites left in place (~20): - `.parse().unwrap_or(...)` defaulting calls (no error path to migrate) - `parse::()` in bitmap.rs (no u8 helper yet) - Non-template variants inside nested closures and Result-returning helpers — these need per-site review and were out of scope here Bump version 0.11.2 -> 0.11.3. --- Cargo.lock | 2 +- Cargo.toml | 2 +- src/command/advanced.rs | 6 +-- src/command/bitmap.rs | 48 +++++++++---------- src/command/cluster.rs | 14 +++--- src/command/geo.rs | 84 ++++++++++++++++---------------- src/command/hash.rs | 18 +++---- src/command/json.rs | 6 +-- src/command/keys.rs | 30 ++++++------ src/command/list.rs | 84 ++++++++++++++++---------------- src/command/mod.rs | 1 + src/command/parse.rs | 28 +++++++++++ src/command/scan.rs | 24 +++++----- src/command/scripting.rs | 18 +++---- src/command/server_cmds.rs | 18 +++---- src/command/set.rs | 18 +++---- src/command/sorted_set.rs | 72 ++++++++++++++-------------- src/command/stream.rs | 98 +++++++++++++++----------------------- src/command/strings.rs | 58 +++++++++++----------- 19 files changed, 318 insertions(+), 311 deletions(-) create mode 100644 src/command/parse.rs diff --git a/Cargo.lock b/Cargo.lock index a105975..394e1bf 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -791,7 +791,7 @@ dependencies = [ [[package]] name = "rcache" -version = "0.11.1" +version = "0.11.2" dependencies = [ "bytes", "crc16", diff --git a/Cargo.toml b/Cargo.toml index f37d81c..8689858 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "rcache" -version = "0.11.2" +version = "0.11.3" edition = "2024" [dependencies] diff --git a/src/command/advanced.rs b/src/command/advanced.rs index bb68978..74a80eb 100644 --- a/src/command/advanced.rs +++ b/src/command/advanced.rs @@ -146,9 +146,9 @@ pub fn cmd_lcs(ctx: &mut CommandContext) -> RespValue { if i >= ctx.args.len() { return RespValue::error("ERR syntax error"); } - min_match_len = match String::from_utf8_lossy(&ctx.args[i]).parse() { - Ok(v) => v, - Err(_) => return RespValue::error("ERR value is not an integer or out of range"), + min_match_len = match super::parse::usize_(&ctx.args[i]) { + Some(v) => v, + None => return RespValue::error("ERR value is not an integer or out of range"), }; } "WITHMATCHLEN" => with_match_len = true, diff --git a/src/command/bitmap.rs b/src/command/bitmap.rs index b247b3a..7409c8b 100644 --- a/src/command/bitmap.rs +++ b/src/command/bitmap.rs @@ -22,9 +22,9 @@ fn check_type(ctx: &mut CommandContext, key: &Bytes) -> Result<(), RespValue> { pub fn cmd_setbit(ctx: &mut CommandContext) -> RespValue { let key = ctx.args[1].clone(); - let offset: usize = match String::from_utf8_lossy(&ctx.args[2]).parse() { - Ok(v) => v, - Err(_) => return RespValue::error("ERR bit offset is not an integer or out of range"), + let offset = match super::parse::usize_(&ctx.args[2]) { + Some(v) => v, + None => return RespValue::error("ERR bit offset is not an integer or out of range"), }; let value: u8 = match String::from_utf8_lossy(&ctx.args[3]).parse::() { Ok(v) if v <= 1 => v, @@ -59,9 +59,9 @@ pub fn cmd_setbit(ctx: &mut CommandContext) -> RespValue { pub fn cmd_getbit(ctx: &mut CommandContext) -> RespValue { let key = ctx.args[1].clone(); - let offset: usize = match String::from_utf8_lossy(&ctx.args[2]).parse() { - Ok(v) => v, - Err(_) => return RespValue::error("ERR bit offset is not an integer or out of range"), + let offset = match super::parse::usize_(&ctx.args[2]) { + Some(v) => v, + None => return RespValue::error("ERR bit offset is not an integer or out of range"), }; if let Err(e) = check_type(ctx, &key) { @@ -100,13 +100,13 @@ pub fn cmd_bitcount(ctx: &mut CommandContext) -> RespValue { return RespValue::integer(count as i64); } - let start: i64 = match String::from_utf8_lossy(&ctx.args[2]).parse() { - Ok(v) => v, - Err(_) => return RespValue::error("ERR value is not an integer or out of range"), + let start = match super::parse::int(&ctx.args[2]) { + Some(v) => v, + None => return RespValue::error("ERR value is not an integer or out of range"), }; - let end: i64 = match String::from_utf8_lossy(&ctx.args[3]).parse() { - Ok(v) => v, - Err(_) => return RespValue::error("ERR value is not an integer or out of range"), + let end = match super::parse::int(&ctx.args[3]) { + Some(v) => v, + None => return RespValue::error("ERR value is not an integer or out of range"), }; let use_bit = ctx.args.len() > 4 && @@ -171,14 +171,14 @@ pub fn cmd_bitpos(ctx: &mut CommandContext) -> RespValue { let has_end = ctx.args.len() > 4; let (start_byte, end_byte) = if ctx.args.len() > 3 { - let start: i64 = match String::from_utf8_lossy(&ctx.args[3]).parse() { - Ok(v) => v, - Err(_) => return RespValue::error("ERR value is not an integer or out of range"), + let start = match super::parse::int(&ctx.args[3]) { + Some(v) => v, + None => return RespValue::error("ERR value is not an integer or out of range"), }; let end: i64 = if ctx.args.len() > 4 { - match String::from_utf8_lossy(&ctx.args[4]).parse() { - Ok(v) => v, - Err(_) => return RespValue::error("ERR value is not an integer or out of range"), + match super::parse::int(&ctx.args[4]) { + Some(v) => v, + None => return RespValue::error("ERR value is not an integer or out of range"), } } else { -1 @@ -344,9 +344,9 @@ pub fn cmd_bitfield(ctx: &mut CommandContext) -> RespValue { Some(v) => v, None => return RespValue::error("ERR bit offset is not an integer or out of range"), }; - let value: i64 = match String::from_utf8_lossy(&ctx.args[i + 3]).parse() { - Ok(v) => v, - Err(_) => return RespValue::error("ERR value is not an integer or out of range"), + let value = match super::parse::int(&ctx.args[i + 3]) { + Some(v) => v, + None => return RespValue::error("ERR value is not an integer or out of range"), }; i += 4; @@ -369,9 +369,9 @@ pub fn cmd_bitfield(ctx: &mut CommandContext) -> RespValue { Some(v) => v, None => return RespValue::error("ERR bit offset is not an integer or out of range"), }; - let increment: i64 = match String::from_utf8_lossy(&ctx.args[i + 3]).parse() { - Ok(v) => v, - Err(_) => return RespValue::error("ERR value is not an integer or out of range"), + let increment = match super::parse::int(&ctx.args[i + 3]) { + Some(v) => v, + None => return RespValue::error("ERR value is not an integer or out of range"), }; i += 4; diff --git a/src/command/cluster.rs b/src/command/cluster.rs index bb55ae7..df85c47 100644 --- a/src/command/cluster.rs +++ b/src/command/cluster.rs @@ -103,8 +103,8 @@ fn cmd_cluster_countkeysinslot(ctx: &mut CommandContext) -> RespValue { "ERR wrong number of arguments for 'cluster|countkeysinslot' command", ); } - let _slot: u16 = match String::from_utf8_lossy(&ctx.args[2]).parse() { - Ok(v) if v < 16384 => v, + let _slot = match super::parse::u64_(&ctx.args[2]) { + Some(v) if v < 16384 => v, _ => return RespValue::error("ERR Invalid or out of range slot"), }; // In standalone mode, we don't track slots; return 0 @@ -117,13 +117,13 @@ fn cmd_cluster_getkeysinslot(ctx: &mut CommandContext) -> RespValue { "ERR wrong number of arguments for 'cluster|getkeysinslot' command", ); } - let _slot: u16 = match String::from_utf8_lossy(&ctx.args[2]).parse() { - Ok(v) if v < 16384 => v, + let _slot = match super::parse::u64_(&ctx.args[2]) { + Some(v) if v < 16384 => v, _ => return RespValue::error("ERR Invalid or out of range slot"), }; - let _count: u64 = match String::from_utf8_lossy(&ctx.args[3]).parse() { - Ok(v) => v, - Err(_) => return RespValue::error("ERR value is not an integer or out of range"), + let _count = match super::parse::u64_(&ctx.args[3]) { + Some(v) => v, + None => return RespValue::error("ERR value is not an integer or out of range"), }; RespValue::array(vec![]) } diff --git a/src/command/geo.rs b/src/command/geo.rs index 94e9758..c9935d1 100644 --- a/src/command/geo.rs +++ b/src/command/geo.rs @@ -138,13 +138,13 @@ pub fn cmd_geoadd(ctx: &mut CommandContext) -> RespValue { let mut triples = Vec::new(); while i + 2 < ctx.args.len() { - let lon: f64 = match String::from_utf8_lossy(&ctx.args[i]).parse() { - Ok(v) => v, - Err(_) => return RespValue::error("ERR value is not a valid float"), + let lon = match super::parse::float(&ctx.args[i]) { + Some(v) => v, + None => return RespValue::error("ERR value is not a valid float"), }; - let lat: f64 = match String::from_utf8_lossy(&ctx.args[i + 1]).parse() { - Ok(v) => v, - Err(_) => return RespValue::error("ERR value is not a valid float"), + let lat = match super::parse::float(&ctx.args[i + 1]) { + Some(v) => v, + None => return RespValue::error("ERR value is not a valid float"), }; if lon < -180.0 || lon > 180.0 || lat < -85.05112878 || lat > 85.05112878 { @@ -274,22 +274,22 @@ pub fn cmd_geosearch(ctx: &mut CommandContext) -> RespValue { "FROMLONLAT" => { i += 1; if i + 1 >= ctx.args.len() { return RespValue::error("ERR syntax error"); } - from_lon = Some(match String::from_utf8_lossy(&ctx.args[i]).parse() { - Ok(v) => v, - Err(_) => return RespValue::error("ERR value is not a valid float"), + from_lon = Some(match super::parse::float(&ctx.args[i]) { + Some(v) => v, + None => return RespValue::error("ERR value is not a valid float"), }); i += 1; - from_lat = Some(match String::from_utf8_lossy(&ctx.args[i]).parse() { - Ok(v) => v, - Err(_) => return RespValue::error("ERR value is not a valid float"), + from_lat = Some(match super::parse::float(&ctx.args[i]) { + Some(v) => v, + None => return RespValue::error("ERR value is not a valid float"), }); } "BYRADIUS" => { i += 1; if i + 1 >= ctx.args.len() { return RespValue::error("ERR syntax error"); } - let radius: f64 = match String::from_utf8_lossy(&ctx.args[i]).parse() { - Ok(v) => v, - Err(_) => return RespValue::error("ERR value is not a valid float"), + let radius = match super::parse::float(&ctx.args[i]) { + Some(v) => v, + None => return RespValue::error("ERR value is not a valid float"), }; i += 1; let unit = String::from_utf8_lossy(&ctx.args[i]).to_lowercase(); @@ -298,14 +298,14 @@ pub fn cmd_geosearch(ctx: &mut CommandContext) -> RespValue { "BYBOX" => { i += 1; if i + 2 >= ctx.args.len() { return RespValue::error("ERR syntax error"); } - let width: f64 = match String::from_utf8_lossy(&ctx.args[i]).parse() { - Ok(v) => v, - Err(_) => return RespValue::error("ERR value is not a valid float"), + let width = match super::parse::float(&ctx.args[i]) { + Some(v) => v, + None => return RespValue::error("ERR value is not a valid float"), }; i += 1; - let height: f64 = match String::from_utf8_lossy(&ctx.args[i]).parse() { - Ok(v) => v, - Err(_) => return RespValue::error("ERR value is not a valid float"), + let height = match super::parse::float(&ctx.args[i]) { + Some(v) => v, + None => return RespValue::error("ERR value is not a valid float"), }; i += 1; let unit = String::from_utf8_lossy(&ctx.args[i]).to_lowercase(); @@ -316,9 +316,9 @@ pub fn cmd_geosearch(ctx: &mut CommandContext) -> RespValue { "COUNT" => { i += 1; if i >= ctx.args.len() { return RespValue::error("ERR syntax error"); } - count = Some(match String::from_utf8_lossy(&ctx.args[i]).parse() { - Ok(v) => v, - Err(_) => return RespValue::error("ERR value is not an integer or out of range"), + count = Some(match super::parse::usize_(&ctx.args[i]) { + Some(v) => v, + None => return RespValue::error("ERR value is not an integer or out of range"), }); // Check for ANY if i + 1 < ctx.args.len() && String::from_utf8_lossy(&ctx.args[i + 1]).to_uppercase() == "ANY" { @@ -459,22 +459,22 @@ pub fn cmd_geosearchstore(ctx: &mut CommandContext) -> RespValue { "FROMLONLAT" => { i += 1; if i + 1 >= ctx.args.len() { return RespValue::error("ERR syntax error"); } - from_lon = Some(match String::from_utf8_lossy(&ctx.args[i]).parse() { - Ok(v) => v, - Err(_) => return RespValue::error("ERR value is not a valid float"), + from_lon = Some(match super::parse::float(&ctx.args[i]) { + Some(v) => v, + None => return RespValue::error("ERR value is not a valid float"), }); i += 1; - from_lat = Some(match String::from_utf8_lossy(&ctx.args[i]).parse() { - Ok(v) => v, - Err(_) => return RespValue::error("ERR value is not a valid float"), + from_lat = Some(match super::parse::float(&ctx.args[i]) { + Some(v) => v, + None => return RespValue::error("ERR value is not a valid float"), }); } "BYRADIUS" => { i += 1; if i + 1 >= ctx.args.len() { return RespValue::error("ERR syntax error"); } - let radius: f64 = match String::from_utf8_lossy(&ctx.args[i]).parse() { - Ok(v) => v, - Err(_) => return RespValue::error("ERR value is not a valid float"), + let radius = match super::parse::float(&ctx.args[i]) { + Some(v) => v, + None => return RespValue::error("ERR value is not a valid float"), }; i += 1; let unit = String::from_utf8_lossy(&ctx.args[i]).to_lowercase(); @@ -483,14 +483,14 @@ pub fn cmd_geosearchstore(ctx: &mut CommandContext) -> RespValue { "BYBOX" => { i += 1; if i + 2 >= ctx.args.len() { return RespValue::error("ERR syntax error"); } - let width: f64 = match String::from_utf8_lossy(&ctx.args[i]).parse() { - Ok(v) => v, - Err(_) => return RespValue::error("ERR value is not a valid float"), + let width = match super::parse::float(&ctx.args[i]) { + Some(v) => v, + None => return RespValue::error("ERR value is not a valid float"), }; i += 1; - let height: f64 = match String::from_utf8_lossy(&ctx.args[i]).parse() { - Ok(v) => v, - Err(_) => return RespValue::error("ERR value is not a valid float"), + let height = match super::parse::float(&ctx.args[i]) { + Some(v) => v, + None => return RespValue::error("ERR value is not a valid float"), }; i += 1; let unit = String::from_utf8_lossy(&ctx.args[i]).to_lowercase(); @@ -501,9 +501,9 @@ pub fn cmd_geosearchstore(ctx: &mut CommandContext) -> RespValue { "COUNT" => { i += 1; if i >= ctx.args.len() { return RespValue::error("ERR syntax error"); } - count = Some(match String::from_utf8_lossy(&ctx.args[i]).parse() { - Ok(v) => v, - Err(_) => return RespValue::error("ERR value is not an integer or out of range"), + count = Some(match super::parse::usize_(&ctx.args[i]) { + Some(v) => v, + None => return RespValue::error("ERR value is not an integer or out of range"), }); if i + 1 < ctx.args.len() && String::from_utf8_lossy(&ctx.args[i + 1]).to_uppercase() == "ANY" { i += 1; diff --git a/src/command/hash.rs b/src/command/hash.rs index 55f26b4..20df011 100644 --- a/src/command/hash.rs +++ b/src/command/hash.rs @@ -184,9 +184,9 @@ pub fn cmd_hgetall(ctx: &mut CommandContext) -> RespValue { pub fn cmd_hincrby(ctx: &mut CommandContext) -> RespValue { let key = ctx.args[1].clone(); let field = ctx.args[2].clone(); - let delta: i64 = match String::from_utf8_lossy(&ctx.args[3]).parse() { - Ok(v) => v, - Err(_) => return RespValue::error("ERR value is not an integer or out of range"), + let delta = match super::parse::int(&ctx.args[3]) { + Some(v) => v, + None => return RespValue::error("ERR value is not an integer or out of range"), }; let hash = match ensure_hash(ctx, &key) { @@ -216,9 +216,9 @@ pub fn cmd_hincrby(ctx: &mut CommandContext) -> RespValue { pub fn cmd_hincrbyfloat(ctx: &mut CommandContext) -> RespValue { let key = ctx.args[1].clone(); let field = ctx.args[2].clone(); - let delta: f64 = match String::from_utf8_lossy(&ctx.args[3]).parse() { - Ok(v) => v, - Err(_) => return RespValue::error("ERR value is not a valid float"), + let delta = match super::parse::float(&ctx.args[3]) { + Some(v) => v, + None => return RespValue::error("ERR value is not a valid float"), }; let hash = match ensure_hash(ctx, &key) { @@ -265,9 +265,9 @@ pub fn cmd_hstrlen(ctx: &mut CommandContext) -> RespValue { pub fn cmd_hrandfield(ctx: &mut CommandContext) -> RespValue { let key = ctx.args[1].clone(); let count = if ctx.args.len() > 2 { - match String::from_utf8_lossy(&ctx.args[2]).parse::() { - Ok(v) => Some(v), - Err(_) => return RespValue::error("ERR value is not an integer or out of range"), + match super::parse::int(&ctx.args[2]) { + Some(v) => Some(v), + None => return RespValue::error("ERR value is not an integer or out of range"), } } else { None diff --git a/src/command/json.rs b/src/command/json.rs index ef0efc5..d0ef5b3 100644 --- a/src/command/json.rs +++ b/src/command/json.rs @@ -562,9 +562,9 @@ pub fn cmd_json_arrpop(ctx: &mut CommandContext) -> RespValue { "$".to_string() }; let index: Option = if ctx.args.len() >= 4 { - match String::from_utf8_lossy(&ctx.args[3]).parse() { - Ok(v) => Some(v), - Err(_) => return RespValue::error("ERR value is not an integer or out of range"), + match super::parse::int(&ctx.args[3]) { + Some(v) => Some(v), + None => return RespValue::error("ERR value is not an integer or out of range"), } } else { None // default: pop last element diff --git a/src/command/keys.rs b/src/command/keys.rs index 1edb81f..d989d6a 100644 --- a/src/command/keys.rs +++ b/src/command/keys.rs @@ -77,9 +77,9 @@ fn apply_expire(ctx: &mut CommandContext, key: &Bytes, new_expire: Instant, nx: pub fn cmd_expire(ctx: &mut CommandContext) -> RespValue { let key = ctx.args[1].clone(); - let secs: i64 = match String::from_utf8_lossy(&ctx.args[2]).parse() { - Ok(v) => v, - Err(_) => return RespValue::error("ERR value is not an integer or out of range"), + let secs = match super::parse::int(&ctx.args[2]) { + Some(v) => v, + None => return RespValue::error("ERR value is not an integer or out of range"), }; if secs < 0 { return RespValue::error("ERR invalid expire time in 'expire' command"); @@ -91,9 +91,9 @@ pub fn cmd_expire(ctx: &mut CommandContext) -> RespValue { pub fn cmd_pexpire(ctx: &mut CommandContext) -> RespValue { let key = ctx.args[1].clone(); - let ms: i64 = match String::from_utf8_lossy(&ctx.args[2]).parse() { - Ok(v) => v, - Err(_) => return RespValue::error("ERR value is not an integer or out of range"), + let ms = match super::parse::int(&ctx.args[2]) { + Some(v) => v, + None => return RespValue::error("ERR value is not an integer or out of range"), }; if ms < 0 { return RespValue::error("ERR invalid expire time in 'pexpire' command"); @@ -105,9 +105,9 @@ pub fn cmd_pexpire(ctx: &mut CommandContext) -> RespValue { pub fn cmd_expireat(ctx: &mut CommandContext) -> RespValue { let key = ctx.args[1].clone(); - let ts: i64 = match String::from_utf8_lossy(&ctx.args[2]).parse() { - Ok(v) => v, - Err(_) => return RespValue::error("ERR value is not an integer or out of range"), + let ts = match super::parse::int(&ctx.args[2]) { + Some(v) => v, + None => return RespValue::error("ERR value is not an integer or out of range"), }; if ts < 0 { return RespValue::error("ERR invalid expire time in 'expireat' command"); @@ -125,9 +125,9 @@ pub fn cmd_expireat(ctx: &mut CommandContext) -> RespValue { pub fn cmd_pexpireat(ctx: &mut CommandContext) -> RespValue { let key = ctx.args[1].clone(); - let ts_ms: i64 = match String::from_utf8_lossy(&ctx.args[2]).parse() { - Ok(v) => v, - Err(_) => return RespValue::error("ERR value is not an integer or out of range"), + let ts_ms = match super::parse::int(&ctx.args[2]) { + Some(v) => v, + None => return RespValue::error("ERR value is not an integer or out of range"), }; if ts_ms < 0 { return RespValue::error("ERR invalid expire time in 'pexpireat' command"); @@ -456,9 +456,9 @@ pub fn cmd_sort(ctx: &mut CommandContext) -> RespValue { /// MOVE key db - move a key to another database pub fn cmd_move(ctx: &mut CommandContext) -> RespValue { let key = ctx.args[1].clone(); - let target_db: usize = match String::from_utf8_lossy(&ctx.args[2]).parse() { - Ok(v) => v, - Err(_) => return RespValue::error("ERR value is not an integer or out of range"), + let target_db = match super::parse::usize_(&ctx.args[2]) { + Some(v) => v, + None => return RespValue::error("ERR value is not an integer or out of range"), }; if target_db >= ctx.store.db_count() { diff --git a/src/command/list.rs b/src/command/list.rs index 6cddec0..f2b7342 100644 --- a/src/command/list.rs +++ b/src/command/list.rs @@ -78,9 +78,9 @@ pub fn cmd_rpushx(ctx: &mut CommandContext) -> RespValue { pub fn cmd_lpop(ctx: &mut CommandContext) -> RespValue { let key = ctx.args[1].clone(); let count = if ctx.args.len() > 2 { - match String::from_utf8_lossy(&ctx.args[2]).parse::() { - Ok(v) => Some(v), - Err(_) => return RespValue::error("ERR value is not an integer or out of range"), + match super::parse::usize_(&ctx.args[2]) { + Some(v) => Some(v), + None => return RespValue::error("ERR value is not an integer or out of range"), } } else { None @@ -127,9 +127,9 @@ pub fn cmd_lpop(ctx: &mut CommandContext) -> RespValue { pub fn cmd_rpop(ctx: &mut CommandContext) -> RespValue { let key = ctx.args[1].clone(); let count = if ctx.args.len() > 2 { - match String::from_utf8_lossy(&ctx.args[2]).parse::() { - Ok(v) => Some(v), - Err(_) => return RespValue::error("ERR value is not an integer or out of range"), + match super::parse::usize_(&ctx.args[2]) { + Some(v) => Some(v), + None => return RespValue::error("ERR value is not an integer or out of range"), } } else { None @@ -194,9 +194,9 @@ pub fn cmd_llen(ctx: &mut CommandContext) -> RespValue { pub fn cmd_lindex(ctx: &mut CommandContext) -> RespValue { let key = ctx.args[1].clone(); - let index: i64 = match String::from_utf8_lossy(&ctx.args[2]).parse() { - Ok(v) => v, - Err(_) => return RespValue::error("ERR value is not an integer or out of range"), + let index = match super::parse::int(&ctx.args[2]) { + Some(v) => v, + None => return RespValue::error("ERR value is not an integer or out of range"), }; let db = ctx.db(); @@ -217,13 +217,13 @@ pub fn cmd_lindex(ctx: &mut CommandContext) -> RespValue { pub fn cmd_lrange(ctx: &mut CommandContext) -> RespValue { let key = ctx.args[1].clone(); - let start: i64 = match String::from_utf8_lossy(&ctx.args[2]).parse() { - Ok(v) => v, - Err(_) => return RespValue::error("ERR value is not an integer or out of range"), + let start = match super::parse::int(&ctx.args[2]) { + Some(v) => v, + None => return RespValue::error("ERR value is not an integer or out of range"), }; - let stop: i64 = match String::from_utf8_lossy(&ctx.args[3]).parse() { - Ok(v) => v, - Err(_) => return RespValue::error("ERR value is not an integer or out of range"), + let stop = match super::parse::int(&ctx.args[3]) { + Some(v) => v, + None => return RespValue::error("ERR value is not an integer or out of range"), }; let db = ctx.db(); @@ -249,9 +249,9 @@ pub fn cmd_lrange(ctx: &mut CommandContext) -> RespValue { pub fn cmd_lset(ctx: &mut CommandContext) -> RespValue { let key = ctx.args[1].clone(); - let index: i64 = match String::from_utf8_lossy(&ctx.args[2]).parse() { - Ok(v) => v, - Err(_) => return RespValue::error("ERR value is not an integer or out of range"), + let index = match super::parse::int(&ctx.args[2]) { + Some(v) => v, + None => return RespValue::error("ERR value is not an integer or out of range"), }; let value = ctx.args[3].clone(); @@ -303,9 +303,9 @@ pub fn cmd_linsert(ctx: &mut CommandContext) -> RespValue { pub fn cmd_lrem(ctx: &mut CommandContext) -> RespValue { let key = ctx.args[1].clone(); - let count: i64 = match String::from_utf8_lossy(&ctx.args[2]).parse() { - Ok(v) => v, - Err(_) => return RespValue::error("ERR value is not an integer or out of range"), + let count = match super::parse::int(&ctx.args[2]) { + Some(v) => v, + None => return RespValue::error("ERR value is not an integer or out of range"), }; let value = ctx.args[3].clone(); @@ -346,13 +346,13 @@ pub fn cmd_lrem(ctx: &mut CommandContext) -> RespValue { pub fn cmd_ltrim(ctx: &mut CommandContext) -> RespValue { let key = ctx.args[1].clone(); - let start: i64 = match String::from_utf8_lossy(&ctx.args[2]).parse() { - Ok(v) => v, - Err(_) => return RespValue::error("ERR value is not an integer or out of range"), + let start = match super::parse::int(&ctx.args[2]) { + Some(v) => v, + None => return RespValue::error("ERR value is not an integer or out of range"), }; - let stop: i64 = match String::from_utf8_lossy(&ctx.args[3]).parse() { - Ok(v) => v, - Err(_) => return RespValue::error("ERR value is not an integer or out of range"), + let stop = match super::parse::int(&ctx.args[3]) { + Some(v) => v, + None => return RespValue::error("ERR value is not an integer or out of range"), }; match get_list(ctx, &key) { @@ -390,26 +390,26 @@ pub fn cmd_lpos(ctx: &mut CommandContext) -> RespValue { "RANK" => { i += 1; if i >= ctx.args.len() { return RespValue::error("ERR syntax error"); } - rank = match String::from_utf8_lossy(&ctx.args[i]).parse() { - Ok(v) => v, - Err(_) => return RespValue::error("ERR value is not an integer or out of range"), + rank = match super::parse::int(&ctx.args[i]) { + Some(v) => v, + None => return RespValue::error("ERR value is not an integer or out of range"), }; if rank == 0 { return RespValue::error("ERR RANK can't be zero"); } } "COUNT" => { i += 1; if i >= ctx.args.len() { return RespValue::error("ERR syntax error"); } - count = Some(match String::from_utf8_lossy(&ctx.args[i]).parse() { - Ok(v) => v, - Err(_) => return RespValue::error("ERR value is not an integer or out of range"), + count = Some(match super::parse::usize_(&ctx.args[i]) { + Some(v) => v, + None => return RespValue::error("ERR value is not an integer or out of range"), }); } "MAXLEN" => { i += 1; if i >= ctx.args.len() { return RespValue::error("ERR syntax error"); } - maxlen = match String::from_utf8_lossy(&ctx.args[i]).parse() { - Ok(v) => v, - Err(_) => return RespValue::error("ERR value is not an integer or out of range"), + maxlen = match super::parse::usize_(&ctx.args[i]) { + Some(v) => v, + None => return RespValue::error("ERR value is not an integer or out of range"), }; } _ => return RespValue::error("ERR syntax error"), @@ -515,9 +515,9 @@ pub fn cmd_lmove(ctx: &mut CommandContext) -> RespValue { } pub fn cmd_lmpop(ctx: &mut CommandContext) -> RespValue { - let numkeys: usize = match String::from_utf8_lossy(&ctx.args[1]).parse() { - Ok(v) => v, - Err(_) => return RespValue::error("ERR value is not an integer or out of range"), + let numkeys = match super::parse::usize_(&ctx.args[1]) { + Some(v) => v, + None => return RespValue::error("ERR value is not an integer or out of range"), }; if ctx.args.len() < 2 + numkeys + 1 { @@ -662,9 +662,9 @@ pub fn cmd_blmpop(ctx: &mut CommandContext) -> RespValue { return RespValue::wrong_arity("blmpop"); } - let numkeys: usize = match String::from_utf8_lossy(&ctx.args[2]).parse() { - Ok(v) => v, - Err(_) => return RespValue::error("ERR value is not an integer or out of range"), + let numkeys = match super::parse::usize_(&ctx.args[2]) { + Some(v) => v, + None => return RespValue::error("ERR value is not an integer or out of range"), }; if ctx.args.len() < 3 + numkeys + 1 { diff --git a/src/command/mod.rs b/src/command/mod.rs index 5e4647a..7a4d601 100644 --- a/src/command/mod.rs +++ b/src/command/mod.rs @@ -1,4 +1,5 @@ mod registry; +mod parse; mod strings; mod keys; pub(crate) mod server_cmds; diff --git a/src/command/parse.rs b/src/command/parse.rs new file mode 100644 index 0000000..6cb6c2b --- /dev/null +++ b/src/command/parse.rs @@ -0,0 +1,28 @@ +//! Lightweight argument parsers shared across command handlers. +//! +//! Replaces the common `String::from_utf8_lossy(&bytes).parse()` chain — which +//! allocates a Cow + an owned String per call and silently accepts malformed +//! UTF-8 — with strict `std::str::from_utf8` followed by `.parse()`. Errors are +//! surfaced as `None` so callers can attach a command-specific RESP error. + +use bytes::Bytes; + +#[inline] +pub fn int(b: &Bytes) -> Option { + std::str::from_utf8(b).ok()?.parse().ok() +} + +#[inline] +pub fn u64_(b: &Bytes) -> Option { + std::str::from_utf8(b).ok()?.parse().ok() +} + +#[inline] +pub fn usize_(b: &Bytes) -> Option { + std::str::from_utf8(b).ok()?.parse().ok() +} + +#[inline] +pub fn float(b: &Bytes) -> Option { + std::str::from_utf8(b).ok()?.parse().ok() +} diff --git a/src/command/scan.rs b/src/command/scan.rs index fea5216..0e46b07 100644 --- a/src/command/scan.rs +++ b/src/command/scan.rs @@ -6,9 +6,9 @@ use crate::storage::db::glob_match; use super::registry::CommandContext; pub fn cmd_scan(ctx: &mut CommandContext) -> RespValue { - let cursor: usize = match String::from_utf8_lossy(&ctx.args[1]).parse() { - Ok(v) => v, - Err(_) => return RespValue::error("ERR value is not an integer or out of range"), + let cursor = match super::parse::usize_(&ctx.args[1]) { + Some(v) => v, + None => return RespValue::error("ERR value is not an integer or out of range"), }; let mut pattern: Option = None; @@ -57,9 +57,9 @@ pub fn cmd_scan(ctx: &mut CommandContext) -> RespValue { pub fn cmd_sscan(ctx: &mut CommandContext) -> RespValue { let key = ctx.args[1].clone(); - let cursor: usize = match String::from_utf8_lossy(&ctx.args[2]).parse() { - Ok(v) => v, - Err(_) => return RespValue::error("ERR value is not an integer or out of range"), + let cursor = match super::parse::usize_(&ctx.args[2]) { + Some(v) => v, + None => return RespValue::error("ERR value is not an integer or out of range"), }; let mut pattern: Option = None; @@ -106,9 +106,9 @@ pub fn cmd_sscan(ctx: &mut CommandContext) -> RespValue { pub fn cmd_hscan(ctx: &mut CommandContext) -> RespValue { let key = ctx.args[1].clone(); - let cursor: usize = match String::from_utf8_lossy(&ctx.args[2]).parse() { - Ok(v) => v, - Err(_) => return RespValue::error("ERR value is not an integer or out of range"), + let cursor = match super::parse::usize_(&ctx.args[2]) { + Some(v) => v, + None => return RespValue::error("ERR value is not an integer or out of range"), }; let mut pattern: Option = None; @@ -163,9 +163,9 @@ pub fn cmd_hscan(ctx: &mut CommandContext) -> RespValue { pub fn cmd_zscan(ctx: &mut CommandContext) -> RespValue { let key = ctx.args[1].clone(); - let cursor: usize = match String::from_utf8_lossy(&ctx.args[2]).parse() { - Ok(v) => v, - Err(_) => return RespValue::error("ERR value is not an integer or out of range"), + let cursor = match super::parse::usize_(&ctx.args[2]) { + Some(v) => v, + None => return RespValue::error("ERR value is not an integer or out of range"), }; let mut pattern: Option = None; diff --git a/src/command/scripting.rs b/src/command/scripting.rs index 4904a22..34ffbf9 100644 --- a/src/command/scripting.rs +++ b/src/command/scripting.rs @@ -10,9 +10,9 @@ pub fn cmd_eval(ctx: &mut CommandContext) -> RespValue { } let script = String::from_utf8_lossy(&ctx.args[1]).to_string(); - let numkeys: usize = match String::from_utf8_lossy(&ctx.args[2]).parse() { - Ok(n) => n, - Err(_) => return RespValue::error("ERR value is not an integer or out of range"), + let numkeys = match super::parse::usize_(&ctx.args[2]) { + Some(n) => n, + None => return RespValue::error("ERR value is not an integer or out of range"), }; if ctx.args.len() < 3 + numkeys { @@ -32,9 +32,9 @@ pub fn cmd_evalsha(ctx: &mut CommandContext) -> RespValue { } let sha = String::from_utf8_lossy(&ctx.args[1]).to_lowercase(); - let numkeys: usize = match String::from_utf8_lossy(&ctx.args[2]).parse() { - Ok(n) => n, - Err(_) => return RespValue::error("ERR value is not an integer or out of range"), + let numkeys = match super::parse::usize_(&ctx.args[2]) { + Some(n) => n, + None => return RespValue::error("ERR value is not an integer or out of range"), }; if ctx.args.len() < 3 + numkeys { @@ -262,9 +262,9 @@ pub fn cmd_fcall(ctx: &mut CommandContext) -> RespValue { } let fname = String::from_utf8_lossy(&ctx.args[1]).to_string(); - let numkeys: usize = match String::from_utf8_lossy(&ctx.args[2]).parse() { - Ok(n) => n, - Err(_) => return RespValue::error("ERR value is not an integer or out of range"), + let numkeys = match super::parse::usize_(&ctx.args[2]) { + Some(n) => n, + None => return RespValue::error("ERR value is not an integer or out of range"), }; if ctx.args.len() < 3 + numkeys { diff --git a/src/command/server_cmds.rs b/src/command/server_cmds.rs index 89af68b..ba5194b 100644 --- a/src/command/server_cmds.rs +++ b/src/command/server_cmds.rs @@ -15,9 +15,9 @@ pub fn cmd_echo(ctx: &mut CommandContext) -> RespValue { } pub fn cmd_select(ctx: &mut CommandContext) -> RespValue { - let index: usize = match String::from_utf8_lossy(&ctx.args[1]).parse() { - Ok(v) => v, - Err(_) => return RespValue::error("ERR value is not an integer or out of range"), + let index = match super::parse::usize_(&ctx.args[1]) { + Some(v) => v, + None => return RespValue::error("ERR value is not an integer or out of range"), }; if index >= ctx.store.db_count() { return RespValue::error("ERR DB index is out of range"); @@ -41,13 +41,13 @@ pub fn cmd_flushall(ctx: &mut CommandContext) -> RespValue { } pub fn cmd_swapdb(ctx: &mut CommandContext) -> RespValue { - let a: usize = match String::from_utf8_lossy(&ctx.args[1]).parse() { - Ok(v) => v, - Err(_) => return RespValue::error("ERR value is not an integer or out of range"), + let a = match super::parse::usize_(&ctx.args[1]) { + Some(v) => v, + None => return RespValue::error("ERR value is not an integer or out of range"), }; - let b: usize = match String::from_utf8_lossy(&ctx.args[2]).parse() { - Ok(v) => v, - Err(_) => return RespValue::error("ERR value is not an integer or out of range"), + let b = match super::parse::usize_(&ctx.args[2]) { + Some(v) => v, + None => return RespValue::error("ERR value is not an integer or out of range"), }; if a >= ctx.store.db_count() || b >= ctx.store.db_count() { return RespValue::error("ERR invalid DB index"); diff --git a/src/command/set.rs b/src/command/set.rs index 5354a9f..f407080 100644 --- a/src/command/set.rs +++ b/src/command/set.rs @@ -122,9 +122,9 @@ pub fn cmd_scard(ctx: &mut CommandContext) -> RespValue { pub fn cmd_srandmember(ctx: &mut CommandContext) -> RespValue { let key = ctx.args[1].clone(); let count = if ctx.args.len() > 2 { - match String::from_utf8_lossy(&ctx.args[2]).parse::() { - Ok(v) => Some(v), - Err(_) => return RespValue::error("ERR value is not an integer or out of range"), + match super::parse::int(&ctx.args[2]) { + Some(v) => Some(v), + None => return RespValue::error("ERR value is not an integer or out of range"), } } else { None @@ -173,9 +173,9 @@ pub fn cmd_srandmember(ctx: &mut CommandContext) -> RespValue { pub fn cmd_spop(ctx: &mut CommandContext) -> RespValue { let key = ctx.args[1].clone(); let count = if ctx.args.len() > 2 { - match String::from_utf8_lossy(&ctx.args[2]).parse::() { - Ok(v) => Some(v), - Err(_) => return RespValue::error("ERR value is not an integer or out of range"), + match super::parse::usize_(&ctx.args[2]) { + Some(v) => Some(v), + None => return RespValue::error("ERR value is not an integer or out of range"), } } else { None @@ -298,9 +298,9 @@ pub fn cmd_sinterstore(ctx: &mut CommandContext) -> RespValue { } pub fn cmd_sintercard(ctx: &mut CommandContext) -> RespValue { - let numkeys: usize = match String::from_utf8_lossy(&ctx.args[1]).parse() { - Ok(v) => v, - Err(_) => return RespValue::error("ERR value is not an integer or out of range"), + let numkeys = match super::parse::usize_(&ctx.args[1]) { + Some(v) => v, + None => return RespValue::error("ERR value is not an integer or out of range"), }; if ctx.args.len() < 2 + numkeys { diff --git a/src/command/sorted_set.rs b/src/command/sorted_set.rs index a05870e..eb8dd38 100644 --- a/src/command/sorted_set.rs +++ b/src/command/sorted_set.rs @@ -203,9 +203,9 @@ pub fn cmd_zmscore(ctx: &mut CommandContext) -> RespValue { pub fn cmd_zincrby(ctx: &mut CommandContext) -> RespValue { let key = ctx.args[1].clone(); - let delta: f64 = match String::from_utf8_lossy(&ctx.args[2]).parse() { - Ok(v) => v, - Err(_) => return RespValue::error("ERR value is not a valid float"), + let delta = match super::parse::float(&ctx.args[2]) { + Some(v) => v, + None => return RespValue::error("ERR value is not a valid float"), }; let member = ctx.args[3].clone(); @@ -373,13 +373,13 @@ pub fn cmd_zrangebyscore(ctx: &mut CommandContext) -> RespValue { pub fn cmd_zrevrange(ctx: &mut CommandContext) -> RespValue { let key = ctx.args[1].clone(); - let start: i64 = match String::from_utf8_lossy(&ctx.args[2]).parse() { - Ok(v) => v, - Err(_) => return RespValue::error("ERR value is not an integer or out of range"), + let start = match super::parse::int(&ctx.args[2]) { + Some(v) => v, + None => return RespValue::error("ERR value is not an integer or out of range"), }; - let stop: i64 = match String::from_utf8_lossy(&ctx.args[3]).parse() { - Ok(v) => v, - Err(_) => return RespValue::error("ERR value is not an integer or out of range"), + let stop = match super::parse::int(&ctx.args[3]) { + Some(v) => v, + None => return RespValue::error("ERR value is not an integer or out of range"), }; let withscores = ctx.args.len() > 4 && String::from_utf8_lossy(&ctx.args[4]).to_uppercase() == "WITHSCORES"; @@ -480,9 +480,9 @@ pub fn cmd_zrevrank(ctx: &mut CommandContext) -> RespValue { pub fn cmd_zpopmin(ctx: &mut CommandContext) -> RespValue { let key = ctx.args[1].clone(); let count: usize = if ctx.args.len() > 2 { - match String::from_utf8_lossy(&ctx.args[2]).parse() { - Ok(v) => v, - Err(_) => return RespValue::error("ERR value is not an integer or out of range"), + match super::parse::usize_(&ctx.args[2]) { + Some(v) => v, + None => return RespValue::error("ERR value is not an integer or out of range"), } } else { 1 @@ -511,9 +511,9 @@ pub fn cmd_zpopmin(ctx: &mut CommandContext) -> RespValue { pub fn cmd_zpopmax(ctx: &mut CommandContext) -> RespValue { let key = ctx.args[1].clone(); let count: usize = if ctx.args.len() > 2 { - match String::from_utf8_lossy(&ctx.args[2]).parse() { - Ok(v) => v, - Err(_) => return RespValue::error("ERR value is not an integer or out of range"), + match super::parse::usize_(&ctx.args[2]) { + Some(v) => v, + None => return RespValue::error("ERR value is not an integer or out of range"), } } else { 1 @@ -626,9 +626,9 @@ enum SetOp { Union, Inter, Diff } fn zstore_op(ctx: &mut CommandContext, op: SetOp) -> RespValue { let dest = ctx.args[1].clone(); - let numkeys: usize = match String::from_utf8_lossy(&ctx.args[2]).parse() { - Ok(v) => v, - Err(_) => return RespValue::error("ERR value is not an integer or out of range"), + let numkeys = match super::parse::usize_(&ctx.args[2]) { + Some(v) => v, + None => return RespValue::error("ERR value is not an integer or out of range"), }; if ctx.args.len() < 3 + numkeys { @@ -891,9 +891,9 @@ pub fn cmd_zdiff(ctx: &mut CommandContext) -> RespValue { } fn zset_op(ctx: &mut CommandContext, op: SetOp, _store: bool) -> RespValue { - let numkeys: usize = match String::from_utf8_lossy(&ctx.args[1]).parse() { - Ok(v) => v, - Err(_) => return RespValue::error("ERR value is not an integer or out of range"), + let numkeys = match super::parse::usize_(&ctx.args[1]) { + Some(v) => v, + None => return RespValue::error("ERR value is not an integer or out of range"), }; if ctx.args.len() < 2 + numkeys { @@ -994,9 +994,9 @@ fn zset_op(ctx: &mut CommandContext, op: SetOp, _store: bool) -> RespValue { } pub fn cmd_zintercard(ctx: &mut CommandContext) -> RespValue { - let numkeys: usize = match String::from_utf8_lossy(&ctx.args[1]).parse() { - Ok(v) => v, - Err(_) => return RespValue::error("ERR value is not an integer or out of range"), + let numkeys = match super::parse::usize_(&ctx.args[1]) { + Some(v) => v, + None => return RespValue::error("ERR value is not an integer or out of range"), }; if ctx.args.len() < 2 + numkeys { @@ -1046,9 +1046,9 @@ pub fn cmd_zintercard(ctx: &mut CommandContext) -> RespValue { } pub fn cmd_zmpop(ctx: &mut CommandContext) -> RespValue { - let numkeys: usize = match String::from_utf8_lossy(&ctx.args[1]).parse() { - Ok(v) => v, - Err(_) => return RespValue::error("ERR value is not an integer or out of range"), + let numkeys = match super::parse::usize_(&ctx.args[1]) { + Some(v) => v, + None => return RespValue::error("ERR value is not an integer or out of range"), }; if ctx.args.len() < 2 + numkeys + 1 { @@ -1163,9 +1163,9 @@ pub fn cmd_bzmpop(ctx: &mut CommandContext) -> RespValue { return RespValue::wrong_arity("bzmpop"); } - let numkeys: usize = match String::from_utf8_lossy(&ctx.args[2]).parse() { - Ok(v) => v, - Err(_) => return RespValue::error("ERR value is not an integer or out of range"), + let numkeys = match super::parse::usize_(&ctx.args[2]) { + Some(v) => v, + None => return RespValue::error("ERR value is not an integer or out of range"), }; if ctx.args.len() < 3 + numkeys + 1 { @@ -1344,13 +1344,13 @@ pub fn cmd_zremrangebylex(ctx: &mut CommandContext) -> RespValue { pub fn cmd_zremrangebyrank(ctx: &mut CommandContext) -> RespValue { let key = ctx.args[1].clone(); - let start: i64 = match String::from_utf8_lossy(&ctx.args[2]).parse() { - Ok(v) => v, - Err(_) => return RespValue::error("ERR value is not an integer or out of range"), + let start = match super::parse::int(&ctx.args[2]) { + Some(v) => v, + None => return RespValue::error("ERR value is not an integer or out of range"), }; - let stop: i64 = match String::from_utf8_lossy(&ctx.args[3]).parse() { - Ok(v) => v, - Err(_) => return RespValue::error("ERR value is not an integer or out of range"), + let stop = match super::parse::int(&ctx.args[3]) { + Some(v) => v, + None => return RespValue::error("ERR value is not an integer or out of range"), }; match get_zset(ctx, &key) { diff --git a/src/command/stream.rs b/src/command/stream.rs index 863827a..a2d602f 100644 --- a/src/command/stream.rs +++ b/src/command/stream.rs @@ -448,14 +448,10 @@ pub fn cmd_xrange(ctx: &mut CommandContext) -> RespValue { if ctx.args.len() >= 6 { let opt = String::from_utf8_lossy(&ctx.args[4]).to_uppercase(); if opt == "COUNT" { - count = Some( - match String::from_utf8_lossy(&ctx.args[5]).parse() { - Ok(v) => v, - Err(_) => { - return RespValue::error("ERR value is not an integer or out of range") - } - }, - ); + count = Some(match super::parse::usize_(&ctx.args[5]) { + Some(v) => v, + None => return RespValue::error("ERR value is not an integer or out of range"), + }); } } @@ -502,14 +498,10 @@ pub fn cmd_xrevrange(ctx: &mut CommandContext) -> RespValue { if ctx.args.len() >= 6 { let opt = String::from_utf8_lossy(&ctx.args[4]).to_uppercase(); if opt == "COUNT" { - count = Some( - match String::from_utf8_lossy(&ctx.args[5]).parse() { - Ok(v) => v, - Err(_) => { - return RespValue::error("ERR value is not an integer or out of range") - } - }, - ); + count = Some(match super::parse::usize_(&ctx.args[5]) { + Some(v) => v, + None => return RespValue::error("ERR value is not an integer or out of range"), + }); } } @@ -612,16 +604,12 @@ pub fn cmd_xread(ctx: &mut CommandContext) -> RespValue { if idx >= ctx.args.len() { return RespValue::error("ERR syntax error"); } - count = Some( - match String::from_utf8_lossy(&ctx.args[idx]).parse() { - Ok(v) => v, - Err(_) => { - return RespValue::error( - "ERR value is not an integer or out of range", - ) - } - }, - ); + count = Some(match super::parse::usize_(&ctx.args[idx]) { + Some(v) => v, + None => { + return RespValue::error("ERR value is not an integer or out of range") + } + }); idx += 1; } "BLOCK" => { @@ -1107,16 +1095,12 @@ pub fn cmd_xreadgroup(ctx: &mut CommandContext) -> RespValue { if idx >= ctx.args.len() { return RespValue::error("ERR syntax error"); } - count = Some( - match String::from_utf8_lossy(&ctx.args[idx]).parse() { - Ok(v) => v, - Err(_) => { - return RespValue::error( - "ERR value is not an integer or out of range", - ) - } - }, - ); + count = Some(match super::parse::usize_(&ctx.args[idx]) { + Some(v) => v, + None => { + return RespValue::error("ERR value is not an integer or out of range") + } + }); idx += 1; } "BLOCK" => { @@ -1372,16 +1356,12 @@ pub fn cmd_xpending(ctx: &mut CommandContext) -> RespValue { if idx >= args_len { return RespValue::error("ERR syntax error"); } - min_idle_val = Some( - match String::from_utf8_lossy(&ctx.args[idx]).parse() { - Ok(v) => v, - Err(_) => { - return RespValue::error( - "ERR value is not an integer or out of range", - ) - } - }, - ); + min_idle_val = Some(match super::parse::u64_(&ctx.args[idx]) { + Some(v) => v, + None => { + return RespValue::error("ERR value is not an integer or out of range") + } + }); idx += 1; } } @@ -1392,9 +1372,9 @@ pub fn cmd_xpending(ctx: &mut CommandContext) -> RespValue { let s_str = String::from_utf8_lossy(&ctx.args[idx]).to_string(); let e_str = String::from_utf8_lossy(&ctx.args[idx + 1]).to_string(); - let cnt: usize = match String::from_utf8_lossy(&ctx.args[idx + 2]).parse() { - Ok(v) => v, - Err(_) => return RespValue::error("ERR value is not an integer or out of range"), + let cnt = match super::parse::usize_(&ctx.args[idx + 2]) { + Some(v) => v, + None => return RespValue::error("ERR value is not an integer or out of range"), }; idx += 3; @@ -1522,9 +1502,9 @@ pub fn cmd_xclaim(ctx: &mut CommandContext) -> RespValue { let key = ctx.args[1].clone(); let group_name = ctx.args[2].clone(); let consumer_name = ctx.args[3].clone(); - let min_idle_time: u64 = match String::from_utf8_lossy(&ctx.args[4]).parse() { - Ok(v) => v, - Err(_) => return RespValue::error("ERR value is not an integer or out of range"), + let min_idle_time = match super::parse::u64_(&ctx.args[4]) { + Some(v) => v, + None => return RespValue::error("ERR value is not an integer or out of range"), }; let mut ids: Vec = Vec::new(); @@ -1616,9 +1596,9 @@ pub fn cmd_xautoclaim(ctx: &mut CommandContext) -> RespValue { let key = ctx.args[1].clone(); let group_name = ctx.args[2].clone(); let consumer_name = ctx.args[3].clone(); - let min_idle_time: u64 = match String::from_utf8_lossy(&ctx.args[4]).parse() { - Ok(v) => v, - Err(_) => return RespValue::error("ERR value is not an integer or out of range"), + let min_idle_time = match super::parse::u64_(&ctx.args[4]) { + Some(v) => v, + None => return RespValue::error("ERR value is not an integer or out of range"), }; let start_str = String::from_utf8_lossy(&ctx.args[5]).to_string(); let start = match parse_stream_id_for_range_start(&start_str) { @@ -1630,11 +1610,9 @@ pub fn cmd_xautoclaim(ctx: &mut CommandContext) -> RespValue { if ctx.args.len() >= 8 { let opt = String::from_utf8_lossy(&ctx.args[6]).to_uppercase(); if opt == "COUNT" { - count = match String::from_utf8_lossy(&ctx.args[7]).parse() { - Ok(v) => v, - Err(_) => { - return RespValue::error("ERR value is not an integer or out of range") - } + count = match super::parse::usize_(&ctx.args[7]) { + Some(v) => v, + None => return RespValue::error("ERR value is not an integer or out of range"), }; } } diff --git a/src/command/strings.rs b/src/command/strings.rs index 6c9e9b2..2aee347 100644 --- a/src/command/strings.rs +++ b/src/command/strings.rs @@ -71,8 +71,8 @@ pub fn cmd_set(ctx: &mut CommandContext) -> RespValue { if i >= ctx.args.len() { return RespValue::error("ERR syntax error"); } - let secs: u64 = match String::from_utf8_lossy(&ctx.args[i]).parse() { - Ok(v) if v > 0 => v, + let secs = match super::parse::u64_(&ctx.args[i]) { + Some(v) if v > 0 => v, _ => return RespValue::error("ERR invalid expire time in 'set' command"), }; ex = Some(Duration::from_secs(secs)); @@ -82,8 +82,8 @@ pub fn cmd_set(ctx: &mut CommandContext) -> RespValue { if i >= ctx.args.len() { return RespValue::error("ERR syntax error"); } - let ms: u64 = match String::from_utf8_lossy(&ctx.args[i]).parse() { - Ok(v) if v > 0 => v, + let ms = match super::parse::u64_(&ctx.args[i]) { + Some(v) if v > 0 => v, _ => return RespValue::error("ERR invalid expire time in 'set' command"), }; ex = Some(Duration::from_millis(ms)); @@ -93,8 +93,8 @@ pub fn cmd_set(ctx: &mut CommandContext) -> RespValue { if i >= ctx.args.len() { return RespValue::error("ERR syntax error"); } - let ts: u64 = match String::from_utf8_lossy(&ctx.args[i]).parse() { - Ok(v) if v > 0 => v, + let ts = match super::parse::u64_(&ctx.args[i]) { + Some(v) if v > 0 => v, _ => return RespValue::error("ERR invalid expire time in 'set' command"), }; let now = SystemTime::now().duration_since(UNIX_EPOCH).unwrap_or_default().as_secs(); @@ -108,8 +108,8 @@ pub fn cmd_set(ctx: &mut CommandContext) -> RespValue { if i >= ctx.args.len() { return RespValue::error("ERR syntax error"); } - let ts_ms: u64 = match String::from_utf8_lossy(&ctx.args[i]).parse() { - Ok(v) if v > 0 => v, + let ts_ms = match super::parse::u64_(&ctx.args[i]) { + Some(v) if v > 0 => v, _ => return RespValue::error("ERR invalid expire time in 'set' command"), }; let now_ms = SystemTime::now().duration_since(UNIX_EPOCH).unwrap_or_default().as_millis() as u64; @@ -188,8 +188,8 @@ pub fn cmd_setnx(ctx: &mut CommandContext) -> RespValue { pub fn cmd_setex(ctx: &mut CommandContext) -> RespValue { let key = ctx.args[1].clone(); - let secs: u64 = match String::from_utf8_lossy(&ctx.args[2]).parse() { - Ok(v) if v > 0 => v, + let secs = match super::parse::u64_(&ctx.args[2]) { + Some(v) if v > 0 => v, _ => return RespValue::error("ERR invalid expire time in 'setex' command"), }; let value = ctx.args[3].clone(); @@ -201,8 +201,8 @@ pub fn cmd_setex(ctx: &mut CommandContext) -> RespValue { pub fn cmd_psetex(ctx: &mut CommandContext) -> RespValue { let key = ctx.args[1].clone(); - let ms: u64 = match String::from_utf8_lossy(&ctx.args[2]).parse() { - Ok(v) if v > 0 => v, + let ms = match super::parse::u64_(&ctx.args[2]) { + Some(v) if v > 0 => v, _ => return RespValue::error("ERR invalid expire time in 'psetex' command"), }; let value = ctx.args[3].clone(); @@ -389,13 +389,13 @@ pub fn cmd_strlen(ctx: &mut CommandContext) -> RespValue { pub fn cmd_getrange(ctx: &mut CommandContext) -> RespValue { let key = ctx.args[1].clone(); - let start: i64 = match String::from_utf8_lossy(&ctx.args[2]).parse() { - Ok(v) => v, - Err(_) => return RespValue::error("ERR value is not an integer or out of range"), + let start = match super::parse::int(&ctx.args[2]) { + Some(v) => v, + None => return RespValue::error("ERR value is not an integer or out of range"), }; - let end: i64 = match String::from_utf8_lossy(&ctx.args[3]).parse() { - Ok(v) => v, - Err(_) => return RespValue::error("ERR value is not an integer or out of range"), + let end = match super::parse::int(&ctx.args[3]) { + Some(v) => v, + None => return RespValue::error("ERR value is not an integer or out of range"), }; let db = ctx.db(); @@ -418,9 +418,9 @@ pub fn cmd_getrange(ctx: &mut CommandContext) -> RespValue { pub fn cmd_setrange(ctx: &mut CommandContext) -> RespValue { let key = ctx.args[1].clone(); - let offset: usize = match String::from_utf8_lossy(&ctx.args[2]).parse() { - Ok(v) => v, - Err(_) => return RespValue::error("ERR value is not an integer or out of range"), + let offset = match super::parse::usize_(&ctx.args[2]) { + Some(v) => v, + None => return RespValue::error("ERR value is not an integer or out of range"), }; let value = ctx.args[3].clone(); @@ -471,24 +471,24 @@ pub fn cmd_getex(ctx: &mut CommandContext) -> RespValue { match opt.as_str() { "EX" => { if ctx.args.len() < 4 { return RespValue::error("ERR syntax error"); } - let secs: u64 = match String::from_utf8_lossy(&ctx.args[3]).parse() { - Ok(v) if v > 0 => v, + let secs = match super::parse::u64_(&ctx.args[3]) { + Some(v) if v > 0 => v, _ => return RespValue::error("ERR invalid expire time in 'getex' command"), }; ctx.db().set_expire(&key, Instant::now() + Duration::from_secs(secs)); } "PX" => { if ctx.args.len() < 4 { return RespValue::error("ERR syntax error"); } - let ms: u64 = match String::from_utf8_lossy(&ctx.args[3]).parse() { - Ok(v) if v > 0 => v, + let ms = match super::parse::u64_(&ctx.args[3]) { + Some(v) if v > 0 => v, _ => return RespValue::error("ERR invalid expire time in 'getex' command"), }; ctx.db().set_expire(&key, Instant::now() + Duration::from_millis(ms)); } "EXAT" => { if ctx.args.len() < 4 { return RespValue::error("ERR syntax error"); } - let ts: u64 = match String::from_utf8_lossy(&ctx.args[3]).parse() { - Ok(v) if v > 0 => v, + let ts = match super::parse::u64_(&ctx.args[3]) { + Some(v) if v > 0 => v, _ => return RespValue::error("ERR invalid expire time in 'getex' command"), }; let now = SystemTime::now().duration_since(UNIX_EPOCH).unwrap_or_default().as_secs(); @@ -501,8 +501,8 @@ pub fn cmd_getex(ctx: &mut CommandContext) -> RespValue { } "PXAT" => { if ctx.args.len() < 4 { return RespValue::error("ERR syntax error"); } - let ts_ms: u64 = match String::from_utf8_lossy(&ctx.args[3]).parse() { - Ok(v) if v > 0 => v, + let ts_ms = match super::parse::u64_(&ctx.args[3]) { + Some(v) if v > 0 => v, _ => return RespValue::error("ERR invalid expire time in 'getex' command"), }; let now_ms = SystemTime::now().duration_since(UNIX_EPOCH).unwrap_or_default().as_millis() as u64; From 8a9f0606c7b1da82684d74c5385de0d205f47f3f Mon Sep 17 00:00:00 2001 From: mack42 Date: Wed, 13 May 2026 07:24:19 -0400 Subject: [PATCH 4/5] SortedSet: drop dummy Bytes value, store scores as BTreeSet MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #29. SortedSetData.scores was BTreeMap where the value was always Bytes::new() — a per-insert no-op allocation kept around so the type could be a Map. Migrate to BTreeSet, which expresses what the code actually wanted (an ordered set of keys) and removes: - one Bytes::new() per insert - the (_, _) tuple destructuring in iter/range/skip paths Iter patterns shift from |(k, _)| -> |k|. .keys() becomes .iter(). External callers (http.rs serializer) updated accordingly. Bump version 0.11.3 -> 0.11.4. --- Cargo.lock | 2 +- Cargo.toml | 2 +- src/http.rs | 2 +- src/storage/types.rs | 23 +++++++++++------------ 4 files changed, 14 insertions(+), 15 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 394e1bf..a6060b9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -791,7 +791,7 @@ dependencies = [ [[package]] name = "rcache" -version = "0.11.2" +version = "0.11.3" dependencies = [ "bytes", "crc16", diff --git a/Cargo.toml b/Cargo.toml index 8689858..d9e9da3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "rcache" -version = "0.11.3" +version = "0.11.4" edition = "2024" [dependencies] diff --git a/src/http.rs b/src/http.rs index 4ee92fe..78cbd34 100644 --- a/src/http.rs +++ b/src/http.rs @@ -420,7 +420,7 @@ fn resp_object_to_json(obj: &crate::storage::RedisObject) -> Value { let members: Vec = z .scores .iter() - .map(|(k, _)| { + .map(|k| { json!({ "member": String::from_utf8_lossy(&k.member).to_string(), "score": k.score, diff --git a/src/storage/types.rs b/src/storage/types.rs index 71051d3..d59b4ad 100644 --- a/src/storage/types.rs +++ b/src/storage/types.rs @@ -1,5 +1,5 @@ use bytes::Bytes; -use std::collections::{BTreeMap, HashMap, HashSet, VecDeque}; +use std::collections::{BTreeMap, BTreeSet, HashMap, HashSet, VecDeque}; use std::fmt; /// Internal Redis data types. @@ -133,10 +133,10 @@ fn estimate_json_size(v: &serde_json::Value) -> usize { #[derive(Debug, Clone)] pub struct SortedSetData { pub members: HashMap, - pub scores: BTreeMap, + pub scores: BTreeSet, } -/// A key for the score-ordered BTreeMap: (score, member) for unique ordering. +/// A key for the score-ordered BTreeSet: (score, member) for unique ordering. #[derive(Debug, Clone)] pub struct ScoreKey { pub score: f64, @@ -169,19 +169,18 @@ impl SortedSetData { pub fn new() -> Self { Self { members: HashMap::new(), - scores: BTreeMap::new(), + scores: BTreeSet::new(), } } /// Add or update a member. Returns true if the member was newly inserted. pub fn insert(&mut self, member: Bytes, score: f64) -> bool { if let Some(old_score) = self.members.insert(member.clone(), score) { - // Remove old score entry self.scores.remove(&ScoreKey { score: old_score, member: member.clone() }); - self.scores.insert(ScoreKey { score, member }, Bytes::new()); + self.scores.insert(ScoreKey { score, member }); false } else { - self.scores.insert(ScoreKey { score, member }, Bytes::new()); + self.scores.insert(ScoreKey { score, member }); true } } @@ -233,7 +232,7 @@ impl SortedSetData { self.scores.iter() .skip(start) .take(stop - start + 1) - .map(|(k, _)| (k.member.clone(), k.score)) + .map(|k| (k.member.clone(), k.score)) .collect() } @@ -254,10 +253,10 @@ impl SortedSetData { }; self.scores.range((min_bound, max_bound)) - .take_while(|(k, _)| { + .take_while(|k| { if max_inclusive { k.score <= max } else { k.score < max } }) - .filter_map(|(k, _)| { + .filter_map(|k| { let in_min = if min_inclusive { k.score >= min } else { k.score > min }; if in_min { Some((k.member.clone(), k.score)) } else { None } }) @@ -271,7 +270,7 @@ impl SortedSetData { /// Pop the member with the minimum score. pub fn pop_min(&mut self) -> Option<(Bytes, f64)> { - let key = self.scores.keys().next()?.clone(); + let key = self.scores.iter().next()?.clone(); self.scores.remove(&key); self.members.remove(&key.member); Some((key.member, key.score)) @@ -279,7 +278,7 @@ impl SortedSetData { /// Pop the member with the maximum score. pub fn pop_max(&mut self) -> Option<(Bytes, f64)> { - let key = self.scores.keys().next_back()?.clone(); + let key = self.scores.iter().next_back()?.clone(); self.scores.remove(&key); self.members.remove(&key.member); Some((key.member, key.score)) From 676fefca3528ff6274dea05142bbcd57d65509b6 Mon Sep 17 00:00:00 2001 From: mack42 Date: Thu, 16 Jul 2026 15:14:45 -0400 Subject: [PATCH 5/5] Security: fix critical vulnerabilities (v0.11.5) Unify ACL enforcement, gate HTTP command execution, sandbox scripting, and bound attacker-controlled allocations. - ACL: make the acl.rs registry the single source of truth so runtime ACL SETUSER changes are actually enforced; drop the disconnected SharedState.acl_users snapshot and dead AclUser/AclMatch types. - HTTP: resolve the bearer token to a user and enforce per-user command and key ACLs on /api/v1/command and the key endpoints (403 on denial). - Scripting: add a 256MB memory limit and 5s wall-clock timeout, block nested EVAL/FCALL/SCRIPT/FUNCTION (recursion crash), and make the _RO variants reject write commands. - Allocation caps: bound SETBIT/BITFIELD bit offsets and SETRANGE length (512MB) with checked arithmetic; cap CMS/TopK width*depth*8 at 256MB. - Connection: cap the per-connection query buffer at 1GB. - AOF: bound multibulk count and bulk length before allocation so a tampered appendonly.aof cannot OOM/panic on startup. --- Cargo.lock | 2 +- Cargo.toml | 2 +- src/command/acl.rs | 131 +++++++++++++++++++++++++++++++++++ src/command/bitmap.rs | 23 ++++-- src/command/mod.rs | 2 +- src/command/probabilistic.rs | 40 ++++++++++- src/command/scripting.rs | 11 +-- src/command/strings.rs | 14 +++- src/http.rs | 98 +++++++++++++++++++------- src/persistence/aof.rs | 27 +++++++- src/scripting_engine.rs | 88 ++++++++++++++++++++--- src/server/connection.rs | 107 +++++++++++----------------- src/server/mod.rs | 77 +------------------- 13 files changed, 433 insertions(+), 189 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a6060b9..5cb4d1e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -791,7 +791,7 @@ dependencies = [ [[package]] name = "rcache" -version = "0.11.3" +version = "0.11.5" dependencies = [ "bytes", "crc16", diff --git a/Cargo.toml b/Cargo.toml index d9e9da3..9245751 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "rcache" -version = "0.11.4" +version = "0.11.5" edition = "2024" [dependencies] diff --git a/src/command/acl.rs b/src/command/acl.rs index 50a2b81..e4e59d8 100644 --- a/src/command/acl.rs +++ b/src/command/acl.rs @@ -429,6 +429,137 @@ struct AclUserEntry { no_pass: bool, } +impl AclUserEntry { + /// Whether this user may run `cmd` (case-insensitive). + fn is_command_allowed(&self, cmd: &str) -> bool { + if !self.enabled { + return false; + } + let cmd_upper = cmd.to_uppercase(); + if self.denied_commands.contains(&cmd_upper) { + return false; + } + if self.all_commands { + return true; + } + self.allowed_commands.contains(&cmd_upper) + } + + /// Whether this user may access `key` under its key patterns. + fn is_key_allowed(&self, key: &str) -> bool { + if self.all_keys { + return true; + } + self.key_patterns + .iter() + .any(|pat| crate::storage::db::glob_match(pat, key)) + } +} + +/// Result of authenticating a username/password against the ACL registry. +pub enum AuthOutcome { + Ok, + WrongPass, + Disabled, + NoUser, +} + +/// Apply the server-level `requirepass` to the default user at startup so the +/// enforcement path (which reads this same registry) requires it. This makes the +/// ACL registry the single source of truth for authentication and authorization. +pub fn init_default_password(requirepass: Option<&str>) { + if let (Some(pass), Ok(mut users)) = (requirepass, ACL_USERS.lock()) { + if let Some(def) = users.get_mut("default") { + use sha2::{Digest, Sha256}; + let hash = format!("{:x}", Sha256::digest(pass.as_bytes())); + if !def.passwords.contains(&hash) { + def.passwords.push(hash); + } + def.no_pass = false; + } + } +} + +/// Authenticate a username/password pair against the ACL registry. +pub fn check_password(username: &str, password: &str) -> AuthOutcome { + match ACL_USERS.lock() { + Ok(users) => match users.get(username) { + Some(u) => { + if !u.enabled { + return AuthOutcome::Disabled; + } + if u.no_pass { + return AuthOutcome::Ok; + } + use sha2::{Digest, Sha256}; + let hash = format!("{:x}", Sha256::digest(password.as_bytes())); + if u.passwords.contains(&hash) { + AuthOutcome::Ok + } else { + AuthOutcome::WrongPass + } + } + None => AuthOutcome::NoUser, + }, + Err(_) => AuthOutcome::WrongPass, + } +} + +/// Whether `username` may run `cmd`. Unknown users are not blocked here (the +/// connection layer gates unauthenticated access separately); the `default` +/// user always exists. +pub fn is_command_allowed(username: &str, cmd: &str) -> bool { + match ACL_USERS.lock() { + Ok(users) => match users.get(username) { + Some(u) => u.is_command_allowed(cmd), + None => true, + }, + Err(_) => false, + } +} + +/// Whether `username` may access `key`. +pub fn is_key_allowed(username: &str, key: &str) -> bool { + match ACL_USERS.lock() { + Ok(users) => match users.get(username) { + Some(u) => u.is_key_allowed(key), + None => true, + }, + Err(_) => false, + } +} + +/// Whether `username`'s key patterns cover every key (no per-key check needed). +pub fn user_has_all_keys(username: &str) -> bool { + match ACL_USERS.lock() { + Ok(users) => users.get(username).map(|u| u.all_keys).unwrap_or(true), + Err(_) => false, + } +} + +/// Whether any enabled user requires a password (used to decide if the HTTP +/// API is open when no `requirepass` is configured). +pub fn any_password_required() -> bool { + match ACL_USERS.lock() { + Ok(users) => users + .values() + .any(|u| u.enabled && !u.no_pass && !u.passwords.is_empty()), + Err(_) => true, + } +} + +/// Resolve an HTTP bearer token to the enabled ACL user whose password it +/// matches, if any. +pub fn http_authenticate(token: &str) -> Option { + use sha2::{Digest, Sha256}; + let hash = format!("{:x}", Sha256::digest(token.as_bytes())); + let users = ACL_USERS.lock().ok()?; + users + .iter() + .find(|(_, u)| u.enabled && !u.no_pass && u.passwords.contains(&hash)) + .map(|(name, _)| name.clone()) +} + static ACL_USERS: std::sync::LazyLock>> = std::sync::LazyLock::new(|| { let mut users = HashMap::new(); diff --git a/src/command/bitmap.rs b/src/command/bitmap.rs index 7409c8b..a3ea99d 100644 --- a/src/command/bitmap.rs +++ b/src/command/bitmap.rs @@ -3,6 +3,13 @@ use crate::protocol::RespValue; use crate::storage::RedisObject; use super::registry::CommandContext; +/// Maximum addressable byte length of a string value (512 MB), matching Redis's +/// proto-max-bulk-len. Bit offsets that would grow a string past this are +/// rejected rather than triggering a multi-GB/TB allocation. +const MAX_STRING_BYTES: usize = 512 * 1024 * 1024; +/// Maximum bit offset, derived from the byte cap. +const MAX_BIT_OFFSET: usize = MAX_STRING_BYTES * 8; + fn get_or_create_string(ctx: &mut CommandContext, key: &Bytes) -> Vec { let db = ctx.db(); match db.get(key) { @@ -23,8 +30,8 @@ fn check_type(ctx: &mut CommandContext, key: &Bytes) -> Result<(), RespValue> { pub fn cmd_setbit(ctx: &mut CommandContext) -> RespValue { let key = ctx.args[1].clone(); let offset = match super::parse::usize_(&ctx.args[2]) { - Some(v) => v, - None => return RespValue::error("ERR bit offset is not an integer or out of range"), + Some(v) if v < MAX_BIT_OFFSET => v, + _ => return RespValue::error("ERR bit offset is not an integer or out of range"), }; let value: u8 = match String::from_utf8_lossy(&ctx.args[3]).parse::() { Ok(v) if v <= 1 => v, @@ -476,12 +483,18 @@ fn parse_encoding(arg: &Bytes) -> Option<(bool, u32)> { fn parse_bitfield_offset(arg: &Bytes, bits: u32) -> Option { let s = String::from_utf8_lossy(arg); - if let Some(rest) = s.strip_prefix('#') { + let offset = if let Some(rest) = s.strip_prefix('#') { let idx: usize = rest.parse().ok()?; - Some(idx * bits as usize) + idx.checked_mul(bits as usize)? } else { - s.parse().ok() + s.parse().ok()? + }; + // Reject offsets that would grow the backing string past the size cap, + // preventing multi-GB allocations in read_bits/write_bits. + if offset >= MAX_BIT_OFFSET { + return None; } + Some(offset) } fn read_bits(data: &[u8], offset: usize, bits: u32, signed: bool) -> i64 { diff --git a/src/command/mod.rs b/src/command/mod.rs index 7a4d601..ee8109a 100644 --- a/src/command/mod.rs +++ b/src/command/mod.rs @@ -14,7 +14,7 @@ mod bitmap; mod hyperloglog; mod geo; mod persistence_cmds; -mod acl; +pub(crate) mod acl; mod scripting; mod replication; mod stream; diff --git a/src/command/probabilistic.rs b/src/command/probabilistic.rs index 57ef76a..d3d641a 100644 --- a/src/command/probabilistic.rs +++ b/src/command/probabilistic.rs @@ -3,6 +3,25 @@ use crate::protocol::RespValue; use crate::storage::RedisObject; use super::registry::CommandContext; +/// Upper bound on the backing-array size (in bytes) any probabilistic structure +/// may allocate (256 MB). Client-supplied width/depth/capacity are validated +/// against this before allocation to prevent multi-GB/TB OOM and integer +/// overflow in the size arithmetic. +const MAX_PROB_BYTES: usize = 256 * 1024 * 1024; + +/// Validate that a `width x depth` grid of u64 counters stays within +/// `MAX_PROB_BYTES`, using checked arithmetic. Returns the byte count on success. +fn checked_counter_bytes(width: u32, depth: u32) -> Option { + let bytes = (width as usize) + .checked_mul(depth as usize)? + .checked_mul(8)?; + if bytes > MAX_PROB_BYTES { + None + } else { + Some(bytes) + } +} + // ============================================================================ // Bloom Filter // ============================================================================ @@ -457,6 +476,10 @@ pub fn cmd_cms_initbydim(ctx: &mut CommandContext) -> RespValue { _ => return RespValue::error("ERR invalid depth"), }; + if checked_counter_bytes(width, depth).is_none() { + return RespValue::error("ERR CMS dimensions too large"); + } + let db = ctx.db(); if db.exists(&key) { return RespValue::error("ERR item exists"); @@ -495,8 +518,13 @@ pub fn cmd_cms_initbyprob(ctx: &mut CommandContext) -> RespValue { // width = ceil(e / error), depth = ceil(ln(1/probability)) let width = (std::f64::consts::E / error).ceil() as u32; let depth = (1.0_f64 / probability).ln().ceil() as u32; + let (width, depth) = (width.max(1), depth.max(1)); - let data = new_cms(width.max(1), depth.max(1)); + if checked_counter_bytes(width, depth).is_none() { + return RespValue::error("ERR CMS dimensions too large"); + } + + let data = new_cms(width, depth); db.set(key, RedisObject::String(Bytes::from(data))); RespValue::ok() } @@ -993,9 +1021,17 @@ pub fn cmd_topk_reserve(ctx: &mut CommandContext) -> RespValue { (w, d, decay) } else { // Defaults: width=8*k, depth=7, decay=0.9 - (8 * k, 7, 0.9) + let w = match k.checked_mul(8) { + Some(w) => w, + None => return RespValue::error("ERR invalid k"), + }; + (w, 7, 0.9) }; + if checked_counter_bytes(width, depth).is_none() { + return RespValue::error("ERR TopK dimensions too large"); + } + let db = ctx.db(); if db.exists(&key) { return RespValue::error("ERR item exists"); diff --git a/src/command/scripting.rs b/src/command/scripting.rs index 34ffbf9..5d049c9 100644 --- a/src/command/scripting.rs +++ b/src/command/scripting.rs @@ -22,7 +22,8 @@ pub fn cmd_eval(ctx: &mut CommandContext) -> RespValue { let keys: Vec = ctx.args[3..3 + numkeys].to_vec(); let argv: Vec = ctx.args[3 + numkeys..].to_vec(); - scripting_engine::execute_script(&script, &keys, &argv, ctx.store, ctx.db_index) + let readonly = String::from_utf8_lossy(&ctx.args[0]).eq_ignore_ascii_case("eval_ro"); + scripting_engine::execute_script(&script, &keys, &argv, ctx.store, ctx.db_index, readonly) } /// EVALSHA sha1 numkeys key [key ...] arg [arg ...] @@ -55,7 +56,8 @@ pub fn cmd_evalsha(ctx: &mut CommandContext) -> RespValue { Some(script_source) => { let keys: Vec = ctx.args[3..3 + numkeys].to_vec(); let argv: Vec = ctx.args[3 + numkeys..].to_vec(); - scripting_engine::execute_script(&script_source, &keys, &argv, ctx.store, ctx.db_index) + let readonly = String::from_utf8_lossy(&ctx.args[0]).eq_ignore_ascii_case("evalsha_ro"); + scripting_engine::execute_script(&script_source, &keys, &argv, ctx.store, ctx.db_index, readonly) } None => RespValue::error(format!("NOSCRIPT No matching script. Please use EVAL.")), } @@ -289,13 +291,14 @@ pub fn cmd_fcall(ctx: &mut CommandContext) -> RespValue { Some(script) => { let keys: Vec = ctx.args[3..3 + numkeys].to_vec(); let argv: Vec = ctx.args[3 + numkeys..].to_vec(); - scripting_engine::execute_script(&script, &keys, &argv, ctx.store, ctx.db_index) + let readonly = String::from_utf8_lossy(&ctx.args[0]).eq_ignore_ascii_case("fcall_ro"); + scripting_engine::execute_script(&script, &keys, &argv, ctx.store, ctx.db_index, readonly) } None => RespValue::error(format!("ERR Function not found")), } } -/// FCALL_RO - same as FCALL but read-only (we don't enforce read-only in this pass) +/// FCALL_RO - same as FCALL but rejects write commands (read-only). pub fn cmd_fcall_ro(ctx: &mut CommandContext) -> RespValue { cmd_fcall(ctx) } diff --git a/src/command/strings.rs b/src/command/strings.rs index 2aee347..484f046 100644 --- a/src/command/strings.rs +++ b/src/command/strings.rs @@ -424,6 +424,19 @@ pub fn cmd_setrange(ctx: &mut CommandContext) -> RespValue { }; let value = ctx.args[3].clone(); + // Cap the resulting length at Redis's 512 MB string limit to prevent an + // attacker-controlled offset from forcing a huge allocation, and to avoid + // usize overflow in `offset + value.len()`. + const MAX_STRING_BYTES: usize = 512 * 1024 * 1024; + let needed = match offset.checked_add(value.len()) { + Some(n) if n <= MAX_STRING_BYTES => n, + _ => { + return RespValue::error( + "ERR string exceeds maximum allowed size (proto-max-bulk-len)", + ) + } + }; + let db = ctx.db(); let mut existing = match db.get(&key) { Some(RedisObject::String(b)) => b.to_vec(), @@ -431,7 +444,6 @@ pub fn cmd_setrange(ctx: &mut CommandContext) -> RespValue { None => Vec::new(), }; - let needed = offset + value.len(); if needed > existing.len() { existing.resize(needed, 0); } diff --git a/src/http.rs b/src/http.rs index 78cbd34..4b29546 100644 --- a/src/http.rs +++ b/src/http.rs @@ -63,11 +63,14 @@ async fn handle_request( let path = req.uri().path().to_string(); // /health is the only unauthenticated endpoint (liveness probe). - if !(method == Method::GET && path == "/health") { - if let Some(resp) = check_auth(&req, &state).await { - return Ok(resp); + let user = if method == Method::GET && path == "/health" { + "default".to_string() + } else { + match check_auth(&req, &state).await { + Ok(u) => u, + Err(resp) => return Ok(resp), } - } + }; let result = match (method, path.as_str()) { (Method::GET, "/health") => handle_health(&state), @@ -75,20 +78,20 @@ async fn handle_request( (Method::GET, "/metrics") => handle_metrics(&state).await, (Method::POST, "/api/v1/command") => { let body = req.collect().await?.to_bytes(); - handle_command(&state, &body).await + handle_command(&state, &user, &body).await } (Method::GET, p) if p.starts_with("/api/v1/") => { let key = &p["/api/v1/".len()..]; - handle_get_key(&state, key).await + handle_get_key(&state, &user, key).await } (Method::PUT, p) if p.starts_with("/api/v1/") => { let key = &p["/api/v1/".len()..]; let body = req.collect().await?.to_bytes(); - handle_put_key(&state, key, &body).await + handle_put_key(&state, &user, key, &body).await } (Method::DELETE, p) if p.starts_with("/api/v1/") => { let key = &p["/api/v1/".len()..]; - handle_delete_key(&state, key).await + handle_delete_key(&state, &user, key).await } _ => json_response(StatusCode::NOT_FOUND, &json!({"error": "not found"})), }; @@ -96,6 +99,11 @@ async fn handle_request( result } +/// 403 response for an authenticated user lacking permission for a command/key. +fn forbidden_response(msg: &str) -> Result>, BoxError> { + json_response(StatusCode::FORBIDDEN, &json!({"error": msg})) +} + /// Validate request authentication. Returns `None` if the request is authorized, /// `Some(401-response)` otherwise. /// @@ -108,15 +116,15 @@ async fn handle_request( async fn check_auth( req: &Request, state: &Arc, -) -> Option>> { +) -> Result>> { + use crate::command::acl; + let has_requirepass = state.config.requirepass.is_some(); - let has_acl_password = { - let users = state.acl_users.lock().await; - users.values().any(|u| !u.no_pass && !u.passwords.is_empty()) - }; + let has_acl_password = acl::any_password_required(); + // Open (no auth configured): act as the unrestricted default user. if !has_requirepass && !has_acl_password { - return None; + return Ok("default".to_string()); } let token = req @@ -128,25 +136,19 @@ async fn check_auth( let token = match token { Some(t) => t, - None => return Some(unauthorized_response()), + None => return Err(unauthorized_response()), }; if let Some(ref req_pass) = state.config.requirepass { if token == *req_pass { - return None; + return Ok("default".to_string()); } } - use sha2::{Digest, Sha256}; - let hash = format!("{:x}", Sha256::digest(token.as_bytes())); - let users = state.acl_users.lock().await; - for user in users.values() { - if user.enabled && user.passwords.contains(&hash) { - return None; - } + match acl::http_authenticate(&token) { + Some(user) => Ok(user), + None => Err(unauthorized_response()), } - - Some(unauthorized_response()) } fn unauthorized_response() -> Response> { @@ -278,8 +280,16 @@ async fn handle_metrics(state: &SharedState) -> Result>, Bo async fn handle_get_key( state: &SharedState, + user: &str, key: &str, ) -> Result>, BoxError> { + use crate::command::acl; + if !acl::is_command_allowed(user, "GET") { + return forbidden_response("this user has no permissions to run the 'get' command"); + } + if !acl::is_key_allowed(user, key) { + return forbidden_response("this user has no permissions to access one of the keys used as arguments"); + } let key_bytes = Bytes::from(key.to_string()); let mut store = state.store.lock().await; let db = store.db_mut(0); @@ -309,9 +319,17 @@ async fn handle_get_key( async fn handle_put_key( state: &SharedState, + user: &str, key: &str, body: &[u8], ) -> Result>, BoxError> { + use crate::command::acl; + if !acl::is_command_allowed(user, "SET") { + return forbidden_response("this user has no permissions to run the 'set' command"); + } + if !acl::is_key_allowed(user, key) { + return forbidden_response("this user has no permissions to access one of the keys used as arguments"); + } let key_bytes = Bytes::from(key.to_string()); let value = Bytes::from(body.to_vec()); let mut store = state.store.lock().await; @@ -324,8 +342,16 @@ async fn handle_put_key( async fn handle_delete_key( state: &SharedState, + user: &str, key: &str, ) -> Result>, BoxError> { + use crate::command::acl; + if !acl::is_command_allowed(user, "DEL") { + return forbidden_response("this user has no permissions to run the 'del' command"); + } + if !acl::is_key_allowed(user, key) { + return forbidden_response("this user has no permissions to access one of the keys used as arguments"); + } let key_bytes = Bytes::from(key.to_string()); let mut store = state.store.lock().await; let db = store.db_mut(0); @@ -337,6 +363,7 @@ async fn handle_delete_key( async fn handle_command( state: &SharedState, + user: &str, body: &[u8], ) -> Result>, BoxError> { let parsed: Value = match serde_json::from_slice(body) { @@ -377,6 +404,27 @@ async fn handle_command( ); } + // Enforce the same ACL command/key gating the RESP path applies, so the + // HTTP command endpoint cannot bypass a user's restrictions. + { + use crate::command::acl; + let cmd_name = String::from_utf8_lossy(&args[0]).to_uppercase(); + if !acl::is_command_allowed(user, &cmd_name) { + return forbidden_response(&format!( + "this user has no permissions to run the '{}' command", + cmd_name.to_lowercase() + )); + } + if args.len() > 1 && !acl::user_has_all_keys(user) { + let key_str = String::from_utf8_lossy(&args[1]); + if !acl::is_key_allowed(user, &key_str) { + return forbidden_response( + "this user has no permissions to access one of the keys used as arguments", + ); + } + } + } + let mut store = state.store.lock().await; let mut ctx = CommandContext { store: &mut store, diff --git a/src/persistence/aof.rs b/src/persistence/aof.rs index 147940f..018afdd 100644 --- a/src/persistence/aof.rs +++ b/src/persistence/aof.rs @@ -7,6 +7,13 @@ use std::time::{Instant, Duration}; use crate::storage::Store; use crate::storage::types::RedisObject; +/// Maximum multibulk element count accepted from an AOF entry (mirrors the +/// RESP parser's limit). Guards replay of a tampered/corrupt AOF against +/// allocation-based DoS. +const AOF_MAX_MULTIBULK: usize = 1_048_576; +/// Maximum bulk-argument length accepted from an AOF entry (512 MB). +const AOF_MAX_BULK: usize = 512 * 1024 * 1024; + /// Append-Only File writer. pub struct AofWriter { path: PathBuf, @@ -502,7 +509,16 @@ fn read_resp_command(reader: &mut BufReader) -> io::Result return Ok(Some(vec![])), }; - let mut args = Vec::with_capacity(count); + // Bound the multibulk count from a possibly-tampered AOF so a huge `*N` + // header can't force a giant reservation (OOM / capacity-overflow panic). + if count > AOF_MAX_MULTIBULK { + return Err(io::Error::new( + io::ErrorKind::InvalidData, + "AOF multibulk count exceeds limit", + )); + } + + let mut args = Vec::with_capacity(count.min(1024)); for _ in 0..count { // Read $N\r\n let mut size_line = String::new(); @@ -518,6 +534,15 @@ fn read_resp_command(reader: &mut BufReader) -> io::Result continue, }; + // Bound the bulk length so a huge `$N` header can't pre-allocate + // gigabytes before any data is read (and `size + 2` can't overflow). + if size > AOF_MAX_BULK { + return Err(io::Error::new( + io::ErrorKind::InvalidData, + "AOF bulk length exceeds limit", + )); + } + // Read exactly `size` bytes + \r\n let mut buf = vec![0u8; size + 2]; reader.read_exact(&mut buf)?; diff --git a/src/scripting_engine.rs b/src/scripting_engine.rs index 6883f16..645bea4 100644 --- a/src/scripting_engine.rs +++ b/src/scripting_engine.rs @@ -164,20 +164,50 @@ impl FunctionLibrary { } } +/// Hard memory ceiling for a single script's Lua VM (256 MB). Prevents a script +/// from allocating until the host is OOM-killed. +const SCRIPT_MEMORY_LIMIT: usize = 256 * 1024 * 1024; + +/// Wall-clock budget for a single script (5 s). Command execution is +/// single-threaded and holds the store lock, so a runaway script would +/// otherwise wedge the whole server. Enforced via an instruction hook. +const SCRIPT_TIMEOUT_MS: u64 = 5_000; + /// Execute a Lua script with the given KEYS and ARGV against the store. /// This creates a sandboxed Lua environment with redis.call() and redis.pcall(). +/// When `readonly` is set (EVAL_RO / EVALSHA_RO / FCALL_RO), write commands +/// invoked from the script are rejected. pub fn execute_script( script: &str, keys: &[Bytes], argv: &[Bytes], store: &mut Store, db_index: usize, + readonly: bool, ) -> RespValue { use mlua::prelude::*; - let lua = match Lua::new() { - lua => lua, - }; + let lua = Lua::new(); + + // Bound the VM's memory so a script cannot exhaust host RAM. + let _ = lua.set_memory_limit(SCRIPT_MEMORY_LIMIT); + + // Abort the script if it runs past its wall-clock budget. The hook fires + // every N instructions; returning an error unwinds the Lua call. + let deadline = std::time::Duration::from_millis(SCRIPT_TIMEOUT_MS); + let started = std::time::Instant::now(); + lua.set_hook( + mlua::HookTriggers::new().every_nth_instruction(100_000), + move |_lua, _debug| { + if started.elapsed() > deadline { + Err(mlua::Error::RuntimeError( + "script exceeded time limit".to_string(), + )) + } else { + Ok(mlua::VmState::Continue) + } + }, + ); // Set up the sandbox: remove dangerous globals let sandbox_result = lua.scope(|_scope| { @@ -263,7 +293,7 @@ pub fn execute_script( } let mut store_ref = store_cell.borrow_mut(); - let resp = execute_redis_command(&mut **store_ref, db_idx, &str_args); + let resp = execute_redis_command(&mut **store_ref, db_idx, &str_args, readonly); resp_to_lua(lua_ctx, &resp) })?; @@ -294,7 +324,7 @@ pub fn execute_script( } let mut store_ref = store_cell.borrow_mut(); - let resp = execute_redis_command(&mut **store_ref, db_idx, &str_args); + let resp = execute_redis_command(&mut **store_ref, db_idx, &str_args, readonly); match &resp { RespValue::Error(e) => { let err_table = lua_ctx.create_table()?; @@ -353,12 +383,34 @@ pub fn execute_script( } /// Execute a Redis command from within a Lua script. -fn execute_redis_command(store: &mut Store, db_index: usize, args: &[String]) -> RespValue { +fn execute_redis_command( + store: &mut Store, + db_index: usize, + args: &[String], + readonly: bool, +) -> RespValue { if args.is_empty() { return RespValue::error("ERR empty command"); } - let _cmd = args[0].to_uppercase(); + let cmd = args[0].to_uppercase(); + + // Block nested scripting: a script calling EVAL/FCALL/etc. can recurse + // without bound and overflow the native stack, aborting the process. + if matches!( + cmd.as_str(), + "EVAL" | "EVALSHA" | "EVAL_RO" | "EVALSHA_RO" | "FCALL" | "FCALL_RO" | "FUNCTION" | "SCRIPT" + ) { + return RespValue::error("ERR This Redis command is not allowed from script"); + } + + // In read-only script context, reject write commands. + if readonly && crate::persistence::aof::is_write_command(&cmd) { + return RespValue::error( + "ERR Write commands are not allowed from read-only scripts.", + ); + } + let byte_args: Vec = args.iter().map(|s| Bytes::from(s.clone())).collect(); // Use the command registry to execute @@ -474,7 +526,7 @@ mod tests { #[test] fn test_execute_simple_script() { let mut store = Store::new(16); - let result = execute_script("return 42", &[], &[], &mut store, 0); + let result = execute_script("return 42", &[], &[], &mut store, 0, false); assert_eq!(result, RespValue::integer(42)); } @@ -489,6 +541,7 @@ mod tests { &argv, &mut store, 0, + false, ); assert_eq!(result, RespValue::bulk_string(Bytes::from("mykey myval"))); } @@ -502,10 +555,28 @@ mod tests { &[], &mut store, 0, + false, ); assert_eq!(result, RespValue::bulk_string(Bytes::from("testval"))); } + #[test] + fn test_readonly_script_rejects_writes() { + let mut store = Store::new(16); + let result = execute_script( + "return redis.call('SET', 'k', 'v')", + &[], + &[], + &mut store, + 0, + true, + ); + match result { + RespValue::Error(e) => assert!(e.contains("read-only")), + other => panic!("expected error, got {:?}", other), + } + } + #[test] fn test_execute_script_pcall_error() { let mut store = Store::new(16); @@ -515,6 +586,7 @@ mod tests { &[], &mut store, 0, + false, ); // pcall should catch the error match result { diff --git a/src/server/connection.rs b/src/server/connection.rs index 7569e22..aaff19c 100644 --- a/src/server/connection.rs +++ b/src/server/connection.rs @@ -13,6 +13,13 @@ use crate::command::CommandContext; use crate::storage::db::glob_match; use super::SharedState; +/// Maximum size of a connection's unprocessed query buffer. A single bulk +/// argument may be up to 512 MB (the parser's `MAX_BULK_LEN`), so this ceiling +/// sits above that to allow one max-size argument while still bounding an +/// attacker who streams a partial/incomplete frame forever. Mirrors Redis's +/// client query-buffer cap. +const MAX_QUERY_BUFFER: usize = 1024 * 1024 * 1024; + /// A stream that may or may not be TLS-wrapped. pub enum MaybeTls { Plain(TcpStream), @@ -59,12 +66,6 @@ impl AsyncWrite for MaybeTls { } } -enum AclMatch { - Ok, - WrongPass, - Disabled, -} - pub struct Connection { stream: MaybeTls, state: Arc, @@ -146,6 +147,12 @@ impl Connection { self.cleanup_pubsub().await; return Ok(()); } + if self.buffer.len() > MAX_QUERY_BUFFER { + let err = RespValue::error("ERR Protocol error: unbalanced/too-large query buffer"); + let _ = self.stream.write_all(&err.serialize()).await; + self.cleanup_pubsub().await; + return Ok(()); + } while let Some(response) = self.try_process_command().await? { let data = response.serialize(); self.stream.write_all(&data).await?; @@ -179,6 +186,12 @@ impl Connection { self.cleanup_pubsub().await; return Ok(()); } + if self.buffer.len() > MAX_QUERY_BUFFER { + let err = RespValue::error("ERR Protocol error: unbalanced/too-large query buffer"); + let _ = self.stream.write_all(&err.serialize()).await; + self.cleanup_pubsub().await; + return Ok(()); + } } } } @@ -328,25 +341,25 @@ impl Connection { } } - // ACL check: verify user is allowed to execute this command + // ACL check: verify user is allowed to execute this command. This reads + // the shared ACL registry that `ACL SETUSER` mutates, so runtime rule + // changes are enforced. { - let acl_users = self.state.acl_users.lock().await; - if let Some(user) = acl_users.get(&self.auth_username) { - if !user.is_command_allowed(&cmd_name) { - return RespValue::error(format!( - "NOPERM this user has no permissions to run the '{}' command", - cmd_name.to_lowercase() - )); - } - // Check key patterns for commands that have keys - if args.len() > 1 && !user.all_keys { - let key_str = String::from_utf8_lossy(&args[1]); - if !user.is_key_allowed(&key_str) { - return RespValue::error( - "NOPERM this user has no permissions to access one of the keys used as arguments" - .to_string(), - ); - } + use crate::command::acl; + if !acl::is_command_allowed(&self.auth_username, &cmd_name) { + return RespValue::error(format!( + "NOPERM this user has no permissions to run the '{}' command", + cmd_name.to_lowercase() + )); + } + // Check key patterns for commands that have keys + if args.len() > 1 && !acl::user_has_all_keys(&self.auth_username) { + let key_str = String::from_utf8_lossy(&args[1]); + if !acl::is_key_allowed(&self.auth_username, &key_str) { + return RespValue::error( + "NOPERM this user has no permissions to access one of the keys used as arguments" + .to_string(), + ); } } } @@ -832,27 +845,8 @@ impl Connection { } } - let acl_match = { - let users = self.state.acl_users.lock().await; - users.get(&username).map(|u| { - if !u.enabled { - AclMatch::Disabled - } else if u.no_pass { - AclMatch::Ok - } else { - use sha2::{Digest, Sha256}; - let hash = format!("{:x}", Sha256::digest(password.as_bytes())); - if u.passwords.contains(&hash) { - AclMatch::Ok - } else { - AclMatch::WrongPass - } - } - }) - }; - - match acl_match { - Some(AclMatch::Ok) => { + match crate::command::acl::check_password(&username, &password) { + crate::command::acl::AuthOutcome::Ok => { self.auth_success(username); RespValue::ok() } @@ -876,27 +870,8 @@ impl Connection { } } - let acl_match = { - let users = self.state.acl_users.lock().await; - users.get(&username).map(|u| { - if !u.enabled { - AclMatch::Disabled - } else if u.no_pass { - AclMatch::Ok - } else { - use sha2::{Digest, Sha256}; - let hash = format!("{:x}", Sha256::digest(password.as_bytes())); - if u.passwords.contains(&hash) { - AclMatch::Ok - } else { - AclMatch::WrongPass - } - } - }) - }; - - match acl_match { - Some(AclMatch::Ok) => { + match crate::command::acl::check_password(&username, &password) { + crate::command::acl::AuthOutcome::Ok => { self.auth_success(username); None } diff --git a/src/server/mod.rs b/src/server/mod.rs index 2831024..a5b441c 100644 --- a/src/server/mod.rs +++ b/src/server/mod.rs @@ -138,66 +138,6 @@ impl PubSubManager { } } -/// ACL user definition. -#[derive(Debug, Clone)] -#[allow(dead_code)] -pub struct AclUser { - pub enabled: bool, - /// SHA-256 hashed passwords. - pub passwords: Vec, - pub allowed_commands: HashSet, - pub denied_commands: HashSet, - pub key_patterns: Vec, - pub channel_patterns: Vec, - pub all_commands: bool, - pub all_keys: bool, - pub no_pass: bool, -} - -impl AclUser { - pub fn default_user() -> Self { - Self { - enabled: true, - passwords: Vec::new(), - allowed_commands: HashSet::new(), - denied_commands: HashSet::new(), - key_patterns: vec!["*".to_string()], - channel_patterns: vec!["*".to_string()], - all_commands: true, - all_keys: true, - no_pass: true, - } - } - - /// Check if a command is allowed for this user. - pub fn is_command_allowed(&self, cmd: &str) -> bool { - if !self.enabled { - return false; - } - let cmd_upper = cmd.to_uppercase(); - if self.denied_commands.contains(&cmd_upper) { - return false; - } - if self.all_commands { - return true; - } - self.allowed_commands.contains(&cmd_upper) - } - - /// Check if a key pattern matches. - pub fn is_key_allowed(&self, key: &str) -> bool { - if self.all_keys { - return true; - } - for pattern in &self.key_patterns { - if crate::storage::db::glob_match(pattern, key) { - return true; - } - } - false - } -} - /// Per-command latency tracking. #[derive(Debug, Clone, Default)] pub struct LatencyStats { @@ -250,8 +190,6 @@ pub struct SharedState { pub script_cache: ScriptCache, /// Function library. pub function_library: FunctionLibrary, - /// ACL user registry. - pub acl_users: Mutex>, /// Per-command latency statistics. pub latency_stats: Mutex>, /// Slow log entries. @@ -353,17 +291,9 @@ impl Server { None }; - // Initialize default ACL user - let mut acl_users = HashMap::new(); - let mut default_user = AclUser::default_user(); - if let Some(ref pass) = self.config.requirepass { - // Hash the requirepass for the default user - use sha2::{Sha256, Digest}; - let hash = format!("{:x}", Sha256::digest(pass.as_bytes())); - default_user.passwords.push(hash); - default_user.no_pass = false; - } - acl_users.insert("default".to_string(), default_user); + // Apply requirepass to the shared ACL registry (the single source of + // truth read by both the RESP and HTTP enforcement paths). + crate::command::acl::init_default_password(self.config.requirepass.as_deref()); let state = Arc::new(SharedState { store: Mutex::new(self.store), @@ -380,7 +310,6 @@ impl Server { connected_clients: AtomicU64::new(0), script_cache: ScriptCache::new(), function_library: FunctionLibrary::new(), - acl_users: Mutex::new(acl_users), latency_stats: Mutex::new(HashMap::new()), slowlog: Mutex::new(Vec::new()), slowlog_next_id: AtomicU64::new(0),