Skip to content

Commit

Permalink
Support all valid vss paths
Browse files Browse the repository at this point in the history
  • Loading branch information
@rafaeling @SebastianSchildt
rafaeling authored and SebastianSchildt committed Oct 18, 2023
1 parent 4fe783b commit 258f349
Show file tree
Hide file tree
Showing 3 changed files with 151 additions and 30 deletions.
56 changes: 55 additions & 1 deletion kuksa_databroker/databroker/src/broker.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ use std::time::SystemTime;

use tracing::{debug, info, warn};

use crate::glob;

#[derive(Debug)]
pub enum UpdateError {
NotFound,
Expand All @@ -46,7 +48,7 @@ pub enum ReadError {
PermissionExpired,
}

#[derive(Debug)]
#[derive(Debug, PartialEq)]
pub enum RegistrationError {
ValidationError,
PermissionDenied,
Expand Down Expand Up @@ -1024,6 +1026,10 @@ impl<'a, 'b> DatabaseWriteAccess<'a, 'b> {
allowed: Option<types::DataValue>,
datapoint: Option<Datapoint>,
) -> Result<i32, RegistrationError> {
if !glob::is_valid_path(name.as_str()) {
return Err(RegistrationError::ValidationError);
}

self.permissions
.can_create(&name)
.map_err(|err| match err {
Expand Down Expand Up @@ -2913,4 +2919,52 @@ mod tests {
}
}
}

#[tokio::test]
async fn test_register_invalid_and_valid_path() {
let broker = DataBroker::default();
let broker = broker.authorized_access(&permissions::ALLOW_ALL);

let error = broker
.add_entry(
"test. signal:3".to_owned(),
DataType::String,
ChangeType::OnChange,
EntryType::Sensor,
"Test signal 3".to_owned(),
Some(DataValue::Int32Array(Vec::from([1, 2, 3, 4]))),
)
.await
.unwrap_err();
assert_eq!(error, RegistrationError::ValidationError);

let id = broker
.add_entry(
"Vehicle._kuksa.databroker.GitVersion.Do_you_not_like_smörgåstårta.tschö_mit_ö.東京_Москва_r#true".to_owned(),
DataType::Bool,
ChangeType::OnChange,
EntryType::Sensor,
"Test datapoint".to_owned(),
Some(DataValue::BoolArray(Vec::from([true]))),
)
.await
.expect("Register datapoint should succeed");
{
match broker.get_entry_by_id(id).await {
Ok(entry) => {
assert_eq!(entry.metadata.id, id);
assert_eq!(entry.metadata.path, "Vehicle._kuksa.databroker.GitVersion.Do_you_not_like_smörgåstårta.tschö_mit_ö.東京_Москва_r#true");
assert_eq!(entry.metadata.data_type, DataType::Bool);
assert_eq!(entry.metadata.description, "Test datapoint");
assert_eq!(
entry.metadata.allowed,
Some(DataValue::BoolArray(Vec::from([true])))
);
}
Err(_) => {
panic!("no metadata returned");
}
}
}
}
}
120 changes: 93 additions & 27 deletions kuksa_databroker/databroker/src/glob.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,21 +40,21 @@ pub fn to_regex_string(glob: &str) -> String {
if glob.starts_with("**.") {
regex_string = regex_string.replace("**\\.", ".+\\.");
} else if glob.starts_with("*.") {
regex_string = regex_string.replace("*\\.", "[^.]+\\.");
regex_string = regex_string.replace("*\\.", "[^.\\s\\:]+\\.");
}

if glob.ends_with(".*") {
regex_string = regex_string.replace("\\.*", "\\.[^.]+");
regex_string = regex_string.replace("\\.*", "\\.[^.\\s\\:]+");
} else if glob.ends_with(".**") {
regex_string = regex_string.replace("\\.**", "\\..*");
}

if glob.contains(".**.") {
regex_string = regex_string.replace("\\.**", "[\\.[A-Z][a-zA-Z0-9]*]*");
regex_string = regex_string.replace("\\.**", "[\\.[^.\\s\\:]+]*");
}

if glob.contains(".*.") {
regex_string = regex_string.replace("\\.*", "\\.[^.]+");
regex_string = regex_string.replace("\\.*", "\\.[^.\\s\\:]+");
}
}

Expand All @@ -77,7 +77,7 @@ lazy_static! {
^ # anchor at start (only match full paths)
# At least one subpath (consisting of either of three):
(?:
[A-Z][a-zA-Z0-9]* # An alphanumeric name (first letter capitalized)
[^\.\s\:]+ # Any character except :, whitespace and .
|
\* # An asterisk
|
Expand All @@ -88,12 +88,12 @@ lazy_static! {
(?:
\. # Separator, literal dot
(?:
[A-Z][a-zA-Z0-9]* # Alphanumeric name (first letter capitalized)
[^\.\s\:]+ # Any character except :, whitespace and .
|
\* # An asterisk
|
\*\* # A double asterisk
)
)+
)*
$ # anchor at end (to match only full paths)
",
Expand All @@ -105,11 +105,40 @@ pub fn is_valid_pattern(input: &str) -> bool {
REGEX_VALID_PATTERN.is_match(input)
}

lazy_static! {
static ref REGEX_VALID_PATH: regex::Regex = regex::Regex::new(
r"(?x)
^ # anchor at start (only match full paths)
# At least one subpath (consisting of either of three):
(?:
[^\.\s\:\*]+ # Any character except :, whitespace and .
)
# which can be followed by ( separator + another subpath )
# repeated any number of times
(?:
\. # Separator, literal dot
(?:
[^\.\s\:\*]+ # Any character except :, whitespace and .
)+
)*
$ # anchor at end (to match only full paths)
",
)
.expect("regex compilation (of static path) should always succeed");
}

pub fn is_valid_path(input: &str) -> bool {
REGEX_VALID_PATH.is_match(input)
}

#[cfg(test)]
mod tests {
use super::*;

static ALL_SIGNALS: &[&str] = &[
"Vehicle._kuksa.databroker.GitVersion",
"Vehicle._kuksa.databroker.CargoVersion",
"Vehicle._kuksa.databroker.GitVersion",
"Vehicle.ADAS.ABS.IsEnabled",
"Vehicle.ADAS.ABS.IsEngaged",
"Vehicle.ADAS.ABS.IsError",
Expand Down Expand Up @@ -1250,7 +1279,7 @@ mod tests {
#[test]
fn test_regex_wildcard_at_end() {
assert!(using_glob("Vehicle.Cabin.Sunroof.*")
.should_equal_regex_pattern(r"^Vehicle\.Cabin\.Sunroof\.[^.]+$"));
.should_equal_regex_pattern(r"^Vehicle\.Cabin\.Sunroof\.[^.\s\:]+$"));
}

#[test]
Expand All @@ -1267,7 +1296,7 @@ mod tests {
#[test]
fn test_regex_single_wildcard_in_middle() {
assert!(using_glob("Vehicle.Cabin.Sunroof.*.Position")
.should_equal_regex_pattern(r"^Vehicle\.Cabin\.Sunroof\.[^.]+\.Position$"));
.should_equal_regex_pattern(r"^Vehicle\.Cabin\.Sunroof\.[^.\s\:]+\.Position$"));
}

#[test]
Expand Down Expand Up @@ -1357,7 +1386,7 @@ mod tests {
#[ignore]
#[test]
fn test_regex_single_wildcard_at_the_beginning() {
assert!(using_glob("*.Sunroof").should_equal_regex_pattern(r"^[^.]+\.Sunroof$"));
assert!(using_glob("*.Sunroof").should_equal_regex_pattern(r"^[^.\s\:]+\.Sunroof$"));
}

#[test]
Expand All @@ -1380,14 +1409,24 @@ mod tests {
.should_match_no_signals());
}

#[test]
fn test_matches_underscore_cases() {
assert!(using_glob("Vehicle._kuksa.**")
.with_signals(ALL_SIGNALS)
.should_match_signals(&[
"Vehicle._kuksa.databroker.GitVersion",
"Vehicle._kuksa.databroker.CargoVersion",
"Vehicle._kuksa.databroker.GitVersion",
],));
}

#[test]
fn test_is_valid_pattern() {
assert!(is_valid_pattern("String.*"));
assert!(is_valid_pattern("String.**"));
assert!(is_valid_pattern("Vehicle.Chassis.Axle.Row2.Wheel.*"));
assert!(is_valid_pattern("String.String.String.String.*"));
assert!(is_valid_pattern("String.String.String.String.**"));
assert!(is_valid_pattern("String.String.String.String"));
assert!(is_valid_pattern("String.String.String.String.String.**"));
assert!(is_valid_pattern("String.String.String.*.String"));
assert!(is_valid_pattern("String.String.String.**.String"));
Expand All @@ -1397,28 +1436,55 @@ mod tests {
assert!(is_valid_pattern(
"String.String.String.String.*.String.String"
));

assert!(is_valid_pattern("String.*.String.String"));
assert!(is_valid_pattern("String.String.**.String.String"));
assert!(is_valid_pattern("String.**.String.String"));
assert!(is_valid_pattern("**.String.String.String.**"));
assert!(is_valid_pattern("**.String.String.String.*"));
assert!(is_valid_pattern("**.String.String.9_tring.*"));
assert!(is_valid_pattern("**.string.String.String.*"));
assert!(is_valid_pattern("**._string.String.String.*"));
assert!(is_valid_pattern("String._kuksa.tring.9tring.*"));
assert!(is_valid_pattern("**.String"));
assert!(is_valid_pattern("*.String.String.String"));
assert!(is_valid_pattern("*.String"));
assert!(!is_valid_pattern("String.String.String."));
assert!(!is_valid_pattern("String.String.String.String.."));
assert!(is_valid_pattern("*.String._"));

assert!(!is_valid_pattern("String.*.String.String.."));
assert!(!is_valid_pattern("*.String.String.String.."));
assert!(!is_valid_pattern("String.**.St ring.String"));
assert!(!is_valid_pattern("String.**:String. String"));
assert!(!is_valid_pattern("String.**.St. .ring.String"));
assert!(!is_valid_pattern("String.**.St. : .ring.String"));
}

#[test]
fn test_is_valid_path() {
assert!(is_valid_path("String.String.String.String"));
assert!(is_valid_path("String._kuksa.tring.9tring"));

assert!(is_valid_path("Vehicle.Con_ñ_de_España,_sí"));
assert!(is_valid_path("Vehicle.Do_you_not_like_smörgåstårta"));
assert!(is_valid_path("Vehicle.tschö_mit_ö"));
assert!(is_valid_path("Vehicle.wie_heißt_das_lied"));
assert!(is_valid_path("Vehicle.東京_Москва_r#true"));

assert!(!is_valid_path("String.String.String."));
assert!(!is_valid_path("String.String.String.String.."));
assert!(!is_valid_path("String:String.String"));
assert!(!is_valid_path("String.St ring.String"));
assert!(!is_valid_path("String:String. String"));
assert!(!is_valid_path("String.St. .ring.String"));
assert!(!is_valid_path("String.St. : .ring.String"));
assert!(!is_valid_path("*.String:String. String"));
}

#[test]
fn test_valid_regex_path() {
assert_eq!(to_regex_string("String.*"), "^String\\.[^.]+$");
assert_eq!(to_regex_string("String.*"), "^String\\.[^.\\s\\:]+$");
assert_eq!(to_regex_string("String.**"), "^String\\..*$");
assert_eq!(
to_regex_string("String.String.String.String.*"),
"^String\\.String\\.String\\.String\\.[^.]+$"
"^String\\.String\\.String\\.String\\.[^.\\s\\:]+$"
);
assert_eq!(
to_regex_string("String.String.String.String.**"),
Expand All @@ -1434,45 +1500,45 @@ mod tests {
);
assert_eq!(
to_regex_string("String.String.String.*.String"),
"^String\\.String\\.String\\.[^.]+\\.String$"
"^String\\.String\\.String\\.[^.\\s\\:]+\\.String$"
);
assert_eq!(
to_regex_string("String.String.String.**.String"),
"^String\\.String\\.String[\\.[A-Z][a-zA-Z0-9]*]*\\.String$"
"^String\\.String\\.String[\\.[^.\\s\\:]+]*\\.String$"
);
assert_eq!(
to_regex_string("String.String.String.String.String.**.String"),
"^String\\.String\\.String\\.String\\.String[\\.[A-Z][a-zA-Z0-9]*]*\\.String$"
"^String\\.String\\.String\\.String\\.String[\\.[^.\\s\\:]+]*\\.String$"
);
assert_eq!(
to_regex_string("String.String.String.String.*.String.String"),
"^String\\.String\\.String\\.String\\.[^.]+\\.String\\.String$"
"^String\\.String\\.String\\.String\\.[^.\\s\\:]+\\.String\\.String$"
);
assert_eq!(
to_regex_string("String.*.String.String"),
"^String\\.[^.]+\\.String\\.String$"
"^String\\.[^.\\s\\:]+\\.String\\.String$"
);
assert_eq!(
to_regex_string("String.String.**.String.String"),
"^String\\.String[\\.[A-Z][a-zA-Z0-9]*]*\\.String\\.String$"
"^String\\.String[\\.[^.\\s\\:]+]*\\.String\\.String$"
);
assert_eq!(
to_regex_string("String.**.String.String"),
"^String[\\.[A-Z][a-zA-Z0-9]*]*\\.String\\.String$"
"^String[\\.[^.\\s\\:]+]*\\.String\\.String$"
);
assert_eq!(
to_regex_string("**.String.String.String.**"),
"^.+\\.String\\.String\\.String\\..*$"
);
assert_eq!(
to_regex_string("**.String.String.String.*"),
"^.+\\.String\\.String\\.String\\.[^.]+$"
"^.+\\.String\\.String\\.String\\.[^.\\s\\:]+$"
);
assert_eq!(to_regex_string("**.String"), "^.+\\.String$");
assert_eq!(to_regex_string("*.String"), "^[^.]+\\.String$");
assert_eq!(to_regex_string("*.String"), "^[^.\\s\\:]+\\.String$");
assert_eq!(
to_regex_string("*.String.String.String"),
"^[^.]+\\.String\\.String\\.String$"
"^[^.\\s\\:]+\\.String\\.String\\.String$"
);
}
}
5 changes: 3 additions & 2 deletions kuksa_databroker/databroker/src/grpc/kuksa_val_v1/val.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,17 +72,18 @@ impl proto::val_server::Val for broker::DataBroker {

// Fill valid_requests structure.
for request in requested {
if !glob::is_valid_pattern(&request.path) {
if request.path.contains('*') && !glob::is_valid_pattern(&request.path) {
errors.push(proto::DataEntryError {
path: request.path,
error: Some(proto::Error {
code: 400,
reason: "bad_request".to_owned(),
message: "Bad Path Request".to_owned(),
message: "Bad Wildcard Pattern Request".to_owned(),
}),
});
continue;
}

let view = proto::View::from_i32(request.view).ok_or_else(|| {
tonic::Status::invalid_argument(format!("Invalid View (id: {}", request.view))
})?;
Expand Down

0 comments on commit 258f349

Please sign in to comment.