Merge pull request #409 from pyfisch/entitytag

feat(entitytag): Add EntityTag comparison, make EntityTag safe to use

src/header/common/etag.rs

@@ -1,5 +1,5 @@
 use header::{EntityTag, Header, HeaderFormat};
-use std::fmt::{self};
+use std::fmt::{self, Display};
 use header::parsing::from_one_raw_str;
 
 /// The `Etag` header.
@@ -28,10 +28,7 @@ impl Header for Etag {
 
 impl HeaderFormat for Etag {
     fn fmt_header(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
-        if self.0.weak {
+        self.0.fmt(fmt)
-            try!(fmt.write_str("W/"));
-        }
-        write!(fmt, "\"{}\"", self.0.tag)
     }
 }
 
@@ -46,34 +43,19 @@ mod tests {
         let mut etag: Option<Etag>;
 
         etag = Header::parse_header([b"\"foobar\"".to_vec()].as_ref());
-        assert_eq!(etag, Some(Etag(EntityTag{
+        assert_eq!(etag, Some(Etag(EntityTag::new(false, "foobar".to_string()))));
-            weak: false,
-            tag: "foobar".to_string()
-        })));
 
         etag = Header::parse_header([b"\"\"".to_vec()].as_ref());
-        assert_eq!(etag, Some(Etag(EntityTag{
+        assert_eq!(etag, Some(Etag(EntityTag::new(false, "".to_string()))));
-            weak: false,
-            tag: "".to_string()
-        })));
 
         etag = Header::parse_header([b"W/\"weak-etag\"".to_vec()].as_ref());
-        assert_eq!(etag, Some(Etag(EntityTag{
+        assert_eq!(etag, Some(Etag(EntityTag::new(true, "weak-etag".to_string()))));
-            weak: true,
-            tag: "weak-etag".to_string()
-        })));
 
         etag = Header::parse_header([b"W/\"\x65\x62\"".to_vec()].as_ref());
-        assert_eq!(etag, Some(Etag(EntityTag{
+        assert_eq!(etag, Some(Etag(EntityTag::new(true, "\u{0065}\u{0062}".to_string()))));
-            weak: true,
-            tag: "\u{0065}\u{0062}".to_string()
-        })));
 
         etag = Header::parse_header([b"W/\"\"".to_vec()].as_ref());
-        assert_eq!(etag, Some(Etag(EntityTag{
+        assert_eq!(etag, Some(Etag(EntityTag::new(true, "".to_string()))));
-            weak: true,
-            tag: "".to_string()
-        })));
     }
 
     #[test]
@@ -102,4 +84,3 @@ mod tests {
 }
 
 bench_header!(bench, Etag, { vec![b"W/\"nonemptytag\"".to_vec()] });
-

src/header/common/if_match.rs

@@ -55,9 +55,9 @@ fn test_parse_header() {
         let a: IfMatch = Header::parse_header(
             [b"\"xyzzy\", \"r2d2xxxx\", \"c3piozzzz\"".to_vec()].as_ref()).unwrap();
         let b = IfMatch::EntityTags(
-            vec![EntityTag{weak:false, tag: "xyzzy".to_string()},
+            vec![EntityTag::new(false, "xyzzy".to_string()),
-                 EntityTag{weak:false, tag: "r2d2xxxx".to_string()},
+                 EntityTag::new(false, "r2d2xxxx".to_string()),
-                 EntityTag{weak:false, tag: "c3piozzzz".to_string()}]);
+                 EntityTag::new(false, "c3piozzzz".to_string())]);
         assert_eq!(a, b);
     }
 }

src/header/common/if_none_match.rs

@@ -67,14 +67,8 @@ mod tests {
 
         if_none_match = Header::parse_header([b"\"foobar\", W/\"weak-etag\"".to_vec()].as_ref());
         let mut entities: Vec<EntityTag> = Vec::new();
-        let foobar_etag = EntityTag {
+        let foobar_etag = EntityTag::new(false, "foobar".to_string());
-            weak: false,
+        let weak_etag = EntityTag::new(true, "weak-etag".to_string());
-            tag: "foobar".to_string()
-        };
-        let weak_etag = EntityTag {
-            weak: true,
-            tag: "weak-etag".to_string()
-        };
         entities.push(foobar_etag);
         entities.push(weak_etag);
         assert_eq!(if_none_match, Some(IfNoneMatch::EntityTags(entities)));

src/header/shared/entity.rs

@@ -1,29 +1,6 @@
 use std::str::FromStr;
 use std::fmt::{self, Display};
 
-/// An entity tag
-///
-/// An Etag consists of a string enclosed by two literal double quotes.
-/// Preceding the first double quote is an optional weakness indicator,
-/// which always looks like this: W/
-/// See also: https://tools.ietf.org/html/rfc7232#section-2.3
-#[derive(Clone, PartialEq, Debug)]
-pub struct EntityTag {
-    /// Weakness indicator for the tag
-    pub weak: bool,
-    /// The opaque string in between the DQUOTEs
-    pub tag: String
-}
-
-impl Display for EntityTag {
-    fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
-        if self.weak {
-            try!(write!(fmt, "{}", "W/"));
-        }
-        write!(fmt, "{}", self.tag)
-    }
-}
-
 // check that each char in the slice is either:
 // 1. %x21, or
 // 2. in the range %x23 to %x7E, or
@@ -38,108 +15,193 @@ fn check_slice_validity(slice: &str) -> bool {
     true
 }
 
+/// An entity tag, defined in [RFC7232](https://tools.ietf.org/html/rfc7232#section-2.3)
+///
+/// An entity tag consists of a string enclosed by two literal double quotes.
+/// Preceding the first double quote is an optional weakness indicator,
+/// which always looks like `W/`. Examples for valid tags are `"xyzzy"` and `W/"xyzzy"`.
+///
+/// # ABNF
+/// ```plain
+/// entity-tag = [ weak ] opaque-tag
+/// weak       = %x57.2F ; "W/", case-sensitive
+/// opaque-tag = DQUOTE *etagc DQUOTE
+/// etagc      = %x21 / %x23-7E / obs-text
+///            ; VCHAR except double quotes, plus obs-text
+/// ```
+///
+/// # Comparison
+/// To check if two entity tags are equivalent in an application always use the `strong_eq` or
+/// `weak_eq` methods based on the context of the Tag. Only use `==` to check if two tags are
+/// identical.
+///
+/// The example below shows the results for a set of entity-tag pairs and
+/// both the weak and strong comparison function results:
+///
+/// | ETag 1  | ETag 2  | Strong Comparison | Weak Comparison |
+/// |---------|---------|-------------------|-----------------|
+/// | `W/"1"` | `W/"1"` | no match          | match           |
+/// | `W/"1"` | `W/"2"` | no match          | no match        |
+/// | `W/"1"` | `"1"`   | no match          | match           |
+/// | `"1"`   | `"1"`   | match             | match           |
+#[derive(Clone, Debug, Eq, PartialEq)]
+pub struct EntityTag {
+    /// Weakness indicator for the tag
+    pub weak: bool,
+    /// The opaque string in between the DQUOTEs
+    tag: String
+}
+
+impl EntityTag {
+    /// Constructs a new EntityTag.
+    /// # Panics
+    /// If the tag contains invalid characters.
+    pub fn new(weak: bool, tag: String) -> EntityTag {
+        match check_slice_validity(&tag) {
+            true => EntityTag { weak: weak, tag: tag },
+            false => panic!("Invalid tag: {:?}", tag),
+        }
+    }
+
+    /// Get the tag.
+    pub fn tag(&self) -> &str {
+        self.tag.as_ref()
+    }
+
+    /// Set the tag.
+    /// # Panics
+    /// If the tag contains invalid characters.
+    pub fn set_tag(&mut self, tag: String) {
+        match check_slice_validity(&tag[..]) {
+            true => self.tag = tag,
+            false => panic!("Invalid tag: {:?}", tag),
+        }
+    }
+
+    /// For strong comparison two entity-tags are equivalent if both are not weak and their
+    /// opaque-tags match character-by-character.
+    pub fn strong_eq(&self, other: &EntityTag) -> bool {
+        self.weak == false && other.weak == false && self.tag == other.tag
+    }
+
+    /// For weak comparison two entity-tags are equivalent if their
+    /// opaque-tags match character-by-character, regardless of either or
+    /// both being tagged as "weak".
+    pub fn weak_eq(&self, other: &EntityTag) -> bool {
+        self.tag == other.tag
+    }
+
+    /// The inverse of `EntityTag.strong_eq()`.
+    pub fn strong_ne(&self, other: &EntityTag) -> bool {
+        !self.strong_eq(other)
+    }
+
+    /// The inverse of `EntityTag.weak_eq()`.
+    pub fn weak_ne(&self, other: &EntityTag) -> bool {
+        !self.weak_eq(other)
+    }
+}
+
+impl Display for EntityTag {
+    fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
+        match self.weak {
+            true => write!(fmt, "W/\"{}\"", self.tag),
+            false => write!(fmt, "\"{}\"", self.tag),
+        }
+    }
+}
+
 impl FromStr for EntityTag {
     type Err = ();
     fn from_str(s: &str) -> Result<EntityTag, ()> {
         let length: usize = s.len();
         let slice = &s[..];
-
         // Early exits:
         // 1. The string is empty, or,
         // 2. it doesn't terminate in a DQUOTE.
         if slice.is_empty() || !slice.ends_with('"') {
             return Err(());
         }
-
         // The etag is weak if its first char is not a DQUOTE.
-        if slice.starts_with('"') /* '"' */ {
+        if slice.starts_with('"') && check_slice_validity(&slice[1..length-1]) {
             // No need to check if the last char is a DQUOTE,
             // we already did that above.
-            if check_slice_validity(&slice[1..length-1]) {
+            return Ok(EntityTag { weak: false, tag: slice[1..length-1].to_string() });
-                return Ok(EntityTag {
+        } else if slice.starts_with("W/\"") && check_slice_validity(&slice[3..length-1]) {
-                    weak: false,
+            return Ok(EntityTag { weak: true, tag: slice[3..length-1].to_string() });
-                    tag: slice[1..length-1].to_string()
-                });
-            } else {
-                return Err(());
-            }
         }
-
-        if slice.starts_with("W/\"") {
-            if check_slice_validity(&slice[3..length-1]) {
-                return Ok(EntityTag {
-                    weak: true,
-                    tag: slice[3..length-1].to_string()
-                });
-            } else {
-                return Err(());
-            }
-        }
-
         Err(())
     }
 }
 
-
 #[cfg(test)]
 mod tests {
     use super::EntityTag;
 
     #[test]
-    fn test_etag_successes() {
+    fn test_etag_parse_success() {
-        // Expected successes
+        // Expected success
-        let mut etag : EntityTag = "\"foobar\"".parse().unwrap();
+        assert_eq!("\"foobar\"".parse().unwrap(), EntityTag::new(false, "foobar".to_string()));
-        assert_eq!(etag, (EntityTag {
+        assert_eq!("\"\"".parse().unwrap(), EntityTag::new(false, "".to_string()));
-            weak: false,
+        assert_eq!("W/\"weaktag\"".parse().unwrap(), EntityTag::new(true, "weaktag".to_string()));
-            tag: "foobar".to_string()
+        assert_eq!("W/\"\x65\x62\"".parse().unwrap(), EntityTag::new(true, "\x65\x62".to_string()));
-        }));
+        assert_eq!("W/\"\"".parse().unwrap(), EntityTag::new(true, "".to_string()));
-
-        etag = "\"\"".parse().unwrap();
-        assert_eq!(etag, EntityTag {
-            weak: false,
-            tag: "".to_string()
-        });
-
-        etag = "W/\"weak-etag\"".parse().unwrap();
-        assert_eq!(etag, EntityTag {
-            weak: true,
-            tag: "weak-etag".to_string()
-        });
-
-        etag = "W/\"\x65\x62\"".parse().unwrap();
-        assert_eq!(etag, EntityTag {
-            weak: true,
-            tag: "\u{0065}\u{0062}".to_string()
-        });
-
-        etag = "W/\"\"".parse().unwrap();
-        assert_eq!(etag, EntityTag {
-            weak: true,
-            tag: "".to_string()
-        });
     }
 
     #[test]
-    fn test_etag_failures() {
+    fn test_etag_parse_failures() {
         // Expected failures
-        let mut etag: Result<EntityTag,()>;
+        assert_eq!("no-dquotes".parse::<EntityTag>(), Err(()));
+        assert_eq!("w/\"the-first-w-is-case-sensitive\"".parse::<EntityTag>(), Err(()));
+        assert_eq!("".parse::<EntityTag>(), Err(()));
+        assert_eq!("\"unmatched-dquotes1".parse::<EntityTag>(), Err(()));
+        assert_eq!("unmatched-dquotes2\"".parse::<EntityTag>(), Err(()));
+        assert_eq!("matched-\"dquotes\"".parse::<EntityTag>(), Err(()));
+    }
 
-        etag = "no-dquotes".parse();
+    #[test]
-        assert_eq!(etag, Err(()));
+    fn test_etag_fmt() {
+        assert_eq!(format!("{}", EntityTag::new(false, "foobar".to_string())), "\"foobar\"");
+        assert_eq!(format!("{}", EntityTag::new(false, "".to_string())), "\"\"");
+        assert_eq!(format!("{}", EntityTag::new(true, "weak-etag".to_string())), "W/\"weak-etag\"");
+        assert_eq!(format!("{}", EntityTag::new(true, "\u{0065}".to_string())), "W/\"\x65\"");
+        assert_eq!(format!("{}", EntityTag::new(true, "".to_string())), "W/\"\"");
+    }
 
-        etag = "w/\"the-first-w-is-case-sensitive\"".parse();
+    #[test]
-        assert_eq!(etag, Err(()));
+    fn test_cmp() {
+        // | ETag 1  | ETag 2  | Strong Comparison | Weak Comparison |
+        // |---------|---------|-------------------|-----------------|
+        // | `W/"1"` | `W/"1"` | no match          | match           |
+        // | `W/"1"` | `W/"2"` | no match          | no match        |
+        // | `W/"1"` | `"1"`   | no match          | match           |
+        // | `"1"`   | `"1"`   | match             | match           |
+        let mut etag1 = EntityTag::new(true, "1".to_string());
+        let mut etag2 = EntityTag::new(true, "1".to_string());
+        assert_eq!(etag1.strong_eq(&etag2), false);
+        assert_eq!(etag1.weak_eq(&etag2), true);
+        assert_eq!(etag1.strong_ne(&etag2), true);
+        assert_eq!(etag1.weak_ne(&etag2), false);
 
-        etag = "".parse();
+        etag1 = EntityTag::new(true, "1".to_string());
-        assert_eq!(etag, Err(()));
+        etag2 = EntityTag::new(true, "2".to_string());
+        assert_eq!(etag1.strong_eq(&etag2), false);
+        assert_eq!(etag1.weak_eq(&etag2), false);
+        assert_eq!(etag1.strong_ne(&etag2), true);
+        assert_eq!(etag1.weak_ne(&etag2), true);
 
-        etag = "\"unmatched-dquotes1".parse();
+        etag1 = EntityTag::new(true, "1".to_string());
-        assert_eq!(etag, Err(()));
+        etag2 = EntityTag::new(false, "1".to_string());
+        assert_eq!(etag1.strong_eq(&etag2), false);
+        assert_eq!(etag1.weak_eq(&etag2), true);
+        assert_eq!(etag1.strong_ne(&etag2), true);
+        assert_eq!(etag1.weak_ne(&etag2), false);
 
-        etag = "unmatched-dquotes2\"".parse();
+        etag1 = EntityTag::new(false, "1".to_string());
-        assert_eq!(etag, Err(()));
+        etag2 = EntityTag::new(false, "1".to_string());
-
+        assert_eq!(etag1.strong_eq(&etag2), true);
-        etag = "matched-\"dquotes\"".parse();
+        assert_eq!(etag1.weak_eq(&etag2), true);
-        assert_eq!(etag, Err(()));
+        assert_eq!(etag1.strong_ne(&etag2), false);
+        assert_eq!(etag1.weak_ne(&etag2), false);
     }
 }