Merge remote-tracking branch 'onionk/rust-protokeyword1-035'

3 files changed, 41 insertions, 1 deletions

diff --git a/changes/bug27687 b/changes/bug27687
new file mode 100644
index 0000000000..8b7903b63e
--- /dev/null
+++ b/changes/bug27687
@@ -0,0 +1,4 @@
+  o Minor bugfixes (rust):
+    - protover parsed and accepted unknown protocol names containing invalid
+      characters outside the range [A-Za-z0-9-]. Fixes bug 27687; bugfix on
+      0.3.3.1-alpha.
diff --git a/src/rust/protover/errors.rs b/src/rust/protover/errors.rs
index 71fbc53e17..f26a48b019 100644
--- a/src/rust/protover/errors.rs
+++ b/src/rust/protover/errors.rs
@@ -18,6 +18,7 @@ pub enum ProtoverError {
     ExceedsExpansionLimit,
     UnknownProtocol,
     ExceedsNameLimit,
+    InvalidProtocol,
 }
 
 /// Descriptive error messages for `ProtoverError` variants.
@@ -48,6 +49,9 @@ impl Display for ProtoverError {
             ProtoverError::ExceedsNameLimit => {
                 write!(f, "An unrecognised protocol name was too long.")
             }
+            ProtoverError::InvalidProtocol => {
+                write!(f, "A protocol name includes invalid characters.")
+            }
         }
     }
 }
diff --git a/src/rust/protover/protover.rs b/src/rust/protover/protover.rs
index 8f99a8a8eb..6fbe7c5dc1 100644
--- a/src/rust/protover/protover.rs
+++ b/src/rust/protover/protover.rs
@@ -89,11 +89,17 @@ impl fmt::Display for UnknownProtocol {
     }
 }
 
+fn is_valid_proto(s: &str) -> bool {
+    s.chars().all(|c| c.is_ascii_alphanumeric() || c == '-')
+}
+
 impl FromStr for UnknownProtocol {
     type Err = ProtoverError;
 
     fn from_str(s: &str) -> Result<Self, Self::Err> {
-        if s.len() <= MAX_PROTOCOL_NAME_LENGTH {
+        if !is_valid_proto(s) {
+            Err(ProtoverError::InvalidProtocol)
+        } else if s.len() <= MAX_PROTOCOL_NAME_LENGTH {
             Ok(UnknownProtocol(s.to_string()))
         } else {
             Err(ProtoverError::ExceedsNameLimit)
@@ -105,6 +111,9 @@ impl UnknownProtocol {
     /// Create an `UnknownProtocol`, ignoring whether or not it
     /// exceeds MAX_PROTOCOL_NAME_LENGTH.
     fn from_str_any_len(s: &str) -> Result<Self, ProtoverError> {
+        if !is_valid_proto(s) {
+            return Err(ProtoverError::InvalidProtocol);
+        }
         Ok(UnknownProtocol(s.to_string()))
     }
 }
@@ -772,6 +781,29 @@ mod test {
 
     use super::*;
 
+    macro_rules! parse_proto {
+        ($e:expr) => {{
+            let proto: Result<UnknownProtocol, _> = $e.parse();
+            let proto2 = UnknownProtocol::from_str_any_len($e);
+            assert_eq!(proto, proto2);
+            proto
+        }};
+    }
+
+    #[test]
+    fn test_protocol_from_str() {
+        assert!(parse_proto!("Cons").is_ok());
+        assert!(parse_proto!("123").is_ok());
+        assert!(parse_proto!("1-2-3").is_ok());
+
+        let err = Err(ProtoverError::InvalidProtocol);
+        assert_eq!(err, parse_proto!("a_b_c"));
+        assert_eq!(err, parse_proto!("a b"));
+        assert_eq!(err, parse_proto!("a,"));
+        assert_eq!(err, parse_proto!("b."));
+        assert_eq!(err, parse_proto!("é"));
+    }
+
     macro_rules! assert_protoentry_is_parseable {
         ($e:expr) => {
             let protoentry: Result<ProtoEntry, ProtoverError> = $e.parse();