Skip to content

Commit 30c3c9b

Browse files
committed
fix: Bring back character validation
1 parent ccedc5d commit 30c3c9b

File tree

1 file changed

+40
-3
lines changed

1 file changed

+40
-3
lines changed

objectstore-types/src/scope.rs

Lines changed: 40 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,13 @@
77
88
use std::fmt::{self, Write};
99

10+
/// Characters allowed in a Scope's key and value.
11+
///
12+
/// These are the URL safe characters, except for `.` which we use as separator between
13+
/// key and value of Scope components in backends.
14+
const ALLOWED_CHARS: &[u8] =
15+
b"ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789_-()$!+'";
16+
1017
/// A single scope value of an object.
1118
///
1219
/// Scopes are used in a hierarchy in object IDs.
@@ -46,7 +53,13 @@ impl Scope {
4653
{
4754
let value = value.to_string();
4855
if name.is_empty() || value.is_empty() {
49-
return Err(InvalidScopeError);
56+
return Err(InvalidScopeError::Empty);
57+
}
58+
59+
for &b in name.as_bytes().iter().chain(value.as_bytes()) {
60+
if !ALLOWED_CHARS.contains(&b) {
61+
return Err(InvalidScopeError::InvalidChar(b.into()));
62+
}
5063
}
5164

5265
Ok(Self {
@@ -68,8 +81,14 @@ impl Scope {
6881

6982
/// An error indicating that a scope is invalid, returned by [`Scope::create`].
7083
#[derive(Debug, thiserror::Error)]
71-
#[error("invalid scope: key and value must be non-empty")]
72-
pub struct InvalidScopeError;
84+
pub enum InvalidScopeError {
85+
/// Indicates that either the key or value is empty.
86+
#[error("key and value must be non-empty")]
87+
Empty,
88+
/// Indicates that the key or value contains an invalid character.
89+
#[error("invalid character '{0}'")]
90+
InvalidChar(char),
91+
}
7392

7493
/// An ordered set of resource scopes.
7594
///
@@ -189,3 +208,21 @@ impl fmt::Display for AsApiPath<'_> {
189208
}
190209
}
191210
}
211+
212+
#[cfg(test)]
213+
mod tests {
214+
use super::*;
215+
216+
/// Regression test to ensure we're not unintentionally adding characters to the allowed set
217+
/// that are required in storage or API paths.
218+
#[test]
219+
fn test_allowed_characters() {
220+
// Storage paths
221+
assert!(!ALLOWED_CHARS.contains(&b'.'));
222+
assert!(!ALLOWED_CHARS.contains(&b'/'));
223+
224+
// API paths
225+
assert!(!ALLOWED_CHARS.contains(&b'='));
226+
assert!(!ALLOWED_CHARS.contains(&b';'));
227+
}
228+
}

0 commit comments

Comments
 (0)