Fix interactions between fairings and auto-HEAD responses.

Fixes #546.
This commit is contained in:
Lukas Abfalterer 2018-02-04 11:07:28 +01:00 committed by Sergio Benitez
parent 9be3c11cdf
commit 3a1d671894
4 changed files with 126 additions and 30 deletions

View File

@ -48,6 +48,15 @@
//! Furthermore, a `Fairing` should take care to act locally so that the actions
//! of other `Fairings` are not jeopardized. For instance, unless it is made
//! abundantly clear, a fairing should not rewrite every request.
//!
//! ## Attention
//!
//! If Rocket receives a `HEAD` request and no appropriate Route is provided,
//! Rocket tries to fullfil this request as it were a `GET` request (Autohandling
//! `HEAD` requests). _Beware_ the request method is set to `GET` on the request which is
//! provided in the [`on_response`](/rocket/fairing/trait.Fairing.html#method.on_response)
//! method.
use {Rocket, Request, Response, Data};
mod fairings;
@ -336,6 +345,11 @@ pub trait Fairing: Send + Sync + 'static {
/// this fairing. The `&Request` parameter is the request that was routed,
/// and the `&mut Response` parameter is the resulting response.
///
/// ## Note
///
/// The body of a `HEAD` request will be stripped off _after_ all response
/// `Fairings` have been performed.
///
/// ## Default Implementation
///
/// The default implementation of this method does nothing.

View File

@ -37,7 +37,7 @@ struct RequestState<'r> {
/// data. This includes the HTTP method, URI, cookies, headers, and more.
#[derive(Clone)]
pub struct Request<'r> {
method: Method,
method: Cell<Method>,
uri: Uri<'r>,
headers: HeaderMap<'r>,
remote: Option<SocketAddr>,
@ -53,7 +53,7 @@ impl<'r> Request<'r> {
method: Method,
uri: U) -> Request<'r> {
Request {
method: method,
method: Cell::new(method),
uri: uri.into(),
headers: HeaderMap::new(),
remote: None,
@ -91,7 +91,7 @@ impl<'r> Request<'r> {
/// ```
#[inline(always)]
pub fn method(&self) -> Method {
self.method
self.method.get()
}
/// Set the method of `self`.
@ -111,7 +111,7 @@ impl<'r> Request<'r> {
/// ```
#[inline(always)]
pub fn set_method(&mut self, method: Method) {
self.method = method;
self.method.set(method);
}
/// Borrow the URI from `self`, which is guaranteed to be an absolute URI.
@ -446,7 +446,7 @@ impl<'r> Request<'r> {
/// ```
pub fn format(&self) -> Option<&MediaType> {
static ANY: MediaType = MediaType::Any;
if self.method.supports_payload() {
if self.method().supports_payload() {
self.content_type().map(|ct| ct.media_type())
} else {
// FIXME: Should we be using `accept_first` or `preferred`? Or
@ -616,6 +616,12 @@ impl<'r> Request<'r> {
*self.state.params.borrow_mut() = route.get_param_indexes(self.uri());
}
/// Set the method of `self`, even if self is a shared reference
#[inline(always)]
pub(crate) fn _set_method(&self, method: Method) {
self.method.set(method);
}
/// Replace all of the cookies in `self` with those in `jar`.
#[inline]
pub(crate) fn set_cookies(&mut self, jar: CookieJar) {
@ -701,7 +707,7 @@ impl<'r> fmt::Display for Request<'r> {
/// Pretty prints a Request. This is primarily used by Rocket's logging
/// infrastructure.
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
write!(f, "{} {}", Paint::green(&self.method), Paint::blue(&self.uri))?;
write!(f, "{} {}", Paint::green(self.method()), Paint::blue(&self.uri))?;
// Print the requests media type when the route specifies a format.
if let Some(media_type) = self.format() {

View File

@ -193,20 +193,46 @@ impl Rocket {
}
}
#[inline]
#[inline(always)]
pub(crate) fn dispatch<'s, 'r>(
&'s self,
request: &'r mut Request<'s>,
data: Data,
data: Data
) -> Response<'r> {
info!("{}:", request);
// Do a bit of preprocessing before routing; run the attached fairings.
self.preprocess_request(request, &data);
// Run the request fairings.
self.fairings.handle_request(request, &data);
// Check if the request is a `HEAD` request.
let was_head_request = request.method() == Method::Head;
// Route the request and run the user's handlers.
let mut response = self.route_and_process(request, data);
// Add the 'rocket' server header to the response and run fairings.
// TODO: If removing Hyper, write out `Date` header too.
response.set_header(Header::new("Server", "Rocket"));
self.fairings.handle_response(request, &mut response);
// Strip the body if this is a `HEAD` request.
if was_head_request {
response.strip_body();
}
response
}
fn route_and_process<'s, 'r>(
&'s self,
request: &'r Request<'s>,
data: Data
) -> Response<'r> {
// Route the request to get a response.
let mut response = match self.route(request, data) {
let response = match self.route(request, data) {
Outcome::Success(mut response) => {
// A user's route responded! Set the cookies.
for cookie in request.cookies().delta() {
@ -216,21 +242,14 @@ impl Rocket {
response
}
Outcome::Forward(data) => {
// Rust thinks `request` is still borrowed here, but it's
// obviously not (data has nothing to do with it), so we
// convince it to give us another mutable reference.
// TODO: Use something that is well defined, like UnsafeCell.
// But that causes variance issues...so wait for NLL.
let request: &'r mut Request<'s> =
unsafe { (&mut *(request as *const _ as *mut _)) };
// There was no matching route. Autohandle `HEAD` requests.
if request.method() == Method::Head {
info_!("Autohandling {} request.", Paint::white("HEAD"));
request.set_method(Method::Get);
let mut response = self.dispatch(request, data);
response.strip_body();
response
// Dispatch the request again with Method `GET`.
request._set_method(Method::Get);
self.route_and_process(request, data)
} else {
self.handle_error(Status::NotFound, request)
}
@ -238,16 +257,6 @@ impl Rocket {
Outcome::Failure(status) => self.handle_error(status, request),
};
// Strip the body if this is a `HEAD` request.
if request.method() == Method::Head {
response.strip_body();
}
// Add the 'rocket' server header to the response and run fairings.
// TODO: If removing Hyper, write out `Date` header too.
response.set_header(Header::new("Server", "Rocket"));
self.fairings.handle_response(request, &mut response);
response
}

View File

@ -0,0 +1,67 @@
#![feature(plugin, decl_macro)]
#![plugin(rocket_codegen)]
extern crate rocket;
const RESPONSE_STRING: &'static str = "{'test': 'dont strip before fairing' }";
#[head("/")]
fn index() -> &'static str {
RESPONSE_STRING
}
#[get("/")]
fn auto() -> &'static str {
RESPONSE_STRING
}
mod fairing_before_head_strip {
use super::*;
use std::sync::atomic::{AtomicUsize, Ordering};
use rocket::fairing::AdHoc;
use rocket::http::Method;
use rocket::local::Client;
use rocket::http::Status;
use rocket::State;
#[derive(Default)]
struct Counter {
get: AtomicUsize,
}
#[test]
fn not_empty_before_fairing() {
let rocket = rocket::ignite()
.mount("/", routes![index])
.attach(AdHoc::on_response(|req, res| {
assert_eq!(req.method(), Method::Head);
assert_eq!(res.body_string(), Some(RESPONSE_STRING.into()));
}));
let client = Client::new(rocket).unwrap();
let response = client.head("/").dispatch();
assert_eq!(response.status(), Status::Ok);
}
#[test]
fn not_empty_before_fairing_autohandeled() {
let counter = Counter::default();
let rocket = rocket::ignite()
.mount("/", routes![auto])
.manage(counter)
.attach(AdHoc::on_request(|req, _| {
let c = req.guard::<State<Counter>>().unwrap();
assert_eq!(c.get.fetch_add(1, Ordering::Release), 0);
}))
.attach(AdHoc::on_response(|req, res| {
assert_eq!(req.method(), Method::Get);
assert_eq!(res.body_string(), Some(RESPONSE_STRING.into()));
}));
let client = Client::new(rocket).unwrap();
let response = client.head("/").dispatch();
assert_eq!(response.status(), Status::Ok);
}
}