Skip to content

Commit 3570b0c

Browse files
joshrotenbergjoshrotenberg
authored
fix: normalize URL path concatenation to prevent double slashes (#282)
- Add normalize_url() method to both CloudClient and EnterpriseClient - Ensures proper URL construction regardless of base URL or path format - Fixes issue where URLs could have double slashes (e.g., https://localhost:9443//v1/cluster) - Add comprehensive tests for all URL normalization cases This makes the libraries more robust and prevents potential issues with certain HTTP servers or proxies that may not handle double slashes correctly.
1 parent 25e8d0e commit 3570b0c

File tree

4 files changed

+162
-17
lines changed

4 files changed

+162
-17
lines changed

‎crates/redis-cloud/src/client.rs‎

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -126,9 +126,16 @@ impl CloudClient {
126126
CloudClientBuilder::new()
127127
}
128128

129+
/// Normalize URL path concatenation to avoid double slashes
130+
fn normalize_url(&self, path: &str) -> String {
131+
let base = self.base_url.trim_end_matches('/');
132+
let path = path.trim_start_matches('/');
133+
format!("{}/{}", base, path)
134+
}
135+
129136
/// Make a GET request with API key authentication
130137
pub async fn get<T: serde::de::DeserializeOwned>(&self, path: &str) -> Result<T> {
131-
let url = format!("{}{}", self.base_url, path);
138+
let url = self.normalize_url(path);
132139

133140
// Redis Cloud API uses these headers for authentication
134141
let response = self
@@ -148,7 +155,7 @@ impl CloudClient {
148155
path: &str,
149156
body: &B,
150157
) -> Result<T> {
151-
let url = format!("{}{}", self.base_url, path);
158+
let url = self.normalize_url(path);
152159

153160
// Same backwards header naming as GET
154161
let response = self
@@ -169,7 +176,7 @@ impl CloudClient {
169176
path: &str,
170177
body: &B,
171178
) -> Result<T> {
172-
let url = format!("{}{}", self.base_url, path);
179+
let url = self.normalize_url(path);
173180

174181
// Same backwards header naming as GET
175182
let response = self
@@ -186,7 +193,7 @@ impl CloudClient {
186193

187194
/// Make a DELETE request
188195
pub async fn delete(&self, path: &str) -> Result<()> {
189-
let url = format!("{}{}", self.base_url, path);
196+
let url = self.normalize_url(path);
190197

191198
// Same backwards header naming as GET
192199
let response = self
@@ -240,7 +247,7 @@ impl CloudClient {
240247
path: &str,
241248
body: serde_json::Value,
242249
) -> Result<serde_json::Value> {
243-
let url = format!("{}{}", self.base_url, path);
250+
let url = self.normalize_url(path);
244251

245252
// Use backwards header names for compatibility
246253
let response = self
@@ -257,7 +264,7 @@ impl CloudClient {
257264

258265
/// Execute raw DELETE request returning any response body
259266
pub async fn delete_raw(&self, path: &str) -> Result<serde_json::Value> {
260-
let url = format!("{}{}", self.base_url, path);
267+
let url = self.normalize_url(path);
261268

262269
// Use backwards header names for compatibility
263270
let response = self

‎crates/redis-cloud/src/lib_tests.rs‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,4 +102,69 @@ mod tests {
102102
};
103103
assert_eq!(err.to_string(), "API error (400): Bad request");
104104
}
105+
106+
#[tokio::test]
107+
async fn test_url_normalization() {
108+
// Test various combinations of base URLs and paths to ensure no double slashes
109+
let test_cases = vec![
110+
(
111+
"https://api.redislabs.com/v1",
112+
"/subscriptions",
113+
"https://api.redislabs.com/v1/subscriptions",
114+
),
115+
(
116+
"https://api.redislabs.com/v1/",
117+
"/subscriptions",
118+
"https://api.redislabs.com/v1/subscriptions",
119+
),
120+
(
121+
"https://api.redislabs.com/v1",
122+
"subscriptions",
123+
"https://api.redislabs.com/v1/subscriptions",
124+
),
125+
(
126+
"https://api.redislabs.com/v1/",
127+
"subscriptions",
128+
"https://api.redislabs.com/v1/subscriptions",
129+
),
130+
(
131+
"https://api.redislabs.com/v1",
132+
"/subscriptions/123/databases",
133+
"https://api.redislabs.com/v1/subscriptions/123/databases",
134+
),
135+
(
136+
"https://api.redislabs.com/v1/",
137+
"/subscriptions/123/databases",
138+
"https://api.redislabs.com/v1/subscriptions/123/databases",
139+
),
140+
];
141+
142+
for (base_url, test_path, _expected) in test_cases {
143+
let mock_server = MockServer::start().await;
144+
145+
// Mock will fail if the URL has double slashes
146+
Mock::given(method("GET"))
147+
.and(path(test_path.trim_start_matches('/')))
148+
.respond_with(
149+
ResponseTemplate::new(200).set_body_json(serde_json::json!({"ok": true})),
150+
)
151+
.mount(&mock_server)
152+
.await;
153+
154+
let client = CloudClient::builder()
155+
.base_url(base_url.replace("https://api.redislabs.com/v1", &mock_server.uri()))
156+
.api_key("test_key")
157+
.api_secret("test_secret")
158+
.build()
159+
.unwrap();
160+
161+
let result: Result<serde_json::Value> = client.get(test_path).await;
162+
assert!(
163+
result.is_ok(),
164+
"Failed for base_url: {}, path: {}",
165+
base_url,
166+
test_path
167+
);
168+
}
169+
}
105170
}

‎crates/redis-enterprise/src/client.rs‎

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,13 @@ impl EnterpriseClient {
110110
EnterpriseClientBuilder::new()
111111
}
112112

113+
/// Normalize URL path concatenation to avoid double slashes
114+
fn normalize_url(&self, path: &str) -> String {
115+
let base = self.base_url.trim_end_matches('/');
116+
let path = path.trim_start_matches('/');
117+
format!("{}/{}", base, path)
118+
}
119+
113120
/// Create a client from environment variables
114121
///
115122
/// Reads configuration from:
@@ -141,7 +148,7 @@ impl EnterpriseClient {
141148

142149
/// Make a GET request
143150
pub async fn get<T: DeserializeOwned>(&self, path: &str) -> Result<T> {
144-
let url = format!("{}{}", self.base_url, path);
151+
let url = self.normalize_url(path);
145152
debug!("GET {}", url);
146153

147154
let response = self
@@ -158,7 +165,7 @@ impl EnterpriseClient {
158165

159166
/// Make a GET request for text content
160167
pub async fn get_text(&self, path: &str) -> Result<String> {
161-
let url = format!("{}{}", self.base_url, path);
168+
let url = self.normalize_url(path);
162169
debug!("GET {} (text)", url);
163170

164171
let response = self
@@ -192,7 +199,7 @@ impl EnterpriseClient {
192199

193200
/// Make a POST request
194201
pub async fn post<B: Serialize, T: DeserializeOwned>(&self, path: &str, body: &B) -> Result<T> {
195-
let url = format!("{}{}", self.base_url, path);
202+
let url = self.normalize_url(path);
196203
debug!("POST {}", url);
197204
trace!("Request body: {:?}", serde_json::to_value(body).ok());
198205

@@ -211,7 +218,7 @@ impl EnterpriseClient {
211218

212219
/// Make a PUT request
213220
pub async fn put<B: Serialize, T: DeserializeOwned>(&self, path: &str, body: &B) -> Result<T> {
214-
let url = format!("{}{}", self.base_url, path);
221+
let url = self.normalize_url(path);
215222
debug!("PUT {}", url);
216223
trace!("Request body: {:?}", serde_json::to_value(body).ok());
217224

@@ -230,7 +237,7 @@ impl EnterpriseClient {
230237

231238
/// Make a DELETE request
232239
pub async fn delete(&self, path: &str) -> Result<()> {
233-
let url = format!("{}{}", self.base_url, path);
240+
let url = self.normalize_url(path);
234241
debug!("DELETE {}", url);
235242

236243
let response = self
@@ -271,7 +278,7 @@ impl EnterpriseClient {
271278

272279
/// POST request for actions that return no content
273280
pub async fn post_action<B: Serialize>(&self, path: &str, body: &B) -> Result<()> {
274-
let url = format!("{}{}", self.base_url, path);
281+
let url = self.normalize_url(path);
275282
debug!("POST {}", url);
276283
trace!("Request body: {:?}", serde_json::to_value(body).ok());
277284

@@ -305,7 +312,7 @@ impl EnterpriseClient {
305312
field_name: &str,
306313
file_name: &str,
307314
) -> Result<T> {
308-
let url = format!("{}{}", self.base_url, path);
315+
let url = self.normalize_url(path);
309316
debug!("POST {} (multipart)", url);
310317

311318
let part = reqwest::multipart::Part::bytes(file_data).file_name(file_name.to_string());
@@ -336,7 +343,7 @@ impl EnterpriseClient {
336343
path: &str,
337344
body: &B,
338345
) -> Result<serde_json::Value> {
339-
let url = format!("{}{}", self.base_url, path);
346+
let url = self.normalize_url(path);
340347

341348
let response = self
342349
.client
@@ -372,7 +379,7 @@ impl EnterpriseClient {
372379
path: &str,
373380
body: serde_json::Value,
374381
) -> Result<serde_json::Value> {
375-
let url = format!("{}{}", self.base_url, path);
382+
let url = self.normalize_url(path);
376383
let response = self
377384
.client
378385
.patch(&url)
@@ -399,7 +406,7 @@ impl EnterpriseClient {
399406

400407
/// Execute raw DELETE request returning any response body
401408
pub async fn delete_raw(&self, path: &str) -> Result<serde_json::Value> {
402-
let url = format!("{}{}", self.base_url, path);
409+
let url = self.normalize_url(path);
403410
let response = self
404411
.client
405412
.delete(&url)
@@ -482,7 +489,7 @@ impl EnterpriseClient {
482489
/// Execute a Redis command on a specific database (internal use only)
483490
/// This uses the /v1/bdbs/{uid}/command endpoint which may not be publicly documented
484491
pub async fn execute_command(&self, db_uid: u32, command: &str) -> Result<serde_json::Value> {
485-
let url = format!("{}/v1/bdbs/{}/command", self.base_url, db_uid);
492+
let url = self.normalize_url(&format!("/v1/bdbs/{}/command", db_uid));
486493
let body = serde_json::json!({
487494
"command": command
488495
});

‎crates/redis-enterprise/src/lib_tests.rs‎

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -452,4 +452,70 @@ mod tests {
452452
assert_eq!(endpoints[1].exclude_proxies, Some(vec![1, 2]));
453453
assert_eq!(endpoints[1].include_proxies, Some(vec![3, 4, 5]));
454454
}
455+
456+
#[tokio::test]
457+
async fn test_url_normalization() {
458+
// Test various combinations of base URLs and paths to ensure no double slashes
459+
let test_cases = vec![
460+
(
461+
"https://localhost:9443",
462+
"/v1/cluster",
463+
"https://localhost:9443/v1/cluster",
464+
),
465+
(
466+
"https://localhost:9443/",
467+
"/v1/cluster",
468+
"https://localhost:9443/v1/cluster",
469+
),
470+
(
471+
"https://localhost:9443",
472+
"v1/cluster",
473+
"https://localhost:9443/v1/cluster",
474+
),
475+
(
476+
"https://localhost:9443/",
477+
"v1/cluster",
478+
"https://localhost:9443/v1/cluster",
479+
),
480+
(
481+
"https://localhost:9443",
482+
"/v1/bdbs/1",
483+
"https://localhost:9443/v1/bdbs/1",
484+
),
485+
(
486+
"https://localhost:9443/",
487+
"/v1/bdbs/1",
488+
"https://localhost:9443/v1/bdbs/1",
489+
),
490+
];
491+
492+
for (base_url, test_path, _expected) in test_cases {
493+
let mock_server = MockServer::start().await;
494+
495+
// Mock will fail if the URL has double slashes
496+
Mock::given(method("GET"))
497+
.and(path(test_path.trim_start_matches('/')))
498+
.and(basic_auth("test", "test"))
499+
.respond_with(
500+
ResponseTemplate::new(200).set_body_json(serde_json::json!({"ok": true})),
501+
)
502+
.mount(&mock_server)
503+
.await;
504+
505+
let client = EnterpriseClient::builder()
506+
.base_url(base_url.replace("https://localhost:9443", &mock_server.uri()))
507+
.username("test")
508+
.password("test")
509+
.build()
510+
.unwrap();
511+
512+
let result: Result<serde_json::Value> = client.get(test_path).await;
513+
assert!(
514+
result.is_ok(),
515+
"Failed for base_url: {}, path: {}",
516+
base_url,
517+
test_path
518+
);
519+
}
520+
}
455521
}

0 commit comments

Comments
 (0)