From a1cc9563f754374225fcb19ffb5a816e3bf4cde3 Mon Sep 17 00:00:00 2001
From: cypherpunks <cypherpunks@torproject.org>
Date: Sat, 18 Aug 2018 13:41:19 +0000
Subject: [PATCH] rust/protover: reject extra commas

The C implementation had gotten this wrong too, in a slightly different way.

Introduced in 5af03c1ef3c4718b79abb1638f5a8c275129530a.

Fixes #27197; bugfix on 0.3.3.3-alpha.
---
 changes/bug27197              |  3 +++
 src/rust/protover/protoset.rs | 22 ++++++++++++++--------
 2 files changed, 17 insertions(+), 8 deletions(-)
 create mode 100644 changes/bug27197

diff --git a/changes/bug27197 b/changes/bug27197
new file mode 100644
index 00000000000..e389f850655
--- /dev/null
+++ b/changes/bug27197
@@ -0,0 +1,3 @@
+  o Minor bugfixes (protover, rust):
+    - Reject extra commas in version string. Fixes bug 27197; bugfix on
+      0.3.3.3-alpha.
diff --git a/src/rust/protover/protoset.rs b/src/rust/protover/protoset.rs
index 027dfba2035..309ec7a281d 100644
--- a/src/rust/protover/protoset.rs
+++ b/src/rust/protover/protoset.rs
@@ -339,13 +339,16 @@ impl FromStr for ProtoSet {
     /// # fn main() { do_test(); }  // wrap the test so we can use the ? operator
     /// ```
     fn from_str(version_string: &str) -> Result<Self, Self::Err> {
+        // If we were passed in an empty string, then return an empty ProtoSet.
+        if version_string.is_empty() {
+            return Ok(ProtoSet::default());
+        }
+
         let mut pairs: Vec<(Version, Version)> = Vec::new();
         let pieces: ::std::str::Split<char> = version_string.split(',');
 
         for p in pieces {
-            if p.is_empty() {
-                continue;
-            } else if p.contains('-') {
+            if p.contains('-') {
                 let mut pair = p.splitn(2, '-');
 
                 let low  = pair.next().ok_or(ProtoverError::Unparseable)?;
@@ -367,11 +370,7 @@ impl FromStr for ProtoSet {
                 pairs.push((v, v));
             }
         }
-        // If we were passed in an empty string, or
-        // simply a comma, or a pile of commas, then return an empty ProtoSet.
-        if pairs.len() == 0 {
-            return Ok(ProtoSet::default());
-        }
+
         ProtoSet::from_slice(&pairs[..])
     }
 }
@@ -536,6 +535,13 @@ mod test {
         assert_eq!(Err(ProtoverError::Unparseable), ProtoSet::from_str("-1"));
     }
 
+    #[test]
+    fn test_versions_from_str_commas() {
+        assert_eq!(Err(ProtoverError::Unparseable), ProtoSet::from_str(","));
+        assert_eq!(Err(ProtoverError::Unparseable), ProtoSet::from_str("1,,2"));
+        assert_eq!(Err(ProtoverError::Unparseable), ProtoSet::from_str("1,2,"));
+    }
+
     #[test]
     fn test_versions_from_str_hyphens() {
         assert_eq!(Err(ProtoverError::Unparseable), ProtoSet::from_str("--1"));