From f73ee3a62e5f23a27f101aa62ba25ddb2ee3d5d6 Mon Sep 17 00:00:00 2001 From: Yong Wen Chua Date: Sun, 16 Jul 2017 14:59:04 +0800 Subject: [PATCH] Refactor to allow for fairings to be even built --- src/lib.rs | 464 +++++++++++++++++++++++----------------------- tests/fairings.rs | 6 +- 2 files changed, 238 insertions(+), 232 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index eec9df0..d07d7ee 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -380,20 +380,6 @@ impl Default for Cors { } } -/// Ad-hoc per route CORS response to requests -/// -/// Note: If you use this, the lifetime parameter `'r` of your `rocket:::response::Responder<'r>` -/// CANNOT be `'static`. This is because the code generated by Rocket will implicitly try to -/// to restrain the `Request` object passed to the route to `&'static Request`, and it is not -/// possible to have such a reference. -/// See [this PR on Rocket](https://github.com/SergioBenitez/Rocket/pull/345). -pub fn respond<'a, 'r: 'a, R: response::Responder<'r>>( - options: State<'a, Cors>, - responder: R, -) -> Responder<'a, 'r, R> { - options.inner().respond(responder) -} - impl Cors { /// Wrap any `Rocket::Response` and respond with CORS headers. /// This is only used for ad-hoc route CORS response @@ -447,8 +433,7 @@ impl fairing::Fairing for Cors { } } - fn on_response(&self, request: &Request, response: &mut rocket::Response) { - } + fn on_response(&self, _: &Request, _: &mut rocket::Response) {} } /// A CORS [Responder](https://rocket.rs/guide/responses/#responder) @@ -486,218 +471,13 @@ impl<'a, 'r: 'a, R: response::Responder<'r>> Responder<'a, 'r, R> { /// Respond to a request fn respond(self, request: &Request) -> response::Result<'r> { - match self.build_cors_response(request) { - Ok(response) => response, + let mut response = self.responder.respond_to(request)?; // handle status errors? + + match build_cors_response(self.options, request, &mut response) { + Ok(()) => Ok(response), Err(e) => response::Responder::respond_to(e, request), } } - - /// Build a CORS response and merge with an existing `rocket::Response` for the request - fn build_cors_response(self, request: &Request) -> Result, Error> { - let original_response = match self.responder.respond_to(request) { - Ok(response) => response, - Err(status) => return Ok(Err(status)), // TODO: Handle this? - }; - - // Existing CORS response? - if Self::has_allow_origin(&original_response) { - return Ok(Ok(original_response)); - } - - // 1. If the Origin header is not present terminate this set of steps. - // The request is outside the scope of this specification. - let origin = Self::origin(request)?; - let origin = match origin { - None => { - // Not a CORS request - return Ok(Ok(original_response)); - } - Some(origin) => origin, - }; - - // Check if the request verb is an OPTION or something else - let cors_response = match request.method() { - Method::Options => { - let method = Self::request_method(request)?; - let headers = Self::request_headers(request)?; - Self::preflight(&self.options, origin, method, headers) - } - _ => Self::actual_request(&self.options, origin), - }?; - - Ok(Ok(cors_response.build(original_response))) - } - - /// Gets the `Origin` request header from the request - fn origin(request: &Request) -> Result, Error> { - match Origin::from_request(request) { - Outcome::Forward(()) => Ok(None), - Outcome::Success(origin) => Ok(Some(origin)), - Outcome::Failure((_, err)) => Err(err), - } - } - - /// Gets the `Access-Control-Request-Method` request header from the request - fn request_method(request: &Request) -> Result, Error> { - match AccessControlRequestMethod::from_request(request) { - Outcome::Forward(()) => Ok(None), - Outcome::Success(method) => Ok(Some(method)), - Outcome::Failure((_, err)) => Err(err), - } - } - - /// Gets the `Access-Control-Request-Headers` request header from the request - fn request_headers(request: &Request) -> Result, Error> { - match AccessControlRequestHeaders::from_request(request) { - Outcome::Forward(()) => Ok(None), - Outcome::Success(geaders) => Ok(Some(geaders)), - Outcome::Failure((_, err)) => Err(err), - } - } - - /// Checks if an existing Response already has the header `Access-Control-Allow-Origin` - fn has_allow_origin(response: &response::Response<'r>) -> bool { - response.headers().get("Access-Control-Allow-Origin").next() != None - } - - /// Construct a preflight response based on the options. Will return an `Err` - /// if any of the preflight checks fail. - /// - /// This implementation references the - /// [W3C recommendation](https://www.w3.org/TR/cors/#resource-preflight-requests). - fn preflight( - options: &Cors, - origin: Origin, - method: Option, - headers: Option, - ) -> Result { - - let response = Response::new(); - - // Note: All header parse failures are dealt with in the `FromRequest` trait implementation - - // 2. If the value of the Origin header is not a case-sensitive match for any of the values - // in list of origins do not set any additional headers and terminate this set of steps. - let response = response.allowed_origin( - &origin, - &options.allowed_origins, - options.send_wildcard, - )?; - - // 3. Let `method` be the value as result of parsing the Access-Control-Request-Method - // header. - // If there is no Access-Control-Request-Method header or if parsing failed, - // do not set any additional headers and terminate this set of steps. - // The request is outside the scope of this specification. - - let method = method.ok_or_else(|| Error::MissingRequestMethod)?; - - // 4. Let header field-names be the values as result of parsing the - // Access-Control-Request-Headers headers. - // If there are no Access-Control-Request-Headers headers - // let header field-names be the empty list. - // If parsing failed do not set any additional headers and terminate this set of steps. - // The request is outside the scope of this specification. - - // 5. If method is not a case-sensitive match for any of the values in list of methods - // do not set any additional headers and terminate this set of steps. - - let response = response.allowed_methods(&method, &options.allowed_methods)?; - - // 6. If any of the header field-names is not a ASCII case-insensitive match for any of the - // values in list of headers do not set any additional headers and terminate this set of - // steps. - let response = if let Some(headers) = headers { - response.allowed_headers(&headers, &options.allowed_headers)? - } else { - response - }; - - // 7. If the resource supports credentials add a single Access-Control-Allow-Origin header, - // with the value of the Origin header as value, and add a - // single Access-Control-Allow-Credentials header with the case-sensitive string "true" as - // value. - // Otherwise, add a single Access-Control-Allow-Origin header, - // with either the value of the Origin header or the string "*" as value. - // Note: The string "*" cannot be used for a resource that supports credentials. - - let response = response.credentials(options.allow_credentials)?; - - // 8. Optionally add a single Access-Control-Max-Age header - // with as value the amount of seconds the user agent is allowed to cache the result of the - // request. - let response = response.max_age(options.max_age); - - // 9. If method is a simple method this step may be skipped. - // Add one or more Access-Control-Allow-Methods headers consisting of - // (a subset of) the list of methods. - // If a method is a simple method it does not need to be listed, but this is not prohibited. - // Since the list of methods can be unbounded, - // simply returning the method indicated by Access-Control-Request-Method - // (if supported) can be enough. - - // Done above - - // 10. If each of the header field-names is a simple header and none is Content-Type, - // this step may be skipped. - // Add one or more Access-Control-Allow-Headers headers consisting of (a subset of) - // the list of headers. - // If a header field name is a simple header and is not Content-Type, - // it is not required to be listed. Content-Type is to be listed as only a - // subset of its values makes it qualify as simple header. - // Since the list of headers can be unbounded, simply returning supported headers - // from Access-Control-Allow-Headers can be enough. - - // Done above -- we do not do anything special with simple headers - - Ok(response) - } - - /// Respond to an actual request based on the settings. - /// If the `Origin` is not provided, then this request was not made by a browser and there is no - /// CORS enforcement. - fn actual_request(options: &Cors, origin: Origin) -> Result { - let response = Response::new(); - - // Note: All header parse failures are dealt with in the `FromRequest` trait implementation - - // 2. If the value of the Origin header is not a case-sensitive match for any of the values - // in list of origins, do not set any additional headers and terminate this set of steps. - // Always matching is acceptable since the list of origins can be unbounded. - - let response = response.allowed_origin( - &origin, - &options.allowed_origins, - options.send_wildcard, - )?; - - // 3. If the resource supports credentials add a single Access-Control-Allow-Origin header, - // with the value of the Origin header as value, and add a - // single Access-Control-Allow-Credentials header with the case-sensitive string "true" as - // value. - // Otherwise, add a single Access-Control-Allow-Origin header, - // with either the value of the Origin header or the string "*" as value. - // Note: The string "*" cannot be used for a resource that supports credentials. - - let response = response.credentials(options.allow_credentials)?; - - // 4. If the list of exposed headers is not empty add one or more - // Access-Control-Expose-Headers headers, with as values the header field names given in - // the list of exposed headers. - // By not adding the appropriate headers resource can also clear the preflight result cache - // of all entries where origin is a case-sensitive match for the value of the Origin header - // and url is a case-sensitive match for the URL of the resource. - - let response = response.exposed_headers( - options - .expose_headers - .iter() - .map(|s| &**s) - .collect::>() - .as_slice(), - ); - Ok(response) - } } impl<'a, 'r: 'a, R: response::Responder<'r>> response::Responder<'r> for Responder<'a, 'r, R> { @@ -874,15 +654,22 @@ impl Response { /// Builds a `rocket::Response` from this struct based off some base `rocket::Response` /// /// This will overwrite any existing CORS headers - #[allow(unused_results)] + #[cfg(test)] fn build<'r>(&self, base: response::Response<'r>) -> response::Response<'r> { let mut response = response::Response::build_from(base).finalize(); + self.merge(&mut response); + response + } + /// Merge CORS headers with an existing `rocket::Response`. + /// + /// This will overwrite any existing CORS headers + fn merge(&self, response: &mut response::Response) { // TODO: We should be able to remove this let origin = match self.allow_origin { None => { // This is not a CORS response - return response; + return; } Some(ref origin) => origin, }; @@ -945,11 +732,230 @@ impl Response { } else { response.remove_header("Vary"); } - - response } } +/// Ad-hoc per route CORS response to requests +/// +/// Note: If you use this, the lifetime parameter `'r` of your `rocket:::response::Responder<'r>` +/// CANNOT be `'static`. This is because the code generated by Rocket will implicitly try to +/// to restrain the `Request` object passed to the route to `&'static Request`, and it is not +/// possible to have such a reference. +/// See [this PR on Rocket](https://github.com/SergioBenitez/Rocket/pull/345). +pub fn respond<'a, 'r: 'a, R: response::Responder<'r>>( + options: State<'a, Cors>, + responder: R, +) -> Responder<'a, 'r, R> { + options.inner().respond(responder) +} + +/// Build a CORS response and merge with an existing `rocket::Response` for the request +fn build_cors_response( + options: &Cors, + request: &Request, + mut response: &mut rocket::Response, +) -> Result<(), Error> { + // Existing CORS response? + if has_allow_origin(response) { + return Ok(()); + } + + // 1. If the Origin header is not present terminate this set of steps. + // The request is outside the scope of this specification. + let origin = origin(request)?; + let origin = match origin { + None => { + // Not a CORS request + return Ok(()); + } + Some(origin) => origin, + }; + + // Check if the request verb is an OPTION or something else + let cors_response = match request.method() { + Method::Options => { + let method = request_method(request)?; + let headers = request_headers(request)?; + preflight(options, origin, method, headers) + } + _ => actual_request(options, origin), + }?; + + cors_response.merge(&mut response); + Ok(()) +} + +/// Gets the `Origin` request header from the request +fn origin(request: &Request) -> Result, Error> { + match Origin::from_request(request) { + Outcome::Forward(()) => Ok(None), + Outcome::Success(origin) => Ok(Some(origin)), + Outcome::Failure((_, err)) => Err(err), + } +} + +/// Gets the `Access-Control-Request-Method` request header from the request +fn request_method(request: &Request) -> Result, Error> { + match AccessControlRequestMethod::from_request(request) { + Outcome::Forward(()) => Ok(None), + Outcome::Success(method) => Ok(Some(method)), + Outcome::Failure((_, err)) => Err(err), + } +} + +/// Gets the `Access-Control-Request-Headers` request header from the request +fn request_headers(request: &Request) -> Result, Error> { + match AccessControlRequestHeaders::from_request(request) { + Outcome::Forward(()) => Ok(None), + Outcome::Success(geaders) => Ok(Some(geaders)), + Outcome::Failure((_, err)) => Err(err), + } +} + +/// Checks if an existing Response already has the header `Access-Control-Allow-Origin` +fn has_allow_origin<'r>(response: &response::Response<'r>) -> bool { + response.headers().get("Access-Control-Allow-Origin").next() != None +} + +/// Construct a preflight response based on the options. Will return an `Err` +/// if any of the preflight checks fail. +/// +/// This implementation references the +/// [W3C recommendation](https://www.w3.org/TR/cors/#resource-preflight-requests). +fn preflight( + options: &Cors, + origin: Origin, + method: Option, + headers: Option, +) -> Result { + + let response = Response::new(); + + // Note: All header parse failures are dealt with in the `FromRequest` trait implementation + + // 2. If the value of the Origin header is not a case-sensitive match for any of the values + // in list of origins do not set any additional headers and terminate this set of steps. + let response = response.allowed_origin( + &origin, + &options.allowed_origins, + options.send_wildcard, + )?; + + // 3. Let `method` be the value as result of parsing the Access-Control-Request-Method + // header. + // If there is no Access-Control-Request-Method header or if parsing failed, + // do not set any additional headers and terminate this set of steps. + // The request is outside the scope of this specification. + + let method = method.ok_or_else(|| Error::MissingRequestMethod)?; + + // 4. Let header field-names be the values as result of parsing the + // Access-Control-Request-Headers headers. + // If there are no Access-Control-Request-Headers headers + // let header field-names be the empty list. + // If parsing failed do not set any additional headers and terminate this set of steps. + // The request is outside the scope of this specification. + + // 5. If method is not a case-sensitive match for any of the values in list of methods + // do not set any additional headers and terminate this set of steps. + + let response = response.allowed_methods(&method, &options.allowed_methods)?; + + // 6. If any of the header field-names is not a ASCII case-insensitive match for any of the + // values in list of headers do not set any additional headers and terminate this set of + // steps. + let response = if let Some(headers) = headers { + response.allowed_headers(&headers, &options.allowed_headers)? + } else { + response + }; + + // 7. If the resource supports credentials add a single Access-Control-Allow-Origin header, + // with the value of the Origin header as value, and add a + // single Access-Control-Allow-Credentials header with the case-sensitive string "true" as + // value. + // Otherwise, add a single Access-Control-Allow-Origin header, + // with either the value of the Origin header or the string "*" as value. + // Note: The string "*" cannot be used for a resource that supports credentials. + + let response = response.credentials(options.allow_credentials)?; + + // 8. Optionally add a single Access-Control-Max-Age header + // with as value the amount of seconds the user agent is allowed to cache the result of the + // request. + let response = response.max_age(options.max_age); + + // 9. If method is a simple method this step may be skipped. + // Add one or more Access-Control-Allow-Methods headers consisting of + // (a subset of) the list of methods. + // If a method is a simple method it does not need to be listed, but this is not prohibited. + // Since the list of methods can be unbounded, + // simply returning the method indicated by Access-Control-Request-Method + // (if supported) can be enough. + + // Done above + + // 10. If each of the header field-names is a simple header and none is Content-Type, + // this step may be skipped. + // Add one or more Access-Control-Allow-Headers headers consisting of (a subset of) + // the list of headers. + // If a header field name is a simple header and is not Content-Type, + // it is not required to be listed. Content-Type is to be listed as only a + // subset of its values makes it qualify as simple header. + // Since the list of headers can be unbounded, simply returning supported headers + // from Access-Control-Allow-Headers can be enough. + + // Done above -- we do not do anything special with simple headers + + Ok(response) +} + +/// Respond to an actual request based on the settings. +/// If the `Origin` is not provided, then this request was not made by a browser and there is no +/// CORS enforcement. +fn actual_request(options: &Cors, origin: Origin) -> Result { + let response = Response::new(); + + // Note: All header parse failures are dealt with in the `FromRequest` trait implementation + + // 2. If the value of the Origin header is not a case-sensitive match for any of the values + // in list of origins, do not set any additional headers and terminate this set of steps. + // Always matching is acceptable since the list of origins can be unbounded. + + let response = response.allowed_origin( + &origin, + &options.allowed_origins, + options.send_wildcard, + )?; + + // 3. If the resource supports credentials add a single Access-Control-Allow-Origin header, + // with the value of the Origin header as value, and add a + // single Access-Control-Allow-Credentials header with the case-sensitive string "true" as + // value. + // Otherwise, add a single Access-Control-Allow-Origin header, + // with either the value of the Origin header or the string "*" as value. + // Note: The string "*" cannot be used for a resource that supports credentials. + + let response = response.credentials(options.allow_credentials)?; + + // 4. If the list of exposed headers is not empty add one or more + // Access-Control-Expose-Headers headers, with as values the header field names given in + // the list of exposed headers. + // By not adding the appropriate headers resource can also clear the preflight result cache + // of all entries where origin is a case-sensitive match for the value of the Origin header + // and url is a case-sensitive match for the URL of the resource. + + let response = response.exposed_headers( + options + .expose_headers + .iter() + .map(|s| &**s) + .collect::>() + .as_slice(), + ); + Ok(response) +} + #[cfg(test)] #[allow(unmounted_route)] mod tests { diff --git a/tests/fairings.rs b/tests/fairings.rs index 549221f..255e9c6 100644 --- a/tests/fairings.rs +++ b/tests/fairings.rs @@ -37,9 +37,9 @@ fn make_cors_options() -> Cors { } fn rocket() -> rocket::Rocket { - rocket::ignite() - .mount("/", routes![cors]) - .attach(make_cors_options()) + rocket::ignite().mount("/", routes![cors]).attach( + make_cors_options(), + ) } #[test]