From 9c21f7f953a5163792e71fb186cab391c45d1bb4 Mon Sep 17 00:00:00 2001 From: Pyfisch Date: Mon, 30 Mar 2015 15:21:32 +0200 Subject: [PATCH] feat(entitytag): Add EntityTag comparison, make EntityTag safe to use Adds strong and weak comparison to EntityTag as described in the RFC, add tests for this. Make EntityTag safe to use by hiding the tag field, this prevents users from inserting malicious values for the tag. Invalid values can screw up header formatting and may allow to insert headers. Introduce EntityTag::new(), .tag() and .set_tag() methods. Fix Display trait for EntityTag. DQUOTES were missing. Remove custom formatting in ETag header. Improve docs. BREAKING_CHANGE: EntityTag.tag is private, use EntityTag.tag() and EntityTag.set_tag("foobar") to access it. --- src/header/common/etag.rs | 33 +--- src/header/common/if_match.rs | 6 +- src/header/common/if_none_match.rs | 10 +- src/header/shared/entity.rs | 248 ++++++++++++++++++----------- 4 files changed, 167 insertions(+), 130 deletions(-) diff --git a/src/header/common/etag.rs b/src/header/common/etag.rs index 85169742..ae376ea3 100644 --- a/src/header/common/etag.rs +++ b/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 { - try!(fmt.write_str("W/")); - } - write!(fmt, "\"{}\"", self.0.tag) + self.0.fmt(fmt) } } @@ -46,34 +43,19 @@ mod tests { let mut etag: Option; etag = Header::parse_header([b"\"foobar\"".to_vec()].as_ref()); - assert_eq!(etag, Some(Etag(EntityTag{ - weak: false, - tag: "foobar".to_string() - }))); + assert_eq!(etag, Some(Etag(EntityTag::new(false, "foobar".to_string())))); etag = Header::parse_header([b"\"\"".to_vec()].as_ref()); - assert_eq!(etag, Some(Etag(EntityTag{ - weak: false, - tag: "".to_string() - }))); + assert_eq!(etag, Some(Etag(EntityTag::new(false, "".to_string())))); etag = Header::parse_header([b"W/\"weak-etag\"".to_vec()].as_ref()); - assert_eq!(etag, Some(Etag(EntityTag{ - weak: true, - tag: "weak-etag".to_string() - }))); + assert_eq!(etag, Some(Etag(EntityTag::new(true, "weak-etag".to_string())))); etag = Header::parse_header([b"W/\"\x65\x62\"".to_vec()].as_ref()); - assert_eq!(etag, Some(Etag(EntityTag{ - weak: true, - tag: "\u{0065}\u{0062}".to_string() - }))); + assert_eq!(etag, Some(Etag(EntityTag::new(true, "\u{0065}\u{0062}".to_string())))); etag = Header::parse_header([b"W/\"\"".to_vec()].as_ref()); - assert_eq!(etag, Some(Etag(EntityTag{ - weak: true, - tag: "".to_string() - }))); + assert_eq!(etag, Some(Etag(EntityTag::new(true, "".to_string())))); } #[test] @@ -102,4 +84,3 @@ mod tests { } bench_header!(bench, Etag, { vec![b"W/\"nonemptytag\"".to_vec()] }); - diff --git a/src/header/common/if_match.rs b/src/header/common/if_match.rs index be7745db..640bc1c5 100644 --- a/src/header/common/if_match.rs +++ b/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()}, - EntityTag{weak:false, tag: "r2d2xxxx".to_string()}, - EntityTag{weak:false, tag: "c3piozzzz".to_string()}]); + vec![EntityTag::new(false, "xyzzy".to_string()), + EntityTag::new(false, "r2d2xxxx".to_string()), + EntityTag::new(false, "c3piozzzz".to_string())]); assert_eq!(a, b); } } diff --git a/src/header/common/if_none_match.rs b/src/header/common/if_none_match.rs index 75169965..6e0c941f 100644 --- a/src/header/common/if_none_match.rs +++ 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 = Vec::new(); - let foobar_etag = EntityTag { - weak: false, - tag: "foobar".to_string() - }; - let weak_etag = EntityTag { - weak: true, - tag: "weak-etag".to_string() - }; + let foobar_etag = EntityTag::new(false, "foobar".to_string()); + let weak_etag = EntityTag::new(true, "weak-etag".to_string()); entities.push(foobar_etag); entities.push(weak_etag); assert_eq!(if_none_match, Some(IfNoneMatch::EntityTags(entities))); diff --git a/src/header/shared/entity.rs b/src/header/shared/entity.rs index 5d193f56..049b1dea 100644 --- a/src/header/shared/entity.rs +++ b/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 { 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() - }); - } else { - return Err(()); - } + return Ok(EntityTag { weak: false, tag: slice[1..length-1].to_string() }); + } else if slice.starts_with("W/\"") && check_slice_validity(&slice[3..length-1]) { + return Ok(EntityTag { weak: true, tag: slice[3..length-1].to_string() }); } - - 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() { - // Expected successes - let mut etag : EntityTag = "\"foobar\"".parse().unwrap(); - assert_eq!(etag, (EntityTag { - weak: false, - tag: "foobar".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() - }); + fn test_etag_parse_success() { + // Expected success + assert_eq!("\"foobar\"".parse().unwrap(), EntityTag::new(false, "foobar".to_string())); + assert_eq!("\"\"".parse().unwrap(), EntityTag::new(false, "".to_string())); + assert_eq!("W/\"weaktag\"".parse().unwrap(), EntityTag::new(true, "weaktag".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())); } #[test] - fn test_etag_failures() { + fn test_etag_parse_failures() { // Expected failures - let mut etag: Result; + assert_eq!("no-dquotes".parse::(), Err(())); + assert_eq!("w/\"the-first-w-is-case-sensitive\"".parse::(), Err(())); + assert_eq!("".parse::(), Err(())); + assert_eq!("\"unmatched-dquotes1".parse::(), Err(())); + assert_eq!("unmatched-dquotes2\"".parse::(), Err(())); + assert_eq!("matched-\"dquotes\"".parse::(), Err(())); + } - etag = "no-dquotes".parse(); - assert_eq!(etag, Err(())); + #[test] + 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(); - assert_eq!(etag, Err(())); + #[test] + 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(); - assert_eq!(etag, Err(())); + etag1 = EntityTag::new(true, "1".to_string()); + 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(); - assert_eq!(etag, Err(())); + etag1 = EntityTag::new(true, "1".to_string()); + 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(); - assert_eq!(etag, Err(())); - - etag = "matched-\"dquotes\"".parse(); - assert_eq!(etag, Err(())); + etag1 = EntityTag::new(false, "1".to_string()); + etag2 = EntityTag::new(false, "1".to_string()); + assert_eq!(etag1.strong_eq(&etag2), true); + assert_eq!(etag1.weak_eq(&etag2), true); + assert_eq!(etag1.strong_ne(&etag2), false); + assert_eq!(etag1.weak_ne(&etag2), false); } }