Skip to content

Commit d203a27

Browse files
committed
Fix infinite loop in redact_api_key when string has multiple api_key params
After replacing a value with [REDACTED], the search offset must advance past the replacement. Without this, out.find("api_key=") re-finds the same position on every iteration and loops forever. Also adds five unit tests covering: no-op, single, multiple occurrences, end-of-string, and already-redacted inputs.
1 parent 219cf72 commit d203a27

File tree

1 file changed

+51
-1
lines changed

1 file changed

+51
-1
lines changed

src/error.rs

Lines changed: 51 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,13 +19,17 @@ fn redact_api_key(s: &str) -> std::borrow::Cow<'_, str> {
1919
return std::borrow::Cow::Borrowed(s);
2020
}
2121
let mut out = s.to_string();
22-
while let Some(pos) = out.find("api_key=") {
22+
let mut search_from = 0;
23+
while let Some(rel) = out[search_from..].find("api_key=") {
24+
let pos = search_from + rel;
2325
let value_start = pos + "api_key=".len();
2426
let value_end = out[value_start..]
2527
.find(['&', ' ', ')', '"', '\''])
2628
.map(|i| value_start + i)
2729
.unwrap_or(out.len());
2830
out.replace_range(value_start..value_end, "[REDACTED]");
31+
// Advance past "api_key=[REDACTED]" so we don't re-examine it.
32+
search_from = value_start + "[REDACTED]".len();
2933
}
3034
std::borrow::Cow::Owned(out)
3135
}
@@ -60,3 +64,49 @@ pub fn exit_code(err: &CliError) -> i32 {
6064
CliError::NetworkError { .. } => 1,
6165
}
6266
}
67+
68+
#[cfg(test)]
69+
mod tests {
70+
use super::*;
71+
72+
#[test]
73+
fn test_redact_no_api_key_is_noop() {
74+
let s = "some error without credentials";
75+
assert!(matches!(redact_api_key(s), std::borrow::Cow::Borrowed(_)));
76+
}
77+
78+
#[test]
79+
fn test_redact_single_occurrence() {
80+
let s = "https://serpapi.com/search.json?q=coffee&api_key=supersecret&engine=google";
81+
let result = redact_api_key(s);
82+
assert!(result.contains("api_key=[REDACTED]"));
83+
assert!(!result.contains("supersecret"));
84+
}
85+
86+
#[test]
87+
fn test_redact_multiple_occurrences_no_infinite_loop() {
88+
// Two api_key params (e.g. malformed URL); must terminate and redact both.
89+
let s = "api_key=first&other=x&api_key=second";
90+
let result = redact_api_key(s);
91+
assert!(result.contains("api_key=[REDACTED]"));
92+
assert!(!result.contains("first"));
93+
assert!(!result.contains("second"));
94+
// Both occurrences replaced
95+
assert_eq!(result.matches("[REDACTED]").count(), 2);
96+
}
97+
98+
#[test]
99+
fn test_redact_at_end_of_string() {
100+
let s = "request failed: api_key=abc123";
101+
let result = redact_api_key(s);
102+
assert_eq!(result, "request failed: api_key=[REDACTED]");
103+
}
104+
105+
#[test]
106+
fn test_redact_already_redacted_does_not_loop() {
107+
// If somehow called twice, [REDACTED] contains no secret and loop terminates.
108+
let s = "api_key=[REDACTED]";
109+
let result = redact_api_key(s);
110+
assert_eq!(result.as_ref(), "api_key=[REDACTED]");
111+
}
112+
}

0 commit comments

Comments
 (0)