From 06c79458012457b2ef6e106b7fbe283a63683e73 Mon Sep 17 00:00:00 2001 From: Rob Ede Date: Thu, 30 Jun 2022 09:19:16 +0100 Subject: [PATCH 01/21] retain previously set vary headers when using compress (#2798) * retain previously set vary headers when using compress --- actix-http/CHANGES.md | 8 +++++--- actix-http/src/encoding/encoder.rs | 2 +- actix-web/src/middleware/compress.rs | 25 +++++++++++++++++++++++++ 3 files changed, 31 insertions(+), 4 deletions(-) diff --git a/actix-http/CHANGES.md b/actix-http/CHANGES.md index dd6051b85..c9ab687f6 100644 --- a/actix-http/CHANGES.md +++ b/actix-http/CHANGES.md @@ -1,13 +1,15 @@ # Changes ## Unreleased - 2022-xx-xx -### Fixed -- Websocket parser no longer throws endless overflow errors after receiving an oversized frame. [#2790] - ### Changed - Minimum supported Rust version (MSRV) is now 1.57 due to transitive `time` dependency. +### Fixed +- Websocket parser no longer throws endless overflow errors after receiving an oversized frame. [#2790] +- Retain previously set Vary headers when using compression encoder. [#2798] + [#2790]: https://github.com/actix/actix-web/pull/2790 +[#2798]: https://github.com/actix/actix-web/pull/2798 ## 3.1.0 - 2022-06-11 diff --git a/actix-http/src/encoding/encoder.rs b/actix-http/src/encoding/encoder.rs index 0bbb1c106..bbe53e8e8 100644 --- a/actix-http/src/encoding/encoder.rs +++ b/actix-http/src/encoding/encoder.rs @@ -257,7 +257,7 @@ fn update_head(encoding: ContentEncoding, head: &mut ResponseHead) { head.headers_mut() .insert(header::CONTENT_ENCODING, encoding.to_header_value()); head.headers_mut() - .insert(header::VARY, HeaderValue::from_static("accept-encoding")); + .append(header::VARY, HeaderValue::from_static("accept-encoding")); head.no_chunking(false); } diff --git a/actix-web/src/middleware/compress.rs b/actix-web/src/middleware/compress.rs index 4fdd74779..ed4291bed 100644 --- a/actix-web/src/middleware/compress.rs +++ b/actix-web/src/middleware/compress.rs @@ -251,6 +251,8 @@ static SUPPORTED_ENCODINGS: Lazy> = Lazy::new(|| { #[cfg(feature = "compress-gzip")] #[cfg(test)] mod tests { + use std::collections::HashSet; + use super::*; use crate::{middleware::DefaultHeaders, test, web, App}; @@ -305,4 +307,27 @@ mod tests { let bytes = test::read_body(res).await; assert_eq!(gzip_decode(bytes), DATA.as_bytes()); } + + #[actix_rt::test] + async fn retains_previously_set_vary_header() { + let app = test::init_service({ + App::new() + .wrap(Compress::default()) + .default_service(web::to(move || { + HttpResponse::Ok() + .insert_header((header::VARY, "x-test")) + .finish() + })) + }) + .await; + + let req = test::TestRequest::default() + .insert_header((header::ACCEPT_ENCODING, "gzip")) + .to_request(); + let res = test::call_service(&app, req).await; + assert_eq!(res.status(), StatusCode::OK); + let vary_headers = res.headers().get_all(header::VARY).collect::>(); + assert!(vary_headers.contains(&HeaderValue::from_static("x-test"))); + assert!(vary_headers.contains(&HeaderValue::from_static("accept-encoding"))); + } } From c6eba2da9b6ead6112433f7d2aaa1f2d19a22395 Mon Sep 17 00:00:00 2001 From: Rob Ede Date: Fri, 1 Jul 2022 06:16:17 +0100 Subject: [PATCH 02/21] prepare actix-http release 3.2.0 (#2801) --- actix-http/CHANGES.md | 3 +++ actix-http/Cargo.toml | 2 +- actix-http/README.md | 4 ++-- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/actix-http/CHANGES.md b/actix-http/CHANGES.md index c9ab687f6..024a731d8 100644 --- a/actix-http/CHANGES.md +++ b/actix-http/CHANGES.md @@ -1,6 +1,9 @@ # Changes ## Unreleased - 2022-xx-xx + + +## 3.2.0 - 2022-06-30 ### Changed - Minimum supported Rust version (MSRV) is now 1.57 due to transitive `time` dependency. diff --git a/actix-http/Cargo.toml b/actix-http/Cargo.toml index 2a4966884..3fccbfa03 100644 --- a/actix-http/Cargo.toml +++ b/actix-http/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "actix-http" -version = "3.1.0" +version = "3.2.0" authors = [ "Nikolay Kim ", "Rob Ede ", diff --git a/actix-http/README.md b/actix-http/README.md index 211f433e8..3179258af 100644 --- a/actix-http/README.md +++ b/actix-http/README.md @@ -3,11 +3,11 @@ > HTTP primitives for the Actix ecosystem. [![crates.io](https://img.shields.io/crates/v/actix-http?label=latest)](https://crates.io/crates/actix-http) -[![Documentation](https://docs.rs/actix-http/badge.svg?version=3.1.0)](https://docs.rs/actix-http/3.1.0) +[![Documentation](https://docs.rs/actix-http/badge.svg?version=3.2.0)](https://docs.rs/actix-http/3.2.0) ![Version](https://img.shields.io/badge/rustc-1.57+-ab6000.svg) ![MIT or Apache 2.0 licensed](https://img.shields.io/crates/l/actix-http.svg)
-[![dependency status](https://deps.rs/crate/actix-http/3.1.0/status.svg)](https://deps.rs/crate/actix-http/3.1.0) +[![dependency status](https://deps.rs/crate/actix-http/3.2.0/status.svg)](https://deps.rs/crate/actix-http/3.2.0) [![Download](https://img.shields.io/crates/d/actix-http.svg)](https://crates.io/crates/actix-http) [![Chat on Discord](https://img.shields.io/discord/771444961383153695?label=chat&logo=discord)](https://discord.gg/NWpN5mmg3x) From 8f9a12ed5d30c1f0a44cbcb5dfb0c3fecd1f6348 Mon Sep 17 00:00:00 2001 From: Rob Ede Date: Fri, 1 Jul 2022 08:23:40 +0100 Subject: [PATCH 03/21] fix parsing ambiguities for HTTP/1.0 requests (#2794) * fix HRS vuln when first CL header is 0 * ignore TE headers in http/1.0 reqs * update changelog * disallow HTTP/1.0 requests without a CL header * fix test * broken fix for http1.0 post requests --- actix-http/CHANGES.md | 4 + actix-http/src/h1/decoder.rs | 156 +++++++++++++++++++++++++++++++---- 2 files changed, 142 insertions(+), 18 deletions(-) diff --git a/actix-http/CHANGES.md b/actix-http/CHANGES.md index 024a731d8..5d441919d 100644 --- a/actix-http/CHANGES.md +++ b/actix-http/CHANGES.md @@ -1,6 +1,10 @@ # Changes ## Unreleased - 2022-xx-xx +### Fixed +- Fix parsing ambiguity in Transfer-Encoding and Content-Length headers for HTTP/1.0 requests. [#2794] + +[#2794]: https://github.com/actix/actix-web/pull/2794 ## 3.2.0 - 2022-06-30 diff --git a/actix-http/src/h1/decoder.rs b/actix-http/src/h1/decoder.rs index a9443997e..91a93d22c 100644 --- a/actix-http/src/h1/decoder.rs +++ b/actix-http/src/h1/decoder.rs @@ -46,6 +46,23 @@ pub(crate) enum PayloadLength { None, } +impl PayloadLength { + /// Returns true if variant is `None`. + fn is_none(&self) -> bool { + matches!(self, Self::None) + } + + /// Returns true if variant is represents zero-length (not none) payload. + fn is_zero(&self) -> bool { + matches!( + self, + PayloadLength::Payload(PayloadType::Payload(PayloadDecoder { + kind: Kind::Length(0) + })) + ) + } +} + pub(crate) trait MessageType: Sized { fn set_connection_type(&mut self, conn_type: Option); @@ -59,6 +76,7 @@ pub(crate) trait MessageType: Sized { &mut self, slice: &Bytes, raw_headers: &[HeaderIndex], + version: Version, ) -> Result { let mut ka = None; let mut has_upgrade_websocket = false; @@ -87,21 +105,23 @@ pub(crate) trait MessageType: Sized { return Err(ParseError::Header); } - header::CONTENT_LENGTH => match value.to_str() { - Ok(s) if s.trim().starts_with('+') => { - debug!("illegal Content-Length: {:?}", s); + header::CONTENT_LENGTH => match value.to_str().map(str::trim) { + Ok(val) if val.starts_with('+') => { + debug!("illegal Content-Length: {:?}", val); return Err(ParseError::Header); } - Ok(s) => { - if let Ok(len) = s.parse::() { - if len != 0 { - content_length = Some(len); - } + + Ok(val) => { + if let Ok(len) = val.parse::() { + // accept 0 lengths here and remove them in `decode` after all + // headers have been processed to prevent request smuggling issues + content_length = Some(len); } else { - debug!("illegal Content-Length: {:?}", s); + debug!("illegal Content-Length: {:?}", val); return Err(ParseError::Header); } } + Err(_) => { debug!("illegal Content-Length: {:?}", value); return Err(ParseError::Header); @@ -114,22 +134,23 @@ pub(crate) trait MessageType: Sized { return Err(ParseError::Header); } - header::TRANSFER_ENCODING => { + header::TRANSFER_ENCODING if version == Version::HTTP_11 => { seen_te = true; - if let Ok(s) = value.to_str().map(str::trim) { - if s.eq_ignore_ascii_case("chunked") { + if let Ok(val) = value.to_str().map(str::trim) { + if val.eq_ignore_ascii_case("chunked") { chunked = true; - } else if s.eq_ignore_ascii_case("identity") { + } else if val.eq_ignore_ascii_case("identity") { // allow silently since multiple TE headers are already checked } else { - debug!("illegal Transfer-Encoding: {:?}", s); + debug!("illegal Transfer-Encoding: {:?}", val); return Err(ParseError::Header); } } else { return Err(ParseError::Header); } } + // connection keep-alive state header::CONNECTION => { ka = if let Ok(conn) = value.to_str().map(str::trim) { @@ -146,6 +167,7 @@ pub(crate) trait MessageType: Sized { None }; } + header::UPGRADE => { if let Ok(val) = value.to_str().map(str::trim) { if val.eq_ignore_ascii_case("websocket") { @@ -153,19 +175,23 @@ pub(crate) trait MessageType: Sized { } } } + header::EXPECT => { let bytes = value.as_bytes(); if bytes.len() >= 4 && &bytes[0..4] == b"100-" { expect = true; } } + _ => {} } headers.append(name, value); } } + self.set_connection_type(ka); + if expect { self.set_expect() } @@ -249,7 +275,22 @@ impl MessageType for Request { let mut msg = Request::new(); // convert headers - let length = msg.set_headers(&src.split_to(len).freeze(), &headers[..h_len])?; + let mut length = + msg.set_headers(&src.split_to(len).freeze(), &headers[..h_len], ver)?; + + // disallow HTTP/1.0 POST requests that do not contain a Content-Length headers + // see https://datatracker.ietf.org/doc/html/rfc1945#section-7.2.2 + if ver == Version::HTTP_10 && method == Method::POST && length.is_none() { + debug!("no Content-Length specified for HTTP/1.0 POST request"); + return Err(ParseError::Header); + } + + // Remove CL value if 0 now that all headers and HTTP/1.0 special cases are processed. + // Protects against some request smuggling attacks. + // See https://github.com/actix/actix-web/issues/2767. + if length.is_zero() { + length = PayloadLength::None; + } // payload decoder let decoder = match length { @@ -337,7 +378,15 @@ impl MessageType for ResponseHead { msg.version = ver; // convert headers - let length = msg.set_headers(&src.split_to(len).freeze(), &headers[..h_len])?; + let mut length = + msg.set_headers(&src.split_to(len).freeze(), &headers[..h_len], ver)?; + + // Remove CL value if 0 now that all headers and HTTP/1.0 special cases are processed. + // Protects against some request smuggling attacks. + // See https://github.com/actix/actix-web/issues/2767. + if length.is_zero() { + length = PayloadLength::None; + } // message payload let decoder = if let PayloadLength::Payload(pl) = length { @@ -606,14 +655,40 @@ mod tests { } #[test] - fn test_parse_post() { - let mut buf = BytesMut::from("POST /test2 HTTP/1.0\r\n\r\n"); + fn parse_h10_post() { + let mut buf = BytesMut::from( + "POST /test1 HTTP/1.0\r\n\ + Content-Length: 3\r\n\ + \r\n\ + abc", + ); + + let mut reader = MessageDecoder::::default(); + let (req, _) = reader.decode(&mut buf).unwrap().unwrap(); + assert_eq!(req.version(), Version::HTTP_10); + assert_eq!(*req.method(), Method::POST); + assert_eq!(req.path(), "/test1"); + + let mut buf = BytesMut::from( + "POST /test2 HTTP/1.0\r\n\ + Content-Length: 0\r\n\ + \r\n", + ); let mut reader = MessageDecoder::::default(); let (req, _) = reader.decode(&mut buf).unwrap().unwrap(); assert_eq!(req.version(), Version::HTTP_10); assert_eq!(*req.method(), Method::POST); assert_eq!(req.path(), "/test2"); + + let mut buf = BytesMut::from( + "POST /test3 HTTP/1.0\r\n\ + \r\n", + ); + + let mut reader = MessageDecoder::::default(); + let err = reader.decode(&mut buf).unwrap_err(); + assert!(err.to_string().contains("Header")) } #[test] @@ -981,6 +1056,17 @@ mod tests { ); expect_parse_err!(&mut buf); + + let mut buf = BytesMut::from( + "GET / HTTP/1.1\r\n\ + Host: example.com\r\n\ + Content-Length: 0\r\n\ + Content-Length: 2\r\n\ + \r\n\ + ab", + ); + + expect_parse_err!(&mut buf); } #[test] @@ -996,6 +1082,40 @@ mod tests { expect_parse_err!(&mut buf); } + #[test] + fn hrs_te_http10() { + // in HTTP/1.0 transfer encoding is ignored and must therefore contain a CL header + + let mut buf = BytesMut::from( + "POST / HTTP/1.0\r\n\ + Host: example.com\r\n\ + Transfer-Encoding: chunked\r\n\ + \r\n\ + 3\r\n\ + aaa\r\n\ + 0\r\n\ + ", + ); + + expect_parse_err!(&mut buf); + } + + #[test] + fn hrs_cl_and_te_http10() { + // in HTTP/1.0 transfer encoding is simply ignored so it's fine to have both + + let mut buf = BytesMut::from( + "GET / HTTP/1.0\r\n\ + Host: example.com\r\n\ + Content-Length: 3\r\n\ + Transfer-Encoding: chunked\r\n\ + \r\n\ + 000", + ); + + parse_ready!(&mut buf); + } + #[test] fn hrs_unknown_transfer_encoding() { let mut buf = BytesMut::from( From 7e990e423fdf4b6b4b8cf24a7bc412351d8e995b Mon Sep 17 00:00:00 2001 From: Rob Ede Date: Fri, 1 Jul 2022 08:24:45 +0100 Subject: [PATCH 04/21] add http/1.0 GET parsing tests --- actix-http/src/h1/decoder.rs | 40 ++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/actix-http/src/h1/decoder.rs b/actix-http/src/h1/decoder.rs index 91a93d22c..c718f26a8 100644 --- a/actix-http/src/h1/decoder.rs +++ b/actix-http/src/h1/decoder.rs @@ -654,6 +654,46 @@ mod tests { assert_eq!(req.path(), "/test"); } + #[test] + fn parse_h10_get() { + let mut buf = BytesMut::from( + "GET /test1 HTTP/1.0\r\n\ + \r\n\ + abc", + ); + + let mut reader = MessageDecoder::::default(); + let (req, _) = reader.decode(&mut buf).unwrap().unwrap(); + assert_eq!(req.version(), Version::HTTP_10); + assert_eq!(*req.method(), Method::GET); + assert_eq!(req.path(), "/test1"); + + let mut buf = BytesMut::from( + "GET /test2 HTTP/1.0\r\n\ + Content-Length: 0\r\n\ + \r\n", + ); + + let mut reader = MessageDecoder::::default(); + let (req, _) = reader.decode(&mut buf).unwrap().unwrap(); + assert_eq!(req.version(), Version::HTTP_10); + assert_eq!(*req.method(), Method::GET); + assert_eq!(req.path(), "/test2"); + + let mut buf = BytesMut::from( + "GET /test3 HTTP/1.0\r\n\ + Content-Length: 3\r\n\ + \r\n + abc", + ); + + let mut reader = MessageDecoder::::default(); + let (req, _) = reader.decode(&mut buf).unwrap().unwrap(); + assert_eq!(req.version(), Version::HTTP_10); + assert_eq!(*req.method(), Method::GET); + assert_eq!(req.path(), "/test3"); + } + #[test] fn parse_h10_post() { let mut buf = BytesMut::from( From e524fc86eaef37c36b40c25c20b9880893c76508 Mon Sep 17 00:00:00 2001 From: Rob Ede Date: Fri, 1 Jul 2022 09:03:57 +0100 Subject: [PATCH 05/21] add HTTP/0.9 rejection test --- actix-http/src/h1/decoder.rs | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/actix-http/src/h1/decoder.rs b/actix-http/src/h1/decoder.rs index c718f26a8..edfc00fd6 100644 --- a/actix-http/src/h1/decoder.rs +++ b/actix-http/src/h1/decoder.rs @@ -654,12 +654,32 @@ mod tests { assert_eq!(req.path(), "/test"); } + #[test] + fn parse_h09_reject() { + let mut buf = BytesMut::from( + "GET /test1 HTTP/0.9\r\n\ + \r\n", + ); + + let mut reader = MessageDecoder::::default(); + reader.decode(&mut buf).unwrap_err(); + + let mut buf = BytesMut::from( + "POST /test2 HTTP/0.9\r\n\ + Content-Length: 3\r\n\ + \r\n + abc", + ); + + let mut reader = MessageDecoder::::default(); + reader.decode(&mut buf).unwrap_err(); + } + #[test] fn parse_h10_get() { let mut buf = BytesMut::from( "GET /test1 HTTP/1.0\r\n\ - \r\n\ - abc", + \r\n", ); let mut reader = MessageDecoder::::default(); From 226ea696ce43283d259cdea316fcc9efca6e25b9 Mon Sep 17 00:00:00 2001 From: Rob Ede Date: Fri, 1 Jul 2022 10:19:28 +0100 Subject: [PATCH 06/21] update dev deps --- actix-http/Cargo.toml | 4 ++-- actix-web/Cargo.toml | 6 +++--- awc/Cargo.toml | 6 +++--- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/actix-http/Cargo.toml b/actix-http/Cargo.toml index 3fccbfa03..abb1e4603 100644 --- a/actix-http/Cargo.toml +++ b/actix-http/Cargo.toml @@ -108,10 +108,10 @@ env_logger = "0.9" futures-util = { version = "0.3.7", default-features = false, features = ["alloc"] } memchr = "2.4" once_cell = "1.9" -rcgen = "0.8" +rcgen = "0.9" regex = "1.3" rustversion = "1" -rustls-pemfile = "0.2" +rustls-pemfile = "1" serde = { version = "1.0", features = ["derive"] } serde_json = "1.0" static_assertions = "1" diff --git a/actix-web/Cargo.toml b/actix-web/Cargo.toml index 8cdf0f611..c58e1604b 100644 --- a/actix-web/Cargo.toml +++ b/actix-web/Cargo.toml @@ -105,14 +105,14 @@ actix-test = { version = "0.1.0-beta.13", features = ["openssl", "rustls"] } awc = { version = "3", features = ["openssl"] } brotli = "3.3.3" -const-str = "0.3" +const-str = "0.4" criterion = { version = "0.3", features = ["html_reports"] } env_logger = "0.9" flate2 = "1.0.13" futures-util = { version = "0.3.7", default-features = false, features = ["std"] } rand = "0.8" -rcgen = "0.8" -rustls-pemfile = "0.2" +rcgen = "0.9" +rustls-pemfile = "1" serde = { version = "1.0", features = ["derive"] } static_assertions = "1" tls-openssl = { package = "openssl", version = "0.10.9" } diff --git a/awc/Cargo.toml b/awc/Cargo.toml index ba0fc14e3..1a69fd49e 100644 --- a/awc/Cargo.toml +++ b/awc/Cargo.toml @@ -102,13 +102,13 @@ actix-utils = "3" actix-web = { version = "4", features = ["openssl"] } brotli = "3.3.3" -const-str = "0.3" +const-str = "0.4" env_logger = "0.9" flate2 = "1.0.13" futures-util = { version = "0.3.7", default-features = false } static_assertions = "1.1" -rcgen = "0.8" -rustls-pemfile = "0.2" +rcgen = "0.9" +rustls-pemfile = "1" tokio = { version = "1.13.1", features = ["rt-multi-thread", "macros"] } zstd = "0.11" From df5257c3730ccf270dbe0799501826fc2d4a1072 Mon Sep 17 00:00:00 2001 From: Rob Ede Date: Fri, 1 Jul 2022 10:21:46 +0100 Subject: [PATCH 07/21] update trust dns resolver --- awc/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/awc/Cargo.toml b/awc/Cargo.toml index 1a69fd49e..9da103cb0 100644 --- a/awc/Cargo.toml +++ b/awc/Cargo.toml @@ -90,7 +90,7 @@ cookie = { version = "0.16", features = ["percent-encode"], optional = true } tls-openssl = { package = "openssl", version = "0.10.9", optional = true } tls-rustls = { package = "rustls", version = "0.20.0", optional = true, features = ["dangerous_configuration"] } -trust-dns-resolver = { version = "0.20.0", optional = true } +trust-dns-resolver = { version = "0.21", optional = true } [dev-dependencies] actix-http = { version = "3", features = ["openssl"] } From b62f1b4ef74fc28881bdf070e35395065189bd26 Mon Sep 17 00:00:00 2001 From: Rob Ede Date: Fri, 1 Jul 2022 12:40:00 +0100 Subject: [PATCH 08/21] migrate deprecated method in docs --- actix-router/src/resource.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/actix-router/src/resource.rs b/actix-router/src/resource.rs index bc082273c..3c6754aeb 100644 --- a/actix-router/src/resource.rs +++ b/actix-router/src/resource.rs @@ -649,7 +649,7 @@ impl ResourceDef { /// resource.capture_match_info_fn( /// path, /// // when env var is not set, reject when path contains "admin" - /// |res| !(!admin_allowed && res.path().contains("admin")), + /// |path| !(!admin_allowed && path.as_str().contains("admin")), /// ) /// } /// From 987067698b11db82f113ae350356cb11d1d7d1f0 Mon Sep 17 00:00:00 2001 From: Rob Ede Date: Fri, 1 Jul 2022 12:45:26 +0100 Subject: [PATCH 09/21] use sparse registry in CI --- .github/workflows/ci-post-merge.yml | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/.github/workflows/ci-post-merge.yml b/.github/workflows/ci-post-merge.yml index 9fce98f4c..d8752d8aa 100644 --- a/.github/workflows/ci-post-merge.yml +++ b/.github/workflows/ci-post-merge.yml @@ -23,6 +23,7 @@ jobs: CI: 1 CARGO_INCREMENTAL: 0 VCPKGRS_DYNAMIC: 1 + CARGO_UNSTABLE_SPARSE_REGISTRY: true steps: - uses: actions/checkout@v2 @@ -86,6 +87,11 @@ jobs: ci_feature_powerset_check: name: Verify Feature Combinations runs-on: ubuntu-latest + + env: + CI: 1 + CARGO_INCREMENTAL: 0 + steps: - uses: actions/checkout@v2 @@ -119,6 +125,11 @@ jobs: nextest: name: nextest runs-on: ubuntu-latest + + env: + CI: 1 + CARGO_INCREMENTAL: 0 + steps: - uses: actions/checkout@v2 From f3f41a0cc70e43564f8243b3ff425195566b5f16 Mon Sep 17 00:00:00 2001 From: Rob Ede Date: Sat, 2 Jul 2022 16:50:54 +0100 Subject: [PATCH 10/21] prepare actix-http release 3.2.1 --- actix-http/CHANGES.md | 3 +++ actix-http/Cargo.toml | 2 +- actix-http/README.md | 4 ++-- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/actix-http/CHANGES.md b/actix-http/CHANGES.md index 5d441919d..7e6604046 100644 --- a/actix-http/CHANGES.md +++ b/actix-http/CHANGES.md @@ -1,6 +1,9 @@ # Changes ## Unreleased - 2022-xx-xx + + +## 3.2.1 - 2022-07-02 ### Fixed - Fix parsing ambiguity in Transfer-Encoding and Content-Length headers for HTTP/1.0 requests. [#2794] diff --git a/actix-http/Cargo.toml b/actix-http/Cargo.toml index abb1e4603..03767ca4e 100644 --- a/actix-http/Cargo.toml +++ b/actix-http/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "actix-http" -version = "3.2.0" +version = "3.2.1" authors = [ "Nikolay Kim ", "Rob Ede ", diff --git a/actix-http/README.md b/actix-http/README.md index 3179258af..787d2f653 100644 --- a/actix-http/README.md +++ b/actix-http/README.md @@ -3,11 +3,11 @@ > HTTP primitives for the Actix ecosystem. [![crates.io](https://img.shields.io/crates/v/actix-http?label=latest)](https://crates.io/crates/actix-http) -[![Documentation](https://docs.rs/actix-http/badge.svg?version=3.2.0)](https://docs.rs/actix-http/3.2.0) +[![Documentation](https://docs.rs/actix-http/badge.svg?version=3.2.1)](https://docs.rs/actix-http/3.2.1) ![Version](https://img.shields.io/badge/rustc-1.57+-ab6000.svg) ![MIT or Apache 2.0 licensed](https://img.shields.io/crates/l/actix-http.svg)
-[![dependency status](https://deps.rs/crate/actix-http/3.2.0/status.svg)](https://deps.rs/crate/actix-http/3.2.0) +[![dependency status](https://deps.rs/crate/actix-http/3.2.1/status.svg)](https://deps.rs/crate/actix-http/3.2.1) [![Download](https://img.shields.io/crates/d/actix-http.svg)](https://crates.io/crates/actix-http) [![Chat on Discord](https://img.shields.io/discord/771444961383153695?label=chat&logo=discord)](https://discord.gg/NWpN5mmg3x) From 2f79daec1608f079802b86a0ff452f380a62a67e Mon Sep 17 00:00:00 2001 From: Rob Ede Date: Sat, 2 Jul 2022 17:05:48 +0100 Subject: [PATCH 11/21] only run tests on stable --- .github/workflows/ci.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 49ad25ccf..15d98aae7 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -68,6 +68,7 @@ jobs: with: { command: ci-check-default } - name: tests + if: matrix.version == 'stable' # temp? timeout-minutes: 60 run: | cargo test --lib --tests -p=actix-router --all-features From e0845d9ad9540818e2a6eec170fa2028d31bf607 Mon Sep 17 00:00:00 2001 From: Rob Ede Date: Sat, 2 Jul 2022 17:12:24 +0100 Subject: [PATCH 12/21] add msrv workarounds to ci --- .github/workflows/ci.yml | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 15d98aae7..10ba660f3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -47,6 +47,12 @@ jobs: profile: minimal override: true + - name: workaround MSRV issues + if: matrix.version != 'stable' + run: | + cargo add const-str@0.3 --dev -p=actix-web + cargo add const-str@0.3 --dev -p=awc + - name: Generate Cargo.lock uses: actions-rs/cargo@v1 with: { command: generate-lockfile } @@ -68,7 +74,6 @@ jobs: with: { command: ci-check-default } - name: tests - if: matrix.version == 'stable' # temp? timeout-minutes: 60 run: | cargo test --lib --tests -p=actix-router --all-features From f7d629a61ae04cc5495da4b4d5daabd254396215 Mon Sep 17 00:00:00 2001 From: Rob Ede Date: Sat, 2 Jul 2022 17:20:46 +0100 Subject: [PATCH 13/21] fix cargo-add in CI --- .github/workflows/ci.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 10ba660f3..35a829ee7 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -50,6 +50,7 @@ jobs: - name: workaround MSRV issues if: matrix.version != 'stable' run: | + cargo install cargo-add cargo add const-str@0.3 --dev -p=actix-web cargo add const-str@0.3 --dev -p=awc From 23ef51609e1e6797ae1905620421f6ebf758a0d6 Mon Sep 17 00:00:00 2001 From: Rob Ede Date: Sat, 2 Jul 2022 17:29:06 +0100 Subject: [PATCH 14/21] s/cargo-add/cargo-edit --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 35a829ee7..f6fafc67c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -50,7 +50,7 @@ jobs: - name: workaround MSRV issues if: matrix.version != 'stable' run: | - cargo install cargo-add + cargo install cargo-edit cargo add const-str@0.3 --dev -p=actix-web cargo add const-str@0.3 --dev -p=awc From 9a2f8450e0bfa8c15db6fe8447a7b25606eb2c8e Mon Sep 17 00:00:00 2001 From: Rob Ede Date: Sat, 2 Jul 2022 17:40:03 +0100 Subject: [PATCH 15/21] install older cargo-edit --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f6fafc67c..cb103e21a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -50,7 +50,7 @@ jobs: - name: workaround MSRV issues if: matrix.version != 'stable' run: | - cargo install cargo-edit + cargo install cargo-edit --version=0.8.0 cargo add const-str@0.3 --dev -p=actix-web cargo add const-str@0.3 --dev -p=awc From 8e2ae8cd40763cbec6bf78a6c9ae35ae0deb01cc Mon Sep 17 00:00:00 2001 From: Rob Ede Date: Sat, 2 Jul 2022 18:38:08 +0100 Subject: [PATCH 16/21] install nextest faster --- .github/workflows/ci-post-merge.yml | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/.github/workflows/ci-post-merge.yml b/.github/workflows/ci-post-merge.yml index d8752d8aa..008ba3296 100644 --- a/.github/workflows/ci-post-merge.yml +++ b/.github/workflows/ci-post-merge.yml @@ -146,11 +146,8 @@ jobs: - name: Cache Dependencies uses: Swatinem/rust-cache@v1.3.0 - - name: Install cargo-nextest - uses: actions-rs/cargo@v1 - with: - command: install - args: cargo-nextest + - name: Install nextest + uses: taiki-e/install-action@nextest - name: Test with cargo-nextest uses: actions-rs/cargo@v1 From 9b51624b2707cb33abad329bd8c5f7ed9ca6ef20 Mon Sep 17 00:00:00 2001 From: Rob Ede Date: Sat, 2 Jul 2022 18:43:19 +0100 Subject: [PATCH 17/21] update cargo-cache to 0.8.2 --- .github/workflows/ci-post-merge.yml | 2 +- .github/workflows/ci.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci-post-merge.yml b/.github/workflows/ci-post-merge.yml index 008ba3296..430c3b1d8 100644 --- a/.github/workflows/ci-post-merge.yml +++ b/.github/workflows/ci-post-merge.yml @@ -81,7 +81,7 @@ jobs: - name: Clear the cargo caches run: | - cargo install cargo-cache --version 0.6.3 --no-default-features --features ci-autoclean + cargo install cargo-cache --version 0.8.2 --no-default-features --features ci-autoclean cargo-cache ci_feature_powerset_check: diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index cb103e21a..c956c2f0e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -90,7 +90,7 @@ jobs: - name: Clear the cargo caches run: | - cargo install cargo-cache --version 0.6.3 --no-default-features --features ci-autoclean + cargo install cargo-cache --version 0.8.2 --no-default-features --features ci-autoclean cargo-cache io-uring: From 75517cce822f9a78eea507229743fbab338eb740 Mon Sep 17 00:00:00 2001 From: Rob Ede Date: Sat, 2 Jul 2022 20:00:59 +0100 Subject: [PATCH 18/21] install cargo hack in CI faster --- .github/workflows/ci-post-merge.yml | 24 +++++++++--------------- .github/workflows/ci.yml | 9 +++------ 2 files changed, 12 insertions(+), 21 deletions(-) diff --git a/.github/workflows/ci-post-merge.yml b/.github/workflows/ci-post-merge.yml index 430c3b1d8..1ee97b591 100644 --- a/.github/workflows/ci-post-merge.yml +++ b/.github/workflows/ci-post-merge.yml @@ -45,18 +45,15 @@ jobs: profile: minimal override: true + - name: Install cargo-hack + uses: taiki-e/install-action@cargo-hack + - name: Generate Cargo.lock uses: actions-rs/cargo@v1 with: { command: generate-lockfile } - name: Cache Dependencies uses: Swatinem/rust-cache@v1.2.0 - - name: Install cargo-hack - uses: actions-rs/cargo@v1 - with: - command: install - args: cargo-hack - - name: check minimal uses: actions-rs/cargo@v1 with: { command: ci-check-min } @@ -102,18 +99,15 @@ jobs: profile: minimal override: true + - name: Install cargo-hack + uses: taiki-e/install-action@cargo-hack + - name: Generate Cargo.lock uses: actions-rs/cargo@v1 with: { command: generate-lockfile } - name: Cache Dependencies uses: Swatinem/rust-cache@v1.2.0 - - name: Install cargo-hack - uses: actions-rs/cargo@v1 - with: - command: install - args: cargo-hack - - name: check feature combinations uses: actions-rs/cargo@v1 with: { command: ci-check-all-feature-powerset } @@ -140,15 +134,15 @@ jobs: profile: minimal override: true + - name: Install nextest + uses: taiki-e/install-action@nextest + - name: Generate Cargo.lock uses: actions-rs/cargo@v1 with: { command: generate-lockfile } - name: Cache Dependencies uses: Swatinem/rust-cache@v1.3.0 - - name: Install nextest - uses: taiki-e/install-action@nextest - - name: Test with cargo-nextest uses: actions-rs/cargo@v1 with: diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c956c2f0e..2ea920808 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -47,6 +47,9 @@ jobs: profile: minimal override: true + - name: Install cargo-hack + uses: taiki-e/install-action@cargo-hack + - name: workaround MSRV issues if: matrix.version != 'stable' run: | @@ -60,12 +63,6 @@ jobs: - name: Cache Dependencies uses: Swatinem/rust-cache@v1.2.0 - - name: Install cargo-hack - uses: actions-rs/cargo@v1 - with: - command: install - args: cargo-hack - - name: check minimal uses: actions-rs/cargo@v1 with: { command: ci-check-min } From 40eab1f091c03ac00c0f1333b2a7a8e336f9f7e8 Mon Sep 17 00:00:00 2001 From: Rob Ede Date: Sat, 2 Jul 2022 20:07:27 +0100 Subject: [PATCH 19/21] simplify simple decoder tests --- actix-http/src/h1/decoder.rs | 151 +++++++++++++---------------------- 1 file changed, 54 insertions(+), 97 deletions(-) diff --git a/actix-http/src/h1/decoder.rs b/actix-http/src/h1/decoder.rs index edfc00fd6..40cabf9cb 100644 --- a/actix-http/src/h1/decoder.rs +++ b/actix-http/src/h1/decoder.rs @@ -844,121 +844,98 @@ mod tests { #[test] fn test_conn_default_1_0() { - let mut buf = BytesMut::from("GET /test HTTP/1.0\r\n\r\n"); - let req = parse_ready!(&mut buf); - + let req = parse_ready!(&mut BytesMut::from("GET /test HTTP/1.0\r\n\r\n")); assert_eq!(req.head().connection_type(), ConnectionType::Close); } #[test] fn test_conn_default_1_1() { - let mut buf = BytesMut::from("GET /test HTTP/1.1\r\n\r\n"); - let req = parse_ready!(&mut buf); - + let req = parse_ready!(&mut BytesMut::from("GET /test HTTP/1.1\r\n\r\n")); assert_eq!(req.head().connection_type(), ConnectionType::KeepAlive); } #[test] fn test_conn_close() { - let mut buf = BytesMut::from( + let req = parse_ready!(&mut BytesMut::from( "GET /test HTTP/1.1\r\n\ connection: close\r\n\r\n", - ); - let req = parse_ready!(&mut buf); - + )); assert_eq!(req.head().connection_type(), ConnectionType::Close); - let mut buf = BytesMut::from( + let req = parse_ready!(&mut BytesMut::from( "GET /test HTTP/1.1\r\n\ connection: Close\r\n\r\n", - ); - let req = parse_ready!(&mut buf); - + )); assert_eq!(req.head().connection_type(), ConnectionType::Close); } #[test] fn test_conn_close_1_0() { - let mut buf = BytesMut::from( + let req = parse_ready!(&mut BytesMut::from( "GET /test HTTP/1.0\r\n\ connection: close\r\n\r\n", - ); - - let req = parse_ready!(&mut buf); - + )); assert_eq!(req.head().connection_type(), ConnectionType::Close); } #[test] fn test_conn_keep_alive_1_0() { - let mut buf = BytesMut::from( + let req = parse_ready!(&mut BytesMut::from( "GET /test HTTP/1.0\r\n\ connection: keep-alive\r\n\r\n", - ); - let req = parse_ready!(&mut buf); - + )); assert_eq!(req.head().connection_type(), ConnectionType::KeepAlive); - let mut buf = BytesMut::from( + let req = parse_ready!(&mut BytesMut::from( "GET /test HTTP/1.0\r\n\ connection: Keep-Alive\r\n\r\n", - ); - let req = parse_ready!(&mut buf); - + )); assert_eq!(req.head().connection_type(), ConnectionType::KeepAlive); } #[test] fn test_conn_keep_alive_1_1() { - let mut buf = BytesMut::from( + let req = parse_ready!(&mut BytesMut::from( "GET /test HTTP/1.1\r\n\ connection: keep-alive\r\n\r\n", - ); - let req = parse_ready!(&mut buf); - + )); assert_eq!(req.head().connection_type(), ConnectionType::KeepAlive); } #[test] fn test_conn_other_1_0() { - let mut buf = BytesMut::from( + let req = parse_ready!(&mut BytesMut::from( "GET /test HTTP/1.0\r\n\ connection: other\r\n\r\n", - ); - let req = parse_ready!(&mut buf); - + )); assert_eq!(req.head().connection_type(), ConnectionType::Close); } #[test] fn test_conn_other_1_1() { - let mut buf = BytesMut::from( + let req = parse_ready!(&mut BytesMut::from( "GET /test HTTP/1.1\r\n\ connection: other\r\n\r\n", - ); - let req = parse_ready!(&mut buf); - + )); assert_eq!(req.head().connection_type(), ConnectionType::KeepAlive); } #[test] fn test_conn_upgrade() { - let mut buf = BytesMut::from( + let req = parse_ready!(&mut BytesMut::from( "GET /test HTTP/1.1\r\n\ upgrade: websockets\r\n\ connection: upgrade\r\n\r\n", - ); - let req = parse_ready!(&mut buf); + )); assert!(req.upgrade()); assert_eq!(req.head().connection_type(), ConnectionType::Upgrade); - let mut buf = BytesMut::from( + let req = parse_ready!(&mut BytesMut::from( "GET /test HTTP/1.1\r\n\ upgrade: Websockets\r\n\ connection: Upgrade\r\n\r\n", - ); - let req = parse_ready!(&mut buf); + )); assert!(req.upgrade()); assert_eq!(req.head().connection_type(), ConnectionType::Upgrade); @@ -966,59 +943,54 @@ mod tests { #[test] fn test_conn_upgrade_connect_method() { - let mut buf = BytesMut::from( + let req = parse_ready!(&mut BytesMut::from( "CONNECT /test HTTP/1.1\r\n\ content-type: text/plain\r\n\r\n", - ); - let req = parse_ready!(&mut buf); + )); assert!(req.upgrade()); } #[test] - fn test_headers_content_length_err_1() { - let mut buf = BytesMut::from( + fn test_headers_bad_content_length() { + // string CL + expect_parse_err!(&mut BytesMut::from( "GET /test HTTP/1.1\r\n\ content-length: line\r\n\r\n", - ); + )); - expect_parse_err!(&mut buf) - } - - #[test] - fn test_headers_content_length_err_2() { - let mut buf = BytesMut::from( + // negative CL + expect_parse_err!(&mut BytesMut::from( "GET /test HTTP/1.1\r\n\ content-length: -1\r\n\r\n", - ); + )); - expect_parse_err!(&mut buf); + // octal CL + // expect_parse_err!(&mut BytesMut::from( + // "GET /test HTTP/1.1\r\n\ + // content-length: 0123\r\n\r\n", + // )); } #[test] fn test_invalid_header() { - let mut buf = BytesMut::from( + expect_parse_err!(&mut BytesMut::from( "GET /test HTTP/1.1\r\n\ test line\r\n\r\n", - ); - - expect_parse_err!(&mut buf); + )); } #[test] fn test_invalid_name() { - let mut buf = BytesMut::from( + expect_parse_err!(&mut BytesMut::from( "GET /test HTTP/1.1\r\n\ test[]: line\r\n\r\n", - ); - - expect_parse_err!(&mut buf); + )); } #[test] fn test_http_request_bad_status_line() { - let mut buf = BytesMut::from("getpath \r\n\r\n"); - expect_parse_err!(&mut buf); + expect_parse_err!(&mut BytesMut::from("getpath \r\n\r\n")); } #[test] @@ -1058,11 +1030,10 @@ mod tests { #[test] fn test_http_request_parser_utf8() { - let mut buf = BytesMut::from( + let req = parse_ready!(&mut BytesMut::from( "GET /test HTTP/1.1\r\n\ x-test: ั‚ะตัั‚\r\n\r\n", - ); - let req = parse_ready!(&mut buf); + )); assert_eq!( req.headers().get("x-test").unwrap().as_bytes(), @@ -1072,24 +1043,18 @@ mod tests { #[test] fn test_http_request_parser_two_slashes() { - let mut buf = BytesMut::from("GET //path HTTP/1.1\r\n\r\n"); - let req = parse_ready!(&mut buf); - + let req = parse_ready!(&mut BytesMut::from("GET //path HTTP/1.1\r\n\r\n")); assert_eq!(req.path(), "//path"); } #[test] fn test_http_request_parser_bad_method() { - let mut buf = BytesMut::from("!12%()+=~$ /get HTTP/1.1\r\n\r\n"); - - expect_parse_err!(&mut buf); + expect_parse_err!(&mut BytesMut::from("!12%()+=~$ /get HTTP/1.1\r\n\r\n")); } #[test] fn test_http_request_parser_bad_version() { - let mut buf = BytesMut::from("GET //get HT/11\r\n\r\n"); - - expect_parse_err!(&mut buf); + expect_parse_err!(&mut BytesMut::from("GET //get HT/11\r\n\r\n")); } #[test] @@ -1106,47 +1071,41 @@ mod tests { #[test] fn hrs_multiple_content_length() { - let mut buf = BytesMut::from( + expect_parse_err!(&mut BytesMut::from( "GET / HTTP/1.1\r\n\ Host: example.com\r\n\ Content-Length: 4\r\n\ Content-Length: 2\r\n\ \r\n\ abcd", - ); + )); - expect_parse_err!(&mut buf); - - let mut buf = BytesMut::from( + expect_parse_err!(&mut BytesMut::from( "GET / HTTP/1.1\r\n\ Host: example.com\r\n\ Content-Length: 0\r\n\ Content-Length: 2\r\n\ \r\n\ ab", - ); - - expect_parse_err!(&mut buf); + )); } #[test] fn hrs_content_length_plus() { - let mut buf = BytesMut::from( + expect_parse_err!(&mut BytesMut::from( "GET / HTTP/1.1\r\n\ Host: example.com\r\n\ Content-Length: +3\r\n\ \r\n\ 000", - ); - - expect_parse_err!(&mut buf); + )); } #[test] fn hrs_te_http10() { // in HTTP/1.0 transfer encoding is ignored and must therefore contain a CL header - let mut buf = BytesMut::from( + expect_parse_err!(&mut BytesMut::from( "POST / HTTP/1.0\r\n\ Host: example.com\r\n\ Transfer-Encoding: chunked\r\n\ @@ -1155,9 +1114,7 @@ mod tests { aaa\r\n\ 0\r\n\ ", - ); - - expect_parse_err!(&mut buf); + )); } #[test] From c0d5d7bdb54a1983942d5a25056d4c669eb03b51 Mon Sep 17 00:00:00 2001 From: Rob Ede Date: Sat, 2 Jul 2022 21:04:37 +0100 Subject: [PATCH 20/21] add octal-ish CL test --- actix-http/src/h1/decoder.rs | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/actix-http/src/h1/decoder.rs b/actix-http/src/h1/decoder.rs index 40cabf9cb..203b6c531 100644 --- a/actix-http/src/h1/decoder.rs +++ b/actix-http/src/h1/decoder.rs @@ -964,12 +964,20 @@ mod tests { "GET /test HTTP/1.1\r\n\ content-length: -1\r\n\r\n", )); + } - // octal CL - // expect_parse_err!(&mut BytesMut::from( - // "GET /test HTTP/1.1\r\n\ - // content-length: 0123\r\n\r\n", - // )); + #[test] + fn octal_ish_cl_parsed_as_decimal() { + let mut buf = BytesMut::from( + "POST /test HTTP/1.1\r\n\ + content-length: 011\r\n\r\n", + ); + let mut reader = MessageDecoder::::default(); + let (_req, pl) = reader.decode(&mut buf).unwrap().unwrap(); + assert!(matches!( + pl, + PayloadType::Payload(pl) if pl == PayloadDecoder::length(11) + )); } #[test] From 8759d79b03510f0cae2aac1cb459e79802c85fc6 Mon Sep 17 00:00:00 2001 From: Roland Fredenhagen Date: Mon, 4 Jul 2022 06:31:49 +0200 Subject: [PATCH 21/21] `routes` macro allowing multiple paths per handler (#2718) * WIP: basic implementation for `routes` macro * chore: changelog, docs, tests * error on missing methods * Apply suggestions from code review Co-authored-by: Igor Aleksanov * update test stderr expectation * add additional tests * fix stderr output * remove useless ResourceType this is dead code from back when .to and .to_async were different ways to add a service Co-authored-by: Igor Aleksanov Co-authored-by: Rob Ede --- actix-web-codegen/CHANGES.md | 3 + actix-web-codegen/Cargo.toml | 2 +- actix-web-codegen/src/lib.rs | 48 ++- actix-web-codegen/src/route.rs | 274 +++++++++++------- actix-web-codegen/tests/test_macro.rs | 76 ++++- actix-web-codegen/tests/trybuild.rs | 4 + .../trybuild/routes-missing-args-fail.rs | 14 + .../trybuild/routes-missing-args-fail.stderr | 21 ++ .../trybuild/routes-missing-method-fail.rs | 13 + .../routes-missing-method-fail.stderr | 15 + actix-web-codegen/tests/trybuild/routes-ok.rs | 23 ++ actix-web/CHANGES.md | 2 + actix-web/src/lib.rs | 1 + 13 files changed, 384 insertions(+), 112 deletions(-) create mode 100644 actix-web-codegen/tests/trybuild/routes-missing-args-fail.rs create mode 100644 actix-web-codegen/tests/trybuild/routes-missing-args-fail.stderr create mode 100644 actix-web-codegen/tests/trybuild/routes-missing-method-fail.rs create mode 100644 actix-web-codegen/tests/trybuild/routes-missing-method-fail.stderr create mode 100644 actix-web-codegen/tests/trybuild/routes-ok.rs diff --git a/actix-web-codegen/CHANGES.md b/actix-web-codegen/CHANGES.md index a85d6c454..6b525a441 100644 --- a/actix-web-codegen/CHANGES.md +++ b/actix-web-codegen/CHANGES.md @@ -1,8 +1,11 @@ # Changes ## Unreleased - 2022-xx-xx +- Add `#[routes]` macro to support multiple paths for one handler. [#2718] - Minimum supported Rust version (MSRV) is now 1.57 due to transitive `time` dependency. +[#2718]: https://github.com/actix/actix-web/pull/2718 + ## 4.0.1 - 2022-06-11 - Fix support for guard paths in route handler macros. [#2771] diff --git a/actix-web-codegen/Cargo.toml b/actix-web-codegen/Cargo.toml index 52094443b..a2aac7e68 100644 --- a/actix-web-codegen/Cargo.toml +++ b/actix-web-codegen/Cargo.toml @@ -18,7 +18,7 @@ proc-macro = true actix-router = "0.5.0" proc-macro2 = "1" quote = "1" -syn = { version = "1", features = ["full", "parsing"] } +syn = { version = "1", features = ["full", "extra-traits"] } [dev-dependencies] actix-macros = "0.2.3" diff --git a/actix-web-codegen/src/lib.rs b/actix-web-codegen/src/lib.rs index 5ca5616b6..4b6dc43c5 100644 --- a/actix-web-codegen/src/lib.rs +++ b/actix-web-codegen/src/lib.rs @@ -46,9 +46,20 @@ //! ``` //! //! # Multiple Path Handlers -//! There are no macros to generate multi-path handlers. Let us know in [this issue]. +//! Acts as a wrapper for multiple single method handler macros. It takes no arguments and +//! delegates those to the macros for the individual methods. See [macro@routes] macro docs. //! -//! [this issue]: https://github.com/actix/actix-web/issues/1709 +//! ``` +//! # use actix_web::HttpResponse; +//! # use actix_web_codegen::routes; +//! #[routes] +//! #[get("/test")] +//! #[get("/test2")] +//! #[delete("/test")] +//! async fn example() -> HttpResponse { +//! HttpResponse::Ok().finish() +//! } +//! ``` //! //! [actix-web attributes docs]: https://docs.rs/actix-web/latest/actix_web/#attributes //! [GET]: macro@get @@ -104,6 +115,39 @@ pub fn route(args: TokenStream, input: TokenStream) -> TokenStream { route::with_method(None, args, input) } +/// Creates resource handler, allowing multiple HTTP methods and paths. +/// +/// # Syntax +/// ```plain +/// #[routes] +/// #[("path", ...)] +/// #[("path", ...)] +/// ... +/// ``` +/// +/// # Attributes +/// The `routes` macro itself has no parameters, but allows specifying the attribute macros for +/// the multiple paths and/or methods, e.g. [`GET`](macro@get) and [`POST`](macro@post). +/// +/// These helper attributes take the same parameters as the [single method handlers](crate#single-method-handler). +/// +/// # Examples +/// ``` +/// # use actix_web::HttpResponse; +/// # use actix_web_codegen::routes; +/// #[routes] +/// #[get("/test")] +/// #[get("/test2")] +/// #[delete("/test")] +/// async fn example() -> HttpResponse { +/// HttpResponse::Ok().finish() +/// } +/// ``` +#[proc_macro_attribute] +pub fn routes(_: TokenStream, input: TokenStream) -> TokenStream { + route::with_methods(input) +} + macro_rules! method_macro { ($variant:ident, $method:ident) => { #[doc = concat!("Creates route handler with `actix_web::guard::", stringify!($variant), "`.")] diff --git a/actix-web-codegen/src/route.rs b/actix-web-codegen/src/route.rs index cae3cbd55..7a0658468 100644 --- a/actix-web-codegen/src/route.rs +++ b/actix-web-codegen/src/route.rs @@ -3,24 +3,12 @@ use std::{collections::HashSet, convert::TryFrom}; use actix_router::ResourceDef; use proc_macro::TokenStream; use proc_macro2::{Span, TokenStream as TokenStream2}; -use quote::{format_ident, quote, ToTokens, TokenStreamExt}; -use syn::{parse_macro_input, AttributeArgs, Ident, LitStr, NestedMeta, Path}; - -enum ResourceType { - Async, - Sync, -} - -impl ToTokens for ResourceType { - fn to_tokens(&self, stream: &mut TokenStream2) { - let ident = format_ident!("to"); - stream.append(ident); - } -} +use quote::{quote, ToTokens, TokenStreamExt}; +use syn::{parse_macro_input, AttributeArgs, Ident, LitStr, Meta, NestedMeta, Path}; macro_rules! method_type { ( - $($variant:ident, $upper:ident,)+ + $($variant:ident, $upper:ident, $lower:ident,)+ ) => { #[derive(Debug, PartialEq, Eq, Hash)] pub enum MethodType { @@ -42,20 +30,27 @@ macro_rules! method_type { _ => Err(format!("Unexpected HTTP method: `{}`", method)), } } + + fn from_path(method: &Path) -> Result { + match () { + $(_ if method.is_ident(stringify!($lower)) => Ok(Self::$variant),)+ + _ => Err(()), + } + } } }; } method_type! { - Get, GET, - Post, POST, - Put, PUT, - Delete, DELETE, - Head, HEAD, - Connect, CONNECT, - Options, OPTIONS, - Trace, TRACE, - Patch, PATCH, + Get, GET, get, + Post, POST, post, + Put, PUT, put, + Delete, DELETE, delete, + Head, HEAD, head, + Connect, CONNECT, connect, + Options, OPTIONS, options, + Trace, TRACE, trace, + Patch, PATCH, patch, } impl ToTokens for MethodType { @@ -90,6 +85,18 @@ impl Args { let mut wrappers = Vec::new(); let mut methods = HashSet::new(); + if args.is_empty() { + return Err(syn::Error::new( + Span::call_site(), + format!( + r#"invalid service definition, expected #[{}("")]"#, + method + .map_or("route", |it| it.as_str()) + .to_ascii_lowercase() + ), + )); + } + let is_route_macro = method.is_none(); if let Some(method) = method { methods.insert(method); @@ -183,55 +190,27 @@ impl Args { } pub struct Route { + /// Name of the handler function being annotated. name: syn::Ident, - args: Args, + + /// Args passed to routing macro. + /// + /// When using `#[routes]`, this will contain args for each specific routing macro. + args: Vec, + + /// AST of the handler function being annotated. ast: syn::ItemFn, - resource_type: ResourceType, /// The doc comment attributes to copy to generated struct, if any. doc_attributes: Vec, } -fn guess_resource_type(typ: &syn::Type) -> ResourceType { - let mut guess = ResourceType::Sync; - - if let syn::Type::ImplTrait(typ) = typ { - for bound in typ.bounds.iter() { - if let syn::TypeParamBound::Trait(bound) = bound { - for bound in bound.path.segments.iter() { - if bound.ident == "Future" { - guess = ResourceType::Async; - break; - } else if bound.ident == "Responder" { - guess = ResourceType::Sync; - break; - } - } - } - } - } - - guess -} - impl Route { pub fn new( args: AttributeArgs, ast: syn::ItemFn, method: Option, ) -> syn::Result { - if args.is_empty() { - return Err(syn::Error::new( - Span::call_site(), - format!( - r#"invalid service definition, expected #[{}("")]"#, - method - .map_or("route", |it| it.as_str()) - .to_ascii_lowercase() - ), - )); - } - let name = ast.sig.ident.clone(); // Try and pull out the doc comments so that we can reapply them to the generated struct. @@ -244,6 +223,7 @@ impl Route { .collect(); let args = Args::new(args, method)?; + if args.methods.is_empty() { return Err(syn::Error::new( Span::call_site(), @@ -251,25 +231,44 @@ impl Route { )); } - let resource_type = if ast.sig.asyncness.is_some() { - ResourceType::Async - } else { - match ast.sig.output { - syn::ReturnType::Default => { - return Err(syn::Error::new_spanned( - ast, - "Function has no return type. Cannot be used as handler", - )); - } - syn::ReturnType::Type(_, ref typ) => guess_resource_type(typ.as_ref()), - } - }; + if matches!(ast.sig.output, syn::ReturnType::Default) { + return Err(syn::Error::new_spanned( + ast, + "Function has no return type. Cannot be used as handler", + )); + } + + Ok(Self { + name, + args: vec![args], + ast, + doc_attributes, + }) + } + + fn multiple(args: Vec, ast: syn::ItemFn) -> syn::Result { + let name = ast.sig.ident.clone(); + + // Try and pull out the doc comments so that we can reapply them to the generated struct. + // Note that multi line doc comments are converted to multiple doc attributes. + let doc_attributes = ast + .attrs + .iter() + .filter(|attr| attr.path.is_ident("doc")) + .cloned() + .collect(); + + if matches!(ast.sig.output, syn::ReturnType::Default) { + return Err(syn::Error::new_spanned( + ast, + "Function has no return type. Cannot be used as handler", + )); + } Ok(Self { name, args, ast, - resource_type, doc_attributes, }) } @@ -280,38 +279,57 @@ impl ToTokens for Route { let Self { name, ast, - args: - Args { + args, + doc_attributes, + } = self; + + let registrations: TokenStream2 = args + .iter() + .map(|args| { + let Args { path, resource_name, guards, wrappers, methods, - }, - resource_type, - doc_attributes, - } = self; - let resource_name = resource_name - .as_ref() - .map_or_else(|| name.to_string(), LitStr::value); - let method_guards = { - let mut others = methods.iter(); - // unwrapping since length is checked to be at least one - let first = others.next().unwrap(); + } = args; + + let resource_name = resource_name + .as_ref() + .map_or_else(|| name.to_string(), LitStr::value); + + let method_guards = { + let mut others = methods.iter(); + + // unwrapping since length is checked to be at least one + let first = others.next().unwrap(); + + if methods.len() > 1 { + quote! { + .guard( + ::actix_web::guard::Any(::actix_web::guard::#first()) + #(.or(::actix_web::guard::#others()))* + ) + } + } else { + quote! { + .guard(::actix_web::guard::#first()) + } + } + }; - if methods.len() > 1 { quote! { - .guard( - ::actix_web::guard::Any(::actix_web::guard::#first()) - #(.or(::actix_web::guard::#others()))* - ) + let __resource = ::actix_web::Resource::new(#path) + .name(#resource_name) + #method_guards + #(.guard(::actix_web::guard::fn_guard(#guards)))* + #(.wrap(#wrappers))* + .to(#name); + + ::actix_web::dev::HttpServiceFactory::register(__resource, __config); } - } else { - quote! { - .guard(::actix_web::guard::#first()) - } - } - }; + }) + .collect(); let stream = quote! { #(#doc_attributes)* @@ -321,14 +339,7 @@ impl ToTokens for Route { impl ::actix_web::dev::HttpServiceFactory for #name { fn register(self, __config: &mut actix_web::dev::AppService) { #ast - let __resource = ::actix_web::Resource::new(#path) - .name(#resource_name) - #method_guards - #(.guard(::actix_web::guard::fn_guard(#guards)))* - #(.wrap(#wrappers))* - .#resource_type(#name); - - ::actix_web::dev::HttpServiceFactory::register(__resource, __config) + #registrations } } }; @@ -357,6 +368,57 @@ pub(crate) fn with_method( } } +pub(crate) fn with_methods(input: TokenStream) -> TokenStream { + let mut ast = match syn::parse::(input.clone()) { + Ok(ast) => ast, + // on parse error, make IDEs happy; see fn docs + Err(err) => return input_and_compile_error(input, err), + }; + + let (methods, others) = ast + .attrs + .into_iter() + .map(|attr| match MethodType::from_path(&attr.path) { + Ok(method) => Ok((method, attr)), + Err(_) => Err(attr), + }) + .partition::, _>(Result::is_ok); + + ast.attrs = others.into_iter().map(Result::unwrap_err).collect(); + + let methods = + match methods + .into_iter() + .map(Result::unwrap) + .map(|(method, attr)| { + attr.parse_meta().and_then(|args| { + if let Meta::List(args) = args { + Args::new(args.nested.into_iter().collect(), Some(method)) + } else { + Err(syn::Error::new_spanned(attr, "Invalid input for macro")) + } + }) + }) + .collect::, _>>() + { + Ok(methods) if methods.is_empty() => return input_and_compile_error( + input, + syn::Error::new( + Span::call_site(), + "The #[routes] macro requires at least one `#[(..)]` attribute.", + ), + ), + Ok(methods) => methods, + Err(err) => return input_and_compile_error(input, err), + }; + + match Route::multiple(methods, ast) { + Ok(route) => route.into_token_stream().into(), + // on macro related error, make IDEs happy; see fn docs + Err(err) => input_and_compile_error(input, err), + } +} + /// Converts the error to a token stream and appends it to the original input. /// /// Returning the original input in addition to the error is good for IDEs which can gracefully diff --git a/actix-web-codegen/tests/test_macro.rs b/actix-web-codegen/tests/test_macro.rs index 55c2417b2..10e906967 100644 --- a/actix-web-codegen/tests/test_macro.rs +++ b/actix-web-codegen/tests/test_macro.rs @@ -8,9 +8,11 @@ use actix_web::{ header::{HeaderName, HeaderValue}, StatusCode, }, - web, App, Error, HttpResponse, Responder, + web, App, Error, HttpRequest, HttpResponse, Responder, +}; +use actix_web_codegen::{ + connect, delete, get, head, options, patch, post, put, route, routes, trace, }; -use actix_web_codegen::{connect, delete, get, head, options, patch, post, put, route, trace}; use futures_core::future::LocalBoxFuture; // Make sure that we can name function as 'config' @@ -89,8 +91,41 @@ async fn route_test() -> impl Responder { HttpResponse::Ok() } +#[routes] +#[get("/routes/test")] +#[get("/routes/test2")] +#[post("/routes/test")] +async fn routes_test() -> impl Responder { + HttpResponse::Ok() +} + +// routes overlap with the more specific route first, therefore accessible +#[routes] +#[get("/routes/overlap/test")] +#[get("/routes/overlap/{foo}")] +async fn routes_overlapping_test(req: HttpRequest) -> impl Responder { + // foo is only populated when route is not /routes/overlap/test + match req.match_info().get("foo") { + None => assert!(req.uri() == "/routes/overlap/test"), + Some(_) => assert!(req.uri() != "/routes/overlap/test"), + } + + HttpResponse::Ok() +} + +// routes overlap with the more specific route last, therefore inaccessible +#[routes] +#[get("/routes/overlap2/{foo}")] +#[get("/routes/overlap2/test")] +async fn routes_overlapping_inaccessible_test(req: HttpRequest) -> impl Responder { + // foo is always populated even when path is /routes/overlap2/test + assert!(req.match_info().get("foo").is_some()); + + HttpResponse::Ok() +} + #[get("/custom_resource_name", name = "custom")] -async fn custom_resource_name_test<'a>(req: actix_web::HttpRequest) -> impl Responder { +async fn custom_resource_name_test<'a>(req: HttpRequest) -> impl Responder { assert!(req.url_for_static("custom").is_ok()); assert!(req.url_for_static("custom_resource_name_test").is_err()); HttpResponse::Ok() @@ -201,6 +236,9 @@ async fn test_body() { .service(patch_test) .service(test_handler) .service(route_test) + .service(routes_overlapping_test) + .service(routes_overlapping_inaccessible_test) + .service(routes_test) .service(custom_resource_name_test) .service(guard_test) }); @@ -258,6 +296,38 @@ async fn test_body() { let response = request.send().await.unwrap(); assert!(!response.status().is_success()); + let request = srv.request(http::Method::GET, srv.url("/routes/test")); + let response = request.send().await.unwrap(); + assert!(response.status().is_success()); + + let request = srv.request(http::Method::GET, srv.url("/routes/test2")); + let response = request.send().await.unwrap(); + assert!(response.status().is_success()); + + let request = srv.request(http::Method::POST, srv.url("/routes/test")); + let response = request.send().await.unwrap(); + assert!(response.status().is_success()); + + let request = srv.request(http::Method::GET, srv.url("/routes/not-set")); + let response = request.send().await.unwrap(); + assert!(response.status().is_client_error()); + + let request = srv.request(http::Method::GET, srv.url("/routes/overlap/test")); + let response = request.send().await.unwrap(); + assert!(response.status().is_success()); + + let request = srv.request(http::Method::GET, srv.url("/routes/overlap/bar")); + let response = request.send().await.unwrap(); + assert!(response.status().is_success()); + + let request = srv.request(http::Method::GET, srv.url("/routes/overlap2/test")); + let response = request.send().await.unwrap(); + assert!(response.status().is_success()); + + let request = srv.request(http::Method::GET, srv.url("/routes/overlap2/bar")); + let response = request.send().await.unwrap(); + assert!(response.status().is_success()); + let request = srv.request(http::Method::GET, srv.url("/custom_resource_name")); let response = request.send().await.unwrap(); assert!(response.status().is_success()); diff --git a/actix-web-codegen/tests/trybuild.rs b/actix-web-codegen/tests/trybuild.rs index 976b3d52c..1f7996fd0 100644 --- a/actix-web-codegen/tests/trybuild.rs +++ b/actix-web-codegen/tests/trybuild.rs @@ -12,6 +12,10 @@ fn compile_macros() { t.compile_fail("tests/trybuild/route-unexpected-method-fail.rs"); t.compile_fail("tests/trybuild/route-malformed-path-fail.rs"); + t.pass("tests/trybuild/routes-ok.rs"); + t.compile_fail("tests/trybuild/routes-missing-method-fail.rs"); + t.compile_fail("tests/trybuild/routes-missing-args-fail.rs"); + t.pass("tests/trybuild/docstring-ok.rs"); t.pass("tests/trybuild/test-runtime.rs"); diff --git a/actix-web-codegen/tests/trybuild/routes-missing-args-fail.rs b/actix-web-codegen/tests/trybuild/routes-missing-args-fail.rs new file mode 100644 index 000000000..65573cf79 --- /dev/null +++ b/actix-web-codegen/tests/trybuild/routes-missing-args-fail.rs @@ -0,0 +1,14 @@ +use actix_web_codegen::*; + +#[routes] +#[get] +async fn index() -> String { + "Hello World!".to_owned() +} + +#[actix_web::main] +async fn main() { + use actix_web::App; + + let srv = actix_test::start(|| App::new().service(index)); +} diff --git a/actix-web-codegen/tests/trybuild/routes-missing-args-fail.stderr b/actix-web-codegen/tests/trybuild/routes-missing-args-fail.stderr new file mode 100644 index 000000000..8efe0682b --- /dev/null +++ b/actix-web-codegen/tests/trybuild/routes-missing-args-fail.stderr @@ -0,0 +1,21 @@ +error: invalid service definition, expected #[get("")] + --> tests/trybuild/routes-missing-args-fail.rs:4:1 + | +4 | #[get] + | ^^^^^^ + | + = note: this error originates in the attribute macro `get` (in Nightly builds, run with -Z macro-backtrace for more info) + +error: Invalid input for macro + --> tests/trybuild/routes-missing-args-fail.rs:4:1 + | +4 | #[get] + | ^^^^^^ + +error[E0277]: the trait bound `fn() -> impl std::future::Future {index}: HttpServiceFactory` is not satisfied + --> tests/trybuild/routes-missing-args-fail.rs:13:55 + | +13 | let srv = actix_test::start(|| App::new().service(index)); + | ------- ^^^^^ the trait `HttpServiceFactory` is not implemented for `fn() -> impl std::future::Future {index}` + | | + | required by a bound introduced by this call diff --git a/actix-web-codegen/tests/trybuild/routes-missing-method-fail.rs b/actix-web-codegen/tests/trybuild/routes-missing-method-fail.rs new file mode 100644 index 000000000..f0271ef44 --- /dev/null +++ b/actix-web-codegen/tests/trybuild/routes-missing-method-fail.rs @@ -0,0 +1,13 @@ +use actix_web_codegen::*; + +#[routes] +async fn index() -> String { + "Hello World!".to_owned() +} + +#[actix_web::main] +async fn main() { + use actix_web::App; + + let srv = actix_test::start(|| App::new().service(index)); +} diff --git a/actix-web-codegen/tests/trybuild/routes-missing-method-fail.stderr b/actix-web-codegen/tests/trybuild/routes-missing-method-fail.stderr new file mode 100644 index 000000000..b3795d74a --- /dev/null +++ b/actix-web-codegen/tests/trybuild/routes-missing-method-fail.stderr @@ -0,0 +1,15 @@ +error: The #[routes] macro requires at least one `#[(..)]` attribute. + --> tests/trybuild/routes-missing-method-fail.rs:3:1 + | +3 | #[routes] + | ^^^^^^^^^ + | + = note: this error originates in the attribute macro `routes` (in Nightly builds, run with -Z macro-backtrace for more info) + +error[E0277]: the trait bound `fn() -> impl std::future::Future {index}: HttpServiceFactory` is not satisfied + --> tests/trybuild/routes-missing-method-fail.rs:12:55 + | +12 | let srv = actix_test::start(|| App::new().service(index)); + | ------- ^^^^^ the trait `HttpServiceFactory` is not implemented for `fn() -> impl std::future::Future {index}` + | | + | required by a bound introduced by this call diff --git a/actix-web-codegen/tests/trybuild/routes-ok.rs b/actix-web-codegen/tests/trybuild/routes-ok.rs new file mode 100644 index 000000000..98b5e553e --- /dev/null +++ b/actix-web-codegen/tests/trybuild/routes-ok.rs @@ -0,0 +1,23 @@ +use actix_web_codegen::*; + +#[routes] +#[get("/")] +#[post("/")] +async fn index() -> String { + "Hello World!".to_owned() +} + +#[actix_web::main] +async fn main() { + use actix_web::App; + + let srv = actix_test::start(|| App::new().service(index)); + + let request = srv.get("/"); + let response = request.send().await.unwrap(); + assert!(response.status().is_success()); + + let request = srv.post("/"); + let response = request.send().await.unwrap(); + assert!(response.status().is_success()); +} diff --git a/actix-web/CHANGES.md b/actix-web/CHANGES.md index 0144cb912..f38282b41 100644 --- a/actix-web/CHANGES.md +++ b/actix-web/CHANGES.md @@ -2,12 +2,14 @@ ## Unreleased - 2022-xx-xx ### Added +- Add `#[routes]` macro to support multiple paths for one handler. [#2718] - Add `ServiceRequest::{parts, request}()` getter methods. [#2786] - Add configuration options for TLS handshake timeout via `HttpServer::{rustls, openssl}_with_config` methods. [#2752] ### Changed - Minimum supported Rust version (MSRV) is now 1.57 due to transitive `time` dependency. +[#2718]: https://github.com/actix/actix-web/pull/2718 [#2752]: https://github.com/actix/actix-web/pull/2752 [#2786]: https://github.com/actix/actix-web/pull/2786 diff --git a/actix-web/src/lib.rs b/actix-web/src/lib.rs index 4eab24cec..8d9e2dbcd 100644 --- a/actix-web/src/lib.rs +++ b/actix-web/src/lib.rs @@ -132,6 +132,7 @@ macro_rules! codegen_reexport { codegen_reexport!(main); codegen_reexport!(test); codegen_reexport!(route); +codegen_reexport!(routes); codegen_reexport!(head); codegen_reexport!(get); codegen_reexport!(post);