Skip to content

Commit

Permalink
Revert b62b4e: strip quotes from cookie value.
Browse files Browse the repository at this point in the history
Some applications expect quotes to be preserved in a cookie's value, as
indicated by the RFC and expected by Servo, while others expect quotes to be
trimmed, as implemented by libraries like Django and Express. This commit
reverts a previous commit that always trimmed quotes and leaves the decision to
the user by introducing a new `value_trimmed()` getter method. The `value()`
method is reverted to the previous behavior, and the `value_trimmed()` strips
quotes.
  • Loading branch information
SergioBenitez committed Sep 20, 2023
1 parent 9a1bf40 commit 89edddf
Show file tree
Hide file tree
Showing 2 changed files with 63 additions and 24 deletions.
53 changes: 53 additions & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -454,19 +454,72 @@ impl<'c> Cookie<'c> {

/// Returns the value of `self`.
///
/// Does not strip surrounding quotes. See [`Cookie::value_trimmed()`] for a
/// version that does.
///
/// # Example
///
/// ```
/// use cookie::Cookie;
///
/// let c = Cookie::new("name", "value");
/// assert_eq!(c.value(), "value");
///
/// let c = Cookie::new("name", "\"value\"");
/// assert_eq!(c.value(), "\"value\"");
/// ```
#[inline]
pub fn value(&self) -> &str {
self.value.to_str(self.cookie_string.as_ref())
}

/// Returns the value of `self` with surrounding double-quotes trimmed.
///
/// This is _not_ the value of the cookie (_that_ is [`Cookie::value()`]).
/// Instead, this is the value with a surrounding pair of double-quotes, if
/// any, trimmed away. Quotes are only trimmed when they form a pair and
/// never otherwise. The trimmed value is never used for any other
/// operations, including equality checking and hashing.
///
/// # Example
///
/// ```
/// use cookie::Cookie;
/// let c0 = Cookie::new("name", "value");
/// assert_eq!(c0.value_trimmed(), "value");
///
/// let c = Cookie::new("name", "\"value\"");
/// assert_eq!(c.value_trimmed(), "value");
/// assert!(c != c0);
///
/// let c = Cookie::new("name", "\"value");
/// assert_eq!(c.value(), "\"value");
/// assert_eq!(c.value_trimmed(), "\"value");
/// assert!(c != c0);
///
/// let c = Cookie::new("name", "\"value\"\"");
/// assert_eq!(c.value(), "\"value\"\"");
/// assert_eq!(c.value_trimmed(), "value\"");
/// assert!(c != c0);
/// ```
#[inline]
pub fn value_trimmed(&self) -> &str {
#[inline(always)]
fn trim_quotes(s: &str) -> &str {
if s.len() < 2 {
return s;
}

let bytes = s.as_bytes();
match (bytes.first(), bytes.last()) {
(Some(b'"'), Some(b'"')) => &s[1..(s.len() - 1)],
_ => s
}
}

trim_quotes(self.value())
}

/// Returns the name and value of `self` as a tuple of `(name, value)`.
///
/// # Example
Expand Down
34 changes: 10 additions & 24 deletions src/parse.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,17 +89,6 @@ fn name_val_decoded(
unreachable!("This function should never be called with 'percent-encode' disabled!")
}

fn trim_quotes(s: &str) -> &str {
if s.len() < 2 {
return s;
}

match (s.chars().next(), s.chars().last()) {
(Some('"'), Some('"')) => &s[1..(s.len() - 1)],
_ => s
}
}

// This function does the real parsing but _does not_ set the `cookie_string` in
// the returned cookie object. This only exists so that the borrow to `s` is
// returned at the end of the call, allowing the `cookie_string` field to be
Expand All @@ -110,10 +99,7 @@ fn parse_inner<'c>(s: &str, decode: bool) -> Result<Cookie<'c>, ParseError> {
// Determine the name = val.
let key_value = attributes.next().expect("first str::split().next() returns Some");
let (name, value) = match key_value.find('=') {
Some(i) => {
let (key, value) = (key_value[..i].trim(), key_value[(i + 1)..].trim());
(key, trim_quotes(value).trim())
},
Some(i) => (key_value[..i].trim(), key_value[(i + 1)..].trim()),
None => return Err(ParseError::MissingPair)
};

Expand Down Expand Up @@ -319,31 +305,31 @@ mod tests {
let expected = Cookie::build("foo", "bar=baz").finish();
assert_eq_parse!("foo=bar=baz", expected);

let expected = Cookie::build("foo", "\"bar\"").finish();
let expected = Cookie::build("foo", "\"\"bar\"\"").finish();
assert_eq_parse!("foo=\"\"bar\"\"", expected);

let expected = Cookie::build("foo", "\"bar").finish();
assert_eq_parse!("foo= \"bar", expected);
assert_eq_parse!("foo=\"bar ", expected);
assert_eq_parse!("foo=\"\"bar\"", expected);
assert_eq_parse!("foo=\"\"bar \"", expected);
assert_eq_parse!("foo=\"\"bar \" ", expected);
assert_ne_parse!("foo=\"\"bar\"", expected);
assert_ne_parse!("foo=\"\"bar \"", expected);
assert_ne_parse!("foo=\"\"bar \" ", expected);

let expected = Cookie::build("foo", "bar\"").finish();
assert_eq_parse!("foo=bar\"", expected);
assert_eq_parse!("foo=\"bar\"\"", expected);
assert_eq_parse!("foo=\" bar\"\"", expected);
assert_eq_parse!("foo=\" bar\" \" ", expected);
assert_ne_parse!("foo=\"bar\"\"", expected);
assert_ne_parse!("foo=\" bar\"\"", expected);
assert_ne_parse!("foo=\" bar\" \" ", expected);

let mut expected = Cookie::build("foo", "bar").finish();
assert_eq_parse!("foo=bar", expected);
assert_eq_parse!("foo = bar", expected);
assert_eq_parse!("foo=\"bar\"", expected);
assert_eq_parse!(" foo=bar ", expected);
assert_eq_parse!(" foo=\"bar \" ", expected);
assert_eq_parse!(" foo=bar ;Domain=", expected);
assert_eq_parse!(" foo=bar ;Domain= ", expected);
assert_eq_parse!(" foo=bar ;Ignored", expected);
assert_ne_parse!("foo=\"bar\"", expected);
assert_ne_parse!(" foo=\"bar \" ", expected);

let mut unexpected = Cookie::build("foo", "bar").http_only(false).finish();
assert_ne_parse!(" foo=bar ;HttpOnly", unexpected);
Expand Down

0 comments on commit 89edddf

Please sign in to comment.