File no longer implements IntoResponse.

The `IntoResponse` implementation of File bypasses the If-None-Match test of its ETag, and is thus a footgun. A test and example demonstrating correct usage has been added.
sammhicks · Jul 22, 2024 · 0db2c14 · 0db2c14
1 parent 8111de0
commit 0db2c14
Show file tree

Hide file tree

Showing 15 changed files with 244 additions and 107 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 - Deprecated and removed `ShutdownMethod`, `Config::shutdown_connection_on_close`, `Config::abort_connection_on_close`, `Socket::abort`
 - `<embassy_net::tcp::TcpSocket as picoserve::io::Socket>::shutdown` now aborts the flush once the socket state is `Closed`
+- [`File`](https://docs.rs/picoserve/latest/picoserve/response/fs/struct.File.html) no longer implements [`IntoResponse`](https://docs.rs/picoserve/latest/picoserve/response/trait.IntoResponse.html)
 
 ### Fixed
 

diff --git a/Cargo.toml b/Cargo.toml
@@ -12,6 +12,7 @@ members = [
     "examples/routing_fallback",
     "examples/server_sent_events",
     "examples/state",
+    "examples/static_content",
     "examples/web_sockets"
 ]
 exclude = [

diff --git a/examples/embassy/set_pico_w_led/src/main.rs b/examples/embassy/set_pico_w_led/src/main.rs
@@ -14,7 +14,7 @@ use embassy_time::Duration;
 use panic_persist as _;
 use picoserve::{
     response::DebugValue,
-    routing::{get, parse_path_segment},
+    routing::{get, get_service, parse_path_segment},
 };
 use rand::Rng;
 use static_cell::make_static;
@@ -152,15 +152,17 @@ async fn main(spawner: embassy_executor::Spawner) {
         picoserve::Router::new()
             .route(
                 "/",
-                get(|| picoserve::response::File::html(include_str!("index.html"))),
+                get_service(picoserve::response::File::html(include_str!("index.html"))),
             )
             .route(
                 "/index.css",
-                get(|| picoserve::response::File::css(include_str!("index.css"))),
+                get_service(picoserve::response::File::css(include_str!("index.css"))),
             )
             .route(
                 "/index.js",
-                get(|| picoserve::response::File::javascript(include_str!("index.js"))),
+                get_service(picoserve::response::File::javascript(include_str!(
+                    "index.js"
+                ))),
             )
             .route(
                 ("/set_led", parse_path_segment()),

diff --git a/examples/embassy/web_sockets/src/main.rs b/examples/embassy/web_sockets/src/main.rs
@@ -12,7 +12,10 @@ use embassy_rp::{
 use embassy_sync::{blocking_mutex::raw::CriticalSectionRawMutex, mutex::Mutex};
 use embassy_time::Duration;
 use panic_persist as _;
-use picoserve::{response::ws, routing::get};
+use picoserve::{
+    response::ws,
+    routing::{get, get_service},
+};
 use rand::Rng;
 use static_cell::make_static;
 
@@ -205,15 +208,17 @@ async fn main(spawner: embassy_executor::Spawner) {
         picoserve::Router::new()
             .route(
                 "/",
-                get(|| picoserve::response::File::html(include_str!("index.html"))),
+                get_service(picoserve::response::File::html(include_str!("index.html"))),
             )
             .route(
                 "/index.css",
-                get(|| picoserve::response::File::css(include_str!("index.css"))),
+                get_service(picoserve::response::File::css(include_str!("index.css"))),
             )
             .route(
                 "/index.js",
-                get(|| picoserve::response::File::javascript(include_str!("index.js"))),
+                get_service(picoserve::response::File::javascript(include_str!(
+                    "index.js"
+                ))),
             )
             .route(
                 "/ws",

diff --git a/examples/form/src/main.rs b/examples/form/src/main.rs
@@ -1,6 +1,6 @@
 use std::time::Duration;
 
-use picoserve::routing::get;
+use picoserve::routing::get_service;
 
 #[derive(serde::Deserialize)]
 struct FormValue {
@@ -14,7 +14,7 @@ async fn main() -> anyhow::Result<()> {
 
     let app = std::rc::Rc::new(picoserve::Router::new().route(
         "/",
-        get(|| picoserve::response::File::html(include_str!("index.html"))).post(
+        get_service(picoserve::response::File::html(include_str!("index.html"))).post(
             |picoserve::extract::Form(FormValue { a, b })| {
                 picoserve::response::DebugValue((("a", a), ("b", b)))
             },

diff --git a/examples/huge_requests/src/main.rs b/examples/huge_requests/src/main.rs
@@ -1,6 +1,6 @@
 use std::time::Duration;
 
-use picoserve::{io::Read, response::IntoResponse, routing::get};
+use picoserve::{io::Read, response::IntoResponse, routing::get_service};
 
 struct MeasureBody;
 
@@ -43,7 +43,7 @@ async fn main() -> anyhow::Result<()> {
     let app = std::rc::Rc::new(
         picoserve::Router::new().route(
             "/",
-            get(|| picoserve::response::File::html(include_str!("index.html")))
+            get_service(picoserve::response::File::html(include_str!("index.html")))
                 .post_service(MeasureBody),
         ),
     );

diff --git a/examples/server_sent_events/src/main.rs b/examples/server_sent_events/src/main.rs
@@ -2,7 +2,7 @@ use std::time::Duration;
 
 use picoserve::{
     response::{self, StatusCode},
-    routing::{get, post},
+    routing::{get, get_service, post},
     ResponseSent,
 };
 
@@ -88,7 +88,7 @@ async fn main() -> anyhow::Result<()> {
             picoserve::Router::new()
                 .route(
                     "/",
-                    get(|| response::File::html(include_str!("index.html"))),
+                    get_service(response::File::html(include_str!("index.html"))),
                 )
                 .route(
                     "/set_message",
@@ -102,35 +102,36 @@ async fn main() -> anyhow::Result<()> {
                     "/events",
                     get(move || response::EventStream(Events(messages_rx.clone()))),
                 )
-                .nest_service("/static", {
-                    const STATIC_FILES: response::Directory = response::Directory {
-                        files: &[],
-                        sub_directories: &[
-                            (
-                                "styles",
-                                response::Directory {
-                                    files: &[(
-                                        "index.css",
-                                        response::File::css(include_str!("index.css")),
-                                    )],
-                                    sub_directories: &[],
-                                },
-                            ),
-                            (
-                                "scripts",
-                                response::Directory {
-                                    files: &[(
-                                        "index.js",
-                                        response::File::css(include_str!("index.js")),
-                                    )],
-                                    sub_directories: &[],
-                                },
-                            ),
-                        ],
-                    };
-
-                    STATIC_FILES
-                }),
+                .nest_service(
+                    "/static",
+                    const {
+                        response::Directory {
+                            files: &[],
+                            sub_directories: &[
+                                (
+                                    "styles",
+                                    response::Directory {
+                                        files: &[(
+                                            "index.css",
+                                            response::File::css(include_str!("index.css")),
+                                        )],
+                                        ..response::Directory::DEFAULT
+                                    },
+                                ),
+                                (
+                                    "scripts",
+                                    response::Directory {
+                                        files: &[(
+                                            "index.js",
+                                            response::File::css(include_str!("index.js")),
+                                        )],
+                                        ..response::Directory::DEFAULT
+                                    },
+                                ),
+                            ],
+                        }
+                    },
+                ),
         );
 
     let config = picoserve::Config::new(picoserve::Timeouts {

diff --git a/examples/static_content/Cargo.toml b/examples/static_content/Cargo.toml
@@ -0,0 +1,13 @@
+[package]
+name = "static_content"
+version = "0.1.0"
+edition = "2021"
+
+# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
+
+[dependencies]
+anyhow = "1.0.72"
+heapless = { version = "0.8.0", features = ["serde"] }
+picoserve = { path = "../..", features = ["tokio"] }
+serde = { version = "1.0.183", features = ["derive"] }
+tokio = { version = "1.31.0", features = ["rt", "io-util", "net", "time", "macros"] }
diff --git a/examples/static_content/src/index.css b/examples/static_content/src/index.css
@@ -0,0 +1,3 @@
+p {
+    color: blue;
+}
diff --git a/examples/static_content/src/index.html b/examples/static_content/src/index.html
@@ -0,0 +1,17 @@
+<!DOCTYPE html>
+<html lang="en">
+
+<head>
+    <meta charset="UTF-8">
+    <meta name="viewport" content="width=device-width, initial-scale=1.0">
+    <title>Picoserve Static Content</title>
+    <link rel="stylesheet" href="/static/index.css">
+</head>
+
+<body>
+    <h1>Picoserve Static Content!</h1>
+
+    <p>This text should be blue</p>
+</body>
+
+</html>
diff --git a/examples/static_content/src/main.rs b/examples/static_content/src/main.rs
@@ -0,0 +1,60 @@
+use std::time::Duration;
+
+use picoserve::{
+    response::{Directory, File},
+    routing::get_service,
+};
+
+#[tokio::main(flavor = "current_thread")]
+async fn main() -> anyhow::Result<()> {
+    let port = 8000;
+
+    let app = std::rc::Rc::new(
+        picoserve::Router::new()
+            .route("/", get_service(File::html(include_str!("index.html"))))
+            .nest_service(
+                "/static",
+                const {
+                    Directory {
+                        files: &[("index.css", File::css(include_str!("index.css")))],
+                        ..Directory::DEFAULT
+                    }
+                },
+            ),
+    );
+
+    let config = picoserve::Config::new(picoserve::Timeouts {
+        start_read_request: Some(Duration::from_secs(5)),
+        read_request: Some(Duration::from_secs(1)),
+        write: Some(Duration::from_secs(1)),
+    })
+    .keep_connection_alive();
+
+    let socket = tokio::net::TcpListener::bind((std::net::Ipv4Addr::LOCALHOST, 8000)).await?;
+
+    println!("http://localhost:{port}/");
+
+    tokio::task::LocalSet::new()
+        .run_until(async {
+            loop {
+                let (stream, remote_address) = socket.accept().await?;
+
+                println!("Connection from {remote_address}");
+
+                let app = app.clone();
+                let config = config.clone();
+
+                tokio::task::spawn_local(async move {
+                    match picoserve::serve(&app, &config, &mut [0; 2048], stream).await {
+                        Ok(handled_requests_count) => {
+                            println!(
+                                "{handled_requests_count} requests handled from {remote_address}"
+                            )
+                        }
+                        Err(err) => println!("{err:?}"),
+                    }
+                });
+            }
+        })
+        .await
+}
diff --git a/examples/web_sockets/src/main.rs b/examples/web_sockets/src/main.rs
@@ -1,6 +1,9 @@
 use std::time::Duration;
 
-use picoserve::{response::ws, routing::get};
+use picoserve::{
+    response::ws,
+    routing::{get, get_service},
+};
 
 struct WebsocketHandler {
     tx: std::rc::Rc<tokio::sync::broadcast::Sender<String>>,
@@ -79,10 +82,11 @@ async fn main() -> anyhow::Result<()> {
         picoserve::Router::new()
             .route(
                 "/",
-                get(|| picoserve::response::File::html(include_str!("index.html"))),
+                get_service(picoserve::response::File::html(include_str!("index.html"))),
             )
-            .nest_service("/static", {
-                const STATIC_FILES: picoserve::response::Directory =
+            .nest_service(
+                "/static",
+                const {
                     picoserve::response::Directory {
                         files: &[
                             (
@@ -94,18 +98,19 @@ async fn main() -> anyhow::Result<()> {
                                 picoserve::response::File::css(include_str!("index.js")),
                             ),
                         ],
-                        sub_directories: &[],
-                    };
-
-                STATIC_FILES
-            })
+                        ..picoserve::response::Directory::DEFAULT
+                    }
+                },
+            )
             .route(
                 "/index.css",
-                get(|| picoserve::response::File::css(include_str!("index.css"))),
+                get_service(picoserve::response::File::css(include_str!("index.css"))),
             )
             .route(
                 "/index.js",
-                get(|| picoserve::response::File::javascript(include_str!("index.js"))),
+                get_service(picoserve::response::File::javascript(include_str!(
+                    "index.js"
+                ))),
             )
             .route(
                 "/ws",