Make 'NamedFile' async. Fix 'Handler' trait.

Previously, 'NamedFile::open()' called a synchronous I/O method. This
commit changes it to instead use tokio's 'File' for async I/O.

To allow this to change, the 'Handler' trait was fixed to enforce that
the lifetime of '&self', the reference to the handler, outlives the
incoming request. As a result, futures returned from a handler can hold
a reference to 'self'.

contrib/lib/src/serve.rs

@@ -276,31 +276,34 @@ impl Into<Vec<Route>> for StaticFiles {
     }
 }
 
-impl Handler for StaticFiles {
-    fn handle<'r>(&self, req: &'r Request<'_>, data: Data) -> HandlerFuture<'r> {
-        fn handle_dir<'r>(opt: Options, r: &'r Request<'_>, d: Data, path: &Path) -> Outcome<'r> {
-            if opt.contains(Options::NormalizeDirs) && !r.uri().path().ends_with('/') {
-                let new_path = r.uri().map_path(|p| p.to_owned() + "/")
+async fn handle_dir<'r, P>(opt: Options, req: &'r Request<'_>, data: Data, path: P) -> Outcome<'r>
+    where P: AsRef<Path>
+{
+    if opt.contains(Options::NormalizeDirs) && !req.uri().path().ends_with('/') {
+        let new_path = req.uri().map_path(|p| p.to_owned() + "/")
             .expect("adding a trailing slash to a known good path results in a valid path")
             .into_owned();
 
-                return Outcome::from_or_forward(r, d, Redirect::permanent(new_path));
+        return Outcome::from_or_forward(req, data, Redirect::permanent(new_path));
     }
 
     if !opt.contains(Options::Index) {
-                return Outcome::forward(d);
+        return Outcome::forward(data);
     }
 
-            let file = NamedFile::open(path.join("index.html")).ok();
-            Outcome::from_or_forward(r, d, file)
-        }
+    let file = NamedFile::open(path.as_ref().join("index.html")).await.ok();
+    Outcome::from_or_forward(req, data, file)
+}
 
+impl Handler for StaticFiles {
+    fn handle<'r, 's: 'r>(&'s self, req: &'r Request<'_>, data: Data) -> HandlerFuture<'r> {
         // If this is not the route with segments, handle it only if the user
         // requested a handling of index files.
         let current_route = req.route().expect("route while handling");
         let is_segments_route = current_route.uri.path().ends_with(">");
         if !is_segments_route {
-            return handle_dir(self.options, req, data, &self.root).pin();
+            let result = handle_dir(self.options, req, data, &self.root);
+            return Box::pin(async move { result.await });
         }
 
         // Otherwise, we're handling segments. Get the segments as a `PathBuf`,
@@ -311,10 +314,12 @@ impl Handler for StaticFiles {
             .and_then(|segments| segments.into_path_buf(allow_dotfiles).ok())
             .map(|path| self.root.join(path));
 
-        match &path {
-            Some(path) if path.is_dir() => handle_dir(self.options, req, data, path),
-            Some(path) => Outcome::from_or_forward(req, data, NamedFile::open(path).ok()),
+        Box::pin(async move {
+            match path {
+                Some(p) if p.is_dir() => handle_dir(self.options, req, data, p).await,
+                Some(p) => Outcome::from_or_forward(req, data, NamedFile::open(p).await.ok()),
                 None => Outcome::forward(data),
-        }.pin()
+            }
+        })
     }
 }

core/lib/src/catcher.rs

@@ -84,11 +84,11 @@ impl Catcher {
     ///
     /// fn handle_404<'r>(req: &'r Request) -> ErrorHandlerFuture<'r> {
     ///    let res = Custom(Status::NotFound, format!("404: {}", req.uri()));
-    ///    res.respond_to(req)
+    ///    Box::pin(async move { res.respond_to(req) })
     /// }
     ///
     /// fn handle_500<'r>(req: &'r Request) -> ErrorHandlerFuture<'r> {
-    ///     "Whoops, we messed up!".respond_to(req)
+    ///     Box::pin(async move{ "Whoops, we messed up!".respond_to(req) })
     /// }
     ///
     /// let not_found_catcher = Catcher::new(404, handle_404);

core/lib/src/handler.rs

@@ -44,18 +44,18 @@ pub type HandlerFuture<'r> = BoxFuture<'r, Outcome<'r>>;
 /// ```rust,no_run
 /// # #[derive(Copy, Clone)] enum Kind { Simple, Intermediate, Complex, }
 /// use rocket::{Request, Data, Route, http::Method};
-/// use rocket::handler::{self, Handler, Outcome};
+/// use rocket::handler::{self, Handler, Outcome, HandlerFuture};
 ///
 /// #[derive(Clone)]
 /// struct CustomHandler(Kind);
 ///
 /// impl Handler for CustomHandler {
-///     fn handle<'r>(&self, req: &'r Request, data: Data) -> Outcome<'r> {
+///     fn handle<'r>(&self, req: &'r Request, data: Data) -> HandlerFuture<'r> {
 ///         match self.0 {
 ///             Kind::Simple => Outcome::from(req, "simple"),
 ///             Kind::Intermediate => Outcome::from(req, "intermediate"),
 ///             Kind::Complex => Outcome::from(req, "complex"),
-///         }
+///         }.pin()
 ///     }
 /// }
 ///
@@ -142,7 +142,7 @@ pub trait Handler: Cloneable + Send + Sync + 'static {
     /// generate a response. Otherwise, if the return value is `Forward(Data)`,
     /// the next matching route is attempted. If there are no other matching
     /// routes, the `404` error catcher is invoked.
-    fn handle<'r>(&self, request: &'r Request<'_>, data: Data) -> HandlerFuture<'r>;
+    fn handle<'r, 's: 'r>(&'s self, request: &'r Request<'_>, data: Data) -> HandlerFuture<'r>;
 }
 
 /// Unfortunate but necessary hack to be able to clone a `Box<Handler>`.
@@ -173,7 +173,7 @@ impl<F: Clone + Sync + Send + 'static> Handler for F
     where for<'r> F: Fn(&'r Request<'_>, Data) -> HandlerFuture<'r>
 {
     #[inline(always)]
-    fn handle<'r>(&self, req: &'r Request<'_>, data: Data) -> HandlerFuture<'r> {
+    fn handle<'r, 's: 'r>(&'s self, req: &'r Request<'_>, data: Data) -> HandlerFuture<'r> {
         self(req, data)
     }
 }

core/lib/src/response/named_file.rs

@@ -1,8 +1,9 @@
-use std::fs::File;
-use std::path::{Path, PathBuf};
 use std::io;
+use std::path::{Path, PathBuf};
 use std::ops::{Deref, DerefMut};
 
+use tokio::fs::File;
+
 use crate::request::Request;
 use crate::response::{self, Responder};
 use crate::http::ContentType;
@@ -26,11 +27,17 @@ impl NamedFile {
     /// ```rust
     /// use rocket::response::NamedFile;
     ///
-    /// # #[allow(unused_variables)]
-    /// let file = NamedFile::open("foo.txt");
+    /// #[allow(unused_variables)]
+    /// # rocket::async_test(async {
+    /// let file = NamedFile::open("foo.txt").await;
+    /// });
     /// ```
-    pub fn open<P: AsRef<Path>>(path: P) -> io::Result<NamedFile> {
-        let file = File::open(path.as_ref())?;
+    pub async fn open<P: AsRef<Path>>(path: P) -> io::Result<NamedFile> {
+        // FIXME: Grab the file size here and prohibit `seek`ing later (or else
+        // the file's effective size may change), to save on the cost of doing
+        // all of those `seek`s to determine the file size. But, what happens if
+        // the file gets changed between now and then?
+        let file = File::open(path.as_ref()).await?;
         Ok(NamedFile(path.as_ref().to_path_buf(), file))
     }
 
@@ -40,18 +47,18 @@ impl NamedFile {
         &self.1
     }
 
-    /// Take the underlying `File`.
-    #[inline(always)]
-    pub fn take_file(self) -> File {
-        self.1
-    }
-
     /// Retrieve a mutable borrow to the underlying `File`.
     #[inline(always)]
     pub fn file_mut(&mut self) -> &mut File {
         &mut self.1
     }
 
+    /// Take the underlying `File`.
+    #[inline(always)]
+    pub fn take_file(self) -> File {
+        self.1
+    }
+
     /// Retrieve the path of this file.
     ///
     /// # Examples
@@ -61,8 +68,8 @@ impl NamedFile {
     /// use rocket::response::NamedFile;
     ///
     /// # #[allow(dead_code)]
-    /// # fn demo_path() -> io::Result<()> {
-    /// let file = NamedFile::open("foo.txt")?;
+    /// # async fn demo_path() -> io::Result<()> {
+    /// let file = NamedFile::open("foo.txt").await?;
     /// assert_eq!(file.path().as_os_str(), "foo.txt");
     /// # Ok(())
     /// # }
@@ -104,55 +111,3 @@ impl DerefMut for NamedFile {
         &mut self.1
     }
 }
-
-impl io::Read for NamedFile {
-    fn read(&mut self, buf: &mut [u8]) -> io::Result<usize> {
-        self.file().read(buf)
-    }
-
-    fn read_to_end(&mut self, buf: &mut Vec<u8>) -> io::Result<usize> {
-        self.file().read_to_end(buf)
-    }
-}
-
-impl io::Write for NamedFile {
-    fn write(&mut self, buf: &[u8]) -> io::Result<usize> {
-        self.file().write(buf)
-    }
-
-    fn flush(&mut self) -> io::Result<()> {
-        self.file().flush()
-    }
-}
-
-impl io::Seek for NamedFile {
-    fn seek(&mut self, pos: io::SeekFrom) -> io::Result<u64> {
-        self.file().seek(pos)
-    }
-}
-
-impl io::Read for &NamedFile {
-    fn read(&mut self, buf: &mut [u8]) -> io::Result<usize> {
-        self.file().read(buf)
-    }
-
-    fn read_to_end(&mut self, buf: &mut Vec<u8>) -> io::Result<usize> {
-        self.file().read_to_end(buf)
-    }
-}
-
-impl io::Write for &NamedFile {
-    fn write(&mut self, buf: &[u8]) -> io::Result<usize> {
-        self.file().write(buf)
-    }
-
-    fn flush(&mut self) -> io::Result<()> {
-        self.file().flush()
-    }
-}
-
-impl io::Seek for &NamedFile {
-    fn seek(&mut self, pos: io::SeekFrom) -> io::Result<u64> {
-        self.file().seek(pos)
-    }
-}

examples/form_kitchen_sink/Cargo.toml

@@ -7,3 +7,4 @@ publish = false
 
 [dependencies]
 rocket = { path = "../../core/lib" }
+rocket_contrib = { path = "../../contrib/lib" }

examples/form_kitchen_sink/src/main.rs

@@ -3,9 +3,10 @@
 #[macro_use] extern crate rocket;
 
 use rocket::request::{Form, FormError, FormDataError};
-use rocket::response::NamedFile;
 use rocket::http::RawStr;
 
+use rocket_contrib::serve::StaticFiles;
+
 #[cfg(test)] mod tests;
 
 #[derive(Debug, FromFormValue)]
@@ -36,12 +37,7 @@ fn sink(sink: Result<Form<FormInput<'_>>, FormError<'_>>) -> String {
     }
 }
 
-#[get("/")]
-fn index() -> Option<NamedFile> {
-    NamedFile::open("static/index.html").ok()
-}
-
 #[rocket::launch]
 fn rocket() -> rocket::Rocket {
-    rocket::ignite().mount("/", routes![index, sink])
+    rocket::ignite().mount("/", routes![sink]).mount("/", StaticFiles::from("static/"))
 }

examples/form_validation/Cargo.toml

@@ -7,3 +7,4 @@ publish = false
 
 [dependencies]
 rocket = { path = "../../core/lib" }
+rocket_contrib = { path = "../../contrib/lib" }

examples/form_validation/src/files.rs

@@ -1,13 +0,0 @@
-use std::path::{Path, PathBuf};
-
-use rocket::response::NamedFile;
-
-#[get("/")]
-pub fn index() -> Option<NamedFile> {
-    NamedFile::open("static/index.html").ok()
-}
-
-#[get("/<file..>", rank = 2)]
-pub fn files(file: PathBuf) -> Option<NamedFile> {
-    NamedFile::open(Path::new("static/").join(file)).ok()
-}

examples/form_validation/src/main.rs

@@ -2,13 +2,14 @@
 
 #[macro_use] extern crate rocket;
 
-mod files;
 #[cfg(test)] mod tests;
 
 use rocket::response::Redirect;
 use rocket::request::{Form, FromFormValue};
 use rocket::http::RawStr;
 
+use rocket_contrib::serve::StaticFiles;
+
 #[derive(Debug)]
 struct StrongPassword<'r>(&'r str);
 
@@ -79,5 +80,6 @@ fn user_page(username: &RawStr) -> String {
 #[rocket::launch]
 fn rocket() -> rocket::Rocket {
     rocket::ignite()
-        .mount("/", routes![files::index, files::files, user_page, login])
+        .mount("/", routes![user_page, login])
+        .mount("/", StaticFiles::from("static/"))
 }

examples/static_files/src/main.rs

@@ -1,11 +1,23 @@
-extern crate rocket;
-extern crate rocket_contrib;
+#[macro_use] extern crate rocket;
 
 #[cfg(test)] mod tests;
 
 use rocket_contrib::serve::StaticFiles;
 
-#[rocket::launch]
-fn rocket() -> rocket::Rocket {
-    rocket::ignite().mount("/", StaticFiles::from("static"))
+// If we wanted or needed to serve files manually, we'd use `NamedFile`. Always
+// prefer to use `StaticFiles`!
+mod manual {
+    use rocket::response::NamedFile;
+
+    #[rocket::get("/rocket-icon.jpg")]
+    pub async fn icon() -> Option<NamedFile> {
+        NamedFile::open("static/rocket-icon.jpg").await.ok()
+    }
+}
+
+#[launch]
+fn rocket() -> rocket::Rocket {
+    rocket::ignite()
+        .mount("/", routes![manual::icon])
+        .mount("/", StaticFiles::from("static"))
 }