From b4df0837ca7b0ab987764531331fb4455d6dd548 Mon Sep 17 00:00:00 2001 From: axon-q Date: Wed, 13 Jun 2018 18:50:47 +0000 Subject: [PATCH 1/8] EtagHasher middleware --- src/middleware/etaghasher.rs | 318 +++++++++++++++++++++++++++++++++++ src/middleware/mod.rs | 1 + 2 files changed, 319 insertions(+) create mode 100644 src/middleware/etaghasher.rs diff --git a/src/middleware/etaghasher.rs b/src/middleware/etaghasher.rs new file mode 100644 index 000000000..0bf6230c3 --- /dev/null +++ b/src/middleware/etaghasher.rs @@ -0,0 +1,318 @@ +//! ETag header and `304 Not Modified` support for HTTP responses +/// +/// The `EtagHasher` middleware generates RFC 7232 ETag headers for HTTP +/// responses, and checks the ETag for a response against those provided +/// in the `If-None-Match` header of the request, if present. In the +/// event of a match, instead of returning the original response, an +/// HTTP `304 Not Modified` response with no content is returned +/// instead. Only response [Body](enum.Body.html)s of type `Binary` are +/// supported; responses with other body types will be left unchanged. +/// +/// ETag values are generated by computing a hash function over the +/// bytes of the body of the original response. Thus, using this +/// middleware amounts to trading CPU resources for bandwidth. Some CPU +/// overhead is incurred by having to compute a hash for each response +/// body, but in return one avoids sending response bodies to requesters +/// that already have the body content cached. +/// +/// An `EtagHasher` instance makes use of two functions, `hash` and +/// `filter`. The `hash` function takes the bytes of the original +/// response body as input and produces an ETag value. The `filter` +/// function takes the original HTTP request and response, and returns +/// `true` if ETag processing should be applied to this response and +/// `false` otherwise. These functions are supplied by the user when the +/// instance is created; the `DefaultHasher` and `DefaultFilter` can be +/// used if desired. Currently `DefaultHasher` computes an SHA-1 hash, +/// but this should not be relied upon. The `DefaultFilter` returns +/// `true` when the request method is `GET` or `HEAD` and the original +/// response status is `200 OK`. If you provide your own `filter`, you +/// will want to check for these conditions as well. +/// +/// ```rust +/// # extern crate actix_web; +/// use actix_web::{http, middleware, App, HttpResponse}; +/// use middleware::etaghasher::{EtagHasher, DefaultHasher, DefaultFilter}; +/// +/// fn main() { +/// let eh = EtagHasher::new(DefaultHasher, DefaultFilter); +/// let app = App::new() +/// .middleware(eh) +/// .resource("/test", |r| { +/// r.method(http::Method::GET).f(|_| HttpResponse::Ok()); +/// }) +/// .finish(); +/// } +/// ``` +/// +/// With custom `hash` and `filter` functions: +/// +/// ```rust +/// # extern crate actix_web; +/// use actix_web::{http, middleware, App, HttpRequest, HttpResponse}; +/// use middleware::etaghasher::EtagHasher; +/// +/// fn main() { +/// let eh = EtagHasher::new( +/// |_input: &[u8]| "static".to_string(), +/// |_req: &HttpRequest<()>, _res: &HttpResponse| true, +/// ); +/// let app = App::new() +/// .middleware(eh) +/// .resource("/test", |r| { +/// r.method(http::Method::GET).f(|_| HttpResponse::Ok()); +/// }) +/// .finish(); +/// } +/// ``` + +use error::Result; +use header::EntityTag; +use httprequest::HttpRequest; +use httpresponse::HttpResponse; +use middleware; + +use std::marker::PhantomData; + +/// `Middleware` for generating ETag headers and returning `304 Not Modified` +/// responses upon receipt of a matching `If-None-Match` request header. + +/// Can produce an ETag value from a byte slice. Per RFC 7232, **must only +/// produce** bytes with hex values `21`, `23-7E`, or greater than or equal +/// to `80`. Producing invalid bytes will result in a panic when the output +/// is converted to an ETag. +pub trait Hasher { + /// Produce an ETag value given a byte slice. + fn hash(&self, input: &[u8]) -> String; +} +/// Can test a (request, response) pair and return `true` or `false` +pub trait RequestFilter { + /// Return `true` if ETag processing should be applied to this + /// `(request, response)` pair and `false` otherwise. A `false` return + /// value will immediately return the original response unchanged. + fn filter(&self, req: &HttpRequest, res: &HttpResponse) -> bool; +} + +// Closure implementations +impl String> Hasher for F { + fn hash(&self, input: &[u8]) -> String { + self(input) + } +} +impl, &HttpResponse) -> bool> RequestFilter for F { + fn filter(&self, req: &HttpRequest, res: &HttpResponse) -> bool { + self(req, res) + } +} + +// Defaults +/// Computes an ETag value from a byte slice using a default cryptographic hash +/// function. +pub struct DefaultHasher; +impl Hasher for DefaultHasher { + fn hash(&self, input: &[u8]) -> String { + use sha1; + let mut h = sha1::Sha1::new(); + h.update(input); + h.digest().to_string() + } +} + +/// Returns `true` when the request method is `GET` or `HEAD` and the +/// original response status is `200 OK`, and `false` otherwise. +pub struct DefaultFilter; +impl RequestFilter for DefaultFilter { + fn filter(&self, req: &HttpRequest, res: &HttpResponse) -> bool { + use http::{Method, StatusCode}; + (*req.method() == Method::GET || *req.method() == Method::HEAD) + && res.status() == StatusCode::OK + } +} + +/// The middleware struct. Contains a Hasher to compute ETag values for byte +/// slices and a filter to determine whether ETag computation and checking +/// should be applied to a particular (request, response) pair. +pub struct EtagHasher +where + S: 'static, + H: Hasher + 'static, + F: RequestFilter + 'static, +{ + hasher: H, + filter: F, + _phantom: PhantomData, +} + +impl EtagHasher +where + S: 'static, + H: Hasher + 'static, + F: RequestFilter + 'static, +{ + /// Create a new middleware struct with the given Hasher and RequestFilter. + pub fn new(hasher: H, filter: F) -> Self { + EtagHasher { + hasher, + filter, + _phantom: PhantomData, + } + } +} + +impl middleware::Middleware for EtagHasher +where + S: 'static, + H: Hasher + 'static, + F: RequestFilter + 'static, +{ + fn response( + &mut self, req: &mut HttpRequest, mut res: HttpResponse, + ) -> Result { + use header; + use Body; + + if !self.filter.filter(req, &res) { + return Ok(middleware::Response::Done(res)); + } + + let e = if let Body::Binary(b) = res.body() { + Some(EntityTag::strong(self.hasher.hash(b.as_ref()))) + } else { + None + }; + + if let Some(etag) = e { + if !none_match(&etag, req) { + let mut not_modified = + HttpResponse::NotModified().set(header::ETag(etag)).finish(); + + // RFC 7232 requires copying over these headers: + copy_header(header::CACHE_CONTROL, &res, &mut not_modified); + copy_header(header::CONTENT_LOCATION, &res, &mut not_modified); + copy_header(header::DATE, &res, &mut not_modified); + copy_header(header::EXPIRES, &res, &mut not_modified); + copy_header(header::VARY, &res, &mut not_modified); + + return Ok(middleware::Response::Done(not_modified)); + } + etag.to_string() + .parse::() + .map(|v| { + res.headers_mut().insert(header::ETAG, v); + }) + .unwrap_or(()); + } + Ok(middleware::Response::Done(res)) + } +} + +#[inline] +fn copy_header(h: ::header::HeaderName, src: &HttpResponse, dst: &mut HttpResponse) { + if let Some(val) = src.headers().get(&h) { + dst.headers_mut().insert(h, val.clone()); + } +} + +// Returns true if `req` doesn't have an `If-None-Match` header matching `req`. +#[inline] +fn none_match(etag: &EntityTag, req: &HttpRequest) -> bool { + use header::IfNoneMatch; + use httpmessage::HttpMessage; + match req.get_header::() { + Some(IfNoneMatch::Items(ref items)) => { + for item in items { + if item.weak_eq(etag) { + return false; + } + } + true + } + Some(IfNoneMatch::Any) => false, + None => true, + } +} + +#[cfg(test)] +mod tests { + use super::*; + use header::ETAG; + use http::StatusCode; + use httpmessage::HttpMessage; + use middleware::Middleware; + use test::{TestRequest, TestServer}; + + const TEST_ETAG: &'static str = "\"a94a8fe5ccb19ba61c4c0873d391e987982fbbd3\""; + struct TestState { + _state: u32, + } + fn test_index(_req: HttpRequest) -> &'static str { + "test" + } + + fn mwres(r: Result) -> HttpResponse { + match r { + Ok(middleware::Response::Done(hr)) => hr, + _ => panic!(), + } + } + + #[test] + fn test_default_create_etag() { + let mut eh = EtagHasher::new(DefaultHasher, DefaultFilter); + let mut req = TestRequest::default().finish(); + let res = HttpResponse::Ok().body("test"); + let res = mwres(eh.response(&mut req, res)); + assert_eq!(res.status(), StatusCode::OK); + assert_eq!(res.headers().get(ETAG).unwrap(), TEST_ETAG); + } + #[test] + fn test_default_with_state_create_etag() { + let state = TestState { _state: 0 }; + let mut eh = EtagHasher::new(DefaultHasher, DefaultFilter); + let mut req = TestRequest::with_state(state).finish(); + let res = HttpResponse::Ok().body("test"); + let res = mwres(eh.response(&mut req, res)); + assert_eq!(res.status(), StatusCode::OK); + assert_eq!(res.headers().get(ETAG).unwrap(), TEST_ETAG); + } + #[test] + fn test_default_none_match() { + let mut eh = EtagHasher::new(DefaultHasher, DefaultFilter); + let mut req = TestRequest::with_header("If-None-Match", "_").finish(); + let res = HttpResponse::Ok().body("test"); + let res = mwres(eh.response(&mut req, res)); + assert_eq!(res.status(), StatusCode::OK); + assert_eq!(res.headers().get(ETAG).unwrap(), TEST_ETAG); + } + #[test] + fn test_default_match() { + let mut eh = EtagHasher::new(DefaultHasher, DefaultFilter); + let mut req = TestRequest::with_header("If-None-Match", TEST_ETAG).finish(); + let res = HttpResponse::Ok().body("test"); + let res = mwres(eh.response(&mut req, res)); + assert_eq!(res.status(), StatusCode::NOT_MODIFIED); + } + #[test] + fn test_custom_match() { + let mut eh = EtagHasher::new( + |_input: &[u8]| "static".to_string(), + |_req: &HttpRequest<()>, _res: &HttpResponse| true, + ); + let mut req = TestRequest::with_header("If-None-Match", "\"static\"").finish(); + let res = HttpResponse::Ok().body("test"); + let res = mwres(eh.response(&mut req, res)); + assert_eq!(res.status(), StatusCode::NOT_MODIFIED); + } + #[test] + fn test_srv_default_create_etag() { + let mut srv = + TestServer::build_with_state(|| TestState { _state: 0 }).start(|app| { + let eh = EtagHasher::new(DefaultHasher, DefaultFilter); + app.middleware(eh).handler(test_index) + }); + + let req = srv.get().finish().unwrap(); + let response = srv.execute(req.send()).unwrap(); + assert!(response.status().is_success()); + assert_eq!(response.headers().get(ETAG).unwrap(), TEST_ETAG); + } +} diff --git a/src/middleware/mod.rs b/src/middleware/mod.rs index 7fd339327..14133e616 100644 --- a/src/middleware/mod.rs +++ b/src/middleware/mod.rs @@ -9,6 +9,7 @@ mod logger; pub mod cors; pub mod csrf; +pub mod etaghasher; mod defaultheaders; mod errhandlers; #[cfg(feature = "session")] From 1598e78de10ca103c2f00392d9698a8eb0a51877 Mon Sep 17 00:00:00 2001 From: axon-q Date: Wed, 13 Jun 2018 23:25:03 +0000 Subject: [PATCH 2/8] re-use hash state --- src/middleware/etaghasher.rs | 39 ++++++++++++++++++++++-------------- 1 file changed, 24 insertions(+), 15 deletions(-) diff --git a/src/middleware/etaghasher.rs b/src/middleware/etaghasher.rs index 0bf6230c3..2cf6ced6b 100644 --- a/src/middleware/etaghasher.rs +++ b/src/middleware/etaghasher.rs @@ -34,7 +34,7 @@ /// use middleware::etaghasher::{EtagHasher, DefaultHasher, DefaultFilter}; /// /// fn main() { -/// let eh = EtagHasher::new(DefaultHasher, DefaultFilter); +/// let eh = EtagHasher::new(DefaultHasher::new(), DefaultFilter); /// let app = App::new() /// .middleware(eh) /// .resource("/test", |r| { @@ -82,7 +82,7 @@ use std::marker::PhantomData; /// is converted to an ETag. pub trait Hasher { /// Produce an ETag value given a byte slice. - fn hash(&self, input: &[u8]) -> String; + fn hash(&mut self, input: &[u8]) -> String; } /// Can test a (request, response) pair and return `true` or `false` pub trait RequestFilter { @@ -93,8 +93,8 @@ pub trait RequestFilter { } // Closure implementations -impl String> Hasher for F { - fn hash(&self, input: &[u8]) -> String { +impl String> Hasher for F { + fn hash(&mut self, input: &[u8]) -> String { self(input) } } @@ -107,13 +107,22 @@ impl, &HttpResponse) -> bool> RequestFilter for F { // Defaults /// Computes an ETag value from a byte slice using a default cryptographic hash /// function. -pub struct DefaultHasher; +pub struct DefaultHasher { + hashstate: ::sha1::Sha1, +} +impl DefaultHasher { + /// Create a new instance. + pub fn new() -> Self { + DefaultHasher { + hashstate: ::sha1::Sha1::new() + } + } +} impl Hasher for DefaultHasher { - fn hash(&self, input: &[u8]) -> String { - use sha1; - let mut h = sha1::Sha1::new(); - h.update(input); - h.digest().to_string() + fn hash(&mut self, input: &[u8]) -> String { + self.hashstate.reset(); + self.hashstate.update(input); + self.hashstate.digest().to_string() } } @@ -257,7 +266,7 @@ mod tests { #[test] fn test_default_create_etag() { - let mut eh = EtagHasher::new(DefaultHasher, DefaultFilter); + let mut eh = EtagHasher::new(DefaultHasher::new(), DefaultFilter); let mut req = TestRequest::default().finish(); let res = HttpResponse::Ok().body("test"); let res = mwres(eh.response(&mut req, res)); @@ -267,7 +276,7 @@ mod tests { #[test] fn test_default_with_state_create_etag() { let state = TestState { _state: 0 }; - let mut eh = EtagHasher::new(DefaultHasher, DefaultFilter); + let mut eh = EtagHasher::new(DefaultHasher::new(), DefaultFilter); let mut req = TestRequest::with_state(state).finish(); let res = HttpResponse::Ok().body("test"); let res = mwres(eh.response(&mut req, res)); @@ -276,7 +285,7 @@ mod tests { } #[test] fn test_default_none_match() { - let mut eh = EtagHasher::new(DefaultHasher, DefaultFilter); + let mut eh = EtagHasher::new(DefaultHasher::new(), DefaultFilter); let mut req = TestRequest::with_header("If-None-Match", "_").finish(); let res = HttpResponse::Ok().body("test"); let res = mwres(eh.response(&mut req, res)); @@ -285,7 +294,7 @@ mod tests { } #[test] fn test_default_match() { - let mut eh = EtagHasher::new(DefaultHasher, DefaultFilter); + let mut eh = EtagHasher::new(DefaultHasher::new(), DefaultFilter); let mut req = TestRequest::with_header("If-None-Match", TEST_ETAG).finish(); let res = HttpResponse::Ok().body("test"); let res = mwres(eh.response(&mut req, res)); @@ -306,7 +315,7 @@ mod tests { fn test_srv_default_create_etag() { let mut srv = TestServer::build_with_state(|| TestState { _state: 0 }).start(|app| { - let eh = EtagHasher::new(DefaultHasher, DefaultFilter); + let eh = EtagHasher::new(DefaultHasher::new(), DefaultFilter); app.middleware(eh).handler(test_index) }); From 97b149f321307332e9af661f8f6769566e17629f Mon Sep 17 00:00:00 2001 From: axon-q Date: Thu, 14 Jun 2018 09:54:22 +0000 Subject: [PATCH 3/8] doc updates --- src/middleware/etaghasher.rs | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/src/middleware/etaghasher.rs b/src/middleware/etaghasher.rs index 2cf6ced6b..132ddc6b5 100644 --- a/src/middleware/etaghasher.rs +++ b/src/middleware/etaghasher.rs @@ -1,6 +1,7 @@ //! ETag header and `304 Not Modified` support for HTTP responses /// -/// The `EtagHasher` middleware generates RFC 7232 ETag headers for HTTP +/// The `EtagHasher` middleware generates [RFC +/// 7232](https://tools.ietf.org/html/rfc7232) ETag headers for HTTP /// responses, and checks the ETag for a response against those provided /// in the `If-None-Match` header of the request, if present. In the /// event of a match, instead of returning the original response, an @@ -15,6 +16,13 @@ /// body, but in return one avoids sending response bodies to requesters /// that already have the body content cached. /// +/// This approach is most useful for dynamically generated responses +/// that don't correspond to a specific external resource (e.g. a +/// file). For such external resources, it's better to generate ETags +/// based on the inherent properties of the resource rather than by +/// hashing the bytes of an HTTP response corresponding to its +/// serialized representation as this middleware does. +/// /// An `EtagHasher` instance makes use of two functions, `hash` and /// `filter`. The `hash` function takes the bytes of the original /// response body as input and produces an ETag value. The `filter` @@ -73,9 +81,6 @@ use middleware; use std::marker::PhantomData; -/// `Middleware` for generating ETag headers and returning `304 Not Modified` -/// responses upon receipt of a matching `If-None-Match` request header. - /// Can produce an ETag value from a byte slice. Per RFC 7232, **must only /// produce** bytes with hex values `21`, `23-7E`, or greater than or equal /// to `80`. Producing invalid bytes will result in a panic when the output @@ -137,9 +142,14 @@ impl RequestFilter for DefaultFilter { } } -/// The middleware struct. Contains a Hasher to compute ETag values for byte -/// slices and a filter to determine whether ETag computation and checking -/// should be applied to a particular (request, response) pair. +/// Middleware for [RFC 7232](https://tools.ietf.org/html/rfc7232) ETag +/// generation and comparison. +/// +/// The `EtagHasher` struct contains a Hasher to compute ETag values for +/// byte slices and a Filter to determine whether ETag computation and +/// checking should be applied to a particular (request, response) +/// pair. Only response [Body](enum.Body.html)s of type `Binary` are +/// supported; responses with other body types will be left unchanged. pub struct EtagHasher where S: 'static, From 024666509e81068a92ad3ec549445170ef73a394 Mon Sep 17 00:00:00 2001 From: axon-q Date: Thu, 14 Jun 2018 09:56:12 +0000 Subject: [PATCH 4/8] rename RequestFilter to Filter --- src/middleware/etaghasher.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/middleware/etaghasher.rs b/src/middleware/etaghasher.rs index 132ddc6b5..b68174c84 100644 --- a/src/middleware/etaghasher.rs +++ b/src/middleware/etaghasher.rs @@ -90,7 +90,7 @@ pub trait Hasher { fn hash(&mut self, input: &[u8]) -> String; } /// Can test a (request, response) pair and return `true` or `false` -pub trait RequestFilter { +pub trait Filter { /// Return `true` if ETag processing should be applied to this /// `(request, response)` pair and `false` otherwise. A `false` return /// value will immediately return the original response unchanged. @@ -103,7 +103,7 @@ impl String> Hasher for F { self(input) } } -impl, &HttpResponse) -> bool> RequestFilter for F { +impl, &HttpResponse) -> bool> Filter for F { fn filter(&self, req: &HttpRequest, res: &HttpResponse) -> bool { self(req, res) } @@ -134,7 +134,7 @@ impl Hasher for DefaultHasher { /// Returns `true` when the request method is `GET` or `HEAD` and the /// original response status is `200 OK`, and `false` otherwise. pub struct DefaultFilter; -impl RequestFilter for DefaultFilter { +impl Filter for DefaultFilter { fn filter(&self, req: &HttpRequest, res: &HttpResponse) -> bool { use http::{Method, StatusCode}; (*req.method() == Method::GET || *req.method() == Method::HEAD) @@ -154,7 +154,7 @@ pub struct EtagHasher where S: 'static, H: Hasher + 'static, - F: RequestFilter + 'static, + F: Filter + 'static, { hasher: H, filter: F, @@ -165,9 +165,9 @@ impl EtagHasher where S: 'static, H: Hasher + 'static, - F: RequestFilter + 'static, + F: Filter + 'static, { - /// Create a new middleware struct with the given Hasher and RequestFilter. + /// Create a new middleware struct with the given Hasher and Filter. pub fn new(hasher: H, filter: F) -> Self { EtagHasher { hasher, @@ -181,7 +181,7 @@ impl middleware::Middleware for EtagHasher where S: 'static, H: Hasher + 'static, - F: RequestFilter + 'static, + F: Filter + 'static, { fn response( &mut self, req: &mut HttpRequest, mut res: HttpResponse, From d3899e5c11dda51f62b67bae9bc9c05c9ea52d6e Mon Sep 17 00:00:00 2001 From: axon-q Date: Thu, 14 Jun 2018 10:27:20 +0000 Subject: [PATCH 5/8] only support GET requests and 200 OK responses --- src/middleware/etaghasher.rs | 45 +++++++++++++++++++++--------------- 1 file changed, 26 insertions(+), 19 deletions(-) diff --git a/src/middleware/etaghasher.rs b/src/middleware/etaghasher.rs index b68174c84..661fd3681 100644 --- a/src/middleware/etaghasher.rs +++ b/src/middleware/etaghasher.rs @@ -1,13 +1,14 @@ //! ETag header and `304 Not Modified` support for HTTP responses /// /// The `EtagHasher` middleware generates [RFC -/// 7232](https://tools.ietf.org/html/rfc7232) ETag headers for HTTP -/// responses, and checks the ETag for a response against those provided -/// in the `If-None-Match` header of the request, if present. In the -/// event of a match, instead of returning the original response, an -/// HTTP `304 Not Modified` response with no content is returned -/// instead. Only response [Body](enum.Body.html)s of type `Binary` are -/// supported; responses with other body types will be left unchanged. +/// 7232](https://tools.ietf.org/html/rfc7232) ETag headers for `200 OK` +/// responses to HTTP `GET` requests, and checks the ETag for a response +/// against those provided in the `If-None-Match` header of the request, +/// if present. In the event of a match, instead of returning the +/// original response, an HTTP `304 Not Modified` response with no +/// content is returned instead. Only response [Body](enum.Body.html)s +/// of type `Binary` are supported; responses with other body types will +/// be left unchanged. /// /// ETag values are generated by computing a hash function over the /// bytes of the body of the original response. Thus, using this @@ -32,9 +33,7 @@ /// instance is created; the `DefaultHasher` and `DefaultFilter` can be /// used if desired. Currently `DefaultHasher` computes an SHA-1 hash, /// but this should not be relied upon. The `DefaultFilter` returns -/// `true` when the request method is `GET` or `HEAD` and the original -/// response status is `200 OK`. If you provide your own `filter`, you -/// will want to check for these conditions as well. +/// `true` for all `(request, response)` pairs. /// /// ```rust /// # extern crate actix_web; @@ -131,14 +130,11 @@ impl Hasher for DefaultHasher { } } -/// Returns `true` when the request method is `GET` or `HEAD` and the -/// original response status is `200 OK`, and `false` otherwise. +/// Returns `true` for every `(request, response)` pair. pub struct DefaultFilter; impl Filter for DefaultFilter { - fn filter(&self, req: &HttpRequest, res: &HttpResponse) -> bool { - use http::{Method, StatusCode}; - (*req.method() == Method::GET || *req.method() == Method::HEAD) - && res.status() == StatusCode::OK + fn filter(&self, _req: &HttpRequest, _res: &HttpResponse) -> bool { + true } } @@ -148,8 +144,17 @@ impl Filter for DefaultFilter { /// The `EtagHasher` struct contains a Hasher to compute ETag values for /// byte slices and a Filter to determine whether ETag computation and /// checking should be applied to a particular (request, response) -/// pair. Only response [Body](enum.Body.html)s of type `Binary` are -/// supported; responses with other body types will be left unchanged. +/// pair. +/// +/// Middleware processing will be performed only if the following +/// conditions hold: +/// +/// * The request method is `GET` +/// * The status of the original response is `200 OK` +/// * The type of the original response [Body](enum.Body.html) is `Binary` +/// +/// If any of these conditions is false, the original response will be +/// passed through unmodified. pub struct EtagHasher where S: 'static, @@ -186,10 +191,12 @@ where fn response( &mut self, req: &mut HttpRequest, mut res: HttpResponse, ) -> Result { + use http::{Method, StatusCode}; use header; use Body; - if !self.filter.filter(req, &res) { + let valid = *req.method() == Method::GET && res.status() == StatusCode::OK; + if !(valid && self.filter.filter(req, &res)) { return Ok(middleware::Response::Done(res)); } From 77e5f6a99bd375c9fd98bfbba1e75dff3996cb7a Mon Sep 17 00:00:00 2001 From: axon-q Date: Thu, 14 Jun 2018 10:30:26 +0000 Subject: [PATCH 6/8] tests: define test body as const --- src/middleware/etaghasher.rs | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/middleware/etaghasher.rs b/src/middleware/etaghasher.rs index 661fd3681..d3a58c11a 100644 --- a/src/middleware/etaghasher.rs +++ b/src/middleware/etaghasher.rs @@ -266,12 +266,13 @@ mod tests { use middleware::Middleware; use test::{TestRequest, TestServer}; + const TEST_BODY: &'static str = "test"; const TEST_ETAG: &'static str = "\"a94a8fe5ccb19ba61c4c0873d391e987982fbbd3\""; struct TestState { _state: u32, } fn test_index(_req: HttpRequest) -> &'static str { - "test" + TEST_BODY } fn mwres(r: Result) -> HttpResponse { @@ -285,7 +286,7 @@ mod tests { fn test_default_create_etag() { let mut eh = EtagHasher::new(DefaultHasher::new(), DefaultFilter); let mut req = TestRequest::default().finish(); - let res = HttpResponse::Ok().body("test"); + let res = HttpResponse::Ok().body(TEST_BODY); let res = mwres(eh.response(&mut req, res)); assert_eq!(res.status(), StatusCode::OK); assert_eq!(res.headers().get(ETAG).unwrap(), TEST_ETAG); @@ -295,7 +296,7 @@ mod tests { let state = TestState { _state: 0 }; let mut eh = EtagHasher::new(DefaultHasher::new(), DefaultFilter); let mut req = TestRequest::with_state(state).finish(); - let res = HttpResponse::Ok().body("test"); + let res = HttpResponse::Ok().body(TEST_BODY); let res = mwres(eh.response(&mut req, res)); assert_eq!(res.status(), StatusCode::OK); assert_eq!(res.headers().get(ETAG).unwrap(), TEST_ETAG); @@ -304,7 +305,7 @@ mod tests { fn test_default_none_match() { let mut eh = EtagHasher::new(DefaultHasher::new(), DefaultFilter); let mut req = TestRequest::with_header("If-None-Match", "_").finish(); - let res = HttpResponse::Ok().body("test"); + let res = HttpResponse::Ok().body(TEST_BODY); let res = mwres(eh.response(&mut req, res)); assert_eq!(res.status(), StatusCode::OK); assert_eq!(res.headers().get(ETAG).unwrap(), TEST_ETAG); @@ -313,7 +314,7 @@ mod tests { fn test_default_match() { let mut eh = EtagHasher::new(DefaultHasher::new(), DefaultFilter); let mut req = TestRequest::with_header("If-None-Match", TEST_ETAG).finish(); - let res = HttpResponse::Ok().body("test"); + let res = HttpResponse::Ok().body(TEST_BODY); let res = mwres(eh.response(&mut req, res)); assert_eq!(res.status(), StatusCode::NOT_MODIFIED); } @@ -324,7 +325,7 @@ mod tests { |_req: &HttpRequest<()>, _res: &HttpResponse| true, ); let mut req = TestRequest::with_header("If-None-Match", "\"static\"").finish(); - let res = HttpResponse::Ok().body("test"); + let res = HttpResponse::Ok().body(TEST_BODY); let res = mwres(eh.response(&mut req, res)); assert_eq!(res.status(), StatusCode::NOT_MODIFIED); } From 2e883cf44f2be8c62670abc90d82742f6febec62 Mon Sep 17 00:00:00 2001 From: axon-q Date: Thu, 14 Jun 2018 11:44:48 +0000 Subject: [PATCH 7/8] tests: test passthrough of unsupported req/res types --- src/middleware/etaghasher.rs | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/src/middleware/etaghasher.rs b/src/middleware/etaghasher.rs index d3a58c11a..769ce3194 100644 --- a/src/middleware/etaghasher.rs +++ b/src/middleware/etaghasher.rs @@ -319,6 +319,37 @@ mod tests { assert_eq!(res.status(), StatusCode::NOT_MODIFIED); } #[test] + fn test_unsupported_messages_unchanged() { + use http; + use Body; + let mut eh = EtagHasher::new(DefaultHasher::new(), DefaultFilter); + + let mut req = TestRequest::default().method(http::Method::HEAD).finish(); + let res = HttpResponse::Ok().body(TEST_BODY); + let res = mwres(eh.response(&mut req, res)); + assert!(res.headers().get(ETAG).is_none()); + + let mut req = TestRequest::default().method(http::Method::POST).finish(); + let res = HttpResponse::Ok().body(TEST_BODY); + let res = mwres(eh.response(&mut req, res)); + assert!(res.headers().get(ETAG).is_none()); + + let mut req = TestRequest::default().method(http::Method::PUT).finish(); + let res = HttpResponse::Ok().body(TEST_BODY); + let res = mwres(eh.response(&mut req, res)); + assert!(res.headers().get(ETAG).is_none()); + + let mut req = TestRequest::default().method(http::Method::PATCH).finish(); + let res = HttpResponse::Ok().body(TEST_BODY); + let res = mwres(eh.response(&mut req, res)); + assert!(res.headers().get(ETAG).is_none()); + + let mut req = TestRequest::default().method(http::Method::GET).finish(); + let res = HttpResponse::Ok().body(Body::Empty); + let res = mwres(eh.response(&mut req, res)); + assert!(res.headers().get(ETAG).is_none()); + } + #[test] fn test_custom_match() { let mut eh = EtagHasher::new( |_input: &[u8]| "static".to_string(), From 7f113ef9e4822bd0c06d64e718a5ccbffcf767e3 Mon Sep 17 00:00:00 2001 From: axon-q Date: Thu, 14 Jun 2018 14:23:19 +0000 Subject: [PATCH 8/8] minor syntax update --- src/middleware/etaghasher.rs | 46 +++++++++++++++++++----------------- 1 file changed, 24 insertions(+), 22 deletions(-) diff --git a/src/middleware/etaghasher.rs b/src/middleware/etaghasher.rs index 769ce3194..0c8706460 100644 --- a/src/middleware/etaghasher.rs +++ b/src/middleware/etaghasher.rs @@ -200,33 +200,35 @@ where return Ok(middleware::Response::Done(res)); } - let e = if let Body::Binary(b) = res.body() { - Some(EntityTag::strong(self.hasher.hash(b.as_ref()))) - } else { - None + // This double match is because we can't do the return in the first match + // as res is still borrowed + let etag = match match res.body() { + Body::Binary(b) => Some(EntityTag::strong(self.hasher.hash(b.as_ref()))), + _ => None, + } { + Some(tag) => tag, + None => return Ok(middleware::Response::Done(res)), }; - if let Some(etag) = e { - if !none_match(&etag, req) { - let mut not_modified = - HttpResponse::NotModified().set(header::ETag(etag)).finish(); + if !none_match(&etag, req) { + let mut not_modified = + HttpResponse::NotModified().set(header::ETag(etag)).finish(); - // RFC 7232 requires copying over these headers: - copy_header(header::CACHE_CONTROL, &res, &mut not_modified); - copy_header(header::CONTENT_LOCATION, &res, &mut not_modified); - copy_header(header::DATE, &res, &mut not_modified); - copy_header(header::EXPIRES, &res, &mut not_modified); - copy_header(header::VARY, &res, &mut not_modified); + // RFC 7232 requires copying over these headers: + copy_header(header::CACHE_CONTROL, &res, &mut not_modified); + copy_header(header::CONTENT_LOCATION, &res, &mut not_modified); + copy_header(header::DATE, &res, &mut not_modified); + copy_header(header::EXPIRES, &res, &mut not_modified); + copy_header(header::VARY, &res, &mut not_modified); - return Ok(middleware::Response::Done(not_modified)); - } - etag.to_string() - .parse::() - .map(|v| { - res.headers_mut().insert(header::ETAG, v); - }) - .unwrap_or(()); + return Ok(middleware::Response::Done(not_modified)); } + etag.to_string() + .parse::() + .map(|v| { + res.headers_mut().insert(header::ETAG, v); + }) + .unwrap_or(()); Ok(middleware::Response::Done(res)) } }