diff --git a/.github/workflows/ci-post-merge.yml b/.github/workflows/ci-post-merge.yml index 9fce98f4c..1ee97b591 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 @@ -44,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 } @@ -80,12 +78,17 @@ 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: name: Verify Feature Combinations runs-on: ubuntu-latest + + env: + CI: 1 + CARGO_INCREMENTAL: 0 + steps: - uses: actions/checkout@v2 @@ -96,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 } @@ -119,6 +119,11 @@ jobs: nextest: name: nextest runs-on: ubuntu-latest + + env: + CI: 1 + CARGO_INCREMENTAL: 0 + steps: - uses: actions/checkout@v2 @@ -129,18 +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 cargo-nextest - uses: actions-rs/cargo@v1 - with: - command: install - args: cargo-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 7bb911f79..de1e1fe18 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -16,7 +16,7 @@ jobs: - { name: macOS, os: macos-latest, triple: x86_64-apple-darwin } - { name: Windows, os: windows-2022, triple: x86_64-pc-windows-msvc } version: - - 1.54.0 # MSRV + - 1.59.0 # MSRV - stable name: ${{ matrix.target.name }} / ${{ matrix.version }} @@ -47,18 +47,22 @@ 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: | + 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 + - 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 } @@ -83,7 +87,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: diff --git a/.github/workflows/upload-doc.yml b/.github/workflows/upload-doc.yml index 94a2ddfbe..07f839e34 100644 --- a/.github/workflows/upload-doc.yml +++ b/.github/workflows/upload-doc.yml @@ -28,8 +28,7 @@ jobs: run: echo '' > target/doc/index.html - name: Deploy to GitHub Pages - uses: JamesIves/github-pages-deploy-action@3.7.1 + uses: JamesIves/github-pages-deploy-action@v4.4.0 with: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - BRANCH: gh-pages - FOLDER: target/doc + folder: target/doc + single-commit: true diff --git a/actix-files/CHANGES.md b/actix-files/CHANGES.md index 7e99c2ae1..a71bf14fa 100644 --- a/actix-files/CHANGES.md +++ b/actix-files/CHANGES.md @@ -1,9 +1,21 @@ # Changes -## Unreleased - 2021-xx-xx +## Unreleased - 2022-xx-xx +- Minimum supported Rust version (MSRV) is now 1.59 due to transitive `time` dependency. + + +## 0.6.2 - 2022-07-23 +- Allow partial range responses for video content to start streaming sooner. [#2817] +- Minimum supported Rust version (MSRV) is now 1.57 due to transitive `time` dependency. + +[#2817]: https://github.com/actix/actix-web/pull/2817 + + +## 0.6.1 - 2022-06-11 - Add `NamedFile::{modified, metadata, content_type, content_disposition, encoding}()` getters. [#2021] - Update `tokio-uring` dependency to `0.3`. - Audio files now use `Content-Disposition: inline` instead of `attachment`. [#2645] +- Minimum supported Rust version (MSRV) is now 1.56 due to transitive `hashbrown` dependency. [#2021]: https://github.com/actix/actix-web/pull/2021 [#2645]: https://github.com/actix/actix-web/pull/2645 diff --git a/actix-files/Cargo.toml b/actix-files/Cargo.toml index 8f856c109..33de0e6d9 100644 --- a/actix-files/Cargo.toml +++ b/actix-files/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "actix-files" -version = "0.6.0" +version = "0.6.2" authors = [ "Nikolay Kim ", "fakeshadow <24548779@qq.com>", @@ -27,7 +27,6 @@ actix-service = "2" actix-utils = "3" actix-web = { version = "4", default-features = false } -askama_escape = "0.10" bitflags = "1" bytes = "1" derive_more = "0.99.5" @@ -38,13 +37,15 @@ mime = "0.3" mime_guess = "2.0.1" percent-encoding = "2.1" pin-project-lite = "0.2.7" +v_htmlescape= "0.15" # experimental-io-uring -tokio-uring = { version = "0.2", optional = true, features = ["bytes"] } -actix-server = "2.1" # ensure matching tokio-uring versions +[target.'cfg(target_os = "linux")'.dependencies] +tokio-uring = { version = "0.3", optional = true, features = ["bytes"] } +actix-server = { version = "2.1", optional = true } # ensure matching tokio-uring versions [dev-dependencies] actix-rt = "2.7" -actix-test = "0.1.0-beta.13" -actix-web = "4.0.0" +actix-test = "0.1" +actix-web = "4" tempfile = "3.2" diff --git a/actix-files/README.md b/actix-files/README.md index 3c4d4443c..a5078c8d5 100644 --- a/actix-files/README.md +++ b/actix-files/README.md @@ -3,11 +3,11 @@ > Static file serving for Actix Web [![crates.io](https://img.shields.io/crates/v/actix-files?label=latest)](https://crates.io/crates/actix-files) -[![Documentation](https://docs.rs/actix-files/badge.svg?version=0.6.0)](https://docs.rs/actix-files/0.6.0) -[![Version](https://img.shields.io/badge/rustc-1.54+-ab6000.svg)](https://blog.rust-lang.org/2021/05/06/Rust-1.54.0.html) +[![Documentation](https://docs.rs/actix-files/badge.svg?version=0.6.2)](https://docs.rs/actix-files/0.6.2) +![Version](https://img.shields.io/badge/rustc-1.59+-ab6000.svg) ![License](https://img.shields.io/crates/l/actix-files.svg)
-[![dependency status](https://deps.rs/crate/actix-files/0.6.0/status.svg)](https://deps.rs/crate/actix-files/0.6.0) +[![dependency status](https://deps.rs/crate/actix-files/0.6.2/status.svg)](https://deps.rs/crate/actix-files/0.6.2) [![Download](https://img.shields.io/crates/d/actix-files.svg)](https://crates.io/crates/actix-files) [![Chat on Discord](https://img.shields.io/discord/771444961383153695?label=chat&logo=discord)](https://discord.gg/NWpN5mmg3x) diff --git a/actix-files/src/directory.rs b/actix-files/src/directory.rs index 32dd6365b..3af53a31a 100644 --- a/actix-files/src/directory.rs +++ b/actix-files/src/directory.rs @@ -1,8 +1,8 @@ use std::{fmt::Write, fs::DirEntry, io, path::Path, path::PathBuf}; use actix_web::{dev::ServiceResponse, HttpRequest, HttpResponse}; -use askama_escape::{escape as escape_html_entity, Html}; use percent_encoding::{utf8_percent_encode, CONTROLS}; +use v_htmlescape::escape as escape_html_entity; /// A directory; responds with the generated directory listing. #[derive(Debug)] @@ -59,7 +59,7 @@ macro_rules! encode_file_url { /// ``` macro_rules! encode_file_name { ($entry:ident) => { - escape_html_entity(&$entry.file_name().to_string_lossy(), Html) + escape_html_entity(&$entry.file_name().to_string_lossy()) }; } diff --git a/actix-files/src/error.rs b/actix-files/src/error.rs index 6682529f8..2f3a36cd1 100644 --- a/actix-files/src/error.rs +++ b/actix-files/src/error.rs @@ -2,7 +2,7 @@ use actix_web::{http::StatusCode, ResponseError}; use derive_more::Display; /// Errors which can occur when serving static files. -#[derive(Display, Debug, PartialEq)] +#[derive(Debug, PartialEq, Eq, Display)] pub enum FilesError { /// Path is not a directory #[allow(dead_code)] @@ -22,7 +22,7 @@ impl ResponseError for FilesError { } #[allow(clippy::enum_variant_names)] -#[derive(Display, Debug, PartialEq)] +#[derive(Debug, PartialEq, Eq, Display)] #[non_exhaustive] pub enum UriSegmentError { /// The segment started with the wrapped invalid character. diff --git a/actix-files/src/named.rs b/actix-files/src/named.rs index 5580e6f7e..1213534c2 100644 --- a/actix-files/src/named.rs +++ b/actix-files/src/named.rs @@ -528,11 +528,26 @@ impl NamedFile { length = ranges[0].length; offset = ranges[0].start; - // don't allow compression middleware to modify partial content - res.insert_header(( - header::CONTENT_ENCODING, - HeaderValue::from_static("identity"), - )); + // When a Content-Encoding header is present in a 206 partial content response + // for video content, it prevents browser video players from starting playback + // before loading the whole video and also prevents seeking. + // + // See: https://github.com/actix/actix-web/issues/2815 + // + // The assumption of this fix is that the video player knows to not send an + // Accept-Encoding header for this request and that downstream middleware will + // not attempt compression for requests without it. + // + // TODO: Solve question around what to do if self.encoding is set and partial + // range is requested. Reject request? Ignoring self.encoding seems wrong, too. + // In practice, it should not come up. + if req.headers().contains_key(&header::ACCEPT_ENCODING) { + // don't allow compression middleware to modify partial content + res.insert_header(( + header::CONTENT_ENCODING, + HeaderValue::from_static("identity"), + )); + } res.insert_header(( header::CONTENT_RANGE, diff --git a/actix-files/src/service.rs b/actix-files/src/service.rs index ec09af01c..d94fd5850 100644 --- a/actix-files/src/service.rs +++ b/actix-files/src/service.rs @@ -23,7 +23,7 @@ impl Deref for FilesService { type Target = FilesServiceInner; fn deref(&self) -> &Self::Target { - &*self.0 + &self.0 } } diff --git a/actix-files/tests/encoding.rs b/actix-files/tests/encoding.rs index 080292af5..7aec25ff9 100644 --- a/actix-files/tests/encoding.rs +++ b/actix-files/tests/encoding.rs @@ -1,11 +1,11 @@ -use actix_files::Files; +use actix_files::{Files, NamedFile}; use actix_web::{ http::{ header::{self, HeaderValue}, StatusCode, }, test::{self, TestRequest}, - App, + web, App, }; #[actix_web::test] @@ -36,3 +36,31 @@ async fn test_utf8_file_contents() { Some(&HeaderValue::from_static("text/plain")), ); } + +#[actix_web::test] +async fn partial_range_response_encoding() { + let srv = test::init_service(App::new().default_service(web::to(|| async { + NamedFile::open_async("./tests/test.binary").await.unwrap() + }))) + .await; + + // range request without accept-encoding returns no content-encoding header + let req = TestRequest::with_uri("/") + .append_header((header::RANGE, "bytes=10-20")) + .to_request(); + let res = test::call_service(&srv, req).await; + assert_eq!(res.status(), StatusCode::PARTIAL_CONTENT); + assert!(!res.headers().contains_key(header::CONTENT_ENCODING)); + + // range request with accept-encoding returns a content-encoding header + let req = TestRequest::with_uri("/") + .append_header((header::RANGE, "bytes=10-20")) + .append_header((header::ACCEPT_ENCODING, "identity")) + .to_request(); + let res = test::call_service(&srv, req).await; + assert_eq!(res.status(), StatusCode::PARTIAL_CONTENT); + assert_eq!( + res.headers().get(header::CONTENT_ENCODING).unwrap(), + "identity" + ); +} diff --git a/actix-http-test/CHANGES.md b/actix-http-test/CHANGES.md index 3b98e0972..028fe3ddc 100644 --- a/actix-http-test/CHANGES.md +++ b/actix-http-test/CHANGES.md @@ -1,8 +1,25 @@ # Changes -## Unreleased - 2021-xx-xx +## Unreleased - 2022-xx-xx +- Minimum supported Rust version (MSRV) is now 1.59. +## 3.0.0 - 2022-07-24 +- `TestServer::stop` is now async and will wait for the server and system to shutdown. [#2442] +- Added `TestServer::client_headers` method. [#2097] +- Update `actix-server` dependency to `2`. +- Update `actix-tls` dependency to `3`. +- Update `bytes` to `1.0`. [#1813] +- Minimum supported Rust version (MSRV) is now 1.57. + +[#2442]: https://github.com/actix/actix-web/pull/2442 +[#2097]: https://github.com/actix/actix-web/pull/2097 +[#1813]: https://github.com/actix/actix-web/pull/1813 + + +
+3.0.0 Pre-Releases + ## 3.0.0-beta.13 - 2022-02-16 - No significant changes since `3.0.0-beta.12`. @@ -68,6 +85,7 @@ [#1813]: https://github.com/actix/actix-web/pull/1813 +
## 2.1.0 - 2020-11-25 - Add ability to set address for `TestServer`. [#1645] diff --git a/actix-http-test/Cargo.toml b/actix-http-test/Cargo.toml index 6f7563ffa..0a9ddf947 100644 --- a/actix-http-test/Cargo.toml +++ b/actix-http-test/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "actix-http-test" -version = "3.0.0-beta.13" +version = "3.0.0" authors = ["Nikolay Kim "] description = "Various helpers for Actix applications to use during testing" keywords = ["http", "web", "framework", "async", "futures"] diff --git a/actix-http-test/README.md b/actix-http-test/README.md index d11ae69b2..25e7c684e 100644 --- a/actix-http-test/README.md +++ b/actix-http-test/README.md @@ -3,11 +3,11 @@ > Various helpers for Actix applications to use during testing. [![crates.io](https://img.shields.io/crates/v/actix-http-test?label=latest)](https://crates.io/crates/actix-http-test) -[![Documentation](https://docs.rs/actix-http-test/badge.svg?version=3.0.0-beta.13)](https://docs.rs/actix-http-test/3.0.0-beta.13) -[![Version](https://img.shields.io/badge/rustc-1.54+-ab6000.svg)](https://blog.rust-lang.org/2021/05/06/Rust-1.54.0.html) +[![Documentation](https://docs.rs/actix-http-test/badge.svg?version=3.0.0)](https://docs.rs/actix-http-test/3.0.0) +![Version](https://img.shields.io/badge/rustc-1.59+-ab6000.svg) ![MIT or Apache 2.0 licensed](https://img.shields.io/crates/l/actix-http-test)
-[![Dependency Status](https://deps.rs/crate/actix-http-test/3.0.0-beta.13/status.svg)](https://deps.rs/crate/actix-http-test/3.0.0-beta.13) +[![Dependency Status](https://deps.rs/crate/actix-http-test/3.0.0/status.svg)](https://deps.rs/crate/actix-http-test/3.0.0) [![Download](https://img.shields.io/crates/d/actix-http-test.svg)](https://crates.io/crates/actix-http-test) [![Chat on Discord](https://img.shields.io/discord/771444961383153695?label=chat&logo=discord)](https://discord.gg/NWpN5mmg3x) diff --git a/actix-http/CHANGES.md b/actix-http/CHANGES.md index 71132c6b2..045ae461f 100644 --- a/actix-http/CHANGES.md +++ b/actix-http/CHANGES.md @@ -1,6 +1,56 @@ # Changes -## Unreleased - 2021-xx-xx +## Unreleased - 2022-xx-xx +### Added +- Implement `MessageBody` for `&mut B` where `B: MessageBody + Unpin`. [#2868] +- Implement `MessageBody` for `Pin` where `B::Target: MessageBody`. [#2868] + +### Performance +- Improve overall performance of operations on `Extensions`. [#2890] + +[#2868]: https://github.com/actix/actix-web/pull/2868 +[#2890]: https://github.com/actix/actix-web/pull/2890 + + +## 3.2.2 - 2022-09-11 +### Changed +- Minimum supported Rust version (MSRV) is now 1.59 due to transitive `time` dependency. + +### Fixed +- Avoid possibility of dispatcher getting stuck while back-pressuring I/O. [#2369] + +[#2369]: https://github.com/actix/actix-web/pull/2369 + + +## 3.2.1 - 2022-07-02 +### 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 +### 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 +### Changed +- Minimum supported Rust version (MSRV) is now 1.56 due to transitive `hashbrown` dependency. + +### Fixed +- Revert broken fix in [#2624] that caused erroneous 500 error responses. Temporarily re-introduces [#2357] bug. [#2779] + +[#2624]: https://github.com/actix/actix-web/pull/2624 +[#2357]: https://github.com/actix/actix-web/issues/2357 +[#2779]: https://github.com/actix/actix-web/pull/2779 ## 3.0.4 - 2022-03-09 @@ -12,14 +62,14 @@ ### Fixed - Allow spaces between header name and colon when parsing responses. [#2684] -[#2684]: https://github.com/actix/actix-web/issues/2684 +[#2684]: https://github.com/actix/actix-web/pull/2684 ## 3.0.2 - 2022-03-05 ### Fixed - Fix encoding camel-case header names with more than one hyphen. [#2683] -[#2683]: https://github.com/actix/actix-web/issues/2683 +[#2683]: https://github.com/actix/actix-web/pull/2683 ## 3.0.1 - 2022-03-04 diff --git a/actix-http/Cargo.toml b/actix-http/Cargo.toml index 6d410e46f..30e436160 100644 --- a/actix-http/Cargo.toml +++ b/actix-http/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "actix-http" -version = "3.0.4" +version = "3.2.2" authors = [ "Nikolay Kim ", "Rob Ede ", @@ -37,7 +37,7 @@ ws = [ "local-channel", "base64", "rand", - "sha-1", + "sha1", ] # TLS via OpenSSL @@ -86,7 +86,7 @@ h2 = { version = "0.3.9", optional = true } local-channel = { version = "0.1", optional = true } base64 = { version = "0.13", optional = true } rand = { version = "0.8", optional = true } -sha-1 = { version = "0.10", optional = true } +sha1 = { version = "0.10", optional = true } # openssl/rustls actix-tls = { version = "3", default-features = false, optional = true } @@ -94,13 +94,13 @@ actix-tls = { version = "3", default-features = false, optional = true } # compress-* brotli = { version = "3.3.3", optional = true } flate2 = { version = "1.0.13", optional = true } -zstd = { version = "0.10", optional = true } +zstd = { version = "0.11", optional = true } [dev-dependencies] -actix-http-test = { version = "3.0.0-beta.13", features = ["openssl"] } +actix-http-test = { version = "3", features = ["openssl"] } actix-server = "2" actix-tls = { version = "3", features = ["openssl"] } -actix-web = "4.0.0" +actix-web = "4" async-stream = "0.3" criterion = { version = "0.3", features = ["html_reports"] } @@ -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-http/README.md b/actix-http/README.md index 14a7013db..994cf97c6 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.0.4)](https://docs.rs/actix-http/3.0.4) -[![Version](https://img.shields.io/badge/rustc-1.54+-ab6000.svg)](https://blog.rust-lang.org/2021/05/06/Rust-1.54.0.html) +[![Documentation](https://docs.rs/actix-http/badge.svg?version=3.2.2)](https://docs.rs/actix-http/3.2.2) +![Version](https://img.shields.io/badge/rustc-1.59+-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.0.4/status.svg)](https://deps.rs/crate/actix-http/3.0.4) +[![dependency status](https://deps.rs/crate/actix-http/3.2.2/status.svg)](https://deps.rs/crate/actix-http/3.2.2) [![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) diff --git a/actix-http/benches/uninit-headers.rs b/actix-http/benches/uninit-headers.rs index 5dfd3bc11..688c64d6e 100644 --- a/actix-http/benches/uninit-headers.rs +++ b/actix-http/benches/uninit-headers.rs @@ -114,11 +114,12 @@ mod _original { use std::mem::MaybeUninit; pub fn parse_headers(src: &mut BytesMut) -> usize { - #![allow(clippy::uninit_assumed_init)] + #![allow(invalid_value, clippy::uninit_assumed_init)] let mut headers: [HeaderIndex; MAX_HEADERS] = unsafe { MaybeUninit::uninit().assume_init() }; + #[allow(invalid_value)] let mut parsed: [httparse::Header<'_>; MAX_HEADERS] = unsafe { MaybeUninit::uninit().assume_init() }; diff --git a/actix-http/src/body/message_body.rs b/actix-http/src/body/message_body.rs index 9090e34d5..0cfaa8653 100644 --- a/actix-http/src/body/message_body.rs +++ b/actix-http/src/body/message_body.rs @@ -120,8 +120,28 @@ pub trait MessageBody { } mod foreign_impls { + use std::ops::DerefMut; + use super::*; + impl MessageBody for &mut B + where + B: MessageBody + Unpin + ?Sized, + { + type Error = B::Error; + + fn size(&self) -> BodySize { + (**self).size() + } + + fn poll_next( + mut self: Pin<&mut Self>, + cx: &mut Context<'_>, + ) -> Poll>> { + Pin::new(&mut **self).poll_next(cx) + } + } + impl MessageBody for Infallible { type Error = Infallible; @@ -179,8 +199,9 @@ mod foreign_impls { } } - impl MessageBody for Pin> + impl MessageBody for Pin where + T: DerefMut + Unpin, B: MessageBody + ?Sized, { type Error = B::Error; @@ -445,6 +466,7 @@ mod tests { use actix_rt::pin; use actix_utils::future::poll_fn; use bytes::{Bytes, BytesMut}; + use futures_util::stream; use super::*; use crate::body::{self, EitherBody}; @@ -481,6 +503,35 @@ mod tests { assert_poll_next_none!(pl); } + #[actix_rt::test] + async fn mut_equivalence() { + assert_eq!(().size(), BodySize::Sized(0)); + assert_eq!(().size(), (&(&mut ())).size()); + + let pl = &mut (); + pin!(pl); + assert_poll_next_none!(pl); + + let pl = &mut Box::new(()); + pin!(pl); + assert_poll_next_none!(pl); + + let mut body = body::SizedStream::new( + 8, + stream::iter([ + Ok::<_, std::io::Error>(Bytes::from("1234")), + Ok(Bytes::from("5678")), + ]), + ); + let body = &mut body; + assert_eq!(body.size(), BodySize::Sized(8)); + pin!(body); + assert_poll_next!(body, Bytes::from_static(b"1234")); + assert_poll_next!(body, Bytes::from_static(b"5678")); + assert_poll_next_none!(body); + } + + #[allow(clippy::let_unit_value)] #[actix_rt::test] async fn test_unit() { let pl = (); @@ -606,4 +657,18 @@ mod tests { let not_body = resp_body.downcast_ref::<()>(); assert!(not_body.is_none()); } + + #[actix_rt::test] + async fn non_owning_to_bytes() { + let mut body = BoxBody::new(()); + let bytes = body::to_bytes(&mut body).await.unwrap(); + assert_eq!(bytes, Bytes::new()); + + let mut body = body::BodyStream::new(stream::iter([ + Ok::<_, std::io::Error>(Bytes::from("1234")), + Ok(Bytes::from("5678")), + ])); + let bytes = body::to_bytes(&mut body).await.unwrap(); + assert_eq!(bytes, Bytes::from_static(b"12345678")); + } } diff --git a/actix-http/src/body/utils.rs b/actix-http/src/body/utils.rs index 194af47f8..0a6fb0c15 100644 --- a/actix-http/src/body/utils.rs +++ b/actix-http/src/body/utils.rs @@ -42,7 +42,7 @@ pub async fn to_bytes(body: B) -> Result { let body = body.as_mut(); match ready!(body.poll_next(cx)) { - Some(Ok(bytes)) => buf.extend_from_slice(&*bytes), + Some(Ok(bytes)) => buf.extend_from_slice(&bytes), None => return Poll::Ready(Ok(())), Some(Err(err)) => return Poll::Ready(Err(err)), } diff --git a/actix-http/src/builder.rs b/actix-http/src/builder.rs index 526a23d53..71b933835 100644 --- a/actix-http/src/builder.rs +++ b/actix-http/src/builder.rs @@ -211,6 +211,7 @@ where /// Finish service configuration and create a HTTP service for HTTP/2 protocol. #[cfg(feature = "http2")] + #[cfg_attr(docsrs, doc(cfg(feature = "http2")))] pub fn h2(self, service: F) -> crate::h2::H2Service where F: IntoServiceFactory, diff --git a/actix-http/src/config.rs b/actix-http/src/config.rs index ac95a2802..c0d297a20 100644 --- a/actix-http/src/config.rs +++ b/actix-http/src/config.rs @@ -35,7 +35,7 @@ impl Default for ServiceConfig { } impl ServiceConfig { - /// Create instance of `ServiceConfig` + /// Create instance of `ServiceConfig`. pub fn new( keep_alive: KeepAlive, client_request_timeout: Duration, 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-http/src/error.rs b/actix-http/src/error.rs index 3fce0a60b..2d443369d 100644 --- a/actix-http/src/error.rs +++ b/actix-http/src/error.rs @@ -294,6 +294,7 @@ impl std::error::Error for PayloadError { PayloadError::Overflow => None, PayloadError::UnknownLength => None, #[cfg(feature = "http2")] + #[cfg_attr(docsrs, doc(cfg(feature = "http2")))] PayloadError::Http2Payload(err) => Some(err), PayloadError::Io(err) => Some(err), } @@ -351,6 +352,7 @@ pub enum DispatchError { /// HTTP/2 error. #[display(fmt = "{}", _0)] #[cfg(feature = "http2")] + #[cfg_attr(docsrs, doc(cfg(feature = "http2")))] H2(h2::Error), /// The first request did not complete within the specified timeout. @@ -388,7 +390,7 @@ impl StdError for DispatchError { /// A set of error that can occur during parsing content type. #[derive(Debug, Display, Error)] -#[cfg_attr(test, derive(PartialEq))] +#[cfg_attr(test, derive(PartialEq, Eq))] #[non_exhaustive] pub enum ContentTypeError { /// Can not parse content type diff --git a/actix-http/src/extensions.rs b/actix-http/src/extensions.rs index 60b769d13..f2047a9ce 100644 --- a/actix-http/src/extensions.rs +++ b/actix-http/src/extensions.rs @@ -1,9 +1,30 @@ use std::{ any::{Any, TypeId}, + collections::HashMap, fmt, + hash::{BuildHasherDefault, Hasher}, }; -use ahash::AHashMap; +/// A hasher for `TypeId`s that takes advantage of its known characteristics. +/// +/// Author of `anymap` crate has done research on the topic: +/// https://github.com/chris-morgan/anymap/blob/2e9a5704/src/lib.rs#L599 +#[derive(Debug, Default)] +struct NoOpHasher(u64); + +impl Hasher for NoOpHasher { + fn write(&mut self, _bytes: &[u8]) { + unimplemented!("This NoOpHasher can only handle u64s") + } + + fn write_u64(&mut self, i: u64) { + self.0 = i; + } + + fn finish(&self) -> u64 { + self.0 + } +} /// A type map for request extensions. /// @@ -11,7 +32,7 @@ use ahash::AHashMap; #[derive(Default)] pub struct Extensions { /// Use AHasher with a std HashMap with for faster lookups on the small `TypeId` keys. - map: AHashMap>, + map: HashMap, BuildHasherDefault>, } impl Extensions { @@ -19,7 +40,7 @@ impl Extensions { #[inline] pub fn new() -> Extensions { Extensions { - map: AHashMap::new(), + map: HashMap::default(), } } diff --git a/actix-http/src/h1/chunked.rs b/actix-http/src/h1/chunked.rs index c1dd4b283..4005ed892 100644 --- a/actix-http/src/h1/chunked.rs +++ b/actix-http/src/h1/chunked.rs @@ -15,7 +15,7 @@ macro_rules! byte ( }) ); -#[derive(Debug, PartialEq, Clone)] +#[derive(Debug, Clone, PartialEq, Eq)] pub(super) enum ChunkedState { Size, SizeLws, diff --git a/actix-http/src/h1/decoder.rs b/actix-http/src/h1/decoder.rs index a9443997e..0b06bfe24 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 { @@ -391,7 +440,7 @@ impl HeaderIndex { } } -#[derive(Debug, Clone, PartialEq)] +#[derive(Debug, Clone, PartialEq, Eq)] /// Chunk type yielded while decoding a payload. pub enum PayloadItem { Chunk(Bytes), @@ -401,7 +450,7 @@ pub enum PayloadItem { /// Decoder that can handle different payload types. /// /// If a message body does not use `Transfer-Encoding`, it should include a `Content-Length`. -#[derive(Debug, Clone, PartialEq)] +#[derive(Debug, Clone, PartialEq, Eq)] pub struct PayloadDecoder { kind: Kind, } @@ -427,7 +476,7 @@ impl PayloadDecoder { } } -#[derive(Debug, Clone, PartialEq)] +#[derive(Debug, Clone, PartialEq, Eq)] enum Kind { /// A reader used when a `Content-Length` header is passed with a positive integer. Length(u64), @@ -606,14 +655,100 @@ mod tests { } #[test] - fn test_parse_post() { - let mut buf = BytesMut::from("POST /test2 HTTP/1.0\r\n\r\n"); + 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", + ); + + 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( + "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] @@ -709,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); @@ -831,59 +943,62 @@ 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) + // negative CL + expect_parse_err!(&mut BytesMut::from( + "GET /test HTTP/1.1\r\n\ + content-length: -1\r\n\r\n", + )); } #[test] - fn test_headers_content_length_err_2() { + fn octal_ish_cl_parsed_as_decimal() { let mut buf = BytesMut::from( - "GET /test HTTP/1.1\r\n\ - content-length: -1\r\n\r\n", + "POST /test HTTP/1.1\r\n\ + content-length: 011\r\n\r\n", ); - - expect_parse_err!(&mut buf); + 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] 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] @@ -923,11 +1038,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(), @@ -937,24 +1051,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] @@ -971,29 +1079,66 @@ 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); + 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", + )); } #[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", + )); + } + + #[test] + fn hrs_te_http10() { + // in HTTP/1.0 transfer encoding is ignored and must therefore contain a CL header + + expect_parse_err!(&mut 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\ + ", + )); + } + + #[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", ); - expect_parse_err!(&mut buf); + parse_ready!(&mut buf); } #[test] diff --git a/actix-http/src/h1/dispatcher.rs b/actix-http/src/h1/dispatcher.rs index dea8a4beb..81090667d 100644 --- a/actix-http/src/h1/dispatcher.rs +++ b/actix-http/src/h1/dispatcher.rs @@ -15,14 +15,14 @@ use bitflags::bitflags; use bytes::{Buf, BytesMut}; use futures_core::ready; use pin_project_lite::pin_project; -use tracing::{debug, error, trace}; +use tracing::{error, trace}; use crate::{ body::{BodySize, BoxBody, MessageBody}, config::ServiceConfig, error::{DispatchError, ParseError, PayloadError}, service::HttpFlow, - ConnectionType, Error, Extensions, OnConnectData, Request, Response, StatusCode, + Error, Extensions, OnConnectData, Request, Response, StatusCode, }; use super::{ @@ -691,74 +691,12 @@ where let can_not_read = !self.can_read(cx); // limit amount of non-processed requests - if pipeline_queue_full { + if pipeline_queue_full || can_not_read { return Ok(false); } let mut this = self.as_mut().project(); - if can_not_read { - debug!("cannot read request payload"); - - if let Some(sender) = &this.payload { - // ...maybe handler does not want to read any more payload... - if let PayloadStatus::Dropped = sender.need_read(cx) { - debug!("handler dropped payload early; attempt to clean connection"); - // ...in which case poll request payload a few times - loop { - match this.codec.decode(this.read_buf)? { - Some(msg) => { - match msg { - // payload decoded did not yield EOF yet - Message::Chunk(Some(_)) => { - // if non-clean connection, next loop iter will detect empty - // read buffer and close connection - } - - // connection is in clean state for next request - Message::Chunk(None) => { - debug!("connection successfully cleaned"); - - // reset dispatcher state - let _ = this.payload.take(); - this.state.set(State::None); - - // break out of payload decode loop - break; - } - - // Either whole payload is read and loop is broken or more data - // was expected in which case connection is closed. In both - // situations dispatcher cannot get here. - Message::Item(_) => { - unreachable!("dispatcher is in payload receive state") - } - } - } - - // not enough info to decide if connection is going to be clean or not - None => { - error!( - "handler did not read whole payload and dispatcher could not \ - drain read buf; return 500 and close connection" - ); - - this.flags.insert(Flags::SHUTDOWN); - let mut res = Response::internal_server_error().drop_body(); - res.head_mut().set_connection_type(ConnectionType::Close); - this.messages.push_back(DispatcherMessage::Error(res)); - *this.error = Some(DispatchError::HandlerDroppedPayload); - return Ok(true); - } - } - } - } - } else { - // can_not_read and no request payload - return Ok(false); - } - } - let mut updated = false; // decode from read buf as many full requests as possible @@ -904,10 +842,7 @@ where if timer.as_mut().poll(cx).is_ready() { // timeout on first request (slow request) return 408 - trace!( - "timed out on slow request; \ - replying with 408 and closing connection" - ); + trace!("timed out on slow request; replying with 408 and closing connection"); let _ = self.as_mut().send_error_response( Response::with_body(StatusCode::REQUEST_TIMEOUT, ()), @@ -1041,9 +976,11 @@ where // // A Request head too large to parse is only checked on `httparse::Status::Partial`. - if this.payload.is_none() { - // When dispatcher has a payload the responsibility of wake up it would be shift - // to h1::payload::Payload. + match this.payload { + // When dispatcher has a payload the responsibility of wake ups is shifted to + // `h1::payload::Payload` unless the payload is needing a read, in which case it + // might not have access to the waker and could result in the dispatcher + // getting stuck until timeout. // // Reason: // Self wake up when there is payload would waste poll and/or result in @@ -1054,7 +991,8 @@ where // read anymore. At this case read_buf could always remain beyond // MAX_BUFFER_SIZE and self wake up would be busy poll dispatcher and // waste resources. - cx.waker().wake_by_ref(); + Some(ref p) if p.need_read(cx) != PayloadStatus::Read => {} + _ => cx.waker().wake_by_ref(), } return Ok(false); diff --git a/actix-http/src/h1/dispatcher_tests.rs b/actix-http/src/h1/dispatcher_tests.rs index 40454d45a..3eea859bf 100644 --- a/actix-http/src/h1/dispatcher_tests.rs +++ b/actix-http/src/h1/dispatcher_tests.rs @@ -637,7 +637,7 @@ async fn expect_handling() { if let DispatcherState::Normal { ref inner } = h1.inner { let io = inner.io.as_ref().unwrap(); - let mut res = (&io.write_buf()[..]).to_owned(); + let mut res = io.write_buf()[..].to_owned(); stabilize_date_header(&mut res); assert_eq!( @@ -699,7 +699,7 @@ async fn expect_eager() { if let DispatcherState::Normal { ref inner } = h1.inner { let io = inner.io.as_ref().unwrap(); - let mut res = (&io.write_buf()[..]).to_owned(); + let mut res = io.write_buf()[..].to_owned(); stabilize_date_header(&mut res); // Despite the content-length header and even though the request payload has not @@ -783,6 +783,9 @@ async fn upgrade_handling() { .await; } +// fix in #2624 reverted temporarily +// complete fix tracked in #2745 +#[ignore] #[actix_rt::test] async fn handler_drop_payload() { let _ = env_logger::try_init(); diff --git a/actix-http/src/h1/payload.rs b/actix-http/src/h1/payload.rs index 5a93e9051..1ed785a1b 100644 --- a/actix-http/src/h1/payload.rs +++ b/actix-http/src/h1/payload.rs @@ -16,7 +16,7 @@ use crate::error::PayloadError; /// max buffer size 32k pub(crate) const MAX_BUFFER_SIZE: usize = 32_768; -#[derive(Debug, PartialEq)] +#[derive(Debug, PartialEq, Eq)] pub enum PayloadStatus { Read, Pause, @@ -252,21 +252,15 @@ impl Inner { #[cfg(test)] mod tests { - use std::panic::{RefUnwindSafe, UnwindSafe}; - use actix_utils::future::poll_fn; use static_assertions::{assert_impl_all, assert_not_impl_any}; use super::*; assert_impl_all!(Payload: Unpin); - assert_not_impl_any!(Payload: Send, Sync, UnwindSafe, RefUnwindSafe); + assert_not_impl_any!(Payload: Send, Sync); assert_impl_all!(Inner: Unpin, Send, Sync); - #[rustversion::before(1.60)] - assert_not_impl_any!(Inner: UnwindSafe, RefUnwindSafe); - #[rustversion::since(1.60)] - assert_impl_all!(Inner: UnwindSafe, RefUnwindSafe); #[actix_rt::test] async fn test_unread_data() { diff --git a/actix-http/src/h1/service.rs b/actix-http/src/h1/service.rs index a791ea8c3..e4d90424d 100644 --- a/actix-http/src/h1/service.rs +++ b/actix-http/src/h1/service.rs @@ -134,6 +134,7 @@ mod openssl { U::InitError: fmt::Debug, { /// Create OpenSSL based service. + #[cfg_attr(docsrs, doc(cfg(feature = "openssl")))] pub fn openssl( self, acceptor: SslAcceptor, @@ -196,6 +197,7 @@ mod rustls { U::InitError: fmt::Debug, { /// Create Rustls based service. + #[cfg_attr(docsrs, doc(cfg(feature = "rustls")))] pub fn rustls( self, config: ServerConfig, diff --git a/actix-http/src/h2/dispatcher.rs b/actix-http/src/h2/dispatcher.rs index 85516cccc..680936f0f 100644 --- a/actix-http/src/h2/dispatcher.rs +++ b/actix-http/src/h2/dispatcher.rs @@ -67,7 +67,7 @@ where timer }) .unwrap_or_else(|| Box::pin(sleep(dur))), - on_flight: false, + in_flight: false, ping_pong: conn.ping_pong().unwrap(), }); @@ -84,9 +84,14 @@ where } struct H2PingPong { - timer: Pin>, - on_flight: bool, + /// Handle to send ping frames from the peer. ping_pong: PingPong, + + /// True when a ping has been sent and is waiting for a reply. + in_flight: bool, + + /// Timeout for pong response. + timer: Pin>, } impl Future for Dispatcher @@ -152,26 +157,28 @@ where }); } Poll::Ready(None) => return Poll::Ready(Ok(())), + Poll::Pending => match this.ping_pong.as_mut() { Some(ping_pong) => loop { - if ping_pong.on_flight { - // When have on flight ping pong. poll pong and and keep alive timer. - // on success pong received update keep alive timer to determine the next timing of - // ping pong. + if ping_pong.in_flight { + // When there is an in-flight ping-pong, poll pong and and keep-alive + // timer. On successful pong received, update keep-alive timer to + // determine the next timing of ping pong. match ping_pong.ping_pong.poll_pong(cx)? { Poll::Ready(_) => { - ping_pong.on_flight = false; + ping_pong.in_flight = false; let dead_line = this.config.keep_alive_deadline().unwrap(); ping_pong.timer.as_mut().reset(dead_line.into()); } Poll::Pending => { - return ping_pong.timer.as_mut().poll(cx).map(|_| Ok(())) + return ping_pong.timer.as_mut().poll(cx).map(|_| Ok(())); } } } else { - // When there is no on flight ping pong. keep alive timer is used to wait for next - // timing of ping pong. Therefore at this point it serves as an interval instead. + // When there is no in-flight ping-pong, keep-alive timer is used to + // wait for next timing of ping-pong. Therefore, at this point it serves + // as an interval instead. ready!(ping_pong.timer.as_mut().poll(cx)); ping_pong.ping_pong.send_ping(Ping::opaque())?; @@ -179,7 +186,7 @@ where let dead_line = this.config.keep_alive_deadline().unwrap(); ping_pong.timer.as_mut().reset(dead_line.into()); - ping_pong.on_flight = true; + ping_pong.in_flight = true; } }, None => return Poll::Pending, @@ -287,13 +294,13 @@ fn prepare_response( _ => {} } - let _ = match size { - BodySize::None | BodySize::Stream => None, + match size { + BodySize::None | BodySize::Stream => {} BodySize::Sized(0) => { #[allow(clippy::declare_interior_mutable_const)] const HV_ZERO: HeaderValue = HeaderValue::from_static("0"); - res.headers_mut().insert(CONTENT_LENGTH, HV_ZERO) + res.headers_mut().insert(CONTENT_LENGTH, HV_ZERO); } BodySize::Sized(len) => { @@ -302,7 +309,7 @@ fn prepare_response( res.headers_mut().insert( CONTENT_LENGTH, HeaderValue::from_str(buf.format(*len)).unwrap(), - ) + ); } }; diff --git a/actix-http/src/h2/mod.rs b/actix-http/src/h2/mod.rs index c8aaaaa5f..39198e0fe 100644 --- a/actix-http/src/h2/mod.rs +++ b/actix-http/src/h2/mod.rs @@ -103,11 +103,9 @@ where #[cfg(test)] mod tests { - use std::panic::{RefUnwindSafe, UnwindSafe}; - use static_assertions::assert_impl_all; use super::*; - assert_impl_all!(Payload: Unpin, Send, Sync, UnwindSafe, RefUnwindSafe); + assert_impl_all!(Payload: Unpin, Send, Sync); } diff --git a/actix-http/src/h2/service.rs b/actix-http/src/h2/service.rs index e526918c7..2a45fc1dc 100644 --- a/actix-http/src/h2/service.rs +++ b/actix-http/src/h2/service.rs @@ -117,6 +117,7 @@ mod openssl { B: MessageBody + 'static, { /// Create OpenSSL based service. + #[cfg_attr(docsrs, doc(cfg(feature = "openssl")))] pub fn openssl( self, acceptor: SslAcceptor, @@ -164,6 +165,7 @@ mod rustls { B: MessageBody + 'static, { /// Create Rustls based service. + #[cfg_attr(docsrs, doc(cfg(feature = "rustls")))] pub fn rustls( self, mut config: ServerConfig, diff --git a/actix-http/src/header/map.rs b/actix-http/src/header/map.rs index 8f6d1cead..28906e835 100644 --- a/actix-http/src/header/map.rs +++ b/actix-http/src/header/map.rs @@ -309,7 +309,7 @@ impl HeaderMap { pub fn get_all(&self, key: impl AsHeaderName) -> std::slice::Iter<'_, HeaderValue> { match self.get_value(key) { Some(value) => value.iter(), - None => (&[]).iter(), + None => [].iter(), } } diff --git a/actix-http/src/header/shared/extended.rs b/actix-http/src/header/shared/extended.rs index 1af9ca20e..cd8adb2bd 100644 --- a/actix-http/src/header/shared/extended.rs +++ b/actix-http/src/header/shared/extended.rs @@ -12,7 +12,7 @@ use crate::header::{Charset, HTTP_VALUE}; /// - A character sequence representing the actual value (`value`), separated by single quotes. /// /// It is defined in [RFC 5987 §3.2](https://datatracker.ietf.org/doc/html/rfc5987#section-3.2). -#[derive(Clone, Debug, PartialEq)] +#[derive(Clone, Debug, PartialEq, Eq)] pub struct ExtendedValue { /// The character set that is used to encode the `value` to a string. pub charset: Charset, diff --git a/actix-http/src/header/shared/quality_item.rs b/actix-http/src/header/shared/quality_item.rs index 71a3bdd53..0b35b5401 100644 --- a/actix-http/src/header/shared/quality_item.rs +++ b/actix-http/src/header/shared/quality_item.rs @@ -147,7 +147,7 @@ mod tests { // copy of encoding from actix-web headers #[allow(clippy::enum_variant_names)] // allow Encoding prefix on EncodingExt - #[derive(Clone, PartialEq, Debug)] + #[derive(Debug, Clone, PartialEq, Eq)] pub enum Encoding { Chunked, Brotli, diff --git a/actix-http/src/lib.rs b/actix-http/src/lib.rs index 360cb86fc..864db4986 100644 --- a/actix-http/src/lib.rs +++ b/actix-http/src/lib.rs @@ -25,6 +25,7 @@ )] #![doc(html_logo_url = "https://actix.rs/img/logo.png")] #![doc(html_favicon_url = "https://actix.rs/favicon.ico")] +#![cfg_attr(docsrs, feature(doc_cfg))] pub use ::http::{uri, uri::Uri}; pub use ::http::{Method, StatusCode, Version}; @@ -39,6 +40,7 @@ pub mod error; mod extensions; pub mod h1; #[cfg(feature = "http2")] +#[cfg_attr(docsrs, doc(cfg(feature = "http2")))] pub mod h2; pub mod header; mod helpers; @@ -53,6 +55,7 @@ mod responses; mod service; pub mod test; #[cfg(feature = "ws")] +#[cfg_attr(docsrs, doc(cfg(feature = "ws")))] pub mod ws; pub use self::builder::HttpServiceBuilder; @@ -69,6 +72,9 @@ pub use self::payload::{BoxedPayloadStream, Payload, PayloadStream}; pub use self::requests::{Request, RequestHead, RequestHeadType}; pub use self::responses::{Response, ResponseBuilder, ResponseHead}; pub use self::service::HttpService; +#[cfg(any(feature = "openssl", feature = "rustls"))] +#[cfg_attr(docsrs, doc(cfg(any(feature = "openssl", feature = "rustls"))))] +pub use self::service::TlsAcceptorConfig; /// A major HTTP protocol version. #[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)] diff --git a/actix-http/src/message.rs b/actix-http/src/message.rs index 5616a4762..7469d74ee 100644 --- a/actix-http/src/message.rs +++ b/actix-http/src/message.rs @@ -3,7 +3,7 @@ use std::{cell::RefCell, ops, rc::Rc}; use bitflags::bitflags; /// Represents various types of connection -#[derive(Copy, Clone, PartialEq, Debug)] +#[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum ConnectionType { /// Close connection after response. Close, diff --git a/actix-http/src/payload.rs b/actix-http/src/payload.rs index ee0128af4..7d476c55f 100644 --- a/actix-http/src/payload.rs +++ b/actix-http/src/payload.rs @@ -97,12 +97,10 @@ where #[cfg(test)] mod tests { - use std::panic::{RefUnwindSafe, UnwindSafe}; - use static_assertions::{assert_impl_all, assert_not_impl_any}; use super::*; assert_impl_all!(Payload: Unpin); - assert_not_impl_any!(Payload: Send, Sync, UnwindSafe, RefUnwindSafe); + assert_not_impl_any!(Payload: Send, Sync); } diff --git a/actix-http/src/requests/request.rs b/actix-http/src/requests/request.rs index 0f8e78d46..ac358e8df 100644 --- a/actix-http/src/requests/request.rs +++ b/actix-http/src/requests/request.rs @@ -113,14 +113,14 @@ impl

Request

{ #[inline] /// Http message part of the request pub fn head(&self) -> &RequestHead { - &*self.head + &self.head } #[inline] #[doc(hidden)] /// Mutable reference to a HTTP message part of the request pub fn head_mut(&mut self) -> &mut RequestHead { - &mut *self.head + &mut self.head } /// Mutable reference to the message's headers. diff --git a/actix-http/src/responses/head.rs b/actix-http/src/responses/head.rs index cb47c4b7a..01bca6c5b 100644 --- a/actix-http/src/responses/head.rs +++ b/actix-http/src/responses/head.rs @@ -237,7 +237,7 @@ mod tests { .await; let mut stream = net::TcpStream::connect(srv.addr()).unwrap(); - let _ = stream + stream .write_all(b"GET /camel HTTP/1.1\r\nConnection: Close\r\n\r\n") .unwrap(); let mut data = vec![]; @@ -251,7 +251,7 @@ mod tests { assert!(memmem::find(&data, b"content-length").is_none()); let mut stream = net::TcpStream::connect(srv.addr()).unwrap(); - let _ = stream + stream .write_all(b"GET /lower HTTP/1.1\r\nConnection: Close\r\n\r\n") .unwrap(); let mut data = vec![]; diff --git a/actix-http/src/responses/response.rs b/actix-http/src/responses/response.rs index ceb158f65..03908d9ce 100644 --- a/actix-http/src/responses/response.rs +++ b/actix-http/src/responses/response.rs @@ -83,13 +83,13 @@ impl Response { /// Returns a reference to the head of this response. #[inline] pub fn head(&self) -> &ResponseHead { - &*self.head + &self.head } /// Returns a mutable reference to the head of this response. #[inline] pub fn head_mut(&mut self) -> &mut ResponseHead { - &mut *self.head + &mut self.head } /// Returns the status code of this response. diff --git a/actix-http/src/service.rs b/actix-http/src/service.rs index f4fe625a3..bcca5b188 100644 --- a/actix-http/src/service.rs +++ b/actix-http/src/service.rs @@ -181,6 +181,25 @@ where } } +/// Configuration options used when accepting TLS connection. +#[cfg(any(feature = "openssl", feature = "rustls"))] +#[cfg_attr(docsrs, doc(cfg(any(feature = "openssl", feature = "rustls"))))] +#[derive(Debug, Default)] +pub struct TlsAcceptorConfig { + pub(crate) handshake_timeout: Option, +} + +#[cfg(any(feature = "openssl", feature = "rustls"))] +impl TlsAcceptorConfig { + /// Set TLS handshake timeout duration. + pub fn handshake_timeout(self, dur: std::time::Duration) -> Self { + Self { + handshake_timeout: Some(dur), + // ..self + } + } +} + #[cfg(feature = "openssl")] mod openssl { use actix_service::ServiceFactoryExt as _; @@ -220,6 +239,7 @@ mod openssl { U::InitError: fmt::Debug, { /// Create OpenSSL based service. + #[cfg_attr(docsrs, doc(cfg(feature = "openssl")))] pub fn openssl( self, acceptor: SslAcceptor, @@ -230,7 +250,29 @@ mod openssl { Error = TlsError, InitError = (), > { - Acceptor::new(acceptor) + self.openssl_with_config(acceptor, TlsAcceptorConfig::default()) + } + + /// Create OpenSSL based service with custom TLS acceptor configuration. + #[cfg_attr(docsrs, doc(cfg(feature = "openssl")))] + pub fn openssl_with_config( + self, + acceptor: SslAcceptor, + tls_acceptor_config: TlsAcceptorConfig, + ) -> impl ServiceFactory< + TcpStream, + Config = (), + Response = (), + Error = TlsError, + InitError = (), + > { + let mut acceptor = Acceptor::new(acceptor); + + if let Some(handshake_timeout) = tls_acceptor_config.handshake_timeout { + acceptor.set_handshake_timeout(handshake_timeout); + } + + acceptor .map_init_err(|_| { unreachable!("TLS acceptor service factory does not error on init") }) @@ -292,9 +334,26 @@ mod rustls { U::InitError: fmt::Debug, { /// Create Rustls based service. + #[cfg_attr(docsrs, doc(cfg(feature = "rustls")))] pub fn rustls( + self, + config: ServerConfig, + ) -> impl ServiceFactory< + TcpStream, + Config = (), + Response = (), + Error = TlsError, + InitError = (), + > { + self.rustls_with_config(config, TlsAcceptorConfig::default()) + } + + /// Create Rustls based service with custom TLS acceptor configuration. + #[cfg_attr(docsrs, doc(cfg(feature = "rustls")))] + pub fn rustls_with_config( self, mut config: ServerConfig, + tls_acceptor_config: TlsAcceptorConfig, ) -> impl ServiceFactory< TcpStream, Config = (), @@ -306,7 +365,13 @@ mod rustls { protos.extend_from_slice(&config.alpn_protocols); config.alpn_protocols = protos; - Acceptor::new(config) + let mut acceptor = Acceptor::new(config); + + if let Some(handshake_timeout) = tls_acceptor_config.handshake_timeout { + acceptor.set_handshake_timeout(handshake_timeout); + } + + acceptor .map_init_err(|_| { unreachable!("TLS acceptor service factory does not error on init") }) diff --git a/actix-http/src/test.rs b/actix-http/src/test.rs index 6212c19d1..3815e64c6 100644 --- a/actix-http/src/test.rs +++ b/actix-http/src/test.rs @@ -19,29 +19,7 @@ use crate::{ Request, }; -/// Test `Request` builder -/// -/// ```ignore -/// # use http::{header, StatusCode}; -/// # use actix_web::*; -/// use actix_web::test::TestRequest; -/// -/// fn index(req: &HttpRequest) -> Response { -/// if let Some(hdr) = req.headers().get(header::CONTENT_TYPE) { -/// Response::Ok().into() -/// } else { -/// Response::BadRequest().into() -/// } -/// } -/// -/// let resp = TestRequest::default().insert_header("content-type", "text/plain") -/// .run(&index) -/// .unwrap(); -/// assert_eq!(resp.status(), StatusCode::OK); -/// -/// let resp = TestRequest::default().run(&index).unwrap(); -/// assert_eq!(resp.status(), StatusCode::BAD_REQUEST); -/// ``` +/// Test `Request` builder. pub struct TestRequest(Option); struct Inner { diff --git a/actix-http/src/ws/codec.rs b/actix-http/src/ws/codec.rs index 3aa325d6a..4a2e741b6 100644 --- a/actix-http/src/ws/codec.rs +++ b/actix-http/src/ws/codec.rs @@ -11,7 +11,7 @@ use super::{ }; /// A WebSocket message. -#[derive(Debug, PartialEq)] +#[derive(Debug, PartialEq, Eq)] pub enum Message { /// Text message. Text(ByteString), @@ -36,7 +36,7 @@ pub enum Message { } /// A WebSocket frame. -#[derive(Debug, PartialEq)] +#[derive(Debug, PartialEq, Eq)] pub enum Frame { /// Text frame. Note that the codec does not validate UTF-8 encoding. Text(Bytes), @@ -58,7 +58,7 @@ pub enum Frame { } /// A WebSocket continuation item. -#[derive(Debug, PartialEq)] +#[derive(Debug, PartialEq, Eq)] pub enum Item { FirstText(Bytes), FirstBinary(Bytes), diff --git a/actix-http/src/ws/frame.rs b/actix-http/src/ws/frame.rs index 17e34e2ba..c7e0427ea 100644 --- a/actix-http/src/ws/frame.rs +++ b/actix-http/src/ws/frame.rs @@ -17,7 +17,6 @@ impl Parser { fn parse_metadata( src: &[u8], server: bool, - max_size: usize, ) -> Result)>, ProtocolError> { let chunk_len = src.len(); @@ -60,20 +59,12 @@ impl Parser { return Ok(None); } let len = u64::from_be_bytes(TryFrom::try_from(&src[idx..idx + 8]).unwrap()); - if len > max_size as u64 { - return Err(ProtocolError::Overflow); - } idx += 8; len as usize } else { len as usize }; - // check for max allowed size - if length > max_size { - return Err(ProtocolError::Overflow); - } - let mask = if server { if chunk_len < idx + 4 { return Ok(None); @@ -98,11 +89,10 @@ impl Parser { max_size: usize, ) -> Result)>, ProtocolError> { // try to parse ws frame metadata - let (idx, finished, opcode, length, mask) = - match Parser::parse_metadata(src, server, max_size)? { - None => return Ok(None), - Some(res) => res, - }; + let (idx, finished, opcode, length, mask) = match Parser::parse_metadata(src, server)? { + None => return Ok(None), + Some(res) => res, + }; // not enough data if src.len() < idx + length { @@ -112,6 +102,13 @@ impl Parser { // remove prefix src.advance(idx); + // check for max allowed size + if length > max_size { + // drop the payload + src.advance(length); + return Err(ProtocolError::Overflow); + } + // no need for body if length == 0 { return Ok(Some((finished, opcode, None))); @@ -316,7 +313,7 @@ mod tests { #[test] fn test_parse_frame_no_mask() { let mut buf = BytesMut::from(&[0b0000_0001u8, 0b0000_0001u8][..]); - buf.extend(&[1u8]); + buf.extend([1u8]); assert!(Parser::parse(&mut buf, true, 1024).is_err()); @@ -329,7 +326,7 @@ mod tests { #[test] fn test_parse_frame_max_size() { let mut buf = BytesMut::from(&[0b0000_0001u8, 0b0000_0010u8][..]); - buf.extend(&[1u8, 1u8]); + buf.extend([1u8, 1u8]); assert!(Parser::parse(&mut buf, true, 1).is_err()); @@ -339,6 +336,30 @@ mod tests { } } + #[test] + fn test_parse_frame_max_size_recoverability() { + let mut buf = BytesMut::new(); + // The first text frame with length == 2, payload doesn't matter. + buf.extend([0b0000_0001u8, 0b0000_0010u8, 0b0000_0000u8, 0b0000_0000u8]); + // Next binary frame with length == 2 and payload == `[0x1111_1111u8, 0x1111_1111u8]`. + buf.extend([0b0000_0010u8, 0b0000_0010u8, 0b1111_1111u8, 0b1111_1111u8]); + + assert_eq!(buf.len(), 8); + assert!(matches!( + Parser::parse(&mut buf, false, 1), + Err(ProtocolError::Overflow) + )); + assert_eq!(buf.len(), 4); + let frame = extract(Parser::parse(&mut buf, false, 2)); + assert!(!frame.finished); + assert_eq!(frame.opcode, OpCode::Binary); + assert_eq!( + frame.payload, + Bytes::from(vec![0b1111_1111u8, 0b1111_1111u8]) + ); + assert_eq!(buf.len(), 0); + } + #[test] fn test_ping_frame() { let mut buf = BytesMut::new(); diff --git a/actix-http/src/ws/mod.rs b/actix-http/src/ws/mod.rs index 568d801a2..75d4ca628 100644 --- a/actix-http/src/ws/mod.rs +++ b/actix-http/src/ws/mod.rs @@ -67,7 +67,7 @@ pub enum ProtocolError { } /// WebSocket handshake errors -#[derive(Debug, Clone, Copy, PartialEq, Display, Error)] +#[derive(Debug, Clone, Copy, PartialEq, Eq, Display, Error)] pub enum HandshakeError { /// Only get method is allowed. #[display(fmt = "Method not allowed.")] diff --git a/actix-http/src/ws/proto.rs b/actix-http/src/ws/proto.rs index 01fe9dd3c..7222168b7 100644 --- a/actix-http/src/ws/proto.rs +++ b/actix-http/src/ws/proto.rs @@ -244,7 +244,7 @@ pub fn hash_key(key: &[u8]) -> [u8; 28] { }; let mut hash_b64 = [0; 28]; - let n = base64::encode_config_slice(&hash, base64::STANDARD, &mut hash_b64); + let n = base64::encode_config_slice(hash, base64::STANDARD, &mut hash_b64); assert_eq!(n, 28); hash_b64 diff --git a/actix-http/tests/test_openssl.rs b/actix-http/tests/test_openssl.rs index 35321ac98..b97b2e45b 100644 --- a/actix-http/tests/test_openssl.rs +++ b/actix-http/tests/test_openssl.rs @@ -2,13 +2,13 @@ extern crate tls_openssl as openssl; -use std::{convert::Infallible, io}; +use std::{convert::Infallible, io, time::Duration}; use actix_http::{ body::{BodyStream, BoxBody, SizedStream}, error::PayloadError, header::{self, HeaderValue}, - Error, HttpService, Method, Request, Response, StatusCode, Version, + Error, HttpService, Method, Request, Response, StatusCode, TlsAcceptorConfig, Version, }; use actix_http_test::test_server; use actix_service::{fn_service, ServiceFactoryExt}; @@ -89,7 +89,10 @@ async fn h2_1() -> io::Result<()> { assert_eq!(req.version(), Version::HTTP_2); ok::<_, Error>(Response::ok()) }) - .openssl(tls_config()) + .openssl_with_config( + tls_config(), + TlsAcceptorConfig::default().handshake_timeout(Duration::from_secs(5)), + ) .map_err(|_| ()) }) .await; diff --git a/actix-http/tests/test_rustls.rs b/actix-http/tests/test_rustls.rs index 550375296..d9ff42b7d 100644 --- a/actix-http/tests/test_rustls.rs +++ b/actix-http/tests/test_rustls.rs @@ -8,13 +8,14 @@ use std::{ net::{SocketAddr, TcpStream as StdTcpStream}, sync::Arc, task::Poll, + time::Duration, }; use actix_http::{ body::{BodyStream, BoxBody, SizedStream}, error::PayloadError, header::{self, HeaderName, HeaderValue}, - Error, HttpService, Method, Request, Response, StatusCode, Version, + Error, HttpService, Method, Request, Response, StatusCode, TlsAcceptorConfig, Version, }; use actix_http_test::test_server; use actix_rt::pin; @@ -40,7 +41,7 @@ where let body = stream.as_mut(); match ready!(body.poll_next(cx)) { - Some(Ok(bytes)) => buf.extend_from_slice(&*bytes), + Some(Ok(bytes)) => buf.extend_from_slice(&bytes), None => return Poll::Ready(Ok(())), Some(Err(err)) => return Poll::Ready(Err(err)), } @@ -160,7 +161,10 @@ async fn h2_1() -> io::Result<()> { assert_eq!(req.version(), Version::HTTP_2); ok::<_, Error>(Response::ok()) }) - .rustls(tls_config()) + .rustls_with_config( + tls_config(), + TlsAcceptorConfig::default().handshake_timeout(Duration::from_secs(5)), + ) }) .await; diff --git a/actix-multipart/CHANGES.md b/actix-multipart/CHANGES.md index 11ec8a64f..655487e54 100644 --- a/actix-multipart/CHANGES.md +++ b/actix-multipart/CHANGES.md @@ -1,6 +1,10 @@ # Changes -## Unreleased - 2021-xx-xx +## Unreleased - 2022-xx-xx +- Minimum supported Rust version (MSRV) is now 1.59 due to transitive `time` dependency. +- `Field::content_type()` now returns `Option<&mime::Mime>` [#2880] + +[#2880]: https://github.com/actix/actix-web/pull/2880 ## 0.4.0 - 2022-02-25 diff --git a/actix-multipart/Cargo.toml b/actix-multipart/Cargo.toml index e93e22941..32ea49a24 100644 --- a/actix-multipart/Cargo.toml +++ b/actix-multipart/Cargo.toml @@ -15,7 +15,7 @@ path = "src/lib.rs" [dependencies] actix-utils = "3" -actix-web = { version = "4.0.0", default-features = false } +actix-web = { version = "4", default-features = false } bytes = "1" derive_more = "0.99.5" @@ -28,7 +28,7 @@ twoway = "0.2" [dev-dependencies] actix-rt = "2.2" -actix-http = "3.0.0" +actix-http = "3" futures-util = { version = "0.3.7", default-features = false, features = ["alloc"] } tokio = { version = "1.8.4", features = ["sync"] } tokio-stream = "0.1" diff --git a/actix-multipart/README.md b/actix-multipart/README.md index 59b9651f1..21999716c 100644 --- a/actix-multipart/README.md +++ b/actix-multipart/README.md @@ -4,7 +4,7 @@ [![crates.io](https://img.shields.io/crates/v/actix-multipart?label=latest)](https://crates.io/crates/actix-multipart) [![Documentation](https://docs.rs/actix-multipart/badge.svg?version=0.4.0)](https://docs.rs/actix-multipart/0.4.0) -[![Version](https://img.shields.io/badge/rustc-1.54+-ab6000.svg)](https://blog.rust-lang.org/2021/05/06/Rust-1.54.0.html) +![Version](https://img.shields.io/badge/rustc-1.59+-ab6000.svg) ![MIT or Apache 2.0 licensed](https://img.shields.io/crates/l/actix-multipart.svg)
[![dependency status](https://deps.rs/crate/actix-multipart/0.4.0/status.svg)](https://deps.rs/crate/actix-multipart/0.4.0) diff --git a/actix-multipart/src/server.rs b/actix-multipart/src/server.rs index 239f7f905..c3757177f 100644 --- a/actix-multipart/src/server.rs +++ b/actix-multipart/src/server.rs @@ -289,10 +289,8 @@ impl InnerMultipart { match self.state { // read until first boundary InnerState::FirstBoundary => { - match InnerMultipart::skip_until_boundary( - &mut *payload, - &self.boundary, - )? { + match InnerMultipart::skip_until_boundary(&mut payload, &self.boundary)? + { Some(eof) => { if eof { self.state = InnerState::Eof; @@ -306,7 +304,7 @@ impl InnerMultipart { } // read boundary InnerState::Boundary => { - match InnerMultipart::read_boundary(&mut *payload, &self.boundary)? { + match InnerMultipart::read_boundary(&mut payload, &self.boundary)? { None => return Poll::Pending, Some(eof) => { if eof { @@ -323,7 +321,7 @@ impl InnerMultipart { // read field headers for next field if self.state == InnerState::Headers { - if let Some(headers) = InnerMultipart::read_headers(&mut *payload)? { + if let Some(headers) = InnerMultipart::read_headers(&mut payload)? { self.state = InnerState::Boundary; headers } else { @@ -361,17 +359,18 @@ impl InnerMultipart { return Poll::Ready(Some(Err(MultipartError::NoContentDisposition))); }; - let ct: mime::Mime = headers + let ct: Option = headers .get(&header::CONTENT_TYPE) .and_then(|ct| ct.to_str().ok()) - .and_then(|ct| ct.parse().ok()) - .unwrap_or(mime::APPLICATION_OCTET_STREAM); + .and_then(|ct| ct.parse().ok()); self.state = InnerState::Boundary; // nested multipart stream is not supported - if ct.type_() == mime::MULTIPART { - return Poll::Ready(Some(Err(MultipartError::Nested))); + if let Some(mime) = &ct { + if mime.type_() == mime::MULTIPART { + return Poll::Ready(Some(Err(MultipartError::Nested))); + } } let field = @@ -399,7 +398,7 @@ impl Drop for InnerMultipart { /// A single field in a multipart stream pub struct Field { - ct: mime::Mime, + ct: Option, cd: ContentDisposition, headers: HeaderMap, inner: Rc>, @@ -410,7 +409,7 @@ impl Field { fn new( safety: Safety, headers: HeaderMap, - ct: mime::Mime, + ct: Option, cd: ContentDisposition, inner: Rc>, ) -> Self { @@ -428,9 +427,13 @@ impl Field { &self.headers } - /// Returns a reference to the field's content (mime) type. - pub fn content_type(&self) -> &mime::Mime { - &self.ct + /// Returns a reference to the field's content (mime) type, if it is supplied by the client. + /// + /// According to [RFC 7578](https://www.rfc-editor.org/rfc/rfc7578#section-4.4), if it is not + /// present, it should default to "text/plain". Note it is the responsibility of the client to + /// provide the appropriate content type, there is no attempt to validate this by the server. + pub fn content_type(&self) -> Option<&mime::Mime> { + self.ct.as_ref() } /// Returns the field's Content-Disposition. @@ -482,7 +485,11 @@ impl Stream for Field { impl fmt::Debug for Field { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - writeln!(f, "\nField: {}", self.ct)?; + if let Some(ct) = &self.ct { + writeln!(f, "\nField: {}", ct)?; + } else { + writeln!(f, "\nField:")?; + } writeln!(f, " boundary: {}", self.inner.borrow().boundary)?; writeln!(f, " headers:")?; for (key, val) in self.headers.iter() { @@ -643,9 +650,9 @@ impl InnerField { let result = if let Some(mut payload) = self.payload.as_ref().unwrap().get_mut(s) { if !self.eof { let res = if let Some(ref mut len) = self.length { - InnerField::read_len(&mut *payload, len) + InnerField::read_len(&mut payload, len) } else { - InnerField::read_stream(&mut *payload, &self.boundary) + InnerField::read_stream(&mut payload, &self.boundary) }; match res { @@ -1024,8 +1031,8 @@ mod tests { assert_eq!(cd.disposition, DispositionType::FormData); assert_eq!(cd.parameters[0], DispositionParam::Name("file".into())); - assert_eq!(field.content_type().type_(), mime::TEXT); - assert_eq!(field.content_type().subtype(), mime::PLAIN); + assert_eq!(field.content_type().unwrap().type_(), mime::TEXT); + assert_eq!(field.content_type().unwrap().subtype(), mime::PLAIN); match field.next().await.unwrap() { Ok(chunk) => assert_eq!(chunk, "test"), @@ -1041,8 +1048,8 @@ mod tests { match multipart.next().await.unwrap() { Ok(mut field) => { - assert_eq!(field.content_type().type_(), mime::TEXT); - assert_eq!(field.content_type().subtype(), mime::PLAIN); + assert_eq!(field.content_type().unwrap().type_(), mime::TEXT); + assert_eq!(field.content_type().unwrap().subtype(), mime::PLAIN); match field.next().await { Some(Ok(chunk)) => assert_eq!(chunk, "data"), @@ -1086,8 +1093,8 @@ mod tests { assert_eq!(cd.disposition, DispositionType::FormData); assert_eq!(cd.parameters[0], DispositionParam::Name("file".into())); - assert_eq!(field.content_type().type_(), mime::TEXT); - assert_eq!(field.content_type().subtype(), mime::PLAIN); + assert_eq!(field.content_type().unwrap().type_(), mime::TEXT); + assert_eq!(field.content_type().unwrap().subtype(), mime::PLAIN); assert_eq!(get_whole_field(&mut field).await, "test"); } @@ -1096,8 +1103,8 @@ mod tests { match multipart.next().await { Some(Ok(mut field)) => { - assert_eq!(field.content_type().type_(), mime::TEXT); - assert_eq!(field.content_type().subtype(), mime::PLAIN); + assert_eq!(field.content_type().unwrap().type_(), mime::TEXT); + assert_eq!(field.content_type().unwrap().subtype(), mime::PLAIN); assert_eq!(get_whole_field(&mut field).await, "data"); } diff --git a/actix-router/CHANGES.md b/actix-router/CHANGES.md index 8e0e4f41e..51e7cbc10 100644 --- a/actix-router/CHANGES.md +++ b/actix-router/CHANGES.md @@ -1,6 +1,13 @@ # Changes -## Unreleased - 2021-xx-xx +## Unreleased - 2022-xx-xx + + +## 0.5.1 - 2022-09-19 +- Correct typo in error string for `i32` deserialization. [#2876] +- Minimum supported Rust version (MSRV) is now 1.59 due to transitive `time` dependency. + +[#2876]: https://github.com/actix/actix-web/pull/2876 ## 0.5.0 - 2022-02-22 diff --git a/actix-router/Cargo.toml b/actix-router/Cargo.toml index 6fcef125d..19d6abc62 100644 --- a/actix-router/Cargo.toml +++ b/actix-router/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "actix-router" -version = "0.5.0" +version = "0.5.1" authors = [ "Nikolay Kim ", "Ali MJ Al-Nasrawy ", @@ -21,18 +21,21 @@ default = ["http"] [dependencies] bytestring = ">=0.1.5, <2" -firestorm = "0.5" -http = { version = "0.2.3", optional = true } +http = { version = "0.2.5", optional = true } regex = "1.5" serde = "1" tracing = { version = "0.1.30", default-features = false, features = ["log"] } [dev-dependencies] criterion = { version = "0.3", features = ["html_reports"] } -firestorm = { version = "0.5", features = ["enable_system_time"] } http = "0.2.5" serde = { version = "1", features = ["derive"] } +percent-encoding = "2.1" [[bench]] name = "router" harness = false + +[[bench]] +name = "quoter" +harness = false diff --git a/actix-router/benches/quoter.rs b/actix-router/benches/quoter.rs new file mode 100644 index 000000000..c18f1620e --- /dev/null +++ b/actix-router/benches/quoter.rs @@ -0,0 +1,52 @@ +use criterion::{black_box, criterion_group, criterion_main, Criterion}; + +use std::borrow::Cow; + +fn compare_quoters(c: &mut Criterion) { + let mut group = c.benchmark_group("Compare Quoters"); + + let quoter = actix_router::Quoter::new(b"", b""); + let path_quoted = (0..=0x7f) + .map(|c| format!("%{:02X}", c)) + .collect::(); + let path_unquoted = ('\u{00}'..='\u{7f}').collect::(); + + group.bench_function("quoter_unquoted", |b| { + b.iter(|| { + for _ in 0..10 { + black_box(quoter.requote(path_unquoted.as_bytes())); + } + }); + }); + + group.bench_function("percent_encode_unquoted", |b| { + b.iter(|| { + for _ in 0..10 { + let decode = percent_encoding::percent_decode(path_unquoted.as_bytes()); + black_box(Into::>::into(decode)); + } + }); + }); + + group.bench_function("quoter_quoted", |b| { + b.iter(|| { + for _ in 0..10 { + black_box(quoter.requote(path_quoted.as_bytes())); + } + }); + }); + + group.bench_function("percent_encode_quoted", |b| { + b.iter(|| { + for _ in 0..10 { + let decode = percent_encoding::percent_decode(path_quoted.as_bytes()); + black_box(Into::>::into(decode)); + } + }); + }); + + group.finish(); +} + +criterion_group!(benches, compare_quoters); +criterion_main!(benches); diff --git a/actix-router/examples/flamegraph.rs b/actix-router/examples/flamegraph.rs deleted file mode 100644 index 798cc22d9..000000000 --- a/actix-router/examples/flamegraph.rs +++ /dev/null @@ -1,169 +0,0 @@ -macro_rules! register { - (brackets) => {{ - register!(finish => "{p1}", "{p2}", "{p3}", "{p4}") - }}; - (finish => $p1:literal, $p2:literal, $p3:literal, $p4:literal) => {{ - let arr = [ - concat!("/authorizations"), - concat!("/authorizations/", $p1), - concat!("/applications/", $p1, "/tokens/", $p2), - concat!("/events"), - concat!("/repos/", $p1, "/", $p2, "/events"), - concat!("/networks/", $p1, "/", $p2, "/events"), - concat!("/orgs/", $p1, "/events"), - concat!("/users/", $p1, "/received_events"), - concat!("/users/", $p1, "/received_events/public"), - concat!("/users/", $p1, "/events"), - concat!("/users/", $p1, "/events/public"), - concat!("/users/", $p1, "/events/orgs/", $p2), - concat!("/feeds"), - concat!("/notifications"), - concat!("/repos/", $p1, "/", $p2, "/notifications"), - concat!("/notifications/threads/", $p1), - concat!("/notifications/threads/", $p1, "/subscription"), - concat!("/repos/", $p1, "/", $p2, "/stargazers"), - concat!("/users/", $p1, "/starred"), - concat!("/user/starred"), - concat!("/user/starred/", $p1, "/", $p2), - concat!("/repos/", $p1, "/", $p2, "/subscribers"), - concat!("/users/", $p1, "/subscriptions"), - concat!("/user/subscriptions"), - concat!("/repos/", $p1, "/", $p2, "/subscription"), - concat!("/user/subscriptions/", $p1, "/", $p2), - concat!("/users/", $p1, "/gists"), - concat!("/gists"), - concat!("/gists/", $p1), - concat!("/gists/", $p1, "/star"), - concat!("/repos/", $p1, "/", $p2, "/git/blobs/", $p3), - concat!("/repos/", $p1, "/", $p2, "/git/commits/", $p3), - concat!("/repos/", $p1, "/", $p2, "/git/refs"), - concat!("/repos/", $p1, "/", $p2, "/git/tags/", $p3), - concat!("/repos/", $p1, "/", $p2, "/git/trees/", $p3), - concat!("/issues"), - concat!("/user/issues"), - concat!("/orgs/", $p1, "/issues"), - concat!("/repos/", $p1, "/", $p2, "/issues"), - concat!("/repos/", $p1, "/", $p2, "/issues/", $p3), - concat!("/repos/", $p1, "/", $p2, "/assignees"), - concat!("/repos/", $p1, "/", $p2, "/assignees/", $p3), - concat!("/repos/", $p1, "/", $p2, "/issues/", $p3, "/comments"), - concat!("/repos/", $p1, "/", $p2, "/issues/", $p3, "/events"), - concat!("/repos/", $p1, "/", $p2, "/labels"), - concat!("/repos/", $p1, "/", $p2, "/labels/", $p3), - concat!("/repos/", $p1, "/", $p2, "/issues/", $p3, "/labels"), - concat!("/repos/", $p1, "/", $p2, "/milestones/", $p3, "/labels"), - concat!("/repos/", $p1, "/", $p2, "/milestones/"), - concat!("/repos/", $p1, "/", $p2, "/milestones/", $p3), - concat!("/emojis"), - concat!("/gitignore/templates"), - concat!("/gitignore/templates/", $p1), - concat!("/meta"), - concat!("/rate_limit"), - concat!("/users/", $p1, "/orgs"), - concat!("/user/orgs"), - concat!("/orgs/", $p1), - concat!("/orgs/", $p1, "/members"), - concat!("/orgs/", $p1, "/members", $p2), - concat!("/orgs/", $p1, "/public_members"), - concat!("/orgs/", $p1, "/public_members/", $p2), - concat!("/orgs/", $p1, "/teams"), - concat!("/teams/", $p1), - concat!("/teams/", $p1, "/members"), - concat!("/teams/", $p1, "/members", $p2), - concat!("/teams/", $p1, "/repos"), - concat!("/teams/", $p1, "/repos/", $p2, "/", $p3), - concat!("/user/teams"), - concat!("/repos/", $p1, "/", $p2, "/pulls"), - concat!("/repos/", $p1, "/", $p2, "/pulls/", $p3), - concat!("/repos/", $p1, "/", $p2, "/pulls/", $p3, "/commits"), - concat!("/repos/", $p1, "/", $p2, "/pulls/", $p3, "/files"), - concat!("/repos/", $p1, "/", $p2, "/pulls/", $p3, "/merge"), - concat!("/repos/", $p1, "/", $p2, "/pulls/", $p3, "/comments"), - concat!("/user/repos"), - concat!("/users/", $p1, "/repos"), - concat!("/orgs/", $p1, "/repos"), - concat!("/repositories"), - concat!("/repos/", $p1, "/", $p2), - concat!("/repos/", $p1, "/", $p2, "/contributors"), - concat!("/repos/", $p1, "/", $p2, "/languages"), - concat!("/repos/", $p1, "/", $p2, "/teams"), - concat!("/repos/", $p1, "/", $p2, "/tags"), - concat!("/repos/", $p1, "/", $p2, "/branches"), - concat!("/repos/", $p1, "/", $p2, "/branches/", $p3), - concat!("/repos/", $p1, "/", $p2, "/collaborators"), - concat!("/repos/", $p1, "/", $p2, "/collaborators/", $p3), - concat!("/repos/", $p1, "/", $p2, "/comments"), - concat!("/repos/", $p1, "/", $p2, "/commits/", $p3, "/comments"), - concat!("/repos/", $p1, "/", $p2, "/commits"), - concat!("/repos/", $p1, "/", $p2, "/commits/", $p3), - concat!("/repos/", $p1, "/", $p2, "/readme"), - concat!("/repos/", $p1, "/", $p2, "/keys"), - concat!("/repos/", $p1, "/", $p2, "/keys", $p3), - concat!("/repos/", $p1, "/", $p2, "/downloads"), - concat!("/repos/", $p1, "/", $p2, "/downloads", $p3), - concat!("/repos/", $p1, "/", $p2, "/forks"), - concat!("/repos/", $p1, "/", $p2, "/hooks"), - concat!("/repos/", $p1, "/", $p2, "/hooks", $p3), - concat!("/repos/", $p1, "/", $p2, "/releases"), - concat!("/repos/", $p1, "/", $p2, "/releases/", $p3), - concat!("/repos/", $p1, "/", $p2, "/releases/", $p3, "/assets"), - concat!("/repos/", $p1, "/", $p2, "/stats/contributors"), - concat!("/repos/", $p1, "/", $p2, "/stats/commit_activity"), - concat!("/repos/", $p1, "/", $p2, "/stats/code_frequency"), - concat!("/repos/", $p1, "/", $p2, "/stats/participation"), - concat!("/repos/", $p1, "/", $p2, "/stats/punch_card"), - concat!("/repos/", $p1, "/", $p2, "/statuses/", $p3), - concat!("/search/repositories"), - concat!("/search/code"), - concat!("/search/issues"), - concat!("/search/users"), - concat!("/legacy/issues/search/", $p1, "/", $p2, "/", $p3, "/", $p4), - concat!("/legacy/repos/search/", $p1), - concat!("/legacy/user/search/", $p1), - concat!("/legacy/user/email/", $p1), - concat!("/users/", $p1), - concat!("/user"), - concat!("/users"), - concat!("/user/emails"), - concat!("/users/", $p1, "/followers"), - concat!("/user/followers"), - concat!("/users/", $p1, "/following"), - concat!("/user/following"), - concat!("/user/following/", $p1), - concat!("/users/", $p1, "/following", $p2), - concat!("/users/", $p1, "/keys"), - concat!("/user/keys"), - concat!("/user/keys/", $p1), - ]; - - arr.to_vec() - }}; -} - -static PATHS: [&str; 5] = [ - "/authorizations", - "/user/repos", - "/repos/rust-lang/rust/stargazers", - "/orgs/rust-lang/public_members/nikomatsakis", - "/repos/rust-lang/rust/releases/1.51.0", -]; - -fn main() { - let mut router = actix_router::Router::::build(); - - for route in register!(brackets) { - router.path(route, true); - } - - let actix = router.finish(); - - if firestorm::enabled() { - firestorm::bench("target", || { - for &route in &PATHS { - let mut path = actix_router::Path::new(route); - actix.recognize(&mut path).unwrap(); - } - }) - .unwrap(); - } -} diff --git a/actix-router/src/de.rs b/actix-router/src/de.rs index efafd08db..458e08930 100644 --- a/actix-router/src/de.rs +++ b/actix-router/src/de.rs @@ -7,7 +7,7 @@ use crate::path::{Path, PathIter}; use crate::{Quoter, ResourcePath}; thread_local! { - static FULL_QUOTER: Quoter = Quoter::new(b"+/%", b""); + static FULL_QUOTER: Quoter = Quoter::new(b"", b""); } macro_rules! unsupported_type { @@ -293,7 +293,7 @@ impl<'de> Deserializer<'de> for Value<'de> { parse_value!(deserialize_bool, visit_bool, "bool"); parse_value!(deserialize_i8, visit_i8, "i8"); parse_value!(deserialize_i16, visit_i16, "i16"); - parse_value!(deserialize_i32, visit_i32, "i16"); + parse_value!(deserialize_i32, visit_i32, "i32"); parse_value!(deserialize_i64, visit_i64, "i64"); parse_value!(deserialize_u8, visit_u8, "u8"); parse_value!(deserialize_u16, visit_u16, "u16"); diff --git a/actix-router/src/path.rs b/actix-router/src/path.rs index dfb645d72..34dabcfbe 100644 --- a/actix-router/src/path.rs +++ b/actix-router/src/path.rs @@ -1,7 +1,6 @@ use std::borrow::Cow; use std::ops::{DerefMut, Index}; -use firestorm::profile_method; use serde::de; use crate::{de::PathDeserializer, Resource, ResourcePath}; @@ -52,7 +51,6 @@ impl Path { /// Returns full path as a string. #[inline] pub fn as_str(&self) -> &str { - profile_method!(as_str); self.path.path() } @@ -61,7 +59,6 @@ impl Path { /// Returns empty string if no more is to be processed. #[inline] pub fn unprocessed(&self) -> &str { - profile_method!(unprocessed); // clamp skip to path length let skip = (self.skip as usize).min(self.as_str().len()); &self.path.path()[skip..] @@ -72,8 +69,6 @@ impl Path { #[deprecated(since = "0.6.0", note = "Use `.as_str()` or `.unprocessed()`.")] #[inline] pub fn path(&self) -> &str { - profile_method!(path); - let skip = self.skip as usize; let path = self.path.path(); if skip <= path.len() { @@ -86,8 +81,6 @@ impl Path { /// Set new path. #[inline] pub fn set(&mut self, path: T) { - profile_method!(set); - self.skip = 0; self.path = path; self.segments.clear(); @@ -96,8 +89,6 @@ impl Path { /// Reset state. #[inline] pub fn reset(&mut self) { - profile_method!(reset); - self.skip = 0; self.segments.clear(); } @@ -105,13 +96,10 @@ impl Path { /// Skip first `n` chars in path. #[inline] pub fn skip(&mut self, n: u16) { - profile_method!(skip); self.skip += n; } pub(crate) fn add(&mut self, name: impl Into>, value: PathItem) { - profile_method!(add); - match value { PathItem::Static(s) => self.segments.push((name.into(), PathItem::Static(s))), PathItem::Segment(begin, end) => self.segments.push(( @@ -127,8 +115,6 @@ impl Path { name: impl Into>, value: impl Into>, ) { - profile_method!(add_static); - self.segments .push((name.into(), PathItem::Static(value.into()))); } @@ -147,8 +133,6 @@ impl Path { /// Get matched parameter by name without type conversion pub fn get(&self, name: &str) -> Option<&str> { - profile_method!(get); - for (seg_name, val) in self.segments.iter() { if name == seg_name { return match val { @@ -167,8 +151,6 @@ impl Path { /// /// If keyed parameter is not available empty string is used as default value. pub fn query(&self, key: &str) -> &str { - profile_method!(query); - if let Some(s) = self.get(key) { s } else { @@ -186,7 +168,6 @@ impl Path { /// Try to deserialize matching parameters to a specified type `U` pub fn load<'de, U: serde::Deserialize<'de>>(&'de self) -> Result { - profile_method!(load); de::Deserialize::deserialize(PathDeserializer::new(self)) } } @@ -261,6 +242,7 @@ mod tests { use super::*; + #[allow(clippy::needless_borrow)] #[test] fn deref_impls() { let mut foo = Path::new("/foo"); diff --git a/actix-router/src/quoter.rs b/actix-router/src/quoter.rs index 8a1e99e1d..6c929d3ac 100644 --- a/actix-router/src/quoter.rs +++ b/actix-router/src/quoter.rs @@ -1,132 +1,89 @@ -#[allow(dead_code)] -const GEN_DELIMS: &[u8] = b":/?#[]@"; - -#[allow(dead_code)] -const SUB_DELIMS_WITHOUT_QS: &[u8] = b"!$'()*,"; - -#[allow(dead_code)] -const SUB_DELIMS: &[u8] = b"!$'()*,+?=;"; - -#[allow(dead_code)] -const RESERVED: &[u8] = b":/?#[]@!$'()*,+?=;"; - -#[allow(dead_code)] -const UNRESERVED: &[u8] = b"abcdefghijklmnopqrstuvwxyz - ABCDEFGHIJKLMNOPQRSTUVWXYZ - 1234567890 - -._~"; - -const ALLOWED: &[u8] = b"abcdefghijklmnopqrstuvwxyz - ABCDEFGHIJKLMNOPQRSTUVWXYZ - 1234567890 - -._~ - !$'()*,"; - -const QS: &[u8] = b"+&=;b"; - -/// A quoter +/// Partial percent-decoding. +/// +/// Performs percent-decoding on a slice but can selectively skip decoding certain sequences. +/// +/// # Examples +/// ``` +/// # use actix_router::Quoter; +/// // + is set as a protected character and will not be decoded... +/// let q = Quoter::new(&[], b"+"); +/// +/// // ...but the other encoded characters (like the hyphen below) will. +/// assert_eq!(q.requote(b"/a%2Db%2Bc").unwrap(), b"/a-b%2Bc"); +/// ``` pub struct Quoter { - /// Simple bit-map of safe values in the 0-127 ASCII range. - safe_table: [u8; 16], - /// Simple bit-map of protected values in the 0-127 ASCII range. - protected_table: [u8; 16], + protected_table: AsciiBitmap, } impl Quoter { - pub fn new(safe: &[u8], protected: &[u8]) -> Quoter { - let mut quoter = Quoter { - safe_table: [0; 16], - protected_table: [0; 16], - }; - - // prepare safe table - for ch in 0..128 { - if ALLOWED.contains(&ch) { - set_bit(&mut quoter.safe_table, ch); - } - - if QS.contains(&ch) { - set_bit(&mut quoter.safe_table, ch); - } - } - - for &ch in safe { - set_bit(&mut quoter.safe_table, ch) - } + /// Constructs a new `Quoter` instance given a set of protected ASCII bytes. + /// + /// The first argument is ignored but is kept for backward compatibility. + /// + /// # Panics + /// Panics if any of the `protected` bytes are not in the 0-127 ASCII range. + pub fn new(_: &[u8], protected: &[u8]) -> Quoter { + let mut protected_table = AsciiBitmap::default(); // prepare protected table for &ch in protected { - set_bit(&mut quoter.safe_table, ch); - set_bit(&mut quoter.protected_table, ch); + protected_table.set_bit(ch); } - quoter + Quoter { protected_table } } - /// Decodes safe percent-encoded sequences from `val`. - /// - /// Returns `None` when no modification to the original byte string was required. - /// - /// Non-ASCII bytes are accepted as valid input. - /// - /// Behavior for invalid/incomplete percent-encoding sequences is unspecified and may include - /// removing the invalid sequence from the output or passing it as-is. - pub fn requote(&self, val: &[u8]) -> Option> { - let mut has_pct = 0; - let mut pct = [b'%', 0, 0]; - let mut idx = 0; - let mut cloned: Option> = None; - - let len = val.len(); - - while idx < len { - let ch = val[idx]; - - if has_pct != 0 { - pct[has_pct] = val[idx]; - has_pct += 1; - - if has_pct == 3 { - has_pct = 0; - let buf = cloned.as_mut().unwrap(); - - if let Some(ch) = hex_pair_to_char(pct[1], pct[2]) { - if ch < 128 { - if bit_at(&self.protected_table, ch) { - buf.extend_from_slice(&pct); - idx += 1; - continue; - } - - if bit_at(&self.safe_table, ch) { - buf.push(ch); - idx += 1; - continue; - } - } - - buf.push(ch); - } else { - buf.extend_from_slice(&pct[..]); - } + /// Decodes the next escape sequence, if any, and advances `val`. + #[inline(always)] + fn decode_next<'a>(&self, val: &mut &'a [u8]) -> Option<(&'a [u8], u8)> { + for i in 0..val.len() { + if let (prev, [b'%', p1, p2, rem @ ..]) = val.split_at(i) { + if let Some(ch) = hex_pair_to_char(*p1, *p2) + // ignore protected ascii bytes + .filter(|&ch| !(ch < 128 && self.protected_table.bit_at(ch))) + { + *val = rem; + return Some((prev, ch)); } - } else if ch == b'%' { - has_pct = 1; - - if cloned.is_none() { - let mut c = Vec::with_capacity(len); - c.extend_from_slice(&val[..idx]); - cloned = Some(c); - } - } else if let Some(ref mut cloned) = cloned { - cloned.push(ch) } - - idx += 1; } - cloned + None + } + + /// Partially percent-decodes the given bytes. + /// + /// Escape sequences of the protected set are *not* decoded. + /// + /// Returns `None` when no modification to the original bytes was required. + /// + /// Invalid/incomplete percent-encoding sequences are passed unmodified. + pub fn requote(&self, val: &[u8]) -> Option> { + let mut remaining = val; + + // early return indicates that no percent-encoded sequences exist and we can skip allocation + let (pre, decoded_char) = self.decode_next(&mut remaining)?; + + // decoded output will always be shorter than the input + let mut decoded = Vec::::with_capacity(val.len()); + + // push first segment and decoded char + decoded.extend_from_slice(pre); + decoded.push(decoded_char); + + // decode and push rest of segments and decoded chars + while let Some((prev, ch)) = self.decode_next(&mut remaining) { + // this ugly conditional achieves +50% perf in cases where this is a tight loop. + if !prev.is_empty() { + decoded.extend_from_slice(prev); + } + decoded.push(ch); + } + + decoded.extend_from_slice(remaining); + + Some(decoded) } pub(crate) fn requote_str_lossy(&self, val: &str) -> Option { @@ -135,24 +92,6 @@ impl Quoter { } } -/// Converts an ASCII character in the hex-encoded set (`0-9`, `A-F`, `a-f`) to its integer -/// representation from `0x0`–`0xF`. -/// -/// - `0x30 ('0') => 0x0` -/// - `0x39 ('9') => 0x9` -/// - `0x41 ('a') => 0xA` -/// - `0x61 ('A') => 0xA` -/// - `0x46 ('f') => 0xF` -/// - `0x66 ('F') => 0xF` -fn from_ascii_hex(v: u8) -> Option { - match v { - b'0'..=b'9' => Some(v - 0x30), // ord('0') == 0x30 - b'A'..=b'F' => Some(v - 0x41 + 10), // ord('A') == 0x41 - b'a'..=b'f' => Some(v - 0x61 + 10), // ord('a') == 0x61 - _ => None, - } -} - /// Decode a ASCII hex-encoded pair to an integer. /// /// Returns `None` if either portion of the decoded pair does not evaluate to a valid hex value. @@ -160,64 +99,52 @@ fn from_ascii_hex(v: u8) -> Option { /// - `0x33 ('3'), 0x30 ('0') => 0x30 ('0')` /// - `0x34 ('4'), 0x31 ('1') => 0x41 ('A')` /// - `0x36 ('6'), 0x31 ('1') => 0x61 ('a')` +#[inline(always)] fn hex_pair_to_char(d1: u8, d2: u8) -> Option { - let (d_high, d_low) = (from_ascii_hex(d1)?, from_ascii_hex(d2)?); + let d_high = char::from(d1).to_digit(16)?; + let d_low = char::from(d2).to_digit(16)?; // left shift high nibble by 4 bits - Some(d_high << 4 | d_low) + Some((d_high as u8) << 4 | (d_low as u8)) } -/// Sets bit in given bit-map to 1=true. -/// -/// # Panics -/// Panics if `ch` index is out of bounds. -fn set_bit(array: &mut [u8], ch: u8) { - array[(ch >> 3) as usize] |= 0b1 << (ch & 0b111) +#[derive(Debug, Default, Clone)] +struct AsciiBitmap { + array: [u8; 16], } -/// Returns true if bit to true in given bit-map. -/// -/// # Panics -/// Panics if `ch` index is out of bounds. -fn bit_at(array: &[u8], ch: u8) -> bool { - array[(ch >> 3) as usize] & (0b1 << (ch & 0b111)) != 0 +impl AsciiBitmap { + /// Sets bit in given bit-map to 1=true. + /// + /// # Panics + /// Panics if `ch` index is out of bounds. + fn set_bit(&mut self, ch: u8) { + self.array[(ch >> 3) as usize] |= 0b1 << (ch & 0b111) + } + + /// Returns true if bit to true in given bit-map. + /// + /// # Panics + /// Panics if `ch` index is out of bounds. + fn bit_at(&self, ch: u8) -> bool { + self.array[(ch >> 3) as usize] & (0b1 << (ch & 0b111)) != 0 + } } #[cfg(test)] mod tests { use super::*; - #[test] - fn hex_encoding() { - let hex = b"0123456789abcdefABCDEF"; - - for i in 0..256 { - let c = i as u8; - if hex.contains(&c) { - assert!(from_ascii_hex(c).is_some()) - } else { - assert!(from_ascii_hex(c).is_none()) - } - } - - let expected = [ - 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 10, 11, 12, 13, 14, 15, - ]; - for i in 0..hex.len() { - assert_eq!(from_ascii_hex(hex[i]).unwrap(), expected[i]); - } - } - #[test] fn custom_quoter() { let q = Quoter::new(b"", b"+"); assert_eq!(q.requote(b"/a%25c").unwrap(), b"/a%c"); - assert_eq!(q.requote(b"/a%2Bc").unwrap(), b"/a%2Bc"); + assert_eq!(q.requote(b"/a%2Bc"), None); let q = Quoter::new(b"%+", b"/"); assert_eq!(q.requote(b"/a%25b%2Bc").unwrap(), b"/a%b+c"); - assert_eq!(q.requote(b"/a%2fb").unwrap(), b"/a%2fb"); - assert_eq!(q.requote(b"/a%2Fb").unwrap(), b"/a%2Fb"); + assert_eq!(q.requote(b"/a%2fb"), None); + assert_eq!(q.requote(b"/a%2Fb"), None); assert_eq!(q.requote(b"/a%0Ab").unwrap(), b"/a\nb"); assert_eq!(q.requote(b"/a%FE\xffb").unwrap(), b"/a\xfe\xffb"); assert_eq!(q.requote(b"/a\xfe\xffb"), None); @@ -233,7 +160,8 @@ mod tests { #[test] fn invalid_sequences() { let q = Quoter::new(b"%+", b"/"); - assert_eq!(q.requote(b"/a%2x%2X%%").unwrap(), b"/a%2x%2X"); + assert_eq!(q.requote(b"/a%2x%2X%%"), None); + assert_eq!(q.requote(b"/a%20%2X%%").unwrap(), b"/a %2X%%"); } #[test] diff --git a/actix-router/src/resource.rs b/actix-router/src/resource.rs index 3d121f369..f198115ad 100644 --- a/actix-router/src/resource.rs +++ b/actix-router/src/resource.rs @@ -5,7 +5,6 @@ use std::{ mem, }; -use firestorm::{profile_fn, profile_method, profile_section}; use regex::{escape, Regex, RegexSet}; use tracing::error; @@ -272,7 +271,6 @@ impl ResourceDef { /// assert!(!resource.is_match("/foo")); /// ``` pub fn new(paths: T) -> Self { - profile_method!(new); Self::construct(paths, false) } @@ -300,7 +298,6 @@ impl ResourceDef { /// assert!(!resource.is_match("/foo")); /// ``` pub fn prefix(paths: T) -> Self { - profile_method!(prefix); ResourceDef::construct(paths, true) } @@ -325,7 +322,6 @@ impl ResourceDef { /// assert!(!resource.is_match("user/123")); /// ``` pub fn root_prefix(path: &str) -> Self { - profile_method!(root_prefix); ResourceDef::prefix(insert_slash(path).into_owned()) } @@ -549,8 +545,6 @@ impl ResourceDef { /// ``` #[inline] pub fn is_match(&self, path: &str) -> bool { - profile_method!(is_match); - // this function could be expressed as: // `self.find_match(path).is_some()` // but this skips some checks and uses potentially faster regex methods @@ -598,8 +592,6 @@ impl ResourceDef { /// assert_eq!(resource.find_match("/profile/1234"), Some(13)); /// ``` pub fn find_match(&self, path: &str) -> Option { - profile_method!(find_match); - match &self.pat_type { PatternType::Static(pattern) => self.static_match(pattern, path), @@ -634,7 +626,6 @@ impl ResourceDef { /// assert_eq!(path.unprocessed(), ""); /// ``` pub fn capture_match_info(&self, resource: &mut R) -> bool { - profile_method!(capture_match_info); self.capture_match_info_fn(resource, |_| true) } @@ -658,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")), /// ) /// } /// @@ -680,53 +671,35 @@ impl ResourceDef { R: Resource, F: FnOnce(&R) -> bool, { - profile_method!(capture_match_info_fn); - let mut segments = <[PathItem; MAX_DYNAMIC_SEGMENTS]>::default(); let path = resource.resource_path(); let path_str = path.unprocessed(); let (matched_len, matched_vars) = match &self.pat_type { - PatternType::Static(pattern) => { - profile_section!(pattern_static_or_prefix); - - match self.static_match(pattern, path_str) { - Some(len) => (len, None), - None => return false, - } - } + PatternType::Static(pattern) => match self.static_match(pattern, path_str) { + Some(len) => (len, None), + None => return false, + }, PatternType::Dynamic(re, names) => { - profile_section!(pattern_dynamic); - - let captures = { - profile_section!(pattern_dynamic_regex_exec); - - match re.captures(path.unprocessed()) { - Some(captures) => captures, - _ => return false, - } + let captures = match re.captures(path.unprocessed()) { + Some(captures) => captures, + _ => return false, }; - { - profile_section!(pattern_dynamic_extract_captures); - - for (no, name) in names.iter().enumerate() { - if let Some(m) = captures.name(name) { - segments[no] = PathItem::Segment(m.start() as u16, m.end() as u16); - } else { - error!("Dynamic path match but not all segments found: {}", name); - return false; - } + for (no, name) in names.iter().enumerate() { + if let Some(m) = captures.name(name) { + segments[no] = PathItem::Segment(m.start() as u16, m.end() as u16); + } else { + error!("Dynamic path match but not all segments found: {}", name); + return false; } - }; + } (captures[1].len(), Some(names)) } PatternType::DynamicSet(re, params) => { - profile_section!(pattern_dynamic_set); - let path = path.unprocessed(); let (pattern, names) = match re.matches(path).into_iter().next() { Some(idx) => ¶ms[idx], @@ -809,7 +782,6 @@ impl ResourceDef { I: IntoIterator, I::Item: AsRef, { - profile_method!(resource_path_from_iter); let mut iter = values.into_iter(); self.build_resource_path(path, |_| iter.next()) } @@ -845,7 +817,6 @@ impl ResourceDef { V: AsRef, S: BuildHasher, { - profile_method!(resource_path_from_map); self.build_resource_path(path, |name| values.get(name)) } @@ -866,8 +837,6 @@ impl ResourceDef { } fn construct(paths: T, is_prefix: bool) -> Self { - profile_method!(construct); - let patterns = paths.patterns(); let (pat_type, segments) = match &patterns { Patterns::Single(pattern) => ResourceDef::parse(pattern, is_prefix, false), @@ -926,8 +895,6 @@ impl ResourceDef { /// # Panics /// Panics if given patterns does not contain a dynamic segment. fn parse_param(pattern: &str) -> (PatternSegment, String, &str, bool) { - profile_method!(parse_param); - const DEFAULT_PATTERN: &str = "[^/]+"; const DEFAULT_PATTERN_TAIL: &str = ".*"; @@ -997,8 +964,6 @@ impl ResourceDef { is_prefix: bool, force_dynamic: bool, ) -> (PatternType, Vec) { - profile_method!(parse); - if !force_dynamic && pattern.find('{').is_none() && !pattern.ends_with('*') { // pattern is static return ( @@ -1131,8 +1096,6 @@ impl From for ResourceDef { } pub(crate) fn insert_slash(path: &str) -> Cow<'_, str> { - profile_fn!(insert_slash); - if !path.is_empty() && !path.starts_with('/') { let mut new_path = String::with_capacity(path.len() + 1); new_path.push('/'); @@ -1540,31 +1503,31 @@ mod tests { fn build_path_list() { let mut s = String::new(); let resource = ResourceDef::new("/user/{item1}/test"); - assert!(resource.resource_path_from_iter(&mut s, &mut (&["user1"]).iter())); + assert!(resource.resource_path_from_iter(&mut s, &mut ["user1"].iter())); assert_eq!(s, "/user/user1/test"); let mut s = String::new(); let resource = ResourceDef::new("/user/{item1}/{item2}/test"); - assert!(resource.resource_path_from_iter(&mut s, &mut (&["item", "item2"]).iter())); + assert!(resource.resource_path_from_iter(&mut s, &mut ["item", "item2"].iter())); assert_eq!(s, "/user/item/item2/test"); let mut s = String::new(); let resource = ResourceDef::new("/user/{item1}/{item2}"); - assert!(resource.resource_path_from_iter(&mut s, &mut (&["item", "item2"]).iter())); + assert!(resource.resource_path_from_iter(&mut s, &mut ["item", "item2"].iter())); assert_eq!(s, "/user/item/item2"); let mut s = String::new(); let resource = ResourceDef::new("/user/{item1}/{item2}/"); - assert!(resource.resource_path_from_iter(&mut s, &mut (&["item", "item2"]).iter())); + assert!(resource.resource_path_from_iter(&mut s, &mut ["item", "item2"].iter())); assert_eq!(s, "/user/item/item2/"); let mut s = String::new(); - assert!(!resource.resource_path_from_iter(&mut s, &mut (&["item"]).iter())); + assert!(!resource.resource_path_from_iter(&mut s, &mut ["item"].iter())); let mut s = String::new(); - assert!(resource.resource_path_from_iter(&mut s, &mut (&["item", "item2"]).iter())); + assert!(resource.resource_path_from_iter(&mut s, &mut ["item", "item2"].iter())); assert_eq!(s, "/user/item/item2/"); - assert!(!resource.resource_path_from_iter(&mut s, &mut (&["item"]).iter())); + assert!(!resource.resource_path_from_iter(&mut s, &mut ["item"].iter())); let mut s = String::new(); assert!(resource.resource_path_from_iter(&mut s, &mut vec!["item", "item2"].iter())); @@ -1641,10 +1604,10 @@ mod tests { let resource = ResourceDef::new("/user/{item1}*"); let mut s = String::new(); - assert!(!resource.resource_path_from_iter(&mut s, &mut (&[""; 0]).iter())); + assert!(!resource.resource_path_from_iter(&mut s, &mut [""; 0].iter())); let mut s = String::new(); - assert!(resource.resource_path_from_iter(&mut s, &mut (&["user1"]).iter())); + assert!(resource.resource_path_from_iter(&mut s, &mut ["user1"].iter())); assert_eq!(s, "/user/user1"); let mut s = String::new(); diff --git a/actix-router/src/resource_path.rs b/actix-router/src/resource_path.rs index 00eb0927c..45948aa2a 100644 --- a/actix-router/src/resource_path.rs +++ b/actix-router/src/resource_path.rs @@ -27,7 +27,7 @@ impl<'a> ResourcePath for &'a str { impl ResourcePath for bytestring::ByteString { fn path(&self) -> &str { - &*self + self } } diff --git a/actix-router/src/router.rs b/actix-router/src/router.rs index f0e598683..064c5e904 100644 --- a/actix-router/src/router.rs +++ b/actix-router/src/router.rs @@ -1,8 +1,6 @@ -use firestorm::profile_method; - use crate::{IntoPatterns, Resource, ResourceDef}; -#[derive(Debug, Copy, Clone, PartialEq)] +#[derive(Debug, Copy, Clone, PartialEq, Eq)] pub struct ResourceId(pub u16); /// Resource router. @@ -30,7 +28,6 @@ impl Router { where R: Resource, { - profile_method!(recognize); self.recognize_fn(resource, |_, _| true) } @@ -39,7 +36,6 @@ impl Router { where R: Resource, { - profile_method!(recognize_mut); self.recognize_mut_fn(resource, |_, _| true) } @@ -55,8 +51,6 @@ impl Router { R: Resource, F: FnMut(&R, &U) -> bool, { - profile_method!(recognize_checked); - for (rdef, val, ctx) in self.routes.iter() { if rdef.capture_match_info_fn(resource, |res| check(res, ctx)) { return Some((val, ResourceId(rdef.id()))); @@ -77,8 +71,6 @@ impl Router { R: Resource, F: FnMut(&R, &U) -> bool, { - profile_method!(recognize_mut_checked); - for (rdef, val, ctx) in self.routes.iter_mut() { if rdef.capture_match_info_fn(resource, |res| check(res, ctx)) { return Some((val, ResourceId(rdef.id()))); @@ -104,7 +96,6 @@ impl RouterBuilder { val: T, ctx: U, ) -> (&mut ResourceDef, &mut T, &mut U) { - profile_method!(push); self.routes.push((rdef, val, ctx)); self.routes .last_mut() @@ -131,7 +122,6 @@ where path: impl IntoPatterns, val: T, ) -> (&mut ResourceDef, &mut T, &mut U) { - profile_method!(path); self.push(ResourceDef::new(path), val, U::default()) } @@ -141,13 +131,11 @@ where prefix: impl IntoPatterns, val: T, ) -> (&mut ResourceDef, &mut T, &mut U) { - profile_method!(prefix); self.push(ResourceDef::prefix(prefix), val, U::default()) } /// Registers resource for [`ResourceDef`]. pub fn rdef(&mut self, rdef: ResourceDef, val: T) -> (&mut ResourceDef, &mut T, &mut U) { - profile_method!(rdef); self.push(rdef, val, U::default()) } } diff --git a/actix-router/src/url.rs b/actix-router/src/url.rs index e7dda3fca..8ac033861 100644 --- a/actix-router/src/url.rs +++ b/actix-router/src/url.rs @@ -3,7 +3,7 @@ use crate::ResourcePath; use crate::Quoter; thread_local! { - static DEFAULT_QUOTER: Quoter = Quoter::new(b"@:", b"%/+"); + static DEFAULT_QUOTER: Quoter = Quoter::new(b"", b"%/+"); } #[derive(Debug, Clone, Default)] diff --git a/actix-test/CHANGES.md b/actix-test/CHANGES.md index 13e75c01a..c8fe54203 100644 --- a/actix-test/CHANGES.md +++ b/actix-test/CHANGES.md @@ -1,6 +1,11 @@ # Changes -## Unreleased - 2021-xx-xx +## Unreleased - 2022-xx-xx +- Minimum supported Rust version (MSRV) is now 1.59 due to transitive `time` dependency. + + +## 0.1.0 - 2022-07-24 +- Minimum supported Rust version (MSRV) is now 1.57 due to transitive `time` dependency. ## 0.1.0-beta.13 - 2022-02-16 diff --git a/actix-test/Cargo.toml b/actix-test/Cargo.toml index 9938be67d..eaea15d47 100644 --- a/actix-test/Cargo.toml +++ b/actix-test/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "actix-test" -version = "0.1.0-beta.13" +version = "0.1.0" authors = [ "Nikolay Kim ", "Rob Ede ", @@ -30,7 +30,7 @@ openssl = ["tls-openssl", "actix-http/openssl", "awc/openssl"] [dependencies] actix-codec = "0.5" actix-http = "3" -actix-http-test = "3.0.0-beta.13" +actix-http-test = "3" actix-rt = "2.1" actix-service = "2" actix-utils = "3" diff --git a/actix-web-actors/CHANGES.md b/actix-web-actors/CHANGES.md index b4844bfa6..33d4712f8 100644 --- a/actix-web-actors/CHANGES.md +++ b/actix-web-actors/CHANGES.md @@ -1,6 +1,7 @@ # Changes -## Unreleased - 2021-xx-xx +## Unreleased - 2022-xx-xx +- Minimum supported Rust version (MSRV) is now 1.57 due to transitive `time` dependency. ## 4.1.0 - 2022-03-02 diff --git a/actix-web-actors/Cargo.toml b/actix-web-actors/Cargo.toml index c939f6ab5..8222fc864 100644 --- a/actix-web-actors/Cargo.toml +++ b/actix-web-actors/Cargo.toml @@ -27,8 +27,11 @@ tokio = { version = "1.13.1", features = ["sync"] } [dev-dependencies] actix-rt = "2.2" -actix-test = "0.1.0-beta.13" +actix-test = "0.1" awc = { version = "3", default-features = false } +actix-web = { version = "4", features = ["macros"] } + +mime = "0.3" env_logger = "0.9" futures-util = { version = "0.3.7", default-features = false } diff --git a/actix-web-actors/README.md b/actix-web-actors/README.md index 357154a86..a0578994c 100644 --- a/actix-web-actors/README.md +++ b/actix-web-actors/README.md @@ -4,7 +4,7 @@ [![crates.io](https://img.shields.io/crates/v/actix-web-actors?label=latest)](https://crates.io/crates/actix-web-actors) [![Documentation](https://docs.rs/actix-web-actors/badge.svg?version=4.1.0)](https://docs.rs/actix-web-actors/4.1.0) -[![Version](https://img.shields.io/badge/rustc-1.54+-ab6000.svg)](https://blog.rust-lang.org/2021/05/06/Rust-1.54.0.html) +![Version](https://img.shields.io/badge/rustc-1.59+-ab6000.svg) ![License](https://img.shields.io/crates/l/actix-web-actors.svg)
[![dependency status](https://deps.rs/crate/actix-web-actors/4.1.0/status.svg)](https://deps.rs/crate/actix-web-actors/4.1.0) diff --git a/actix-web-actors/src/context.rs b/actix-web-actors/src/context.rs index d83969ff7..f7b11c780 100644 --- a/actix-web-actors/src/context.rs +++ b/actix-web-actors/src/context.rs @@ -14,6 +14,58 @@ use futures_core::Stream; use tokio::sync::oneshot::Sender; /// Execution context for HTTP actors +/// +/// # Example +/// +/// A demonstration of [server-sent events](https://developer.mozilla.org/docs/Web/API/Server-sent_events) using actors: +/// +/// ```no_run +/// use std::time::Duration; +/// +/// use actix::{Actor, AsyncContext}; +/// use actix_web::{get, http::header, App, HttpResponse, HttpServer}; +/// use actix_web_actors::HttpContext; +/// use bytes::Bytes; +/// +/// struct MyActor { +/// count: usize, +/// } +/// +/// impl Actor for MyActor { +/// type Context = HttpContext; +/// +/// fn started(&mut self, ctx: &mut Self::Context) { +/// ctx.run_later(Duration::from_millis(100), Self::write); +/// } +/// } +/// +/// impl MyActor { +/// fn write(&mut self, ctx: &mut HttpContext) { +/// self.count += 1; +/// if self.count > 3 { +/// ctx.write_eof() +/// } else { +/// ctx.write(Bytes::from(format!("event: count\ndata: {}\n\n", self.count))); +/// ctx.run_later(Duration::from_millis(100), Self::write); +/// } +/// } +/// } +/// +/// #[get("/")] +/// async fn index() -> HttpResponse { +/// HttpResponse::Ok() +/// .insert_header(header::ContentType(mime::TEXT_EVENT_STREAM)) +/// .streaming(HttpContext::create(MyActor { count: 0 })) +/// } +/// +/// #[actix_web::main] +/// async fn main() -> std::io::Result<()> { +/// HttpServer::new(|| App::new().service(index)) +/// .bind(("127.0.0.1", 8080))? +/// .run() +/// .await +/// } +/// ``` pub struct HttpContext where A: Actor>, @@ -210,7 +262,7 @@ mod tests { type Context = HttpContext; fn started(&mut self, ctx: &mut Self::Context) { - ctx.run_later(Duration::from_millis(100), |slf, ctx| slf.write(ctx)); + ctx.run_later(Duration::from_millis(100), Self::write); } } @@ -221,7 +273,7 @@ mod tests { ctx.write_eof() } else { ctx.write(Bytes::from(format!("LINE-{}", self.count))); - ctx.run_later(Duration::from_millis(100), |slf, ctx| slf.write(ctx)); + ctx.run_later(Duration::from_millis(100), Self::write); } } } diff --git a/actix-web-actors/src/lib.rs b/actix-web-actors/src/lib.rs index 70c957020..106bc5202 100644 --- a/actix-web-actors/src/lib.rs +++ b/actix-web-actors/src/lib.rs @@ -1,4 +1,59 @@ //! Actix actors support for Actix Web. +//! +//! # Examples +//! +//! ```no_run +//! use actix::{Actor, StreamHandler}; +//! use actix_web::{get, web, App, Error, HttpRequest, HttpResponse, HttpServer}; +//! use actix_web_actors::ws; +//! +//! /// Define Websocket actor +//! struct MyWs; +//! +//! impl Actor for MyWs { +//! type Context = ws::WebsocketContext; +//! } +//! +//! /// Handler for ws::Message message +//! impl StreamHandler> for MyWs { +//! fn handle(&mut self, msg: Result, ctx: &mut Self::Context) { +//! match msg { +//! Ok(ws::Message::Ping(msg)) => ctx.pong(&msg), +//! Ok(ws::Message::Text(text)) => ctx.text(text), +//! Ok(ws::Message::Binary(bin)) => ctx.binary(bin), +//! _ => (), +//! } +//! } +//! } +//! +//! #[get("/ws")] +//! async fn index(req: HttpRequest, stream: web::Payload) -> Result { +//! ws::start(MyWs, &req, stream) +//! } +//! +//! #[actix_web::main] +//! async fn main() -> std::io::Result<()> { +//! HttpServer::new(|| App::new().service(index)) +//! .bind(("127.0.0.1", 8080))? +//! .run() +//! .await +//! } +//! ``` +//! +//! # Documentation & Community Resources +//! In addition to this API documentation, several other resources are available: +//! +//! * [Website & User Guide](https://actix.rs/) +//! * [Documentation for `actix_web`](actix_web) +//! * [Examples Repository](https://github.com/actix/examples) +//! * [Community Chat on Discord](https://discord.gg/NWpN5mmg3x) +//! +//! To get started navigating the API docs, you may consider looking at the following pages first: +//! +//! * [`ws`]: This module provides actor support for WebSockets. +//! +//! * [`HttpContext`]: This struct provides actor support for streaming HTTP responses. +//! #![deny(rust_2018_idioms, nonstandard_style)] #![warn(future_incompatible)] diff --git a/actix-web-actors/src/ws.rs b/actix-web-actors/src/ws.rs index 6fde10192..9a4880159 100644 --- a/actix-web-actors/src/ws.rs +++ b/actix-web-actors/src/ws.rs @@ -1,4 +1,60 @@ //! Websocket integration. +//! +//! # Examples +//! +//! ```no_run +//! use actix::{Actor, StreamHandler}; +//! use actix_web::{get, web, App, Error, HttpRequest, HttpResponse, HttpServer}; +//! use actix_web_actors::ws; +//! +//! /// Define Websocket actor +//! struct MyWs; +//! +//! impl Actor for MyWs { +//! type Context = ws::WebsocketContext; +//! } +//! +//! /// Handler for ws::Message message +//! impl StreamHandler> for MyWs { +//! fn handle(&mut self, msg: Result, ctx: &mut Self::Context) { +//! match msg { +//! Ok(ws::Message::Ping(msg)) => ctx.pong(&msg), +//! Ok(ws::Message::Text(text)) => ctx.text(text), +//! Ok(ws::Message::Binary(bin)) => ctx.binary(bin), +//! _ => (), +//! } +//! } +//! } +//! +//! #[get("/ws")] +//! async fn websocket(req: HttpRequest, stream: web::Payload) -> Result { +//! ws::start(MyWs, &req, stream) +//! } +//! +//! const MAX_FRAME_SIZE: usize = 16_384; // 16KiB +//! +//! #[get("/custom-ws")] +//! async fn custom_websocket(req: HttpRequest, stream: web::Payload) -> Result { +//! // Create a Websocket session with a specific max frame size, and protocols. +//! ws::WsResponseBuilder::new(MyWs, &req, stream) +//! .frame_size(MAX_FRAME_SIZE) +//! .protocols(&["A", "B"]) +//! .start() +//! } +//! +//! #[actix_web::main] +//! async fn main() -> std::io::Result<()> { +//! HttpServer::new(|| { +//! App::new() +//! .service(websocket) +//! .service(custom_websocket) +//! }) +//! .bind(("127.0.0.1", 8080))? +//! .run() +//! .await +//! } +//! ``` +//! use std::{ collections::VecDeque, @@ -41,20 +97,51 @@ use tokio::sync::oneshot; /// /// # Examples /// -/// Create a Websocket session response with default configuration. -/// ```ignore -/// WsResponseBuilder::new(WsActor, &req, stream).start() -/// ``` +/// ```no_run +/// # use actix::{Actor, StreamHandler}; +/// # use actix_web::{get, web, App, Error, HttpRequest, HttpResponse, HttpServer}; +/// # use actix_web_actors::ws; +/// # +/// # struct MyWs; +/// # +/// # impl Actor for MyWs { +/// # type Context = ws::WebsocketContext; +/// # } +/// # +/// # /// Handler for ws::Message message +/// # impl StreamHandler> for MyWs { +/// # fn handle(&mut self, msg: Result, ctx: &mut Self::Context) {} +/// # } +/// # +/// #[get("/ws")] +/// async fn websocket(req: HttpRequest, stream: web::Payload) -> Result { +/// ws::WsResponseBuilder::new(MyWs, &req, stream).start() +/// } /// -/// Create a Websocket session with a specific max frame size, [`Codec`], and protocols. -/// ```ignore /// const MAX_FRAME_SIZE: usize = 16_384; // 16KiB /// -/// ws::WsResponseBuilder::new(WsActor, &req, stream) -/// .codec(Codec::new()) -/// .protocols(&["A", "B"]) -/// .frame_size(MAX_FRAME_SIZE) -/// .start() +/// #[get("/custom-ws")] +/// async fn custom_websocket(req: HttpRequest, stream: web::Payload) -> Result { +/// // Create a Websocket session with a specific max frame size, codec, and protocols. +/// ws::WsResponseBuilder::new(MyWs, &req, stream) +/// .codec(actix_http::ws::Codec::new()) +/// // This will overwrite the codec's max frame-size +/// .frame_size(MAX_FRAME_SIZE) +/// .protocols(&["A", "B"]) +/// .start() +/// } +/// # +/// # #[actix_web::main] +/// # async fn main() -> std::io::Result<()> { +/// # HttpServer::new(|| { +/// # App::new() +/// # .service(websocket) +/// # .service(custom_websocket) +/// # }) +/// # .bind(("127.0.0.1", 8080))? +/// # .run() +/// # .await +/// # } /// ``` pub struct WsResponseBuilder<'a, A, T> where diff --git a/actix-web-codegen/CHANGES.md b/actix-web-codegen/CHANGES.md index 8ee787c0a..cb37bfdb0 100644 --- a/actix-web-codegen/CHANGES.md +++ b/actix-web-codegen/CHANGES.md @@ -1,6 +1,20 @@ # Changes -## Unreleased - 2021-xx-xx +## Unreleased - 2022-xx-xx + + +## 4.1.0 - 2022-09-11 +- Add `#[routes]` macro to support multiple paths for one handler. [#2718] +- Minimum supported Rust version (MSRV) is now 1.59 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] +- Minimum supported Rust version (MSRV) is now 1.56 due to transitive `hashbrown` dependency. + +[#2771]: https://github.com/actix/actix-web/pull/2771 ## 4.0.0 - 2022-02-24 diff --git a/actix-web-codegen/Cargo.toml b/actix-web-codegen/Cargo.toml index 0d8b86459..2477364a6 100644 --- a/actix-web-codegen/Cargo.toml +++ b/actix-web-codegen/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "actix-web-codegen" -version = "4.0.0" +version = "4.1.0" description = "Routing and runtime macros for Actix Web" homepage = "https://actix.rs" repository = "https://github.com/actix/actix-web.git" @@ -15,17 +15,17 @@ edition = "2018" proc-macro = true [dependencies] -actix-router = "0.5.0" +actix-router = "0.5" 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" actix-rt = "2.2" -actix-test = "0.1.0-beta.13" -actix-utils = "3.0.0" -actix-web = "4.0.0" +actix-test = "0.1" +actix-utils = "3" +actix-web = "4" futures-core = { version = "0.3.7", default-features = false, features = ["alloc"] } trybuild = "1" diff --git a/actix-web-codegen/README.md b/actix-web-codegen/README.md index 439beadb4..821236e47 100644 --- a/actix-web-codegen/README.md +++ b/actix-web-codegen/README.md @@ -3,11 +3,11 @@ > Routing and runtime macros for Actix Web. [![crates.io](https://img.shields.io/crates/v/actix-web-codegen?label=latest)](https://crates.io/crates/actix-web-codegen) -[![Documentation](https://docs.rs/actix-web-codegen/badge.svg?version=4.0.0)](https://docs.rs/actix-web-codegen/4.0.0) -[![Version](https://img.shields.io/badge/rustc-1.54+-ab6000.svg)](https://blog.rust-lang.org/2021/05/06/Rust-1.54.0.html) +[![Documentation](https://docs.rs/actix-web-codegen/badge.svg?version=4.1.0)](https://docs.rs/actix-web-codegen/4.1.0) +![Version](https://img.shields.io/badge/rustc-1.59+-ab6000.svg) ![License](https://img.shields.io/crates/l/actix-web-codegen.svg)
-[![dependency status](https://deps.rs/crate/actix-web-codegen/4.0.0/status.svg)](https://deps.rs/crate/actix-web-codegen/4.0.0) +[![dependency status](https://deps.rs/crate/actix-web-codegen/4.1.0/status.svg)](https://deps.rs/crate/actix-web-codegen/4.1.0) [![Download](https://img.shields.io/crates/d/actix-web-codegen.svg)](https://crates.io/crates/actix-web-codegen) [![Chat on Discord](https://img.shields.io/discord/771444961383153695?label=chat&logo=discord)](https://discord.gg/NWpN5mmg3x) 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 cb1ba1ef6..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}; - -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 { @@ -77,7 +72,7 @@ impl TryFrom<&syn::LitStr> for MethodType { struct Args { path: syn::LitStr, resource_name: Option, - guards: Vec, + guards: Vec, wrappers: Vec, methods: HashSet, } @@ -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); @@ -121,7 +128,7 @@ impl Args { } } else if nv.path.is_ident("guard") { if let syn::Lit::Str(lit) = nv.lit { - guards.push(Ident::new(&lit.value(), Span::call_site())); + guards.push(lit.parse::()?); } else { return Err(syn::Error::new_spanned( nv.lit, @@ -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 769cf2bc3..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,13 +91,61 @@ 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() } +mod guard_module { + use actix_web::{guard::GuardContext, http::header}; + + pub fn guard(ctx: &GuardContext) -> bool { + ctx.header::() + .map(|h| h.preference() == "image/*") + .unwrap_or(false) + } +} + +#[get("/test/guard", guard = "guard_module::guard")] +async fn guard_test() -> impl Responder { + HttpResponse::Ok() +} + pub struct ChangeStatusCode; impl Transform for ChangeStatusCode @@ -186,7 +236,11 @@ 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) }); let request = srv.request(http::Method::GET, srv.url("/test")); let response = request.send().await.unwrap(); @@ -242,9 +296,47 @@ 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()); + + let request = srv + .request(http::Method::GET, srv.url("/test/guard")) + .insert_header(("Accept", "image/*")); + let response = request.send().await.unwrap(); + assert!(response.status().is_success()); } #[actix_rt::test] diff --git a/actix-web-codegen/tests/trybuild.rs b/actix-web-codegen/tests/trybuild.rs index b2d9ce186..26aec7d28 100644 --- a/actix-web-codegen/tests/trybuild.rs +++ b/actix-web-codegen/tests/trybuild.rs @@ -1,4 +1,4 @@ -#[rustversion::stable(1.54)] // MSRV +#[rustversion::stable(1.59)] // MSRV #[test] fn compile_macros() { let t = trybuild::TestCases::new(); @@ -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/route-duplicate-method-fail.stderr b/actix-web-codegen/tests/trybuild/route-duplicate-method-fail.stderr index 90cff1b1c..7eac84f3e 100644 --- a/actix-web-codegen/tests/trybuild/route-duplicate-method-fail.stderr +++ b/actix-web-codegen/tests/trybuild/route-duplicate-method-fail.stderr @@ -1,11 +1,19 @@ error: HTTP method defined more than once: `GET` - --> $DIR/route-duplicate-method-fail.rs:3:35 + --> tests/trybuild/route-duplicate-method-fail.rs:3:35 | 3 | #[route("/", method="GET", method="GET")] | ^^^^^ -error[E0277]: the trait bound `fn() -> impl std::future::Future {index}: HttpServiceFactory` is not satisfied - --> $DIR/route-duplicate-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}` +error[E0277]: the trait bound `fn() -> impl std::future::Future {index}: HttpServiceFactory` is not satisfied + --> tests/trybuild/route-duplicate-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 + | +note: required by a bound in `App::::service` + --> $WORKSPACE/actix-web/src/app.rs + | + | F: HttpServiceFactory + 'static, + | ^^^^^^^^^^^^^^^^^^ required by this bound in `App::::service` diff --git a/actix-web-codegen/tests/trybuild/route-missing-method-fail.stderr b/actix-web-codegen/tests/trybuild/route-missing-method-fail.stderr index b1cefafde..bc8497c10 100644 --- a/actix-web-codegen/tests/trybuild/route-missing-method-fail.stderr +++ b/actix-web-codegen/tests/trybuild/route-missing-method-fail.stderr @@ -6,8 +6,16 @@ error: The #[route(..)] macro requires at least one `method` attribute | = note: this error originates in the attribute macro `route` (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/route-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}` +error[E0277]: the trait bound `fn() -> impl std::future::Future {index}: HttpServiceFactory` is not satisfied + --> tests/trybuild/route-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 + | +note: required by a bound in `App::::service` + --> $WORKSPACE/actix-web/src/app.rs + | + | F: HttpServiceFactory + 'static, + | ^^^^^^^^^^^^^^^^^^ required by this bound in `App::::service` diff --git a/actix-web-codegen/tests/trybuild/route-unexpected-method-fail.stderr b/actix-web-codegen/tests/trybuild/route-unexpected-method-fail.stderr index dda366067..3df5d9f5a 100644 --- a/actix-web-codegen/tests/trybuild/route-unexpected-method-fail.stderr +++ b/actix-web-codegen/tests/trybuild/route-unexpected-method-fail.stderr @@ -1,11 +1,19 @@ error: Unexpected HTTP method: `UNEXPECTED` - --> $DIR/route-unexpected-method-fail.rs:3:21 + --> tests/trybuild/route-unexpected-method-fail.rs:3:21 | 3 | #[route("/", method="UNEXPECTED")] | ^^^^^^^^^^^^ -error[E0277]: the trait bound `fn() -> impl std::future::Future {index}: HttpServiceFactory` is not satisfied - --> $DIR/route-unexpected-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}` +error[E0277]: the trait bound `fn() -> impl std::future::Future {index}: HttpServiceFactory` is not satisfied + --> tests/trybuild/route-unexpected-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 + | +note: required by a bound in `App::::service` + --> $WORKSPACE/actix-web/src/app.rs + | + | F: HttpServiceFactory + 'static, + | ^^^^^^^^^^^^^^^^^^ required by this bound in `App::::service` 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..785d6f326 --- /dev/null +++ b/actix-web-codegen/tests/trybuild/routes-missing-args-fail.stderr @@ -0,0 +1,27 @@ +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 + | +note: required by a bound in `App::::service` + --> $WORKSPACE/actix-web/src/app.rs + | + | F: HttpServiceFactory + 'static, + | ^^^^^^^^^^^^^^^^^^ required by this bound in `App::::service` 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..38a6d2f9b --- /dev/null +++ b/actix-web-codegen/tests/trybuild/routes-missing-method-fail.stderr @@ -0,0 +1,21 @@ +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 + | +note: required by a bound in `App::::service` + --> $WORKSPACE/actix-web/src/app.rs + | + | F: HttpServiceFactory + 'static, + | ^^^^^^^^^^^^^^^^^^ required by this bound in `App::::service` 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 bd2252059..a4ba16e36 100644 --- a/actix-web/CHANGES.md +++ b/actix-web/CHANGES.md @@ -1,6 +1,53 @@ # Changelog -## Unreleased - 2021-xx-xx +## Unreleased - 2022-xx-xx +### Added +- Add `ContentDisposition::attachment` constructor. [#2867] +- Add `ErrorHandlers::default_handler()` (as well as `default_handler_{server, client}()`) to make registering handlers for groups of response statuses easier. [#2784] + +[#2784]: https://github.com/actix/actix-web/pull/2784 +[#2867]: https://github.com/actix/actix-web/pull/2867 + + +## 4.2.1 - 2022-09-12 +### Fixed +- Bump minimum version of `actix-http` dependency to fix compatibility issue. [#2871] + +[#2871]: https://github.com/actix/actix-web/pull/2871 + + +## 4.2.0 - 2022-09-11 +### 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.59 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 + + +## 4.1.0 - 2022-06-11 +### Added +- Add `ServiceRequest::extract()` to make it easier to use extractors when writing middlewares. [#2647] +- Add `Route::wrap()` to allow individual routes to use middleware. [#2725] +- Add `ServiceConfig::default_service()`. [#2338] [#2743] +- Implement `ResponseError` for `std::convert::Infallible` + +### Changed +- Minimum supported Rust version (MSRV) is now 1.56 due to transitive `hashbrown` dependency. + +### Fixed +- Clear connection-level data on `HttpRequest` drop. [#2742] + +[#2338]: https://github.com/actix/actix-web/pull/2338 +[#2647]: https://github.com/actix/actix-web/pull/2647 +[#2725]: https://github.com/actix/actix-web/pull/2725 +[#2742]: https://github.com/actix/actix-web/pull/2742 +[#2743]: https://github.com/actix/actix-web/pull/2743 ### Added diff --git a/actix-web/Cargo.toml b/actix-web/Cargo.toml index 093c000b4..c7b54bd46 100644 --- a/actix-web/Cargo.toml +++ b/actix-web/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "actix-web" -version = "4.0.1" +version = "4.2.1" authors = [ "Nikolay Kim ", "Rob Ede ", @@ -59,7 +59,7 @@ rustls = ["actix-http/rustls", "actix-tls/accept", "actix-tls/rustls"] # Don't rely on these whatsoever. They may disappear at anytime. __compress = [] -# io-uring feature only avaiable for Linux OSes. +# io-uring feature only available for Linux OSes. experimental-io-uring = ["actix-server/io-uring"] [dependencies] @@ -71,24 +71,25 @@ actix-service = "2" actix-utils = "3" actix-tls = { version = "3", default-features = false, optional = true } -actix-http = { version = "3", features = ["http2", "ws"] } +actix-http = { version = "3.2.2", features = ["http2", "ws"] } actix-router = "0.5" -actix-web-codegen = { version = "4", optional = true } +actix-web-codegen = { version = "4.1", optional = true } ahash = "0.7" bytes = "1" bytestring = "1" cfg-if = "1" cookie = { version = "0.16", features = ["percent-encode"], optional = true } -derive_more = "0.99.5" +derive_more = "0.99.8" encoding_rs = "0.8" futures-core = { version = "0.3.7", default-features = false } futures-util = { version = "0.3.7", default-features = false } +http = "0.2.8" itoa = "1" language-tags = "0.3" -once_cell = "1.5" log = "0.4" mime = "0.3" +once_cell = "1.5" pin-project-lite = "0.2.7" regex = "1.5.5" serde = "1.0" @@ -101,24 +102,24 @@ url = "2.1" [dev-dependencies] actix-files = "0.6" -actix-test = { version = "0.1.0-beta.13", features = ["openssl", "rustls"] } +actix-test = { version = "0.1", 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" } tls-rustls = { package = "rustls", version = "0.20.0" } tokio = { version = "1.13.1", features = ["rt-multi-thread", "macros"] } -zstd = "0.10" +zstd = "0.11" [[test]] name = "test_server" diff --git a/actix-web/MIGRATION-4.0.md b/actix-web/MIGRATION-4.0.md index 7192d0bc6..fbeae0680 100644 --- a/actix-web/MIGRATION-4.0.md +++ b/actix-web/MIGRATION-4.0.md @@ -31,6 +31,7 @@ Headings marked with :warning: are **breaking behavioral changes**. They will pr - [Returning `HttpResponse` synchronously](#returning-httpresponse-synchronously) - [`#[actix_web::main]` and `#[tokio::main]`](#actix_webmain-and-tokiomain) - [`web::block`](#webblock) +- ## MSRV @@ -483,3 +484,24 @@ The `web::block` helper has changed return type from roughly `async fn(fn() -> R - let n: u32 = web::block(|| Ok(123)).await?; + let n: u32 = web::block(|| Ok(123)).await??; ``` + +## `HttpResponse` as a `ResponseError` + +The implementation of `ResponseError` for `HttpResponse` has been removed. + +It was common in v3 to use `HttpResponse` as an error type in fallible handlers. The problem is that `HttpResponse` contains no knowledge or reference to the source error. Being able to guarantee that an "error" response actually contains an error reference makes middleware and other parts of Actix Web more effective. + +The error response builders in the `error` module were available in v3 but are now the best method for simple error responses without requiring you to implement the trait on your own custom error types. These builders can receive simple strings and third party errors that can not implement the `ResponseError` trait. + +A few common patterns are affected by this change: + +```diff +- Err(HttpResponse::InternalServerError().finish()) ++ Err(error::ErrorInternalServerError("reason")) + +- Err(HttpResponse::InternalServerError().body(third_party_error.to_string())) ++ Err(error::ErrorInternalServerError(err)) + +- .map_err(|err| HttpResponse::InternalServerError().finish())? ++ .map_err(error::ErrorInternalServerError)? +``` diff --git a/actix-web/README.md b/actix-web/README.md index 957fb47b8..65076e0b8 100644 --- a/actix-web/README.md +++ b/actix-web/README.md @@ -6,10 +6,10 @@

[![crates.io](https://img.shields.io/crates/v/actix-web?label=latest)](https://crates.io/crates/actix-web) -[![Documentation](https://docs.rs/actix-web/badge.svg?version=4.0.1)](https://docs.rs/actix-web/4.0.1) -![MSRV](https://img.shields.io/badge/rustc-1.54+-ab6000.svg) +[![Documentation](https://docs.rs/actix-web/badge.svg?version=4.2.1)](https://docs.rs/actix-web/4.2.1) +![MSRV](https://img.shields.io/badge/rustc-1.59+-ab6000.svg) ![MIT or Apache 2.0 licensed](https://img.shields.io/crates/l/actix-web.svg) -[![Dependency Status](https://deps.rs/crate/actix-web/4.0.1/status.svg)](https://deps.rs/crate/actix-web/4.0.1) +[![Dependency Status](https://deps.rs/crate/actix-web/4.2.1/status.svg)](https://deps.rs/crate/actix-web/4.2.1)
[![CI](https://github.com/actix/actix-web/actions/workflows/ci.yml/badge.svg)](https://github.com/actix/actix-web/actions/workflows/ci.yml) [![codecov](https://codecov.io/gh/actix/actix-web/branch/master/graph/badge.svg)](https://codecov.io/gh/actix/actix-web) @@ -33,7 +33,7 @@ - SSL support using OpenSSL or Rustls - Middlewares ([Logger, Session, CORS, etc](https://actix.rs/docs/middleware/)) - Integrates with the [`awc` HTTP client](https://docs.rs/awc/) -- Runs on stable Rust 1.54+ +- Runs on stable Rust 1.59+ ## Documentation @@ -79,6 +79,7 @@ async fn main() -> std::io::Result<()> { - [Application State](https://github.com/actix/examples/tree/master/basics/state) - [JSON Handling](https://github.com/actix/examples/tree/master/json/json) - [Multipart Streams](https://github.com/actix/examples/tree/master/forms/multipart) +- [MongoDB Integration](https://github.com/actix/examples/tree/master/databases/mongodb) - [Diesel Integration](https://github.com/actix/examples/tree/master/databases/diesel) - [SQLite Integration](https://github.com/actix/examples/tree/master/databases/sqlite) - [Postgres Integration](https://github.com/actix/examples/tree/master/databases/postgres) diff --git a/actix-web/src/app.rs b/actix-web/src/app.rs index d2df72714..ec37ff8a4 100644 --- a/actix-web/src/app.rs +++ b/actix-web/src/app.rs @@ -60,7 +60,7 @@ where /// [`HttpRequest::app_data`](crate::HttpRequest::app_data) method at runtime. /// /// # [`Data`] - /// Any [`Data`] type added here can utilize it's extractor implementation in handlers. + /// Any [`Data`] type added here can utilize its extractor implementation in handlers. /// Types not wrapped in `Data` cannot use this extractor. See [its docs](Data) for more /// about its usage and patterns. /// @@ -185,10 +185,17 @@ where F: FnOnce(&mut ServiceConfig), { let mut cfg = ServiceConfig::new(); + f(&mut cfg); + self.services.extend(cfg.services); self.external.extend(cfg.external); self.extensions.extend(cfg.app_data); + + if let Some(default) = cfg.default { + self.default = Some(default); + } + self } @@ -267,7 +274,6 @@ where { let svc = svc .into_factory() - .map(|res| res.map_into_boxed_body()) .map_init_err(|e| log::error!("Can not construct default service: {:?}", e)); self.default = Some(Rc::new(boxed::factory(svc))); @@ -308,7 +314,7 @@ where /// Registers an app-wide middleware. /// - /// Registers middleware, in the form of a middleware compo nen t (type), that runs during + /// Registers middleware, in the form of a middleware component (type), that runs during /// inbound and/or outbound processing in the request life-cycle (request -> response), /// modifying request/response as necessary, across all requests managed by the `App`. /// @@ -676,7 +682,7 @@ mod tests { "/test", web::get().to(|req: HttpRequest| { HttpResponse::Ok() - .body(req.url_for("youtube", &["12345"]).unwrap().to_string()) + .body(req.url_for("youtube", ["12345"]).unwrap().to_string()) }), ), ) diff --git a/actix-web/src/app_service.rs b/actix-web/src/app_service.rs index 3ef31ac75..0b5ba2ab6 100644 --- a/actix-web/src/app_service.rs +++ b/actix-web/src/app_service.rs @@ -173,7 +173,7 @@ impl AppInitServiceState { #[inline] pub(crate) fn rmap(&self) -> &ResourceMap { - &*self.rmap + &self.rmap } #[inline] @@ -257,7 +257,7 @@ impl ServiceFactory for AppRoutingFactory { type Future = LocalBoxFuture<'static, Result>; fn new_service(&self, _: ()) -> Self::Future { - // construct all services factory future with it's resource def and guards. + // construct all services factory future with its resource def and guards. let factory_fut = join_all(self.services.iter().map(|(path, factory, guards)| { let path = path.clone(); let guards = guards.borrow_mut().take().unwrap_or_default(); diff --git a/actix-web/src/config.rs b/actix-web/src/config.rs index 77fba18ed..68bea34ca 100644 --- a/actix-web/src/config.rs +++ b/actix-web/src/config.rs @@ -1,33 +1,32 @@ -use std::net::SocketAddr; -use std::rc::Rc; +use std::{net::SocketAddr, rc::Rc}; -use actix_http::Extensions; -use actix_router::ResourceDef; -use actix_service::{boxed, IntoServiceFactory, ServiceFactory}; +use actix_service::{boxed, IntoServiceFactory, ServiceFactory, ServiceFactoryExt as _}; -use crate::data::Data; -use crate::error::Error; -use crate::guard::Guard; -use crate::resource::Resource; -use crate::rmap::ResourceMap; -use crate::route::Route; -use crate::service::{ - AppServiceFactory, HttpServiceFactory, ServiceFactoryWrapper, ServiceRequest, - ServiceResponse, +use crate::{ + data::Data, + dev::{Extensions, ResourceDef}, + error::Error, + guard::Guard, + resource::Resource, + rmap::ResourceMap, + route::Route, + service::{ + AppServiceFactory, BoxedHttpServiceFactory, HttpServiceFactory, ServiceFactoryWrapper, + ServiceRequest, ServiceResponse, + }, }; type Guards = Vec>; -type HttpNewService = boxed::BoxServiceFactory<(), ServiceRequest, ServiceResponse, Error, ()>; /// Application configuration pub struct AppService { config: AppConfig, root: bool, - default: Rc, + default: Rc, #[allow(clippy::type_complexity)] services: Vec<( ResourceDef, - HttpNewService, + BoxedHttpServiceFactory, Option, Option>, )>, @@ -35,7 +34,7 @@ pub struct AppService { impl AppService { /// Crate server settings instance. - pub(crate) fn new(config: AppConfig, default: Rc) -> Self { + pub(crate) fn new(config: AppConfig, default: Rc) -> Self { AppService { config, default, @@ -56,7 +55,7 @@ impl AppService { AppConfig, Vec<( ResourceDef, - HttpNewService, + BoxedHttpServiceFactory, Option, Option>, )>, @@ -81,7 +80,7 @@ impl AppService { } /// Returns default handler factory. - pub fn default_service(&self) -> Rc { + pub fn default_service(&self) -> Rc { self.default.clone() } @@ -154,6 +153,16 @@ impl AppConfig { } impl Default for AppConfig { + /// Returns the default AppConfig. + /// Note: The included socket address is "127.0.0.1". + /// + /// 127.0.0.1: non-routable meta address that denotes an unknown, invalid or non-applicable target. + /// If you need a service only accessed by itself, use a loopback address. + /// A loopback address for IPv4 is any loopback address that begins with "127". + /// Loopback addresses should be only used to test your application locally. + /// The default configuration provides a loopback address. + /// + /// 0.0.0.0: if configured to use this special address, the application will listen to any IP address configured on the machine. fn default() -> Self { AppConfig::new( false, @@ -187,6 +196,7 @@ pub struct ServiceConfig { pub(crate) services: Vec>, pub(crate) external: Vec, pub(crate) app_data: Extensions, + pub(crate) default: Option>, } impl ServiceConfig { @@ -195,6 +205,7 @@ impl ServiceConfig { services: Vec::new(), external: Vec::new(), app_data: Extensions::new(), + default: None, } } @@ -215,6 +226,29 @@ impl ServiceConfig { self } + /// Default service to be used if no matching resource could be found. + /// + /// Counterpart to [`App::default_service()`](crate::App::default_service). + pub fn default_service(&mut self, f: F) -> &mut Self + where + F: IntoServiceFactory, + U: ServiceFactory< + ServiceRequest, + Config = (), + Response = ServiceResponse, + Error = Error, + > + 'static, + U::InitError: std::fmt::Debug, + { + let svc = f + .into_factory() + .map_init_err(|err| log::error!("Can not construct default service: {:?}", err)); + + self.default = Some(Rc::new(boxed::factory(svc))); + + self + } + /// Run external configuration as part of the application building process /// /// Counterpart to [`App::configure()`](crate::App::configure) that allows for easy nesting. @@ -310,7 +344,7 @@ mod tests { "/test", web::get().to(|req: HttpRequest| { HttpResponse::Ok() - .body(req.url_for("youtube", &["12345"]).unwrap().to_string()) + .body(req.url_for("youtube", ["12345"]).unwrap().to_string()) }), ), ) @@ -322,6 +356,38 @@ mod tests { assert_eq!(body, Bytes::from_static(b"https://youtube.com/watch/12345")); } + #[actix_rt::test] + async fn registers_default_service() { + let srv = init_service( + App::new() + .configure(|cfg| { + cfg.default_service( + web::get().to(|| HttpResponse::NotFound().body("four oh four")), + ); + }) + .service(web::scope("/scoped").configure(|cfg| { + cfg.default_service( + web::get().to(|| HttpResponse::NotFound().body("scoped four oh four")), + ); + })), + ) + .await; + + // app registers default service + let req = TestRequest::with_uri("/path/i/did/not-configure").to_request(); + let resp = call_service(&srv, req).await; + assert_eq!(resp.status(), StatusCode::NOT_FOUND); + let body = read_body(resp).await; + assert_eq!(body, Bytes::from_static(b"four oh four")); + + // scope registers default service + let req = TestRequest::with_uri("/scoped/path/i/did/not-configure").to_request(); + let resp = call_service(&srv, req).await; + assert_eq!(resp.status(), StatusCode::NOT_FOUND); + let body = read_body(resp).await; + assert_eq!(body, Bytes::from_static(b"scoped four oh four")); + } + #[actix_rt::test] async fn test_service() { let srv = init_service(App::new().configure(|cfg| { diff --git a/actix-web/src/data.rs b/actix-web/src/data.rs index ce7b1fee6..89104a1ac 100644 --- a/actix-web/src/data.rs +++ b/actix-web/src/data.rs @@ -5,10 +5,7 @@ use actix_utils::future::{err, ok, Ready}; use futures_core::future::LocalBoxFuture; use serde::Serialize; -use crate::{ - dev::Payload, error::ErrorInternalServerError, extract::FromRequest, request::HttpRequest, - Error, -}; +use crate::{dev::Payload, error, Error, FromRequest, HttpRequest}; /// Data factory. pub(crate) trait DataFactory { @@ -121,7 +118,7 @@ impl Deref for Data { impl Clone for Data { fn clone(&self) -> Data { - Data(self.0.clone()) + Data(Arc::clone(&self.0)) } } @@ -160,7 +157,7 @@ impl FromRequest for Data { req.match_name().unwrap_or_else(|| req.path()) ); - err(ErrorInternalServerError( + err(error::ErrorInternalServerError( "Requested application data is not configured correctly. \ View/enable debug logs for more details.", )) diff --git a/actix-web/src/error/error.rs b/actix-web/src/error/error.rs index 3d3978dde..3a5a128f6 100644 --- a/actix-web/src/error/error.rs +++ b/actix-web/src/error/error.rs @@ -51,12 +51,6 @@ impl StdError for Error { } } -impl From for Error { - fn from(val: std::convert::Infallible) -> Self { - match val {} - } -} - /// `Error` for any error that implements `ResponseError` impl From for Error { fn from(err: T) -> Error { diff --git a/actix-web/src/error/mod.rs b/actix-web/src/error/mod.rs index 6095cd5d2..604c539f3 100644 --- a/actix-web/src/error/mod.rs +++ b/actix-web/src/error/mod.rs @@ -42,7 +42,7 @@ pub struct BlockingError; impl ResponseError for crate::error::BlockingError {} /// Errors which can occur when attempting to generate resource uri. -#[derive(Debug, PartialEq, Display, Error, From)] +#[derive(Debug, PartialEq, Eq, Display, Error, From)] #[non_exhaustive] pub enum UrlGenerationError { /// Resource not found. diff --git a/actix-web/src/error/response_error.rs b/actix-web/src/error/response_error.rs index 0b8a82ce8..7d2c06154 100644 --- a/actix-web/src/error/response_error.rs +++ b/actix-web/src/error/response_error.rs @@ -1,6 +1,7 @@ //! `ResponseError` trait and foreign impls. use std::{ + convert::Infallible, error::Error as StdError, fmt, io::{self, Write as _}, @@ -54,6 +55,15 @@ downcast_dyn!(ResponseError); impl ResponseError for Box {} +impl ResponseError for Infallible { + fn status_code(&self) -> StatusCode { + match *self {} + } + fn error_response(&self) -> HttpResponse { + match *self {} + } +} + #[cfg(feature = "openssl")] impl ResponseError for actix_tls::accept::openssl::reexports::Error {} diff --git a/actix-web/src/extract.rs b/actix-web/src/extract.rs index a8b3d4565..d4f5cc91f 100644 --- a/actix-web/src/extract.rs +++ b/actix-web/src/extract.rs @@ -18,9 +18,11 @@ use crate::{dev::Payload, Error, HttpRequest}; /// A type that implements [`FromRequest`] is called an **extractor** and can extract data from /// the request. Some types that implement this trait are: [`Json`], [`Header`], and [`Path`]. /// +/// Check out [`ServiceRequest::extract`](crate::dev::ServiceRequest::extract) if you want to +/// leverage extractors when implementing middlewares. +/// /// # Configuration /// An extractor can be customized by injecting the corresponding configuration with one of: -/// /// - [`App::app_data()`][crate::App::app_data] /// - [`Scope::app_data()`][crate::Scope::app_data] /// - [`Resource::app_data()`][crate::Resource::app_data] @@ -64,13 +66,29 @@ pub trait FromRequest: Sized { /// The associated error which can be returned. type Error: Into; - /// Future that resolves to a Self. + /// Future that resolves to a `Self`. + /// + /// To use an async function or block, the futures must be boxed. The following snippet will be + /// common when creating async/await extractors (that do not consume the body). + /// + /// ```ignore + /// type Future = Pin>>>; + /// // or + /// type Future = futures_util::future::LocalBoxFuture<'static, Result>; + /// + /// fn from_request(req: HttpRequest, ...) -> Self::Future { + /// let req = req.clone(); + /// Box::pin(async move { + /// ... + /// }) + /// } + /// ``` type Future: Future>; - /// Create a Self from request parts asynchronously. + /// Create a `Self` from request parts asynchronously. fn from_request(req: &HttpRequest, payload: &mut Payload) -> Self::Future; - /// Create a Self from request head asynchronously. + /// Create a `Self` from request head asynchronously. /// /// This method is short for `T::from_request(req, &mut Payload::None)`. fn extract(req: &HttpRequest) -> Self::Future { @@ -78,9 +96,9 @@ pub trait FromRequest: Sized { } } -/// Optionally extract a field from the request +/// Optionally extract from the request. /// -/// If the FromRequest for T fails, return None rather than returning an error response +/// If the inner `T::from_request` returns an error, handler will receive `None` instead. /// /// # Examples /// ``` @@ -165,9 +183,10 @@ where } } -/// Optionally extract a field from the request or extract the Error if unsuccessful +/// Extract from the request, passing error type through to handler. /// -/// If the `FromRequest` for T fails, inject Err into handler rather than returning an error response +/// If the inner `T::from_request` returns an error, allow handler to receive the error rather than +/// immediately returning an error response. /// /// # Examples /// ``` diff --git a/actix-web/src/guard.rs b/actix-web/src/guard.rs index 9f7514644..ef1301075 100644 --- a/actix-web/src/guard.rs +++ b/actix-web/src/guard.rs @@ -254,7 +254,7 @@ impl Guard for AllGuard { } } -/// Wraps a guard and inverts the outcome of it's `Guard` implementation. +/// Wraps a guard and inverts the outcome of its `Guard` implementation. /// /// # Examples /// The handler below will be called for any request method apart from `GET`. @@ -459,7 +459,7 @@ impl Guard for HostGuard { return scheme == req_host_uri_scheme; } - // TODO: is the the correct behavior? + // TODO: is this the correct behavior? // falls through if scheme cannot be determined } diff --git a/actix-web/src/handler.rs b/actix-web/src/handler.rs index cf86cb38b..522a48b82 100644 --- a/actix-web/src/handler.rs +++ b/actix-web/src/handler.rs @@ -37,7 +37,7 @@ use crate::{ /// Thanks to Rust's type system, Actix Web can infer the function parameter types. During the /// extraction step, the parameter types are described as a tuple type, [`from_request`] is run on /// that tuple, and the `Handler::call` implementation for that particular function arity -/// destructures the tuple into it's component types and calls your handler function with them. +/// destructures the tuple into its component types and calls your handler function with them. /// /// In pseudo-code the process looks something like this: /// ```ignore diff --git a/actix-web/src/http/header/cache_control.rs b/actix-web/src/http/header/cache_control.rs index 490d36558..37629313e 100644 --- a/actix-web/src/http/header/cache_control.rs +++ b/actix-web/src/http/header/cache_control.rs @@ -176,7 +176,7 @@ impl str::FromStr for CacheDirective { _ => match s.find('=') { Some(idx) if idx + 1 < s.len() => { - match (&s[..idx], (&s[idx + 1..]).trim_matches('"')) { + match (&s[..idx], s[idx + 1..].trim_matches('"')) { ("max-age", secs) => secs.parse().map(MaxAge).map_err(Some), ("max-stale", secs) => secs.parse().map(MaxStale).map_err(Some), ("min-fresh", secs) => secs.parse().map(MinFresh).map_err(Some), diff --git a/actix-web/src/http/header/content_disposition.rs b/actix-web/src/http/header/content_disposition.rs index 8b7101aa1..f743302a2 100644 --- a/actix-web/src/http/header/content_disposition.rs +++ b/actix-web/src/http/header/content_disposition.rs @@ -10,9 +10,10 @@ //! - Browser conformance tests at: //! - IANA assignment: +use std::fmt::{self, Write}; + use once_cell::sync::Lazy; use regex::Regex; -use std::fmt::{self, Write}; use super::{ExtendedValue, Header, TryIntoHeaderValue, Writer}; use crate::http::header; @@ -36,7 +37,7 @@ fn split_once_and_trim(haystack: &str, needle: char) -> (&str, &str) { } /// The implied disposition of the content of the HTTP body. -#[derive(Clone, Debug, PartialEq)] +#[derive(Debug, Clone, PartialEq, Eq)] pub enum DispositionType { /// Inline implies default processing. Inline, @@ -78,7 +79,7 @@ impl<'a> From<&'a str> for DispositionType { /// assert!(param.is_filename()); /// assert_eq!(param.as_filename().unwrap(), "sample.txt"); /// ``` -#[derive(Clone, Debug, PartialEq)] +#[derive(Debug, Clone, PartialEq, Eq)] #[allow(clippy::large_enum_variant)] pub enum DispositionParam { /// For [`DispositionType::FormData`] (i.e. *multipart/form-data*), the name of an field from @@ -301,7 +302,7 @@ impl DispositionParam { /// change to match local file system conventions if applicable, and do not use directory path /// information that may be present. /// See [RFC 2183 §2.3](https://datatracker.ietf.org/doc/html/rfc2183#section-2.3). -#[derive(Clone, Debug, PartialEq)] +#[derive(Debug, Clone, PartialEq, Eq)] pub struct ContentDisposition { /// The disposition type pub disposition: DispositionType, @@ -311,16 +312,36 @@ pub struct ContentDisposition { } impl ContentDisposition { + /// Constructs a Content-Disposition header suitable for downloads. + /// + /// # Examples + /// ``` + /// use actix_web::http::header::{ContentDisposition, TryIntoHeaderValue as _}; + /// + /// let cd = ContentDisposition::attachment("files.zip"); + /// + /// let cd_val = cd.try_into_value().unwrap(); + /// assert_eq!(cd_val, "attachment; filename=\"files.zip\""); + /// ``` + pub fn attachment(filename: impl Into) -> Self { + Self { + disposition: DispositionType::Attachment, + parameters: vec![DispositionParam::Filename(filename.into())], + } + } + /// Parse a raw Content-Disposition header value. pub fn from_raw(hv: &header::HeaderValue) -> Result { // `header::from_one_raw_str` invokes `hv.to_str` which assumes `hv` contains only visible // ASCII characters. So `hv.as_bytes` is necessary here. let hv = String::from_utf8(hv.as_bytes().to_vec()) .map_err(|_| crate::error::ParseError::Header)?; + let (disp_type, mut left) = split_once_and_trim(hv.as_str().trim(), ';'); if disp_type.is_empty() { return Err(crate::error::ParseError::Header); } + let mut cd = ContentDisposition { disposition: disp_type.into(), parameters: Vec::new(), diff --git a/actix-web/src/http/header/if_range.rs b/actix-web/src/http/header/if_range.rs index b845fb3bf..eb3632a4d 100644 --- a/actix-web/src/http/header/if_range.rs +++ b/actix-web/src/http/header/if_range.rs @@ -57,7 +57,7 @@ use crate::HttpMessage; /// IfRange::Date(fetched.into()) /// ); /// ``` -#[derive(Clone, Debug, PartialEq)] +#[derive(Clone, Debug, PartialEq, Eq)] pub enum IfRange { /// The entity-tag the client has of the resource. EntityTag(EntityTag), diff --git a/actix-web/src/http/header/macros.rs b/actix-web/src/http/header/macros.rs index 25f40a52b..b40eca03b 100644 --- a/actix-web/src/http/header/macros.rs +++ b/actix-web/src/http/header/macros.rs @@ -224,10 +224,11 @@ macro_rules! common_header { // List header, one or more items with "*" option ($(#[$attrs:meta])*($id:ident, $name:expr) => {Any / ($item:ty)+}) => { $(#[$attrs])* - #[derive(Clone, Debug, PartialEq)] + #[derive(Clone, Debug, PartialEq, Eq)] pub enum $id { /// Any value is a match Any, + /// Only the listed items are a match Items(Vec<$item>), } diff --git a/actix-web/src/http/header/range.rs b/actix-web/src/http/header/range.rs index 68028f53a..2326bb19c 100644 --- a/actix-web/src/http/header/range.rs +++ b/actix-web/src/http/header/range.rs @@ -53,7 +53,7 @@ use super::{Header, HeaderName, HeaderValue, InvalidHeaderValue, TryIntoHeaderVa /// builder.insert_header(Range::bytes(1, 100)); /// builder.insert_header(Range::bytes_multi(vec![(1, 100), (200, 300)])); /// ``` -#[derive(PartialEq, Clone, Debug)] +#[derive(Debug, Clone, PartialEq, Eq)] pub enum Range { /// Byte range. Bytes(Vec), diff --git a/actix-web/src/info.rs b/actix-web/src/info.rs index 77b98110e..7c685406e 100644 --- a/actix-web/src/info.rs +++ b/actix-web/src/info.rs @@ -2,7 +2,6 @@ use std::{convert::Infallible, net::SocketAddr}; use actix_utils::future::{err, ok, Ready}; use derive_more::{Display, Error}; -use once_cell::sync::Lazy; use crate::{ dev::{AppConfig, Payload, RequestHead}, @@ -13,12 +12,9 @@ use crate::{ FromRequest, HttpRequest, ResponseError, }; -static X_FORWARDED_FOR: Lazy = - Lazy::new(|| HeaderName::from_static("x-forwarded-for")); -static X_FORWARDED_HOST: Lazy = - Lazy::new(|| HeaderName::from_static("x-forwarded-host")); -static X_FORWARDED_PROTO: Lazy = - Lazy::new(|| HeaderName::from_static("x-forwarded-proto")); +static X_FORWARDED_FOR: HeaderName = HeaderName::from_static("x-forwarded-for"); +static X_FORWARDED_HOST: HeaderName = HeaderName::from_static("x-forwarded-host"); +static X_FORWARDED_PROTO: HeaderName = HeaderName::from_static("x-forwarded-proto"); /// Trim whitespace then any quote marks. fn unquote(val: &str) -> &str { @@ -117,21 +113,21 @@ impl ConnectionInfo { } let scheme = scheme - .or_else(|| first_header_value(req, &*X_FORWARDED_PROTO)) + .or_else(|| first_header_value(req, &X_FORWARDED_PROTO)) .or_else(|| req.uri.scheme().map(Scheme::as_str)) .or_else(|| Some("https").filter(|_| cfg.secure())) .unwrap_or("http") .to_owned(); let host = host - .or_else(|| first_header_value(req, &*X_FORWARDED_HOST)) + .or_else(|| first_header_value(req, &X_FORWARDED_HOST)) .or_else(|| req.headers.get(&header::HOST)?.to_str().ok()) .or_else(|| req.uri.authority().map(Authority::as_str)) .unwrap_or_else(|| cfg.host()) .to_owned(); let realip_remote_addr = realip_remote_addr - .or_else(|| first_header_value(req, &*X_FORWARDED_FOR)) + .or_else(|| first_header_value(req, &X_FORWARDED_FOR)) .map(str::to_owned); let peer_addr = req.peer_addr.map(|addr| addr.ip().to_string()); 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); diff --git a/actix-web/src/middleware/authors-guide.md b/actix-web/src/middleware/authors-guide.md index 344523a1a..a8d1edea4 100644 --- a/actix-web/src/middleware/authors-guide.md +++ b/actix-web/src/middleware/authors-guide.md @@ -11,3 +11,6 @@ ## Error Propagation ## When To (Not) Use Middleware + +## Author's References +- `EitherBody` + when is middleware appropriate: https://discord.com/channels/771444961383153695/952016890723729428 diff --git a/actix-web/src/middleware/compress.rs b/actix-web/src/middleware/compress.rs index 4fdd74779..51b44c6ef 100644 --- a/actix-web/src/middleware/compress.rs +++ b/actix-web/src/middleware/compress.rs @@ -220,37 +220,32 @@ static SUPPORTED_ENCODINGS_STRING: Lazy = Lazy::new(|| { encoding.join(", ") }); -static SUPPORTED_ENCODINGS: Lazy> = Lazy::new(|| { - let mut encodings = vec![Encoding::identity()]; - +static SUPPORTED_ENCODINGS: &[Encoding] = &[ + Encoding::identity(), #[cfg(feature = "compress-brotli")] { - encodings.push(Encoding::brotli()); - } - + Encoding::brotli() + }, #[cfg(feature = "compress-gzip")] { - encodings.push(Encoding::gzip()); - encodings.push(Encoding::deflate()); - } - + Encoding::gzip() + }, + #[cfg(feature = "compress-gzip")] + { + Encoding::deflate() + }, #[cfg(feature = "compress-zstd")] { - encodings.push(Encoding::zstd()); - } - - assert!( - !encodings.is_empty(), - "encodings can not be empty unless __compress feature has been explicitly enabled by itself" - ); - - encodings -}); + Encoding::zstd() + }, +]; // move cfg(feature) to prevents_double_compressing if more tests are added #[cfg(feature = "compress-gzip")] #[cfg(test)] mod tests { + use std::collections::HashSet; + use super::*; use crate::{middleware::DefaultHeaders, test, web, App}; @@ -305,4 +300,28 @@ 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); + #[allow(clippy::mutable_key_type)] + 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"))); + } } diff --git a/actix-web/src/middleware/err_handlers.rs b/actix-web/src/middleware/err_handlers.rs index f74220cd2..3a4e44a2c 100644 --- a/actix-web/src/middleware/err_handlers.rs +++ b/actix-web/src/middleware/err_handlers.rs @@ -30,11 +30,25 @@ pub enum ErrorHandlerResponse { type ErrorHandler = dyn Fn(ServiceResponse) -> Result>; +type DefaultHandler = Option>>; + /// Middleware for registering custom status code based error handlers. /// -/// Register handlers with the `ErrorHandlers::handler()` method to register a custom error handler +/// Register handlers with the [`ErrorHandlers::handler()`] method to register a custom error handler /// for a given status code. Handlers can modify existing responses or create completely new ones. /// +/// To register a default handler, use the [`ErrorHandlers::default_handler()`] method. This +/// handler will be used only if a response has an error status code (400-599) that isn't covered by +/// a more specific handler (set with the [`handler()`][ErrorHandlers::handler] method). See examples +/// below. +/// +/// To register a default for only client errors (400-499) or only server errors (500-599), use the +/// [`ErrorHandlers::default_handler_client()`] and [`ErrorHandlers::default_handler_server()`] +/// methods, respectively. +/// +/// Any response with a status code that isn't covered by a specific handler or a default handler +/// will pass by unchanged by this middleware. +/// /// # Examples /// ``` /// use actix_web::http::{header, StatusCode}; @@ -53,7 +67,70 @@ type ErrorHandler = dyn Fn(ServiceResponse) -> Result(mut res: dev::ServiceResponse) -> Result> { +/// res.response_mut().headers_mut().insert( +/// header::CONTENT_TYPE, +/// header::HeaderValue::from_static("Error"), +/// ); +/// Ok(ErrorHandlerResponse::Response(res.map_into_left_body())) +/// } +/// +/// fn handle_bad_request(mut res: dev::ServiceResponse) -> Result> { +/// res.response_mut().headers_mut().insert( +/// header::CONTENT_TYPE, +/// header::HeaderValue::from_static("Bad Request Error"), +/// ); +/// Ok(ErrorHandlerResponse::Response(res.map_into_left_body())) +/// } +/// +/// // Bad Request errors will hit `handle_bad_request()`, while all other errors will hit +/// // `add_error_header()`. The order in which the methods are called is not meaningful. +/// let app = App::new() +/// .wrap( +/// ErrorHandlers::new() +/// .default_handler(add_error_header) +/// .handler(StatusCode::BAD_REQUEST, handle_bad_request) +/// ) +/// .service(web::resource("/").route(web::get().to(HttpResponse::InternalServerError))); +/// ``` +/// Alternatively, you can set default handlers for only client or only server errors: +/// +/// ```rust +/// # use actix_web::http::{header, StatusCode}; +/// # use actix_web::middleware::{ErrorHandlerResponse, ErrorHandlers}; +/// # use actix_web::{dev, web, App, HttpResponse, Result}; +/// # fn add_error_header(mut res: dev::ServiceResponse) -> Result> { +/// # res.response_mut().headers_mut().insert( +/// # header::CONTENT_TYPE, +/// # header::HeaderValue::from_static("Error"), +/// # ); +/// # Ok(ErrorHandlerResponse::Response(res.map_into_left_body())) +/// # } +/// # fn handle_bad_request(mut res: dev::ServiceResponse) -> Result> { +/// # res.response_mut().headers_mut().insert( +/// # header::CONTENT_TYPE, +/// # header::HeaderValue::from_static("Bad Request Error"), +/// # ); +/// # Ok(ErrorHandlerResponse::Response(res.map_into_left_body())) +/// # } +/// // Bad request errors will hit `handle_bad_request()`, other client errors will hit +/// // `add_error_header()`, and server errors will pass through unchanged +/// let app = App::new() +/// .wrap( +/// ErrorHandlers::new() +/// .default_handler_client(add_error_header) // or .default_handler_server +/// .handler(StatusCode::BAD_REQUEST, handle_bad_request) +/// ) +/// .service(web::resource("/").route(web::get().to(HttpResponse::InternalServerError))); +/// ``` pub struct ErrorHandlers { + default_client: DefaultHandler, + default_server: DefaultHandler, handlers: Handlers, } @@ -62,6 +139,8 @@ type Handlers = Rc>>>; impl Default for ErrorHandlers { fn default() -> Self { ErrorHandlers { + default_client: Default::default(), + default_server: Default::default(), handlers: Default::default(), } } @@ -83,6 +162,66 @@ impl ErrorHandlers { .insert(status, Box::new(handler)); self } + + /// Register a default error handler. + /// + /// Any request with a status code that hasn't been given a specific other handler (by calling + /// [`.handler()`][ErrorHandlers::handler]) will fall back on this. + /// + /// Note that this will overwrite any default handlers previously set by calling + /// [`.default_handler_client()`][ErrorHandlers::default_handler_client] or + /// [`.default_handler_server()`][ErrorHandlers::default_handler_server], but not any set by + /// calling [`.handler()`][ErrorHandlers::handler]. + pub fn default_handler(self, handler: F) -> Self + where + F: Fn(ServiceResponse) -> Result> + 'static, + { + let handler = Rc::new(handler); + Self { + default_server: Some(handler.clone()), + default_client: Some(handler), + ..self + } + } + + /// Register a handler on which to fall back for client error status codes (400-499). + pub fn default_handler_client(self, handler: F) -> Self + where + F: Fn(ServiceResponse) -> Result> + 'static, + { + Self { + default_client: Some(Rc::new(handler)), + ..self + } + } + + /// Register a handler on which to fall back for server error status codes (500-599). + pub fn default_handler_server(self, handler: F) -> Self + where + F: Fn(ServiceResponse) -> Result> + 'static, + { + Self { + default_server: Some(Rc::new(handler)), + ..self + } + } + + /// Selects the most appropriate handler for the given status code. + /// + /// If the `handlers` map has an entry for that status code, that handler is returned. + /// Otherwise, fall back on the appropriate default handler. + fn get_handler<'a>( + status: &StatusCode, + default_client: Option<&'a ErrorHandler>, + default_server: Option<&'a ErrorHandler>, + handlers: &'a Handlers, + ) -> Option<&'a ErrorHandler> { + handlers + .get(status) + .map(|h| h.as_ref()) + .or_else(|| status.is_client_error().then(|| default_client).flatten()) + .or_else(|| status.is_server_error().then(|| default_server).flatten()) + } } impl Transform for ErrorHandlers @@ -99,13 +238,24 @@ where fn new_transform(&self, service: S) -> Self::Future { let handlers = self.handlers.clone(); - Box::pin(async move { Ok(ErrorHandlersMiddleware { service, handlers }) }) + let default_client = self.default_client.clone(); + let default_server = self.default_server.clone(); + Box::pin(async move { + Ok(ErrorHandlersMiddleware { + service, + default_client, + default_server, + handlers, + }) + }) } } #[doc(hidden)] pub struct ErrorHandlersMiddleware { service: S, + default_client: DefaultHandler, + default_server: DefaultHandler, handlers: Handlers, } @@ -123,8 +273,15 @@ where fn call(&self, req: ServiceRequest) -> Self::Future { let handlers = self.handlers.clone(); + let default_client = self.default_client.clone(); + let default_server = self.default_server.clone(); let fut = self.service.call(req); - ErrorHandlersFuture::ServiceFuture { fut, handlers } + ErrorHandlersFuture::ServiceFuture { + fut, + default_client, + default_server, + handlers, + } } } @@ -137,6 +294,8 @@ pin_project! { ServiceFuture { #[pin] fut: Fut, + default_client: DefaultHandler, + default_server: DefaultHandler, handlers: Handlers, }, ErrorHandlerFuture { @@ -153,10 +312,22 @@ where fn poll(mut self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll { match self.as_mut().project() { - ErrorHandlersProj::ServiceFuture { fut, handlers } => { + ErrorHandlersProj::ServiceFuture { + fut, + default_client, + default_server, + handlers, + } => { let res = ready!(fut.poll(cx))?; + let status = res.status(); - match handlers.get(&res.status()) { + let handler = ErrorHandlers::get_handler( + &status, + default_client.as_mut().map(|f| Rc::as_ref(f)), + default_server.as_mut().map(|f| Rc::as_ref(f)), + handlers, + ); + match handler { Some(handler) => match handler(res)? { ErrorHandlerResponse::Response(res) => Poll::Ready(Ok(res)), ErrorHandlerResponse::Future(fut) => { @@ -166,7 +337,6 @@ where self.poll(cx) } }, - None => Poll::Ready(Ok(res.map_into_left_body())), } } @@ -298,4 +468,117 @@ mod tests { "error in error handler" ); } + + #[actix_rt::test] + async fn default_error_handler() { + #[allow(clippy::unnecessary_wraps)] + fn error_handler(mut res: ServiceResponse) -> Result> { + res.response_mut() + .headers_mut() + .insert(CONTENT_TYPE, HeaderValue::from_static("0001")); + Ok(ErrorHandlerResponse::Response(res.map_into_left_body())) + } + + let make_mw = |status| async move { + ErrorHandlers::new() + .default_handler(error_handler) + .new_transform(test::status_service(status).into_service()) + .await + .unwrap() + }; + let mw_server = make_mw(StatusCode::INTERNAL_SERVER_ERROR).await; + let mw_client = make_mw(StatusCode::BAD_REQUEST).await; + + let resp = + test::call_service(&mw_client, TestRequest::default().to_srv_request()).await; + assert_eq!(resp.headers().get(CONTENT_TYPE).unwrap(), "0001"); + + let resp = + test::call_service(&mw_server, TestRequest::default().to_srv_request()).await; + assert_eq!(resp.headers().get(CONTENT_TYPE).unwrap(), "0001"); + } + + #[actix_rt::test] + async fn default_handlers_separate_client_server() { + #[allow(clippy::unnecessary_wraps)] + fn error_handler_client( + mut res: ServiceResponse, + ) -> Result> { + res.response_mut() + .headers_mut() + .insert(CONTENT_TYPE, HeaderValue::from_static("0001")); + Ok(ErrorHandlerResponse::Response(res.map_into_left_body())) + } + + #[allow(clippy::unnecessary_wraps)] + fn error_handler_server( + mut res: ServiceResponse, + ) -> Result> { + res.response_mut() + .headers_mut() + .insert(CONTENT_TYPE, HeaderValue::from_static("0002")); + Ok(ErrorHandlerResponse::Response(res.map_into_left_body())) + } + + let make_mw = |status| async move { + ErrorHandlers::new() + .default_handler_server(error_handler_server) + .default_handler_client(error_handler_client) + .new_transform(test::status_service(status).into_service()) + .await + .unwrap() + }; + let mw_server = make_mw(StatusCode::INTERNAL_SERVER_ERROR).await; + let mw_client = make_mw(StatusCode::BAD_REQUEST).await; + + let resp = + test::call_service(&mw_client, TestRequest::default().to_srv_request()).await; + assert_eq!(resp.headers().get(CONTENT_TYPE).unwrap(), "0001"); + + let resp = + test::call_service(&mw_server, TestRequest::default().to_srv_request()).await; + assert_eq!(resp.headers().get(CONTENT_TYPE).unwrap(), "0002"); + } + + #[actix_rt::test] + async fn default_handlers_specialization() { + #[allow(clippy::unnecessary_wraps)] + fn error_handler_client( + mut res: ServiceResponse, + ) -> Result> { + res.response_mut() + .headers_mut() + .insert(CONTENT_TYPE, HeaderValue::from_static("0001")); + Ok(ErrorHandlerResponse::Response(res.map_into_left_body())) + } + + #[allow(clippy::unnecessary_wraps)] + fn error_handler_specific( + mut res: ServiceResponse, + ) -> Result> { + res.response_mut() + .headers_mut() + .insert(CONTENT_TYPE, HeaderValue::from_static("0003")); + Ok(ErrorHandlerResponse::Response(res.map_into_left_body())) + } + + let make_mw = |status| async move { + ErrorHandlers::new() + .default_handler_client(error_handler_client) + .handler(StatusCode::UNPROCESSABLE_ENTITY, error_handler_specific) + .new_transform(test::status_service(status).into_service()) + .await + .unwrap() + }; + let mw_client = make_mw(StatusCode::BAD_REQUEST).await; + let mw_specific = make_mw(StatusCode::UNPROCESSABLE_ENTITY).await; + + let resp = + test::call_service(&mw_client, TestRequest::default().to_srv_request()).await; + assert_eq!(resp.headers().get(CONTENT_TYPE).unwrap(), "0001"); + + let resp = + test::call_service(&mw_specific, TestRequest::default().to_srv_request()).await; + assert_eq!(resp.headers().get(CONTENT_TYPE).unwrap(), "0003"); + } } diff --git a/actix-web/src/request.rs b/actix-web/src/request.rs index 5545cf982..6a32bf838 100644 --- a/actix-web/src/request.rs +++ b/actix-web/src/request.rs @@ -219,7 +219,7 @@ impl HttpRequest { /// for urls that do not contain variable parts. pub fn url_for_static(&self, name: &str) -> Result { const NO_PARAMS: [&str; 0] = []; - self.url_for(name, &NO_PARAMS) + self.url_for(name, NO_PARAMS) } /// Get a reference to a `ResourceMap` of current application. @@ -253,7 +253,7 @@ impl HttpRequest { #[inline] pub fn connection_info(&self) -> Ref<'_, ConnectionInfo> { if !self.extensions().contains::() { - let info = ConnectionInfo::new(self.head(), &*self.app_config()); + let info = ConnectionInfo::new(self.head(), self.app_config()); self.extensions_mut().insert(info); } @@ -306,11 +306,12 @@ impl HttpRequest { #[inline] fn app_state(&self) -> &AppInitServiceState { - &*self.inner.app_state + &self.inner.app_state } /// Load request cookies. #[cfg(feature = "cookies")] + #[cfg_attr(docsrs, doc(cfg(feature = "cookies")))] pub fn cookies(&self) -> Result>>, CookieParseError> { use actix_http::header::COOKIE; @@ -334,6 +335,7 @@ impl HttpRequest { /// Return request cookie. #[cfg(feature = "cookies")] + #[cfg_attr(docsrs, doc(cfg(feature = "cookies")))] pub fn cookie(&self, name: &str) -> Option> { if let Ok(cookies) = self.cookies() { for cookie in cookies.iter() { @@ -381,12 +383,16 @@ impl Drop for HttpRequest { inner.app_data.truncate(1); // Inner is borrowed mut here and; get req data mutably to reduce borrow check. Also - // we know the req_data Rc will not have any cloned at this point to unwrap is okay. + // we know the req_data Rc will not have any clones at this point to unwrap is okay. Rc::get_mut(&mut inner.extensions) .unwrap() .get_mut() .clear(); + // We can't use the same trick as req data because the conn_data is held by the + // dispatcher, too. + inner.conn_data = None; + // a re-borrow of pool is necessary here. let req = Rc::clone(&self.inner); self.app_state().pool().push(req); @@ -577,14 +583,14 @@ mod tests { .to_http_request(); assert_eq!( - req.url_for("unknown", &["test"]), + req.url_for("unknown", ["test"]), Err(UrlGenerationError::ResourceNotFound) ); assert_eq!( - req.url_for("index", &["test"]), + req.url_for("index", ["test"]), Err(UrlGenerationError::NotEnoughElements) ); - let url = req.url_for("index", &["test", "html"]); + let url = req.url_for("index", ["test", "html"]); assert_eq!( url.ok().unwrap().as_str(), "http://www.rust-lang.org/user/test.html" @@ -640,7 +646,7 @@ mod tests { rmap.add(&mut rdef, None); let req = TestRequest::default().rmap(rmap).to_http_request(); - let url = req.url_for("youtube", &["oHg5SJYRHA0"]); + let url = req.url_for("youtube", ["oHg5SJYRHA0"]); assert_eq!( url.ok().unwrap().as_str(), "https://youtube.com/watch/oHg5SJYRHA0" @@ -761,10 +767,8 @@ mod tests { assert_eq!(body, Bytes::from_static(b"1")); } - // allow deprecated App::data - #[allow(deprecated)] #[actix_rt::test] - async fn test_extensions_dropped() { + async fn test_app_data_dropped() { struct Tracker { pub dropped: bool, } @@ -780,7 +784,7 @@ mod tests { let tracker = Rc::new(RefCell::new(Tracker { dropped: false })); { let tracker2 = Rc::clone(&tracker); - let srv = init_service(App::new().data(10u32).service(web::resource("/").to( + let srv = init_service(App::new().service(web::resource("/").to( move |req: HttpRequest| { req.extensions_mut().insert(Foo { tracker: Rc::clone(&tracker2), diff --git a/actix-web/src/response/builder.rs b/actix-web/src/response/builder.rs index f50aad9f4..120d4c358 100644 --- a/actix-web/src/response/builder.rs +++ b/actix-web/src/response/builder.rs @@ -457,7 +457,7 @@ mod tests { assert_eq!(ct, HeaderValue::from_static("application/json")); assert_body_eq!(res, br#"["v1","v2","v3"]"#); - let res = HttpResponse::Ok().json(&["v1", "v2", "v3"]); + let res = HttpResponse::Ok().json(["v1", "v2", "v3"]); let ct = res.headers().get(CONTENT_TYPE).unwrap(); assert_eq!(ct, HeaderValue::from_static("application/json")); assert_body_eq!(res, br#"["v1","v2","v3"]"#); diff --git a/actix-web/src/response/response.rs b/actix-web/src/response/response.rs index 630acc3f2..ead8badba 100644 --- a/actix-web/src/response/response.rs +++ b/actix-web/src/response/response.rs @@ -343,7 +343,7 @@ mod response_fut_impl { // Future is only implemented for BoxBody payload type because it's the most useful for making // simple handlers without async blocks. Making it generic over all MessageBody types requires a - // future impl on Response which would cause it's body field to be, undesirably, Option. + // future impl on Response which would cause its body field to be, undesirably, Option. // // This impl is not particularly efficient due to the Response construction and should probably // not be invoked if performance is important. Prefer an async fn/block in such cases. diff --git a/actix-web/src/rmap.rs b/actix-web/src/rmap.rs index 6a1a187b2..6e10717c3 100644 --- a/actix-web/src/rmap.rs +++ b/actix-web/src/rmap.rs @@ -449,12 +449,12 @@ mod tests { let req = req.to_http_request(); let url = rmap - .url_for(&req, "post", &["u123", "foobar"]) + .url_for(&req, "post", ["u123", "foobar"]) .unwrap() .to_string(); assert_eq!(url, "http://localhost:8888/user/u123/post/foobar"); - assert!(rmap.url_for(&req, "missing", &["u123"]).is_err()); + assert!(rmap.url_for(&req, "missing", ["u123"]).is_err()); } #[test] @@ -490,7 +490,7 @@ mod tests { assert_eq!(url.path(), OUTPUT); assert!(rmap.url_for(&req, "external.2", INPUT).is_err()); - assert!(rmap.url_for(&req, "external.2", &[""]).is_err()); + assert!(rmap.url_for(&req, "external.2", [""]).is_err()); } #[test] @@ -524,7 +524,7 @@ mod tests { let req = req.to_http_request(); assert_eq!( - rmap.url_for(&req, "duck", &["abcd"]).unwrap().to_string(), + rmap.url_for(&req, "duck", ["abcd"]).unwrap().to_string(), "https://duck.com/abcd" ); } @@ -552,9 +552,9 @@ mod tests { let req = crate::test::TestRequest::default().to_http_request(); - let url = rmap.url_for(&req, "nested", &[""; 0]).unwrap().to_string(); + let url = rmap.url_for(&req, "nested", [""; 0]).unwrap().to_string(); assert_eq!(url, "http://localhost:8080/bar/nested"); - assert!(rmap.url_for(&req, "missing", &["u123"]).is_err()); + assert!(rmap.url_for(&req, "missing", ["u123"]).is_err()); } } diff --git a/actix-web/src/route.rs b/actix-web/src/route.rs index 0410b99dd..b37128f2c 100644 --- a/actix-web/src/route.rs +++ b/actix-web/src/route.rs @@ -1,15 +1,17 @@ use std::{mem, rc::Rc}; -use actix_http::Method; +use actix_http::{body::MessageBody, Method}; use actix_service::{ + apply, boxed::{self, BoxService}, - fn_service, Service, ServiceFactory, ServiceFactoryExt, + fn_service, Service, ServiceFactory, ServiceFactoryExt, Transform, }; use futures_core::future::LocalBoxFuture; use crate::{ guard::{self, Guard}, handler::{handler_service, Handler}, + middleware::Compat, service::{BoxedHttpServiceFactory, ServiceRequest, ServiceResponse}, Error, FromRequest, HttpResponse, Responder, }; @@ -35,6 +37,31 @@ impl Route { } } + /// Registers a route middleware. + /// + /// `mw` is a middleware component (type), that can modify the requests and responses handled by + /// this `Route`. + /// + /// See [`App::wrap`](crate::App::wrap) for more details. + #[doc(alias = "middleware")] + #[doc(alias = "use")] // nodejs terminology + pub fn wrap(self, mw: M) -> Route + where + M: Transform< + BoxService, + ServiceRequest, + Response = ServiceResponse, + Error = Error, + InitError = (), + > + 'static, + B: MessageBody + 'static, + { + Route { + service: boxed::factory(apply(Compat::new(mw), self.service)), + guards: self.guards, + } + } + pub(crate) fn take_guards(&mut self) -> Vec> { mem::take(Rc::get_mut(&mut self.guards).unwrap()) } @@ -246,11 +273,15 @@ mod tests { use futures_core::future::LocalBoxFuture; use serde::Serialize; - use crate::dev::{always_ready, fn_factory, fn_service, Service}; - use crate::http::{header, Method, StatusCode}; - use crate::service::{ServiceRequest, ServiceResponse}; - use crate::test::{call_service, init_service, read_body, TestRequest}; - use crate::{error, web, App, HttpResponse}; + use crate::{ + dev::{always_ready, fn_factory, fn_service, Service}, + error, + http::{header, Method, StatusCode}, + middleware::{DefaultHeaders, Logger}, + service::{ServiceRequest, ServiceResponse}, + test::{call_service, init_service, read_body, TestRequest}, + web, App, HttpResponse, + }; #[derive(Serialize, PartialEq, Debug)] struct MyObject { @@ -323,6 +354,44 @@ mod tests { assert_eq!(body, Bytes::from_static(b"{\"name\":\"test\"}")); } + #[actix_rt::test] + async fn route_middleware() { + let srv = init_service( + App::new() + .route("/", web::get().to(HttpResponse::Ok).wrap(Logger::default())) + .service( + web::resource("/test") + .route(web::get().to(HttpResponse::Ok)) + .route( + web::post() + .to(HttpResponse::Created) + .wrap(DefaultHeaders::new().add(("x-test", "x-posted"))), + ) + .route( + web::delete() + .to(HttpResponse::Accepted) + // logger changes body type, proving Compat is not needed + .wrap(Logger::default()), + ), + ), + ) + .await; + + let req = TestRequest::get().uri("/test").to_request(); + let res = call_service(&srv, req).await; + assert_eq!(res.status(), StatusCode::OK); + assert!(!res.headers().contains_key("x-test")); + + let req = TestRequest::post().uri("/test").to_request(); + let res = call_service(&srv, req).await; + assert_eq!(res.status(), StatusCode::CREATED); + assert_eq!(res.headers().get("x-test").unwrap(), "x-posted"); + + let req = TestRequest::delete().uri("/test").to_request(); + let res = call_service(&srv, req).await; + assert_eq!(res.status(), StatusCode::ACCEPTED); + } + #[actix_rt::test] async fn test_service_handler() { struct HelloWorld; diff --git a/actix-web/src/scope.rs b/actix-web/src/scope.rs index 0fcc83d70..9af05674b 100644 --- a/actix-web/src/scope.rs +++ b/actix-web/src/scope.rs @@ -40,7 +40,7 @@ type Guards = Vec>; /// use actix_web::{web, App, HttpResponse}; /// /// let app = App::new().service( -/// web::scope("/{project_id}/") +/// web::scope("/{project_id}") /// .service(web::resource("/path1").to(|| async { "OK" })) /// .service(web::resource("/path2").route(web::get().to(|| HttpResponse::Ok()))) /// .service(web::resource("/path3").route(web::head().to(HttpResponse::MethodNotAllowed))) @@ -198,6 +198,10 @@ where .get_or_insert_with(Extensions::new) .extend(cfg.app_data); + if let Some(default) = cfg.default { + self.default = Some(default); + } + self } @@ -1129,7 +1133,7 @@ mod tests { "/", web::get().to(|req: HttpRequest| { HttpResponse::Ok() - .body(req.url_for("youtube", &["xxxxxx"]).unwrap().to_string()) + .body(req.url_for("youtube", ["xxxxxx"]).unwrap().to_string()) }), ); })); @@ -1148,8 +1152,7 @@ mod tests { let srv = init_service(App::new().service(web::scope("/a").service( web::scope("/b").service(web::resource("/c/{stuff}").name("c").route( web::get().to(|req: HttpRequest| { - HttpResponse::Ok() - .body(format!("{}", req.url_for("c", &["12345"]).unwrap())) + HttpResponse::Ok().body(format!("{}", req.url_for("c", ["12345"]).unwrap())) }), )), ))) diff --git a/actix-web/src/server.rs b/actix-web/src/server.rs index 99812600c..3a8897f11 100644 --- a/actix-web/src/server.rs +++ b/actix-web/src/server.rs @@ -18,6 +18,9 @@ use actix_tls::accept::openssl::reexports::{AlpnError, SslAcceptor, SslAcceptorB #[cfg(feature = "rustls")] use actix_tls::accept::rustls::reexports::ServerConfig as RustlsServerConfig; +#[cfg(any(feature = "openssl", feature = "rustls"))] +use actix_http::TlsAcceptorConfig; + use crate::{config::AppConfig, Error}; struct Socket { @@ -30,23 +33,30 @@ struct Config { keep_alive: KeepAlive, client_request_timeout: Duration, client_disconnect_timeout: Duration, + #[cfg(any(feature = "openssl", feature = "rustls"))] + tls_handshake_timeout: Option, } /// An HTTP Server. /// /// Create new HTTP server with application factory. /// +/// # HTTP/2 +/// Currently, HTTP/2 is only supported when using TLS (HTTPS). See `bind_rustls` or `bind_openssl`. +/// +/// # Examples /// ```no_run /// use actix_web::{web, App, HttpResponse, HttpServer}; /// -/// #[actix_rt::main] +/// #[actix_web::main] /// async fn main() -> std::io::Result<()> { -/// HttpServer::new( -/// || App::new() -/// .service(web::resource("/").to(|| HttpResponse::Ok()))) -/// .bind("127.0.0.1:59090")? -/// .run() -/// .await +/// HttpServer::new(|| { +/// App::new() +/// .service(web::resource("/").to(|| async { "hello world" })) +/// }) +/// .bind(("127.0.0.1", 8080))? +/// .run() +/// .await /// } /// ``` pub struct HttpServer @@ -92,6 +102,8 @@ where keep_alive: KeepAlive::default(), client_request_timeout: Duration::from_secs(5), client_disconnect_timeout: Duration::from_secs(1), + #[cfg(any(feature = "rustls", feature = "openssl"))] + tls_handshake_timeout: None, })), backlog: 1024, sockets: Vec::new(), @@ -101,13 +113,135 @@ where } } + /// Sets number of workers to start (per bind address). + /// + /// By default, the number of available physical CPUs is used as the worker count. + pub fn workers(mut self, num: usize) -> Self { + self.builder = self.builder.workers(num); + self + } + + /// Sets server keep-alive preference. + /// + /// By default keep-alive is set to 5 seconds. + pub fn keep_alive>(self, val: T) -> Self { + self.config.lock().unwrap().keep_alive = val.into(); + self + } + + /// Sets the maximum number of pending connections. + /// + /// This refers to the number of clients that can be waiting to be served. Exceeding this number + /// results in the client getting an error when attempting to connect. It should only affect + /// servers under significant load. + /// + /// Generally set in the 64–2048 range. Default value is 2048. + /// + /// This method will have no effect if called after a `bind()`. + pub fn backlog(mut self, backlog: u32) -> Self { + self.backlog = backlog; + self.builder = self.builder.backlog(backlog); + self + } + + /// Sets the per-worker maximum number of concurrent connections. + /// + /// All socket listeners will stop accepting connections when this limit is reached for + /// each worker. + /// + /// By default max connections is set to a 25k. + pub fn max_connections(mut self, num: usize) -> Self { + self.builder = self.builder.max_concurrent_connections(num); + self + } + + /// Sets the per-worker maximum concurrent TLS connection limit. + /// + /// All listeners will stop accepting connections when this limit is reached. It can be used to + /// limit the global TLS CPU usage. + /// + /// By default max connections is set to a 256. + #[allow(unused_variables)] + pub fn max_connection_rate(self, num: usize) -> Self { + #[cfg(any(feature = "rustls", feature = "openssl"))] + actix_tls::accept::max_concurrent_tls_connect(num); + self + } + + /// Sets max number of threads for each worker's blocking task thread pool. + /// + /// One thread pool is set up **per worker**; not shared across workers. + /// + /// By default set to 512 divided by the number of workers. + pub fn worker_max_blocking_threads(mut self, num: usize) -> Self { + self.builder = self.builder.worker_max_blocking_threads(num); + self + } + + /// Sets server client timeout for first request. + /// + /// Defines a timeout for reading client request head. If a client does not transmit the entire + /// set headers within this time, the request is terminated with a 408 (Request Timeout) error. + /// + /// To disable timeout set value to 0. + /// + /// By default client timeout is set to 5000 milliseconds. + pub fn client_request_timeout(self, dur: Duration) -> Self { + self.config.lock().unwrap().client_request_timeout = dur; + self + } + + #[doc(hidden)] + #[deprecated(since = "4.0.0", note = "Renamed to `client_request_timeout`.")] + pub fn client_timeout(self, dur: Duration) -> Self { + self.client_request_timeout(dur) + } + + /// Sets server connection shutdown timeout. + /// + /// Defines a timeout for connection shutdown. If a shutdown procedure does not complete within + /// this time, the request is dropped. + /// + /// To disable timeout set value to 0. + /// + /// By default client timeout is set to 5000 milliseconds. + pub fn client_disconnect_timeout(self, dur: Duration) -> Self { + self.config.lock().unwrap().client_disconnect_timeout = dur; + self + } + + /// Sets TLS handshake timeout. + /// + /// Defines a timeout for TLS handshake. If the TLS handshake does not complete within this + /// time, the connection is closed. + /// + /// By default handshake timeout is set to 3000 milliseconds. + #[cfg(any(feature = "openssl", feature = "rustls"))] + #[cfg_attr(docsrs, doc(cfg(any(feature = "openssl", feature = "rustls"))))] + pub fn tls_handshake_timeout(self, dur: Duration) -> Self { + self.config + .lock() + .unwrap() + .tls_handshake_timeout + .replace(dur); + + self + } + + #[doc(hidden)] + #[deprecated(since = "4.0.0", note = "Renamed to `client_disconnect_timeout`.")] + pub fn client_shutdown(self, dur: u64) -> Self { + self.client_disconnect_timeout(Duration::from_millis(dur)) + } + /// Sets function that will be called once before each connection is handled. + /// /// It will receive a `&std::any::Any`, which contains underlying connection type and an /// [Extensions] container so that connection data can be accessed in middleware and handlers. /// /// # Connection Types - /// - `actix_tls::accept::openssl::TlsStream` when using openssl. - /// - `actix_tls::accept::rustls::TlsStream` when using rustls. + /// - `actix_tls::accept::openssl::TlsStream` when using OpenSSL. + /// - `actix_tls::accept::rustls::TlsStream` when using Rustls. /// - `actix_web::rt::net::TcpStream` when no encryption is used. /// /// See the `on_connect` example for additional details. @@ -126,140 +260,35 @@ where } } - /// Set number of workers to start. + /// Sets server host name. /// - /// By default, the number of available physical CPUs is used as the worker count. - pub fn workers(mut self, num: usize) -> Self { - self.builder = self.builder.workers(num); - self - } - - /// Set the maximum number of pending connections. + /// Host name is used by application router as a hostname for url generation. Check + /// [`ConnectionInfo`](crate::dev::ConnectionInfo::host()) docs for more info. /// - /// This refers to the number of clients that can be waiting to be served. - /// Exceeding this number results in the client getting an error when - /// attempting to connect. It should only affect servers under significant - /// load. - /// - /// Generally set in the 64-2048 range. Default value is 2048. - /// - /// This method should be called before `bind()` method call. - pub fn backlog(mut self, backlog: u32) -> Self { - self.backlog = backlog; - self.builder = self.builder.backlog(backlog); - self - } - - /// Sets the maximum per-worker number of concurrent connections. - /// - /// All socket listeners will stop accepting connections when this limit is reached for - /// each worker. - /// - /// By default max connections is set to a 25k. - pub fn max_connections(mut self, num: usize) -> Self { - self.builder = self.builder.max_concurrent_connections(num); - self - } - - /// Sets the maximum per-worker concurrent connection establish process. - /// - /// All listeners will stop accepting connections when this limit is reached. It can be used to - /// limit the global TLS CPU usage. - /// - /// By default max connections is set to a 256. - #[allow(unused_variables)] - pub fn max_connection_rate(self, num: usize) -> Self { - #[cfg(any(feature = "rustls", feature = "openssl"))] - actix_tls::accept::max_concurrent_tls_connect(num); - self - } - - /// Set max number of threads for each worker's blocking task thread pool. - /// - /// One thread pool is set up **per worker**; not shared across workers. - /// - /// By default set to 512 / workers. - pub fn worker_max_blocking_threads(mut self, num: usize) -> Self { - self.builder = self.builder.worker_max_blocking_threads(num); - self - } - - /// Set server keep-alive setting. - /// - /// By default keep alive is set to a 5 seconds. - pub fn keep_alive>(self, val: T) -> Self { - self.config.lock().unwrap().keep_alive = val.into(); - self - } - - /// Set server client timeout in milliseconds for first request. - /// - /// Defines a timeout for reading client request header. If a client does not transmit - /// the entire set headers within this time, the request is terminated with - /// the 408 (Request Time-out) error. - /// - /// To disable timeout set value to 0. - /// - /// By default client timeout is set to 5000 milliseconds. - pub fn client_request_timeout(self, dur: Duration) -> Self { - self.config.lock().unwrap().client_request_timeout = dur; - self - } - - #[doc(hidden)] - #[deprecated(since = "4.0.0", note = "Renamed to `client_request_timeout`.")] - pub fn client_timeout(self, dur: Duration) -> Self { - self.client_request_timeout(dur) - } - - /// Set server connection shutdown timeout in milliseconds. - /// - /// Defines a timeout for shutdown connection. If a shutdown procedure does not complete - /// within this time, the request is dropped. - /// - /// To disable timeout set value to 0. - /// - /// By default client timeout is set to 5000 milliseconds. - pub fn client_disconnect_timeout(self, dur: Duration) -> Self { - self.config.lock().unwrap().client_disconnect_timeout = dur; - self - } - - #[doc(hidden)] - #[deprecated(since = "4.0.0", note = "Renamed to `client_disconnect_timeout`.")] - pub fn client_shutdown(self, dur: u64) -> Self { - self.client_disconnect_timeout(Duration::from_millis(dur)) - } - - /// Set server host name. - /// - /// Host name is used by application router as a hostname for url generation. - /// Check [ConnectionInfo](super::dev::ConnectionInfo::host()) - /// documentation for more information. - /// - /// By default host name is set to a "localhost" value. + /// By default, hostname is set to "localhost". pub fn server_hostname>(self, val: T) -> Self { self.config.lock().unwrap().host = Some(val.as_ref().to_owned()); self } - /// Stop Actix `System` after server shutdown. + /// Flags the `System` to exit after server shutdown. + /// + /// Does nothing when running under `#[tokio::main]` runtime. pub fn system_exit(mut self) -> Self { self.builder = self.builder.system_exit(); self } - /// Disable signal handling + /// Disables signal handling. pub fn disable_signals(mut self) -> Self { self.builder = self.builder.disable_signals(); self } - /// Timeout for graceful workers shutdown. + /// Sets timeout for graceful worker shutdown of workers. /// - /// After receiving a stop signal, workers have this much time to finish - /// serving requests. Workers still alive after the timeout are force - /// dropped. + /// After receiving a stop signal, workers have this much time to finish serving requests. + /// Workers still alive after the timeout are force dropped. /// /// By default shutdown timeout sets to 30 seconds. pub fn shutdown_timeout(mut self, sec: u64) -> Self { @@ -267,33 +296,141 @@ where self } - /// Get addresses of bound sockets. + /// Returns addresses of bound sockets. pub fn addrs(&self) -> Vec { self.sockets.iter().map(|s| s.addr).collect() } - /// Get addresses of bound sockets and the scheme for it. + /// Returns addresses of bound sockets and the scheme for it. /// - /// This is useful when the server is bound from different sources - /// with some sockets listening on HTTP and some listening on HTTPS - /// and the user should be presented with an enumeration of which - /// socket requires which protocol. + /// This is useful when the server is bound from different sources with some sockets listening + /// on HTTP and some listening on HTTPS and the user should be presented with an enumeration of + /// which socket requires which protocol. pub fn addrs_with_scheme(&self) -> Vec<(net::SocketAddr, &str)> { self.sockets.iter().map(|s| (s.addr, s.scheme)).collect() } - /// Use listener for accepting incoming connection requests + /// Resolves socket address(es) and binds server to created listener(s). /// - /// HttpServer does not change any configuration for TcpListener, - /// it needs to be configured before passing it to listen() method. + /// # Hostname Resolution + /// When `addr` includes a hostname, it is possible for this method to bind to both the IPv4 and + /// IPv6 addresses that result from a DNS lookup. You can test this by passing `localhost:8080` + /// and noting that the server binds to `127.0.0.1:8080` _and_ `[::1]:8080`. To bind additional + /// addresses, call this method multiple times. + /// + /// Note that, if a DNS lookup is required, resolving hostnames is a blocking operation. + /// + /// # Typical Usage + /// In general, use `127.0.0.1:` when testing locally and `0.0.0.0:` when deploying + /// (with or without a reverse proxy or load balancer) so that the server is accessible. + /// + /// # Errors + /// Returns an `io::Error` if: + /// - `addrs` cannot be resolved into one or more socket addresses; + /// - all the resolved socket addresses are already bound. + /// + /// # Example + /// ``` + /// # use actix_web::{App, HttpServer}; + /// # fn inner() -> std::io::Result<()> { + /// HttpServer::new(|| App::new()) + /// .bind(("127.0.0.1", 8080))? + /// .bind("[::1]:9000")? + /// # ; Ok(()) } + /// ``` + pub fn bind(mut self, addrs: A) -> io::Result { + let sockets = self.bind2(addrs)?; + + for lst in sockets { + self = self.listen(lst)?; + } + + Ok(self) + } + + fn bind2(&self, addrs: A) -> io::Result> { + let mut err = None; + let mut success = false; + let mut sockets = Vec::new(); + + for addr in addrs.to_socket_addrs()? { + match create_tcp_listener(addr, self.backlog) { + Ok(lst) => { + success = true; + sockets.push(lst); + } + Err(e) => err = Some(e), + } + } + + if success { + Ok(sockets) + } else if let Some(e) = err.take() { + Err(e) + } else { + Err(io::Error::new( + io::ErrorKind::Other, + "Can not bind to address.", + )) + } + } + + /// Resolves socket address(es) and binds server to created listener(s) for TLS connections + /// using Rustls. + /// + /// See [`bind()`](Self::bind) for more details on `addrs` argument. + /// + /// ALPN protocols "h2" and "http/1.1" are added to any configured ones. + #[cfg(feature = "rustls")] + #[cfg_attr(docsrs, doc(cfg(feature = "rustls")))] + pub fn bind_rustls( + mut self, + addrs: A, + config: RustlsServerConfig, + ) -> io::Result { + let sockets = self.bind2(addrs)?; + for lst in sockets { + self = self.listen_rustls_inner(lst, config.clone())?; + } + Ok(self) + } + + /// Resolves socket address(es) and binds server to created listener(s) for TLS connections + /// using OpenSSL. + /// + /// See [`bind()`](Self::bind) for more details on `addrs` argument. + /// + /// ALPN protocols "h2" and "http/1.1" are added to any configured ones. + #[cfg(feature = "openssl")] + #[cfg_attr(docsrs, doc(cfg(feature = "openssl")))] + pub fn bind_openssl
(mut self, addrs: A, builder: SslAcceptorBuilder) -> io::Result + where + A: net::ToSocketAddrs, + { + let sockets = self.bind2(addrs)?; + let acceptor = openssl_acceptor(builder)?; + + for lst in sockets { + self = self.listen_openssl_inner(lst, acceptor.clone())?; + } + + Ok(self) + } + + /// Binds to existing listener for accepting incoming connection requests. + /// + /// No changes are made to `lst`'s configuration. Ensure it is configured properly before + /// passing ownership to `listen()`. pub fn listen(mut self, lst: net::TcpListener) -> io::Result { let cfg = self.config.clone(); let factory = self.factory.clone(); let addr = lst.local_addr().unwrap(); + self.sockets.push(Socket { addr, scheme: "http", }); + let on_connect_fn = self.on_connect_fn.clone(); self.builder = @@ -326,69 +463,13 @@ where Ok(self) } - #[cfg(feature = "openssl")] - /// Use listener for accepting incoming tls connection requests + /// Binds to existing listener for accepting incoming TLS connection requests using Rustls. /// - /// This method sets alpn protocols to "h2" and "http/1.1" - pub fn listen_openssl( - self, - lst: net::TcpListener, - builder: SslAcceptorBuilder, - ) -> io::Result { - self.listen_ssl_inner(lst, openssl_acceptor(builder)?) - } - - #[cfg(feature = "openssl")] - fn listen_ssl_inner( - mut self, - lst: net::TcpListener, - acceptor: SslAcceptor, - ) -> io::Result { - let factory = self.factory.clone(); - let cfg = self.config.clone(); - let addr = lst.local_addr().unwrap(); - self.sockets.push(Socket { - addr, - scheme: "https", - }); - - let on_connect_fn = self.on_connect_fn.clone(); - - self.builder = - self.builder - .listen(format!("actix-web-service-{}", addr), lst, move || { - let c = cfg.lock().unwrap(); - let host = c.host.clone().unwrap_or_else(|| format!("{}", addr)); - - let svc = HttpService::build() - .keep_alive(c.keep_alive) - .client_request_timeout(c.client_request_timeout) - .client_disconnect_timeout(c.client_disconnect_timeout) - .local_addr(addr); - - let svc = if let Some(handler) = on_connect_fn.clone() { - svc.on_connect_ext(move |io: &_, ext: _| (handler)(io as &dyn Any, ext)) - } else { - svc - }; - - let fac = factory() - .into_factory() - .map_err(|err| err.into().error_response()); - - svc.finish(map_config(fac, move |_| { - AppConfig::new(true, host.clone(), addr) - })) - .openssl(acceptor.clone()) - })?; - - Ok(self) - } - + /// See [`listen()`](Self::listen) for more details on the `lst` argument. + /// + /// ALPN protocols "h2" and "http/1.1" are added to any configured ones. #[cfg(feature = "rustls")] - /// Use listener for accepting incoming tls connection requests - /// - /// This method prepends alpn protocols "h2" and "http/1.1" to configured ones + #[cfg_attr(docsrs, doc(cfg(feature = "rustls")))] pub fn listen_rustls( self, lst: net::TcpListener, @@ -434,91 +515,139 @@ where .into_factory() .map_err(|err| err.into().error_response()); + let acceptor_config = match c.tls_handshake_timeout { + Some(dur) => TlsAcceptorConfig::default().handshake_timeout(dur), + None => TlsAcceptorConfig::default(), + }; + svc.finish(map_config(fac, move |_| { AppConfig::new(true, host.clone(), addr) })) - .rustls(config.clone()) + .rustls_with_config(config.clone(), acceptor_config) })?; Ok(self) } - /// The socket address to bind + /// Binds to existing listener for accepting incoming TLS connection requests using OpenSSL. /// - /// To bind multiple addresses this method can be called multiple times. - pub fn bind(mut self, addr: A) -> io::Result { - let sockets = self.bind2(addr)?; - - for lst in sockets { - self = self.listen(lst)?; - } - - Ok(self) - } - - fn bind2(&self, addr: A) -> io::Result> { - let mut err = None; - let mut success = false; - let mut sockets = Vec::new(); - - for addr in addr.to_socket_addrs()? { - match create_tcp_listener(addr, self.backlog) { - Ok(lst) => { - success = true; - sockets.push(lst); - } - Err(e) => err = Some(e), - } - } - - if success { - Ok(sockets) - } else if let Some(e) = err.take() { - Err(e) - } else { - Err(io::Error::new( - io::ErrorKind::Other, - "Can not bind to address.", - )) - } + /// See [`listen()`](Self::listen) for more details on the `lst` argument. + /// + /// ALPN protocols "h2" and "http/1.1" are added to any configured ones. + #[cfg(feature = "openssl")] + #[cfg_attr(docsrs, doc(cfg(feature = "openssl")))] + pub fn listen_openssl( + self, + lst: net::TcpListener, + builder: SslAcceptorBuilder, + ) -> io::Result { + self.listen_openssl_inner(lst, openssl_acceptor(builder)?) } #[cfg(feature = "openssl")] - /// Start listening for incoming tls connections. - /// - /// This method sets alpn protocols to "h2" and "http/1.1" - pub fn bind_openssl(mut self, addr: A, builder: SslAcceptorBuilder) -> io::Result - where - A: net::ToSocketAddrs, - { - let sockets = self.bind2(addr)?; - let acceptor = openssl_acceptor(builder)?; - - for lst in sockets { - self = self.listen_ssl_inner(lst, acceptor.clone())?; - } - - Ok(self) - } - - #[cfg(feature = "rustls")] - /// Start listening for incoming tls connections. - /// - /// This method prepends alpn protocols "h2" and "http/1.1" to configured ones - pub fn bind_rustls( + fn listen_openssl_inner( mut self, - addr: A, - config: RustlsServerConfig, + lst: net::TcpListener, + acceptor: SslAcceptor, ) -> io::Result { - let sockets = self.bind2(addr)?; - for lst in sockets { - self = self.listen_rustls_inner(lst, config.clone())?; - } + let factory = self.factory.clone(); + let cfg = self.config.clone(); + let addr = lst.local_addr().unwrap(); + self.sockets.push(Socket { + addr, + scheme: "https", + }); + + let on_connect_fn = self.on_connect_fn.clone(); + + self.builder = + self.builder + .listen(format!("actix-web-service-{}", addr), lst, move || { + let c = cfg.lock().unwrap(); + let host = c.host.clone().unwrap_or_else(|| format!("{}", addr)); + + let svc = HttpService::build() + .keep_alive(c.keep_alive) + .client_request_timeout(c.client_request_timeout) + .client_disconnect_timeout(c.client_disconnect_timeout) + .local_addr(addr); + + let svc = if let Some(handler) = on_connect_fn.clone() { + svc.on_connect_ext(move |io: &_, ext: _| (handler)(io as &dyn Any, ext)) + } else { + svc + }; + + let fac = factory() + .into_factory() + .map_err(|err| err.into().error_response()); + + // false positive lint (?) + #[allow(clippy::significant_drop_in_scrutinee)] + let acceptor_config = match c.tls_handshake_timeout { + Some(dur) => TlsAcceptorConfig::default().handshake_timeout(dur), + None => TlsAcceptorConfig::default(), + }; + + svc.finish(map_config(fac, move |_| { + AppConfig::new(true, host.clone(), addr) + })) + .openssl_with_config(acceptor.clone(), acceptor_config) + })?; + Ok(self) } + /// Opens Unix Domain Socket (UDS) from `uds` path and binds server to created listener. + #[cfg(unix)] + pub fn bind_uds(mut self, uds_path: A) -> io::Result + where + A: AsRef, + { + use actix_http::Protocol; + use actix_rt::net::UnixStream; + use actix_service::{fn_service, ServiceFactoryExt as _}; + + let cfg = self.config.clone(); + let factory = self.factory.clone(); + let socket_addr = + net::SocketAddr::new(net::IpAddr::V4(net::Ipv4Addr::new(127, 0, 0, 1)), 8080); + + self.sockets.push(Socket { + scheme: "http", + addr: socket_addr, + }); + + self.builder = self.builder.bind_uds( + format!("actix-web-service-{:?}", uds_path.as_ref()), + uds_path, + move || { + let c = cfg.lock().unwrap(); + let config = AppConfig::new( + false, + c.host.clone().unwrap_or_else(|| format!("{}", socket_addr)), + socket_addr, + ); + + let fac = factory() + .into_factory() + .map_err(|err| err.into().error_response()); + + fn_service(|io: UnixStream| async { Ok((io, Protocol::Http1, None)) }).and_then( + HttpService::build() + .keep_alive(c.keep_alive) + .client_request_timeout(c.client_request_timeout) + .client_disconnect_timeout(c.client_disconnect_timeout) + .finish(map_config(fac, move |_| config.clone())), + ) + }, + )?; + + Ok(self) + } + + /// Binds to existing Unix Domain Socket (UDS) listener. #[cfg(unix)] - /// Start listening for unix domain (UDS) connections on existing listener. pub fn listen_uds(mut self, lst: std::os::unix::net::UnixListener) -> io::Result { use actix_http::Protocol; use actix_rt::net::UnixStream; @@ -565,54 +694,6 @@ where })?; Ok(self) } - - /// Start listening for incoming unix domain connections. - #[cfg(unix)] - pub fn bind_uds(mut self, addr: A) -> io::Result - where - A: AsRef, - { - use actix_http::Protocol; - use actix_rt::net::UnixStream; - use actix_service::{fn_service, ServiceFactoryExt as _}; - - let cfg = self.config.clone(); - let factory = self.factory.clone(); - let socket_addr = - net::SocketAddr::new(net::IpAddr::V4(net::Ipv4Addr::new(127, 0, 0, 1)), 8080); - - self.sockets.push(Socket { - scheme: "http", - addr: socket_addr, - }); - - self.builder = self.builder.bind_uds( - format!("actix-web-service-{:?}", addr.as_ref()), - addr, - move || { - let c = cfg.lock().unwrap(); - let config = AppConfig::new( - false, - c.host.clone().unwrap_or_else(|| format!("{}", socket_addr)), - socket_addr, - ); - - let fac = factory() - .into_factory() - .map_err(|err| err.into().error_response()); - - fn_service(|io: UnixStream| async { Ok((io, Protocol::Http1, None)) }).and_then( - HttpService::build() - .keep_alive(c.keep_alive) - .client_request_timeout(c.client_request_timeout) - .client_disconnect_timeout(c.client_disconnect_timeout) - .finish(map_config(fac, move |_| config.clone())), - ) - }, - )?; - - Ok(self) - } } impl HttpServer @@ -628,25 +709,16 @@ where { /// Start listening for incoming connections. /// - /// This method starts number of HTTP workers in separate threads. - /// For each address this method starts separate thread which does - /// `accept()` in a loop. + /// # Workers + /// This method starts a number of HTTP workers in separate threads. The number of workers in a + /// set is defined by [`workers()`](Self::workers) or, by default, the number of the machine's + /// physical cores. One worker set is created for each socket address to be bound. For example, + /// if workers is set to 4, and there are 2 addresses to bind, then 8 worker threads will be + /// spawned. /// - /// This methods panics if no socket address can be bound or an `Actix` system is not yet - /// configured. - /// - /// ```no_run - /// use std::io; - /// use actix_web::{web, App, HttpResponse, HttpServer}; - /// - /// #[actix_rt::main] - /// async fn main() -> io::Result<()> { - /// HttpServer::new(|| App::new().service(web::resource("/").to(|| HttpResponse::Ok()))) - /// .bind("127.0.0.1:0")? - /// .run() - /// .await - /// } - /// ``` + /// # Panics + /// This methods panics if no socket addresses were successfully bound or if no Tokio runtime + /// is set up. pub fn run(self) -> Server { self.builder.run() } diff --git a/actix-web/src/service.rs b/actix-web/src/service.rs index 426e9d62b..0ad92d8a1 100644 --- a/actix-web/src/service.rs +++ b/actix-web/src/service.rs @@ -24,7 +24,7 @@ use crate::{ guard::{Guard, GuardContext}, info::ConnectionInfo, rmap::ResourceMap, - Error, HttpRequest, HttpResponse, + Error, FromRequest, HttpRequest, HttpResponse, }; pub(crate) type BoxedHttpService = BoxService, Error>; @@ -95,6 +95,43 @@ impl ServiceRequest { (&mut self.req, &mut self.payload) } + /// Returns immutable accessors to inner parts. + #[inline] + pub fn parts(&self) -> (&HttpRequest, &Payload) { + (&self.req, &self.payload) + } + + /// Returns immutable accessor to inner [`HttpRequest`]. + #[inline] + pub fn request(&self) -> &HttpRequest { + &self.req + } + + /// Derives a type from this request using an [extractor](crate::FromRequest). + /// + /// Returns the `T` extractor's `Future` type which can be `await`ed. This is particularly handy + /// when you want to use an extractor in a middleware implementation. + /// + /// # Examples + /// ``` + /// use actix_web::{ + /// dev::{ServiceRequest, ServiceResponse}, + /// web::Path, Error + /// }; + /// + /// async fn my_helper(mut srv_req: ServiceRequest) -> Result { + /// let path = srv_req.extract::>().await?; + /// // [...] + /// # todo!() + /// } + /// ``` + pub fn extract(&mut self) -> ::Future + where + T: FromRequest, + { + T::from_request(&self.req, &mut self.payload) + } + /// Construct request from parts. pub fn from_parts(req: HttpRequest, payload: Payload) -> Self { #[cfg(debug_assertions)] diff --git a/actix-web/src/test/test_request.rs b/actix-web/src/test/test_request.rs index a368d873f..e81561d17 100644 --- a/actix-web/src/test/test_request.rs +++ b/actix-web/src/test/test_request.rs @@ -33,7 +33,7 @@ use crate::cookie::{Cookie, CookieJar}; /// use actix_web::{test, HttpRequest, HttpResponse, HttpMessage}; /// use actix_web::http::{header, StatusCode}; /// -/// async fn index(req: HttpRequest) -> HttpResponse { +/// async fn handler(req: HttpRequest) -> HttpResponse { /// if let Some(hdr) = req.headers().get(header::CONTENT_TYPE) { /// HttpResponse::Ok().into() /// } else { @@ -45,14 +45,15 @@ use crate::cookie::{Cookie, CookieJar}; /// # // force rustdoc to display the correct thing and also compile check the test /// # async fn _test() {} /// async fn test_index() { -/// let req = test::TestRequest::default().insert_header(header::ContentType::plaintext()) +/// let req = test::TestRequest::default() +/// .insert_header(header::ContentType::plaintext()) /// .to_http_request(); /// -/// let resp = index(req).await; +/// let resp = handler(req).await; /// assert_eq!(resp.status(), StatusCode::OK); /// /// let req = test::TestRequest::default().to_http_request(); -/// let resp = index(req).await; +/// let resp = handler(req).await; /// assert_eq!(resp.status(), StatusCode::BAD_REQUEST); /// } /// ``` diff --git a/actix-web/src/types/either.rs b/actix-web/src/types/either.rs index 0eafb9e43..119dd0d62 100644 --- a/actix-web/src/types/either.rs +++ b/actix-web/src/types/either.rs @@ -49,7 +49,7 @@ use crate::{ /// ``` /// /// # Responder -/// It may be desireable to use a concrete type for a response with multiple branches. As long as +/// It may be desirable to use a concrete type for a response with multiple branches. As long as /// both types implement `Responder`, so will the `Either` type, enabling it to be used as a /// handler's return type. /// @@ -73,7 +73,7 @@ use crate::{ /// } /// } /// ``` -#[derive(Debug, PartialEq)] +#[derive(Debug, PartialEq, Eq)] pub enum Either { /// A value of type `L`. Left(L), diff --git a/actix-web/src/types/json.rs b/actix-web/src/types/json.rs index 8fdbfafa4..4eab55175 100644 --- a/actix-web/src/types/json.rs +++ b/actix-web/src/types/json.rs @@ -290,7 +290,7 @@ const DEFAULT_CONFIG: JsonConfig = JsonConfig { impl Default for JsonConfig { fn default() -> Self { - DEFAULT_CONFIG.clone() + DEFAULT_CONFIG } } diff --git a/actix-web/src/types/path.rs b/actix-web/src/types/path.rs index 0fcac2c19..a90c912f6 100644 --- a/actix-web/src/types/path.rs +++ b/actix-web/src/types/path.rs @@ -134,6 +134,7 @@ where /// ``` #[derive(Clone, Default)] pub struct PathConfig { + #[allow(clippy::type_complexity)] err_handler: Option Error + Send + Sync>>, } @@ -183,6 +184,7 @@ mod tests { assert!(Path::::from_request(&req, &mut pl).await.is_err()); } + #[allow(clippy::let_unit_value)] #[actix_rt::test] async fn test_tuple_extract() { let resource = ResourceDef::new("/{key}/{value}/"); diff --git a/actix-web/src/types/payload.rs b/actix-web/src/types/payload.rs index b47a39e97..f17a4ed6d 100644 --- a/actix-web/src/types/payload.rs +++ b/actix-web/src/types/payload.rs @@ -113,7 +113,7 @@ pub struct BytesExtractFut { body_fut: HttpMessageBody, } -impl<'a> Future for BytesExtractFut { +impl Future for BytesExtractFut { type Output = Result; fn poll(mut self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll { @@ -167,7 +167,7 @@ pub struct StringExtractFut { encoding: &'static Encoding, } -impl<'a> Future for StringExtractFut { +impl Future for StringExtractFut { type Output = Result; fn poll(mut self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll { @@ -271,7 +271,7 @@ const DEFAULT_CONFIG: PayloadConfig = PayloadConfig { impl Default for PayloadConfig { fn default() -> Self { - DEFAULT_CONFIG.clone() + DEFAULT_CONFIG } } diff --git a/actix-web/src/types/query.rs b/actix-web/src/types/query.rs index 97d17123d..e71b886f2 100644 --- a/actix-web/src/types/query.rs +++ b/actix-web/src/types/query.rs @@ -169,6 +169,7 @@ impl FromRequest for Query { /// ``` #[derive(Clone, Default)] pub struct QueryConfig { + #[allow(clippy::type_complexity)] err_handler: Option Error + Send + Sync>>, } diff --git a/actix-web/src/types/readlines.rs b/actix-web/src/types/readlines.rs index 6c456e21c..8a775a073 100644 --- a/actix-web/src/types/readlines.rs +++ b/actix-web/src/types/readlines.rs @@ -20,7 +20,7 @@ use crate::{ /// Stream that reads request line by line. pub struct Readlines { stream: Payload, - buff: BytesMut, + buf: BytesMut, limit: usize, checked_buff: bool, encoding: &'static Encoding, @@ -41,7 +41,7 @@ where Readlines { stream: req.take_payload(), - buff: BytesMut::with_capacity(262_144), + buf: BytesMut::with_capacity(262_144), limit: 262_144, checked_buff: true, err: None, @@ -58,7 +58,7 @@ where fn err(err: ReadlinesError) -> Self { Readlines { stream: Payload::None, - buff: BytesMut::new(), + buf: BytesMut::new(), limit: 262_144, checked_buff: true, encoding: UTF_8, @@ -84,7 +84,7 @@ where // check if there is a newline in the buffer if !this.checked_buff { let mut found: Option = None; - for (ind, b) in this.buff.iter().enumerate() { + for (ind, b) in this.buf.iter().enumerate() { if *b == b'\n' { found = Some(ind); break; @@ -96,13 +96,13 @@ where return Poll::Ready(Some(Err(ReadlinesError::LimitOverflow))); } let line = if this.encoding == UTF_8 { - str::from_utf8(&this.buff.split_to(ind + 1)) + str::from_utf8(&this.buf.split_to(ind + 1)) .map_err(|_| ReadlinesError::EncodingError)? .to_owned() } else { this.encoding .decode_without_bom_handling_and_without_replacement( - &this.buff.split_to(ind + 1), + &this.buf.split_to(ind + 1), ) .map(Cow::into_owned) .ok_or(ReadlinesError::EncodingError)? @@ -141,32 +141,32 @@ where .ok_or(ReadlinesError::EncodingError)? }; // extend buffer with rest of the bytes; - this.buff.extend_from_slice(&bytes); + this.buf.extend_from_slice(&bytes); this.checked_buff = false; return Poll::Ready(Some(Ok(line))); } - this.buff.extend_from_slice(&bytes); + this.buf.extend_from_slice(&bytes); Poll::Pending } None => { - if this.buff.is_empty() { + if this.buf.is_empty() { return Poll::Ready(None); } - if this.buff.len() > this.limit { + if this.buf.len() > this.limit { return Poll::Ready(Some(Err(ReadlinesError::LimitOverflow))); } let line = if this.encoding == UTF_8 { - str::from_utf8(&this.buff) + str::from_utf8(&this.buf) .map_err(|_| ReadlinesError::EncodingError)? .to_owned() } else { this.encoding - .decode_without_bom_handling_and_without_replacement(&this.buff) + .decode_without_bom_handling_and_without_replacement(&this.buf) .map(Cow::into_owned) .ok_or(ReadlinesError::EncodingError)? }; - this.buff.clear(); + this.buf.clear(); Poll::Ready(Some(Ok(line))) } diff --git a/actix-web/tests/test_server.rs b/actix-web/tests/test_server.rs index bd8934061..270223d69 100644 --- a/actix-web/tests/test_server.rs +++ b/actix-web/tests/test_server.rs @@ -799,34 +799,36 @@ async fn test_server_cookies() { let res = req.send().await.unwrap(); assert!(res.status().is_success()); - let first_cookie = Cookie::build("first", "first_value") - .http_only(true) - .finish(); - let second_cookie = Cookie::new("second", "first_value"); + { + let first_cookie = Cookie::build("first", "first_value") + .http_only(true) + .finish(); + let second_cookie = Cookie::new("second", "first_value"); - let cookies = res.cookies().expect("To have cookies"); - assert_eq!(cookies.len(), 3); - if cookies[0] == first_cookie { - assert_eq!(cookies[1], second_cookie); - } else { - assert_eq!(cookies[0], second_cookie); - assert_eq!(cookies[1], first_cookie); - } + let cookies = res.cookies().expect("To have cookies"); + assert_eq!(cookies.len(), 3); + if cookies[0] == first_cookie { + assert_eq!(cookies[1], second_cookie); + } else { + assert_eq!(cookies[0], second_cookie); + assert_eq!(cookies[1], first_cookie); + } - let first_cookie = first_cookie.to_string(); - let second_cookie = second_cookie.to_string(); - // Check that we have exactly two instances of raw cookie headers - let cookies = res - .headers() - .get_all(http::header::SET_COOKIE) - .map(|header| header.to_str().expect("To str").to_string()) - .collect::>(); - assert_eq!(cookies.len(), 3); - if cookies[0] == first_cookie { - assert_eq!(cookies[1], second_cookie); - } else { - assert_eq!(cookies[0], second_cookie); - assert_eq!(cookies[1], first_cookie); + let first_cookie = first_cookie.to_string(); + let second_cookie = second_cookie.to_string(); + // Check that we have exactly two instances of raw cookie headers + let cookies = res + .headers() + .get_all(http::header::SET_COOKIE) + .map(|header| header.to_str().expect("To str").to_string()) + .collect::>(); + assert_eq!(cookies.len(), 3); + if cookies[0] == first_cookie { + assert_eq!(cookies[1], second_cookie); + } else { + assert_eq!(cookies[0], second_cookie); + assert_eq!(cookies[1], first_cookie); + } } srv.stop().await; diff --git a/awc/CHANGES.md b/awc/CHANGES.md index ebc0dbe61..7892d9339 100644 --- a/awc/CHANGES.md +++ b/awc/CHANGES.md @@ -1,6 +1,18 @@ # Changes -## Unreleased - 2021-xx-xx +## Unreleased - 2022-xx-xx +### Changed +- Minimum supported Rust version (MSRV) is now 1.59 due to transitive `time` dependency. + + +## 3.0.1 - 2022-08-25 +### Changed +- Minimum supported Rust version (MSRV) is now 1.57 due to transitive `time` dependency. + +### Fixed +- Fixed handling of redirection requests that begin with `//`. [#2840] + +[#2840]: https://github.com/actix/actix-web/pull/2840 ## 3.0.0 - 2022-03-07 diff --git a/awc/Cargo.toml b/awc/Cargo.toml index 9dd29e4b7..2f0027725 100644 --- a/awc/Cargo.toml +++ b/awc/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "awc" -version = "3.0.0" +version = "3.0.1" authors = [ "Nikolay Kim ", "fakeshadow <24548779@qq.com>", @@ -90,27 +90,27 @@ 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"] } -actix-http-test = { version = "3.0.0-beta.13", features = ["openssl"] } +actix-http-test = { version = "3", features = ["openssl"] } actix-server = "2" -actix-test = { version = "0.1.0-beta.13", features = ["openssl", "rustls"] } +actix-test = { version = "0.1", features = ["openssl", "rustls"] } actix-tls = { version = "3", features = ["openssl", "rustls"] } 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.10" +zstd = "0.11" [[example]] name = "client" diff --git a/awc/README.md b/awc/README.md index db70f7332..9f47e663b 100644 --- a/awc/README.md +++ b/awc/README.md @@ -3,9 +3,9 @@ > Async HTTP and WebSocket client library. [![crates.io](https://img.shields.io/crates/v/awc?label=latest)](https://crates.io/crates/awc) -[![Documentation](https://docs.rs/awc/badge.svg?version=3.0.0)](https://docs.rs/awc/3.0.0) +[![Documentation](https://docs.rs/awc/badge.svg?version=3.0.1)](https://docs.rs/awc/3.0.1) ![MIT or Apache 2.0 licensed](https://img.shields.io/crates/l/awc) -[![Dependency Status](https://deps.rs/crate/awc/3.0.0/status.svg)](https://deps.rs/crate/awc/3.0.0) +[![Dependency Status](https://deps.rs/crate/awc/3.0.1/status.svg)](https://deps.rs/crate/awc/3.0.1) [![Chat on Discord](https://img.shields.io/discord/771444961383153695?label=chat&logo=discord)](https://discord.gg/NWpN5mmg3x) ## Documentation & Resources diff --git a/awc/src/builder.rs b/awc/src/builder.rs index 16a4e9cb5..c101d18f0 100644 --- a/awc/src/builder.rs +++ b/awc/src/builder.rs @@ -165,7 +165,7 @@ where /// Add default header. /// - /// Headers added by this method get added to every request unless overriden by . + /// Headers added by this method get added to every request unless overridden by other methods. /// /// # Panics /// Panics if header name or value is invalid. diff --git a/awc/src/client/pool.rs b/awc/src/client/pool.rs index cc3e4d7c0..5655b5845 100644 --- a/awc/src/client/pool.rs +++ b/awc/src/client/pool.rs @@ -97,7 +97,7 @@ where type Target = ConnectionPoolInnerPriv; fn deref(&self) -> &Self::Target { - &*self.0 + &self.0 } } diff --git a/awc/src/middleware/redirect.rs b/awc/src/middleware/redirect.rs index d48822168..67ef5d76f 100644 --- a/awc/src/middleware/redirect.rs +++ b/awc/src/middleware/redirect.rs @@ -257,6 +257,16 @@ fn build_next_uri(res: &ClientResponse, prev_uri: &Uri) -> Result = scheme.as_bytes().to_vec(); + full_url.push(b':'); + full_url.extend(location.as_bytes()); + + return Uri::try_from(full_url) + .map_err(|_| SendRequestError::Url(InvalidUrl::MissingScheme)); + } // when scheme or authority is missing treat the location value as path and query // recover error where location does not have leading slash let path = if location.as_bytes().starts_with(b"/") { @@ -588,6 +598,41 @@ mod tests { assert_eq!(res.status().as_u16(), 200); } + #[actix_rt::test] + async fn test_double_slash_redirect() { + let client = ClientBuilder::new() + .disable_redirects() + .wrap(Redirect::new().max_redirect_times(10)) + .finish(); + + let srv = actix_test::start(|| { + App::new() + .service(web::resource("/test").route(web::to(|| async { + Ok::<_, Error>(HttpResponse::BadRequest()) + }))) + .service( + web::resource("/").route(web::to(|req: HttpRequest| async move { + Ok::<_, Error>( + HttpResponse::Found() + .append_header(( + "location", + format!( + "//localhost:{}/test", + req.app_config().local_addr().port() + ) + .as_str(), + )) + .finish(), + ) + })), + ) + }); + + let res = client.get(srv.url("/")).send().await.unwrap(); + + assert_eq!(res.status().as_u16(), 400); + } + #[actix_rt::test] async fn test_remove_sensitive_headers() { fn gen_headers() -> header::HeaderMap { diff --git a/awc/src/ws.rs b/awc/src/ws.rs index d8ed4c879..b316f68b4 100644 --- a/awc/src/ws.rs +++ b/awc/src/ws.rs @@ -321,7 +321,7 @@ impl WebsocketsRequest { // Generate a random key for the `Sec-WebSocket-Key` header which is a base64-encoded // (see RFC 4648 §4) value that, when decoded, is 16 bytes in length (RFC 6455 §1.3). let sec_key: [u8; 16] = rand::random(); - let key = base64::encode(&sec_key); + let key = base64::encode(sec_key); self.head.headers.insert( header::SEC_WEBSOCKET_KEY, @@ -513,7 +513,7 @@ mod tests { .origin("test-origin") .max_frame_size(100) .server_mode() - .protocols(&["v1", "v2"]) + .protocols(["v1", "v2"]) .set_header_if_none(header::CONTENT_TYPE, "json") .set_header_if_none(header::CONTENT_TYPE, "text") .cookie(Cookie::build("cookie1", "value1").finish()); diff --git a/awc/tests/test_client.rs b/awc/tests/test_client.rs index 165d8faf0..c4b468eeb 100644 --- a/awc/tests/test_client.rs +++ b/awc/tests/test_client.rs @@ -707,8 +707,7 @@ async fn client_cookie_handling() { async move { // Check cookies were sent correctly - let res: Result<(), Error> = req - .cookie("cookie1") + req.cookie("cookie1") .ok_or(()) .and_then(|c1| { if c1.value() == "value1" { @@ -725,16 +724,10 @@ async fn client_cookie_handling() { Err(()) } }) - .map_err(|_| Error::from(IoError::from(ErrorKind::NotFound))); + .map_err(|_| Error::from(IoError::from(ErrorKind::NotFound)))?; - if let Err(e) = res { - Err(e) - } else { - // Send some cookies back - Ok::<_, Error>( - HttpResponse::Ok().cookie(cookie1).cookie(cookie2).finish(), - ) - } + // Send some cookies back + Ok::<_, Error>(HttpResponse::Ok().cookie(cookie1).cookie(cookie2).finish()) } }), ) diff --git a/clippy.toml b/clippy.toml index ece14b8d2..abe19b3a0 100644 --- a/clippy.toml +++ b/clippy.toml @@ -1 +1 @@ -msrv = "1.54" +msrv = "1.59" diff --git a/scripts/bump b/scripts/bump index 209e2281d..33ea52010 100755 --- a/scripts/bump +++ b/scripts/bump @@ -98,7 +98,7 @@ rm -f $README_FILE.bak echo "manifest, changelog, and readme updated" echo echo "check other references:" -rg --glob='**/Cargo.toml' "\ +rg --glob='**/{Cargo.toml,README.md}' "\ ${PACKAGE_NAME} ?= ?\"[^\"]+\"\ |${PACKAGE_NAME} ?=.*version ?= ?\"([^\"]+)\"\ |package ?= ?\"${PACKAGE_NAME}\".*version ?= ?\"([^\"]+)\"\ diff --git a/scripts/unreleased b/scripts/unreleased index e664c0879..6738ad090 100755 --- a/scripts/unreleased +++ b/scripts/unreleased @@ -45,6 +45,8 @@ unreleased_for() { cat "$CHANGE_CHUNK_FILE" } -for f in $(fd --absolute-path 'CHANGE\w+.md'); do - unreleased_for $(dirname $f) +files=$(fd --threads=1 --min-depth=2 --absolute-path 'CHANGE\w+.md') + +for f in $files; do + unreleased_for $(dirname $f) || true done