Skip to content

Commit

Permalink
feat(entitytag): Add EntityTag comparison, make EntityTag safe to use
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
pyfisch committed Mar 30, 2015
1 parent 4c5a42b commit 9c21f7f
Show file tree
Hide file tree
Showing 4 changed files with 168 additions and 131 deletions.
33 changes: 7 additions & 26 deletions src/header/common/etag.rs
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -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)
}
}

Expand All @@ -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{
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]
Expand Down Expand Up @@ -102,4 +84,3 @@ mod tests {
}

bench_header!(bench, Etag, { vec![b"W/\"nonemptytag\"".to_vec()] });

6 changes: 3 additions & 3 deletions src/header/common/if_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Expand Down
10 changes: 2 additions & 8 deletions src/header/common/if_none_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
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)));
Expand Down
250 changes: 156 additions & 94 deletions src/header/shared/entity.rs
Original file line number Diff line number Diff line change
@@ -1,145 +1,207 @@
use std::str::FromStr;
use std::fmt::{self, Display};

/// An entity tag
// check that each char in the slice is either:
// 1. %x21, or
// 2. in the range %x23 to %x7E, or
// 3. in the range %x80 to %xFF
fn check_slice_validity(slice: &str) -> bool {
for c in slice.bytes() {
match c {
b'\x21' | b'\x23' ... b'\x7e' | b'\x80' ... b'\xff' => (),
_ => { return false; }
}
}
true
}

/// An entity tag, defined in [RFC7232](https://tools.ietf.org/html/rfc7232#section-2.3)
///
/// An Etag consists of a string enclosed by two literal double quotes.
/// 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 this: W/
/// See also: https://tools.ietf.org/html/rfc7232#section-2.3
#[derive(Clone, PartialEq, Debug)]
/// 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
pub tag: String
tag: String
}

impl Display for EntityTag {
fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
if self.weak {
try!(write!(fmt, "{}", "W/"));
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),
}
write!(fmt, "{}", self.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)
}
}

// check that each char in the slice is either:
// 1. %x21, or
// 2. in the range %x23 to %x7E, or
// 3. in the range %x80 to %xFF
fn check_slice_validity(slice: &str) -> bool {
for c in slice.bytes() {
match c {
b'\x21' | b'\x23' ... b'\x7e' | b'\x80' ... b'\xff' => (),
_ => { return false; }
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),
}
}
true
}

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()
});
} 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(());
}
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() });
}

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<EntityTag,()>;

etag = "no-dquotes".parse();
assert_eq!(etag, Err(()));

etag = "w/\"the-first-w-is-case-sensitive\"".parse();
assert_eq!(etag, Err(()));

etag = "".parse();
assert_eq!(etag, Err(()));

etag = "\"unmatched-dquotes1".parse();
assert_eq!(etag, Err(()));
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 = "unmatched-dquotes2\"".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 = "matched-\"dquotes\"".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);

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);

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);

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);
}
}

0 comments on commit 9c21f7f

Please sign in to comment.