Skip to content
This repository has been archived by the owner on Dec 18, 2024. It is now read-only.

Support all valid vss paths #678

Merged
merged 1 commit into from
Oct 18, 2023
Merged

Support all valid vss paths #678

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 55 additions & 1 deletion kuksa_databroker/databroker/src/broker.rs
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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