Merge 98e0cc4049 into 5041cd1c65

* Make 'ws' feature of actix-http optional

* Update CHANGES.md

* Update actix-web-actors

* Update CHANGES.md

* nits

* nits

---------

Co-authored-by: Rob Ede <robjtede@icloud.com>

actix-http/CHANGES.md

@@ -2,6 +2,8 @@
 
 ## Unreleased
 
+- Properly wake Payload receivers when feeding errors or EOF
+
 ## 3.11.1
 
 - Prevent more hangs after client disconnects.

actix-http/Cargo.toml

@@ -156,7 +156,7 @@ serde_json = "1.0"
 static_assertions = "1"
 tls-openssl = { package = "openssl", version = "0.10.55" }
 tls-rustls_023 = { package = "rustls", version = "0.23" }
-tokio = { version = "1.38.2", features = ["net", "rt", "macros"] }
+tokio = { version = "1.38.2", features = ["net", "rt", "macros", "sync"] }
 
 [lints]
 workspace = true

actix-http/src/h1/payload.rs

@@ -200,11 +200,13 @@ impl Inner {
     #[inline]
     fn set_error(&mut self, err: PayloadError) {
         self.err = Some(err);
+        self.wake();
     }
 
     #[inline]
     fn feed_eof(&mut self) {
         self.eof = true;
+        self.wake();
     }
 
     #[inline]
@@ -253,8 +255,13 @@ impl Inner {
 
 #[cfg(test)]
 mod tests {
+    use std::{task::Poll, time::Duration};
+
+    use actix_rt::time::timeout;
     use actix_utils::future::poll_fn;
+    use futures_util::{FutureExt, StreamExt};
     use static_assertions::{assert_impl_all, assert_not_impl_any};
+    use tokio::sync::oneshot;
 
     use super::*;
 
@@ -263,6 +270,67 @@ mod tests {
 
     assert_impl_all!(Inner: Unpin, Send, Sync);
 
+    const WAKE_TIMEOUT: Duration = Duration::from_secs(2);
+
+    fn prepare_waking_test(
+        mut payload: Payload,
+        expected: Option<Result<(), ()>>,
+    ) -> (oneshot::Receiver<()>, actix_rt::task::JoinHandle<()>) {
+        let (tx, rx) = oneshot::channel();
+
+        let handle = actix_rt::spawn(async move {
+            // Make sure to poll once to set the waker
+            poll_fn(|cx| {
+                assert!(payload.poll_next_unpin(cx).is_pending());
+                Poll::Ready(())
+            })
+            .await;
+            tx.send(()).unwrap();
+
+            // actix-rt is single-threaded, so this won't race with `rx.await`
+            let mut pend_once = false;
+            poll_fn(|_| {
+                if pend_once {
+                    Poll::Ready(())
+                } else {
+                    // Return pending without storing wakers, we already did on the previous
+                    // `poll_fn`, now this task will only continue if the `sender` wakes us
+                    pend_once = true;
+                    Poll::Pending
+                }
+            })
+            .await;
+
+            let got = payload.next().now_or_never().unwrap();
+            match expected {
+                Some(Ok(_)) => assert!(got.unwrap().is_ok()),
+                Some(Err(_)) => assert!(got.unwrap().is_err()),
+                None => assert!(got.is_none()),
+            }
+        });
+        (rx, handle)
+    }
+
+    #[actix_rt::test]
+    async fn wake_on_error() {
+        let (mut sender, payload) = Payload::create(false);
+        let (rx, handle) = prepare_waking_test(payload, Some(Err(())));
+
+        rx.await.unwrap();
+        sender.set_error(PayloadError::Incomplete(None));
+        timeout(WAKE_TIMEOUT, handle).await.unwrap().unwrap();
+    }
+
+    #[actix_rt::test]
+    async fn wake_on_eof() {
+        let (mut sender, payload) = Payload::create(false);
+        let (rx, handle) = prepare_waking_test(payload, None);
+
+        rx.await.unwrap();
+        sender.feed_eof();
+        timeout(WAKE_TIMEOUT, handle).await.unwrap().unwrap();
+    }
+
     #[actix_rt::test]
     async fn test_unread_data() {
         let (_, mut payload) = Payload::create(false);

actix-http/src/header/into_value.rs

@@ -1,7 +1,7 @@
 //! [`TryIntoHeaderValue`] trait and implementations.
 
 use bytes::Bytes;
-use http::{header::InvalidHeaderValue, Error as HttpError, HeaderValue};
+use http::{header::InvalidHeaderValue, Error as HttpError, HeaderValue, Uri};
 use mime::Mime;
 
 /// An interface for types that can be converted into a [`HeaderValue`].
@@ -129,3 +129,12 @@ impl TryIntoHeaderValue for Mime {
         HeaderValue::from_str(self.as_ref())
     }
 }
+
+impl TryIntoHeaderValue for Uri {
+    type Error = InvalidHeaderValue;
+
+    #[inline]
+    fn try_into_value(self) -> Result<HeaderValue, Self::Error> {
+        HeaderValue::from_str(&self.to_string())
+    }
+}

actix-web-actors/Cargo.toml

@@ -24,7 +24,7 @@ allowed_external_types = [
 actix = { version = ">=0.12, <0.14", default-features = false }
 actix-codec = "0.5"
 actix-http = "3"
-actix-web = { version = "4", default-features = false }
+actix-web = { version = "4", default-features = false, features = ["ws"] }
 
 bytes = "1"
 bytestring = "1"

actix-web/CHANGES.md

@@ -4,6 +4,7 @@
 
 - `actix_web::response::builder::HttpResponseBuilder::streaming()` now sets `Content-Type` to `application/octet-stream` if `Content-Type` does not exist.
 - `actix_web::response::builder::HttpResponseBuilder::streaming()` now calls `actix_web::response::builder::HttpResponseBuilder::no_chunking()` if `Content-Length` is set by user.
+- Add `ws` crate feature (on-by-default) which forwards to `actix-http` and guards some of its `ResponseError` impls.
 
 ## 4.11.0
 
@@ -35,6 +36,10 @@
 - On Windows, an error is now returned from `HttpServer::bind()` (or TLS variants) when binding to a socket that's already in use.
 - Update `brotli` dependency to `7`.
 - Minimum supported Rust version (MSRV) is now 1.75.
+- Add `TryIntoHeaderValue` for `Uri` type.
+- Add `http::header::ContentLocation` typed header.
+- Add `http::header::Location` typed header.
+- Add `http::header::Referer` typed header.
 
 ## 4.9.0
 

actix-web/Cargo.toml

@@ -67,6 +67,7 @@ default = [
   "http2",
   "unicode",
   "compat",
+  "ws",
 ]
 
 # Brotli algorithm content-encoding support
@@ -85,9 +86,12 @@ cookies = ["dep:cookie"]
 # Secure & signed cookies
 secure-cookies = ["cookies", "cookie/secure"]
 
-# HTTP/2 support (including h2c).
+# HTTP/2 support (including h2c)
 http2 = ["actix-http/http2"]
 
+# WebSocket support
+ws = ["actix-http/ws"]
+
 # TLS via OpenSSL
 openssl = ["__tls", "http2", "actix-http/openssl", "actix-tls/accept", "actix-tls/openssl"]
 
@@ -131,7 +135,7 @@ actix-service = "2"
 actix-tls = { version = "3.4", default-features = false, optional = true }
 actix-utils = "3"
 
-actix-http = { version = "3.11", features = ["ws"] }
+actix-http = "3.11"
 actix-router = { version = "0.5.3", default-features = false, features = ["http"] }
 actix-web-codegen = { version = "4.3", optional = true, default-features = false }
 

actix-web/src/error/response_error.rs

@@ -7,7 +7,6 @@ use std::{
     io::{self, Write as _},
 };
 
-use actix_http::Response;
 use bytes::BytesMut;
 
 use crate::{
@@ -126,20 +125,24 @@ impl ResponseError for actix_http::error::PayloadError {
     }
 }
 
-impl ResponseError for actix_http::ws::ProtocolError {}
-
 impl ResponseError for actix_http::error::ContentTypeError {
     fn status_code(&self) -> StatusCode {
         StatusCode::BAD_REQUEST
     }
 }
 
+#[cfg(feature = "ws")]
 impl ResponseError for actix_http::ws::HandshakeError {
     fn error_response(&self) -> HttpResponse<BoxBody> {
-        Response::from(self).map_into_boxed_body().into()
+        actix_http::Response::from(self)
+            .map_into_boxed_body()
+            .into()
     }
 }
 
+#[cfg(feature = "ws")]
+impl ResponseError for actix_http::ws::ProtocolError {}
+
 #[cfg(test)]
 mod tests {
     use super::*;

actix-web/src/http/header/content_location.rs

@@ -0,0 +1,36 @@
+use super::{Uri, CONTENT_LOCATION};
+
+crate::http::header::common_header! {
+    /// `Content-Location` header, defined
+    /// in [RFC 9110 §8.7](https://datatracker.ietf.org/doc/html/rfc9110#section-8.7)
+    ///
+    /// The "Content-Location" header field references a URI that can be used
+    /// as an identifier for a specific resource corresponding to the
+    /// representation in this message's content.
+    ///
+    /// # ABNF
+    /// ```plain
+    /// Content-Location = absolute-URI / partial-URI
+    /// ```
+    ///
+    /// # Example Values
+    /// * `http://www.example.org/hypertext/Overview.html`
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// use actix_web::HttpResponse;
+    /// use actix_http::Uri;
+    /// use actix_web::http::header::ContentLocation;
+    ///
+    /// let mut builder = HttpResponse::Created();
+    /// builder.insert_header(
+    ///     ContentLocation("http://www.example.org".parse::<Uri>().unwrap())
+    /// );
+    /// ```
+    (ContentLocation, CONTENT_LOCATION) => [Uri]
+
+    test_parse_and_format {
+        crate::http::header::common_header_test!(test1, [b"http://www.example.org/hypertext/Overview.html"]);
+    }
+}

actix-web/src/http/header/location.rs

@@ -0,0 +1,37 @@
+use super::{Uri, LOCATION};
+
+crate::http::header::common_header! {
+    /// `Location` header, defined
+    /// in [RFC 9110 §10.2.2](https://datatracker.ietf.org/doc/html/rfc9110#section-10.2.2)
+    ///
+    /// The "Location" header field is used in some responses to refer to a
+    /// specific resource in relation to the response. The type of relationship
+    /// is defined by the combination of request method and status code
+    /// semantics.
+    ///
+    /// # ABNF
+    /// ```plain
+    /// Location = URI-reference
+    /// ```
+    ///
+    /// # Example Values
+    /// * `http://www.example.org/hypertext/Overview.html`
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// use actix_web::HttpResponse;
+    /// use actix_http::Uri;
+    /// use actix_web::http::header::Location;
+    ///
+    /// let mut builder = HttpResponse::Ok();
+    /// builder.insert_header(
+    ///     Location("http://www.example.org".parse::<Uri>().unwrap())
+    /// );
+    /// ```
+    (Location, LOCATION) => [Uri]
+
+    test_parse_and_format {
+        crate::http::header::common_header_test!(test1, [b"http://www.example.org/hypertext/Overview.html"]);
+    }
+}

actix-web/src/http/header/mod.rs

@@ -14,6 +14,7 @@ use std::fmt;
 // - the few typed headers from actix-http
 // - header parsing utils
 pub use actix_http::header::*;
+use actix_http::Uri;
 use bytes::{Bytes, BytesMut};
 
 mod accept;
@@ -25,6 +26,7 @@ mod cache_control;
 mod content_disposition;
 mod content_language;
 mod content_length;
+mod content_location;
 mod content_range;
 mod content_type;
 mod date;
@@ -38,9 +40,11 @@ mod if_none_match;
 mod if_range;
 mod if_unmodified_since;
 mod last_modified;
+mod location;
 mod macros;
 mod preference;
 mod range;
+mod referer;
 
 #[cfg(test)]
 pub(crate) use self::macros::common_header_test;
@@ -55,6 +59,7 @@ pub use self::{
     content_disposition::{ContentDisposition, DispositionParam, DispositionType},
     content_language::ContentLanguage,
     content_length::ContentLength,
+    content_location::ContentLocation,
     content_range::{ContentRange, ContentRangeSpec},
     content_type::ContentType,
     date::Date,
@@ -68,8 +73,10 @@ pub use self::{
     if_range::IfRange,
     if_unmodified_since::IfUnmodifiedSince,
     last_modified::LastModified,
+    location::Location,
     preference::Preference,
     range::{ByteRangeSpec, Range},
+    referer::Referer,
 };
 
 /// Format writer ([`fmt::Write`]) for a [`BytesMut`].

actix-web/src/http/header/referer.rs

@@ -0,0 +1,36 @@
+use super::{Uri, REFERER};
+
+crate::http::header::common_header! {
+    /// `Referer` header, defined
+    /// in [RFC 9110 §10.1.3](https://datatracker.ietf.org/doc/html/rfc9110#section-10.1.3)
+    ///
+    /// The "Referer" (sic) header field allows the user agent to specify a URI
+    /// reference for the resource from which the target URI was obtained (i.e.,
+    /// the "referrer", though the field name is misspelled).
+    ///
+    /// # ABNF
+    /// ```plain
+    /// Referer = absolute-URI / partial-URI
+    /// ```
+    ///
+    /// # Example Values
+    /// * `http://www.example.org/hypertext/Overview.html`
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// use actix_web::HttpResponse;
+    /// use actix_http::Uri;
+    /// use actix_web::http::header::Referer;
+    ///
+    /// let mut builder = HttpResponse::Ok();
+    /// builder.insert_header(
+    ///     Referer("http://www.example.org".parse::<Uri>().unwrap())
+    /// );
+    /// ```
+    (Referer, REFERER) => [Uri]
+
+    test_parse_and_format {
+        crate::http::header::common_header_test!(test1, [b"http://www.example.org/hypertext/Overview.html"]);
+    }
+}