Skip to content
This repository was archived by the owner on Jun 21, 2020. It is now read-only.

Adds support for q-values, some refactoring #1

Merged
merged 6 commits into from
May 10, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ readme = "README.md"
edition = "2018"

[dependencies]
derive_is_enum_variant = "0.1.1"
failure = "0.1.3"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fairingrey Should we be injecting failure as a dependency for library? If it is used, other users need to pull it as well. IIRC most libraries does not use failure.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure, you'll have to ask @yoshuawuyts. It was already present when I started working on this PR 😅

I would probably remark that there might be a better way to do error handling, but I don't have any strong opinions on it. FWIW, per failure's best practices we do implement Fail for our Error type so it's easier to work with: https://docs.rs/failure/0.1.5/failure/

http = "0.1.13"
derive_is_enum_variant = "0.1.1"

[dev-dependencies]
2 changes: 0 additions & 2 deletions rustfmt.toml

This file was deleted.

64 changes: 32 additions & 32 deletions src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,59 +21,59 @@ pub type Result<T> = result::Result<T, failure::Error>;
/// [`Error`]: std.struct.Error.html
#[derive(Debug, Fail)]
pub enum ErrorKind {
/// Invalid header encoding.
#[fail(display = "Invalid header encoding.")]
InvalidEncoding,
/// The encoding scheme is unknown.
#[fail(display = "Uknown encoding scheme.")]
UnknownEncoding,
/// Any error not part of this list.
#[fail(display = "Generic error.")]
Other,
/// Invalid header encoding.
#[fail(display = "Invalid header encoding.")]
InvalidEncoding,
/// The encoding scheme is unknown.
#[fail(display = "Unknown encoding scheme.")]
UnknownEncoding,
/// Any error not part of this list.
#[fail(display = "Generic error.")]
Other,
}

/// A specialized [`Error`] type for this crate's operations.
///
/// [`Error`]: https://doc.rust-lang.org/nightly/std/error/trait.Error.html
#[derive(Debug)]
pub struct Error {
inner: Context<ErrorKind>,
inner: Context<ErrorKind>,
}

impl Error {
/// Access the [`ErrorKind`] member.
///
/// [`ErrorKind`]: enum.ErrorKind.html
pub fn kind(&self) -> &ErrorKind {
&*self.inner.get_context()
}
/// Access the [`ErrorKind`] member.
///
/// [`ErrorKind`]: enum.ErrorKind.html
pub fn kind(&self) -> &ErrorKind {
&*self.inner.get_context()
}
}

impl Fail for Error {
fn cause(&self) -> Option<&dyn Fail> {
self.inner.cause()
}
fn cause(&self) -> Option<&dyn Fail> {
self.inner.cause()
}

fn backtrace(&self) -> Option<&Backtrace> {
self.inner.backtrace()
}
fn backtrace(&self) -> Option<&Backtrace> {
self.inner.backtrace()
}
}

impl Display for Error {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
Display::fmt(&self.inner, f)
}
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
Display::fmt(&self.inner, f)
}
}

impl From<ErrorKind> for Error {
fn from(kind: ErrorKind) -> Error {
let inner = Context::new(kind);
Error { inner }
}
fn from(kind: ErrorKind) -> Error {
let inner = Context::new(kind);
Error { inner }
}
}

impl From<Context<ErrorKind>> for Error {
fn from(inner: Context<ErrorKind>) -> Error {
Error { inner }
}
fn from(inner: Context<ErrorKind>) -> Error {
Error { inner }
}
}
121 changes: 92 additions & 29 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
#![deny(missing_docs)]
#![cfg_attr(test, deny(warnings))]

//! ## Example
//! ## Examples
//! ```rust
//! # use failure::Error;
//! use http::header::{HeaderMap, HeaderValue, ACCEPT_ENCODING};
Expand All @@ -14,46 +14,109 @@
//! headers.insert(ACCEPT_ENCODING, HeaderValue::from_str("gzip, deflate, br")?);
//!
//! let encoding = accept_encoding::parse(&headers)?;
//! assert!(encoding.is_gzip());
//! # Ok(())}
//! ```
//!
//! ```rust
//! # use failure::Error;
//! use http::header::{HeaderMap, HeaderValue, ACCEPT_ENCODING};
//!
//! # fn main () -> Result<(), failure::Error> {
//! let mut headers = HeaderMap::new();
//! headers.insert(ACCEPT_ENCODING, HeaderValue::from_str("gzip;q=0.5, deflate;q=0.9, br;q=1.0")?);
//!
//! let encoding = accept_encoding::parse(&headers)?;
//! assert!(encoding.is_brotli());
//! # Ok(())}
//! ```

mod error;

pub use crate::error::{Error, ErrorKind, Result};
use derive_is_enum_variant::is_enum_variant;
use http::header::{ACCEPT_ENCODING, HeaderMap};
use failure::{ResultExt};
pub use crate::error::{Error, Result, ErrorKind};
use failure::ResultExt;
use http::header::{HeaderMap, HeaderValue, ACCEPT_ENCODING};

/// Encoding levels.
#[derive(Debug, Clone, is_enum_variant)]
#[derive(Debug, Clone, Copy, Eq, PartialEq, is_enum_variant)]
pub enum Encoding {
/// Gzip is the best encoding present.
Gzip,
/// Deflate is the best encoding present.
Deflate,
/// Brotli is the best encoding is present.
Brotli,
/// No encoding is present.
None,
/// Gzip is the most preferred encoding present.
Gzip,
/// Deflate is the most preferred encoding present.
Deflate,
/// Brotli is the most preferred encoding present.
Brotli,
/// No encoding is preferred.
Identity,
/// No preference is expressed on which encoding to use. Either the `Accept-Encoding` header is not present, or `*` is set as the most preferred encoding.
None,
}

impl Encoding {
/// Parses a given string into its corresponding encoding.
fn parse(s: &str) -> Result<Encoding> {
match s {
"gzip" => Ok(Encoding::Gzip),
"deflate" => Ok(Encoding::Deflate),
"br" => Ok(Encoding::Brotli),
"identity" => Ok(Encoding::Identity),
"*" => Ok(Encoding::None),
_ => Err(ErrorKind::UnknownEncoding)?,
}
}

/// Converts the encoding into its' corresponding header value.
///
/// Note that [`Encoding::None`] will return a HeaderValue with the content `*`.
/// This is likely not what you want if you are using this to generate the `Content-Encoding` header to be included in an encoded response.
pub fn to_header_value(self) -> HeaderValue {
match self {
Encoding::Gzip => HeaderValue::from_str("gzip").unwrap(),
Encoding::Deflate => HeaderValue::from_str("deflate").unwrap(),
Encoding::Brotli => HeaderValue::from_str("br").unwrap(),
Encoding::Identity => HeaderValue::from_str("identity").unwrap(),
Encoding::None => HeaderValue::from_str("*").unwrap(),
}
}
}

/// Parse a set of HTTP headers into an `Encoding`.
pub fn parse(headers: &HeaderMap) -> Result<Encoding> {
let header = match headers.get(ACCEPT_ENCODING) {
Some(header) => header,
None => return Ok(Encoding::None),
};

let string = header.to_str().context(ErrorKind::InvalidEncoding)?;

if string.contains("br") {
Ok(Encoding::Brotli)
} else if string.contains("deflate") {
Ok(Encoding::Deflate)
} else if string.contains("gzip") {
Ok(Encoding::Gzip)
} else {
Err(ErrorKind::UnknownEncoding)?
}
let mut preferred_encoding = Encoding::None;
let mut max_qval = 0.0;

for header_value in headers.get_all(ACCEPT_ENCODING).iter() {
let header_value = header_value.to_str().context(ErrorKind::InvalidEncoding)?;
for v in header_value.split(',').map(str::trim) {
let mut v = v.splitn(2, ";q=");
let encoding = v.next().unwrap();

match Encoding::parse(encoding) {
Ok(encoding) => {
if let Some(qval) = v.next() {
let qval = match qval.parse::<f32>() {
Ok(f) => f,
Err(_) => return Err(ErrorKind::InvalidEncoding)?,
};
if (qval - 1.0f32).abs() < 0.01 {
preferred_encoding = encoding;
break;
} else if qval > 1.0 {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we move this branch as the last branch?

I think error handling branches can come at last.

Copy link
Contributor Author

@fairingrey fairingrey May 11, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a little bit difficult to track the comments here since this PR was merged already, but I'll reply here anyways.

Actually, there is a slip of logic here that would require the predicate there to actually come first instead of last (since qval can technically be 1.001 and it would still pass, even though ideally it should fail since a q-value shouldn't be greater than 1.0 flat).

This is actually addressed indirectly through the refactoring on the second PR I've made: #2

Since the logic in that function runs first anyway.

return Err(ErrorKind::InvalidEncoding)?; // q-values over 1 are unacceptable
} else if qval > max_qval {
preferred_encoding = encoding;
max_qval = qval;
}
} else {
preferred_encoding = encoding;
break;
}
}
Err(_) => continue, // ignore unknown encodings for now
}
}
}

Ok(preferred_encoding)
}
75 changes: 73 additions & 2 deletions tests/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,79 @@ extern crate accept_encoding;
extern crate failure;

use failure::Error;
use http::header::{HeaderMap, HeaderValue, ACCEPT_ENCODING};

#[test]
fn should_work() -> Result<(), Error> {
Ok(())
fn single_encoding() -> Result<(), Error> {
let mut headers = HeaderMap::new();
headers.insert(ACCEPT_ENCODING, HeaderValue::from_str("gzip")?);

let encoding = accept_encoding::parse(&headers)?;
assert!(encoding.is_gzip());

Ok(())
}

#[test]
fn multiple_encodings() -> Result<(), Error> {
let mut headers = HeaderMap::new();
headers.insert(ACCEPT_ENCODING, HeaderValue::from_str("gzip, deflate, br")?);

let encoding = accept_encoding::parse(&headers)?;
assert!(encoding.is_gzip());

Ok(())
}

#[test]
fn single_encoding_with_qval() -> Result<(), Error> {
let mut headers = HeaderMap::new();
headers.insert(ACCEPT_ENCODING, HeaderValue::from_str("deflate;q=1.0")?);

let encoding = accept_encoding::parse(&headers)?;
assert!(encoding.is_deflate());

Ok(())
}

#[test]
fn multiple_encodings_with_qval_1() -> Result<(), Error> {
let mut headers = HeaderMap::new();
headers.insert(
ACCEPT_ENCODING,
HeaderValue::from_str("deflate, gzip;q=1.0, *;q=0.5")?,
);

let encoding = accept_encoding::parse(&headers)?;
assert!(encoding.is_deflate());

Ok(())
}

#[test]
fn multiple_encodings_with_qval_2() -> Result<(), Error> {
let mut headers = HeaderMap::new();
headers.insert(
ACCEPT_ENCODING,
HeaderValue::from_str("gzip;q=0.5, deflate;q=1.0, *;q=0.5")?,
);

let encoding = accept_encoding::parse(&headers)?;
assert!(encoding.is_deflate());

Ok(())
}

#[test]
fn multiple_encodings_with_qval_3() -> Result<(), Error> {
let mut headers = HeaderMap::new();
headers.insert(
ACCEPT_ENCODING,
HeaderValue::from_str("gzip;q=0.5, deflate;q=0.75, *;q=1.0")?,
);

let encoding = accept_encoding::parse(&headers)?;
assert!(encoding.is_none());

Ok(())
}