diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index cc34e2bc6..b779b33fa 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -1,6 +1,8 @@ -## PR Type -What kind of change does this PR make? + + +## PR Type + INSERT_PR_TYPE @@ -13,6 +15,7 @@ Check your PR fulfills the following: - [ ] Tests for the changes have been added / updated. - [ ] Documentation comments have been added / updated. - [ ] A changelog entry has been made for the appropriate packages. +- [ ] Format code with the latest stable rustfmt ## Overview diff --git a/.github/workflows/bench.yml b/.github/workflows/bench.yml index ce8a7da7e..754091dde 100644 --- a/.github/workflows/bench.yml +++ b/.github/workflows/bench.yml @@ -1,6 +1,11 @@ name: Benchmark (Linux) -on: [push, pull_request] +on: + pull_request: + types: [opened, synchronize, reopened] + push: + branches: + - master jobs: check_benchmark: diff --git a/.github/workflows/clippy-fmt.yml b/.github/workflows/clippy-fmt.yml new file mode 100644 index 000000000..fb1ed7f32 --- /dev/null +++ b/.github/workflows/clippy-fmt.yml @@ -0,0 +1,32 @@ +on: + pull_request: + types: [opened, synchronize, reopened] + +name: Clippy and rustfmt Check +jobs: + clippy_check: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + + - uses: actions-rs/toolchain@v1 + with: + toolchain: stable + components: rustfmt + override: true + - name: Check with rustfmt + uses: actions-rs/cargo@v1 + with: + command: fmt + args: --all -- --check + + - uses: actions-rs/toolchain@v1 + with: + toolchain: nightly + components: clippy + override: true + - name: Check with Clippy + uses: actions-rs/clippy-check@v1 + with: + token: ${{ secrets.GITHUB_TOKEN }} + args: --all-features --all --tests diff --git a/.github/workflows/linux.yml b/.github/workflows/linux.yml index 7529c8494..d49874ce3 100644 --- a/.github/workflows/linux.yml +++ b/.github/workflows/linux.yml @@ -1,6 +1,11 @@ name: CI (Linux) -on: [push, pull_request] +on: + pull_request: + types: [opened, synchronize, reopened] + push: + branches: + - master jobs: build_and_test: @@ -8,7 +13,7 @@ jobs: fail-fast: false matrix: version: - - 1.41.1 # MSRV + - 1.42.0 # MSRV - stable - nightly diff --git a/.github/workflows/macos.yml b/.github/workflows/macos.yml index 6c360bacc..0683fcc8c 100644 --- a/.github/workflows/macos.yml +++ b/.github/workflows/macos.yml @@ -1,6 +1,11 @@ name: CI (macOS) -on: [push, pull_request] +on: + pull_request: + types: [opened, synchronize, reopened] + push: + branches: + - master jobs: build_and_test: diff --git a/.github/workflows/windows.yml b/.github/workflows/windows.yml index 5fd785fad..c09c7cea3 100644 --- a/.github/workflows/windows.yml +++ b/.github/workflows/windows.yml @@ -1,6 +1,11 @@ name: CI (Windows) -on: [push, pull_request] +on: + pull_request: + types: [opened, synchronize, reopened] + push: + branches: + - master env: VCPKGRS_DYNAMIC: 1 diff --git a/CHANGES.md b/CHANGES.md index 149ff8fcb..d80e87946 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,6 +1,19 @@ # Changes ## Unreleased - 2020-xx-xx +### Changed +* `PayloadConfig` is now also considered in `Bytes` and `String` extractors when set + using `App::data`. [#1610] +* `web::Path` now has a public representation: `web::Path(pub T)` that enables + destructuring. [#1594] +* MSRV is now 1.42.0. + +### Fixed +* Memory leak of app data in pooled requests. [#1609] + +[#1594]: https://github.com/actix/actix-web/pull/1594 +[#1609]: https://github.com/actix/actix-web/pull/1609 +[#1610]: https://github.com/actix/actix-web/pull/1610 ## 3.0.0-beta.1 - 2020-07-13 diff --git a/MIGRATION.md b/MIGRATION.md index d2e9735fb..7459b5b2d 100644 --- a/MIGRATION.md +++ b/MIGRATION.md @@ -12,6 +12,26 @@ * `BodySize::Sized64` variant has been removed. `BodySize::Sized` now receives a `u64` instead of a `usize`. +* Code that was using `path.` to access a `web::Path<(A, B, C)>`s elements now needs to use + destructuring or `.into_inner()`. For example: + + ```rust + // Previously: + async fn some_route(path: web::Path<(String, String)>) -> String { + format!("Hello, {} {}", path.0, path.1) + } + + // Now (this also worked before): + async fn some_route(path: web::Path<(String, String)>) -> String { + let (first_name, last_name) = path.into_inner(); + format!("Hello, {} {}", first_name, last_name) + } + // Or (this wasn't previously supported): + async fn some_route(web::Path((first_name, last_name)): web::Path<(String, String)>) -> String { + format!("Hello, {} {}", first_name, last_name) + } + ``` + ## 2.0.0 * `HttpServer::start()` renamed to `HttpServer::run()`. It also possible to diff --git a/README.md b/README.md index 4d6bac29c..a42a1a6f8 100644 --- a/README.md +++ b/README.md @@ -7,7 +7,7 @@ [![crates.io](https://meritbadge.herokuapp.com/actix-web)](https://crates.io/crates/actix-web) [![Documentation](https://docs.rs/actix-web/badge.svg)](https://docs.rs/actix-web) -[![Version](https://img.shields.io/badge/rustc-1.41+-lightgray.svg)](https://blog.rust-lang.org/2020/02/27/Rust-1.41.1.html) +[![Version](https://img.shields.io/badge/rustc-1.42+-ab6000.svg)](https://blog.rust-lang.org/2020/03/12/Rust-1.42.html) ![License](https://img.shields.io/crates/l/actix-web.svg)
[![Build Status](https://travis-ci.org/actix/actix-web.svg?branch=master)](https://travis-ci.org/actix/actix-web) @@ -32,7 +32,7 @@ * Middlewares ([Logger, Session, CORS, etc](https://actix.rs/docs/middleware/)) * Includes an async [HTTP client](https://actix.rs/actix-web/actix_web/client/index.html) * Supports [Actix actor framework](https://github.com/actix/actix) -* Runs on stable Rust 1.41+ +* Runs on stable Rust 1.42+ ## Documentation @@ -61,8 +61,8 @@ Code: use actix_web::{get, web, App, HttpServer, Responder}; #[get("/{id}/{name}/index.html")] -async fn index(info: web::Path<(u32, String)>) -> impl Responder { - format!("Hello {}! id:{}", info.1, info.0) +async fn index(web::Path((id, name)): web::Path<(u32, String)>) -> impl Responder { + format!("Hello {}! id:{}", name, id) } #[actix_web::main] diff --git a/actix-files/CHANGES.md b/actix-files/CHANGES.md index 5d1845e37..75d616ff9 100644 --- a/actix-files/CHANGES.md +++ b/actix-files/CHANGES.md @@ -1,11 +1,12 @@ # Changes -## [unreleased] - xxx +## [Unreleased] - 2020-xx-xx +## [0.3.0-beta.1] - 2020-07-15 * Update `v_htmlescape` to 0.10 +* Update `actix-web` and `actix-http` dependencies to beta.1 ## [0.3.0-alpha.1] - 2020-05-23 - * Update `actix-web` and `actix-http` dependencies to alpha * Fix some typos in the docs * Bump minimum supported Rust version to 1.40 diff --git a/actix-files/Cargo.toml b/actix-files/Cargo.toml index b7ba75960..379ba8f89 100644 --- a/actix-files/Cargo.toml +++ b/actix-files/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "actix-files" -version = "0.3.0-alpha.1" +version = "0.3.0-beta.1" authors = ["Nikolay Kim "] description = "Static files support for actix web." readme = "README.md" @@ -17,8 +17,8 @@ name = "actix_files" path = "src/lib.rs" [dependencies] -actix-web = { version = "3.0.0-alpha.3", default-features = false } -actix-http = "2.0.0-alpha.4" +actix-web = { version = "3.0.0-beta.1", default-features = false } +actix-http = "2.0.0-beta.1" actix-service = "1.0.1" bitflags = "1" bytes = "0.5.3" @@ -33,4 +33,4 @@ v_htmlescape = "0.10" [dev-dependencies] actix-rt = "1.0.0" -actix-web = { version = "3.0.0-alpha.3", features = ["openssl"] } +actix-web = { version = "3.0.0-beta.1", features = ["openssl"] } diff --git a/actix-files/src/lib.rs b/actix-files/src/lib.rs index 76c68ce25..ae0204a71 100644 --- a/actix-files/src/lib.rs +++ b/actix-files/src/lib.rs @@ -26,7 +26,6 @@ use actix_web::{web, FromRequest, HttpRequest, HttpResponse}; use bytes::Bytes; use futures_core::Stream; use futures_util::future::{ok, ready, Either, FutureExt, LocalBoxFuture, Ready}; -use mime; use mime_guess::from_ext; use percent_encoding::{utf8_percent_encode, CONTROLS}; use v_htmlescape::escape as escape_html_entity; @@ -250,6 +249,8 @@ pub struct Files { renderer: Rc, mime_override: Option>, file_flags: named::Flags, + // FIXME: Should re-visit later. + #[allow(clippy::redundant_allocation)] guards: Option>>, } @@ -462,6 +463,8 @@ pub struct FilesService { renderer: Rc, mime_override: Option>, file_flags: named::Flags, + // FIXME: Should re-visit later. + #[allow(clippy::redundant_allocation)] guards: Option>>, } @@ -501,11 +504,8 @@ impl Service for FilesService { // execute user defined guards (**guard).check(req.head()) } else { - // default behaviour - match *req.method() { - Method::HEAD | Method::GET => true, - _ => false, - } + // default behavior + matches!(*req.method(), Method::HEAD | Method::GET) }; if !is_method_valid { @@ -952,9 +952,7 @@ mod tests { #[actix_rt::test] async fn test_named_file_content_range_headers() { - let srv = test::start(|| { - App::new().service(Files::new("/", ".")) - }); + let srv = test::start(|| App::new().service(Files::new("/", "."))); // Valid range header let response = srv @@ -979,9 +977,7 @@ mod tests { #[actix_rt::test] async fn test_named_file_content_length_headers() { - let srv = test::start(|| { - App::new().service(Files::new("/", ".")) - }); + let srv = test::start(|| App::new().service(Files::new("/", "."))); // Valid range header let response = srv @@ -1020,15 +1016,9 @@ mod tests { #[actix_rt::test] async fn test_head_content_length_headers() { - let srv = test::start(|| { - App::new().service(Files::new("/", ".")) - }); + let srv = test::start(|| App::new().service(Files::new("/", "."))); - let response = srv - .head("/tests/test.binary") - .send() - .await - .unwrap(); + let response = srv.head("/tests/test.binary").send().await.unwrap(); let content_length = response .headers() @@ -1097,12 +1087,10 @@ mod tests { #[actix_rt::test] async fn test_named_file_content_encoding() { let mut srv = test::init_service(App::new().wrap(Compress::default()).service( - web::resource("/").to(|| { - async { - NamedFile::open("Cargo.toml") - .unwrap() - .set_content_encoding(header::ContentEncoding::Identity) - } + web::resource("/").to(|| async { + NamedFile::open("Cargo.toml") + .unwrap() + .set_content_encoding(header::ContentEncoding::Identity) }), )) .await; @@ -1119,12 +1107,10 @@ mod tests { #[actix_rt::test] async fn test_named_file_content_encoding_gzip() { let mut srv = test::init_service(App::new().wrap(Compress::default()).service( - web::resource("/").to(|| { - async { - NamedFile::open("Cargo.toml") - .unwrap() - .set_content_encoding(header::ContentEncoding::Gzip) - } + web::resource("/").to(|| async { + NamedFile::open("Cargo.toml") + .unwrap() + .set_content_encoding(header::ContentEncoding::Gzip) }), )) .await; diff --git a/actix-files/src/named.rs b/actix-files/src/named.rs index 6ee561a4b..12da722d2 100644 --- a/actix-files/src/named.rs +++ b/actix-files/src/named.rs @@ -8,7 +8,6 @@ use std::time::{SystemTime, UNIX_EPOCH}; use std::os::unix::fs::MetadataExt; use bitflags::bitflags; -use mime; use mime_guess::from_path; use actix_http::body::SizedStream; @@ -90,7 +89,7 @@ impl NamedFile { }; let ct = from_path(&path).first_or_octet_stream(); - let disposition_type = match ct.type_() { + let disposition = match ct.type_() { mime::IMAGE | mime::TEXT | mime::VIDEO => DispositionType::Inline, _ => DispositionType::Attachment, }; @@ -104,8 +103,8 @@ impl NamedFile { })) } let cd = ContentDisposition { - disposition: disposition_type, - parameters: parameters, + disposition, + parameters, }; (ct, cd) }; diff --git a/actix-http/CHANGES.md b/actix-http/CHANGES.md index a185a9f81..b2b816d54 100644 --- a/actix-http/CHANGES.md +++ b/actix-http/CHANGES.md @@ -1,6 +1,18 @@ # Changes -## [Unreleased] - xxx +## Unreleased - 2020-xx-xx + + +## 2.0.0-beta.2 - 2020-07-21 +### Fixed +* Potential UB in h1 decoder using uninitialized memory. [#1614] + +### Changed +* Fix illegal chunked encoding. [#1615] + +[#1614]: https://github.com/actix/actix-web/pull/1614 +[#1615]: https://github.com/actix/actix-web/pull/1615 + ## [2.0.0-beta.1] - 2020-07-11 diff --git a/actix-http/Cargo.toml b/actix-http/Cargo.toml index 232b5c3f0..7f07f67c3 100644 --- a/actix-http/Cargo.toml +++ b/actix-http/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "actix-http" -version = "2.0.0-beta.1" +version = "2.0.0-beta.2" authors = ["Nikolay Kim "] description = "Actix HTTP primitives" readme = "README.md" @@ -103,3 +103,7 @@ harness = false [[bench]] name = "status-line" harness = false + +[[bench]] +name = "uninit-headers" +harness = false diff --git a/actix-http/benches/uninit-headers.rs b/actix-http/benches/uninit-headers.rs new file mode 100644 index 000000000..83e74171c --- /dev/null +++ b/actix-http/benches/uninit-headers.rs @@ -0,0 +1,137 @@ +use criterion::{criterion_group, criterion_main, Criterion}; + +use bytes::BytesMut; + +// A Miri run detects UB, seen on this playground: +// https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=f5d9aa166aa48df8dca05fce2b6c3915 + +fn bench_header_parsing(c: &mut Criterion) { + c.bench_function("Original (Unsound) [short]", |b| { + b.iter(|| { + let mut buf = BytesMut::from(REQ_SHORT); + _original::parse_headers(&mut buf); + }) + }); + + c.bench_function("New (safe) [short]", |b| { + b.iter(|| { + let mut buf = BytesMut::from(REQ_SHORT); + _new::parse_headers(&mut buf); + }) + }); + + c.bench_function("Original (Unsound) [realistic]", |b| { + b.iter(|| { + let mut buf = BytesMut::from(REQ); + _original::parse_headers(&mut buf); + }) + }); + + c.bench_function("New (safe) [realistic]", |b| { + b.iter(|| { + let mut buf = BytesMut::from(REQ); + _new::parse_headers(&mut buf); + }) + }); +} + +criterion_group!(benches, bench_header_parsing); +criterion_main!(benches); + +const MAX_HEADERS: usize = 96; + +const EMPTY_HEADER_ARRAY: [httparse::Header<'static>; MAX_HEADERS] = + [httparse::EMPTY_HEADER; MAX_HEADERS]; + +#[derive(Clone, Copy)] +struct HeaderIndex { + name: (usize, usize), + value: (usize, usize), +} + +const EMPTY_HEADER_INDEX: HeaderIndex = HeaderIndex { + name: (0, 0), + value: (0, 0), +}; + +const EMPTY_HEADER_INDEX_ARRAY: [HeaderIndex; MAX_HEADERS] = + [EMPTY_HEADER_INDEX; MAX_HEADERS]; + +impl HeaderIndex { + fn record( + bytes: &[u8], + headers: &[httparse::Header<'_>], + indices: &mut [HeaderIndex], + ) { + let bytes_ptr = bytes.as_ptr() as usize; + for (header, indices) in headers.iter().zip(indices.iter_mut()) { + let name_start = header.name.as_ptr() as usize - bytes_ptr; + let name_end = name_start + header.name.len(); + indices.name = (name_start, name_end); + let value_start = header.value.as_ptr() as usize - bytes_ptr; + let value_end = value_start + header.value.len(); + indices.value = (value_start, value_end); + } + } +} + +// test cases taken from: +// https://github.com/seanmonstar/httparse/blob/master/benches/parse.rs + +const REQ_SHORT: &'static [u8] = b"\ +GET / HTTP/1.0\r\n\ +Host: example.com\r\n\ +Cookie: session=60; user_id=1\r\n\r\n"; + +const REQ: &'static [u8] = b"\ +GET /wp-content/uploads/2010/03/hello-kitty-darth-vader-pink.jpg HTTP/1.1\r\n\ +Host: www.kittyhell.com\r\n\ +User-Agent: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; ja-JP-mac; rv:1.9.2.3) Gecko/20100401 Firefox/3.6.3 Pathtraq/0.9\r\n\ +Accept: text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8\r\n\ +Accept-Language: ja,en-us;q=0.7,en;q=0.3\r\n\ +Accept-Encoding: gzip,deflate\r\n\ +Accept-Charset: Shift_JIS,utf-8;q=0.7,*;q=0.7\r\n\ +Keep-Alive: 115\r\n\ +Connection: keep-alive\r\n\ +Cookie: wp_ozh_wsa_visits=2; wp_ozh_wsa_visit_lasttime=xxxxxxxxxx; __utma=xxxxxxxxx.xxxxxxxxxx.xxxxxxxxxx.xxxxxxxxxx.xxxxxxxxxx.x; __utmz=xxxxxxxxx.xxxxxxxxxx.x.x.utmccn=(referral)|utmcsr=reader.livedoor.com|utmcct=/reader/|utmcmd=referral|padding=under256\r\n\r\n"; + +mod _new { + use super::*; + + pub fn parse_headers(src: &mut BytesMut) -> usize { + let mut headers: [HeaderIndex; MAX_HEADERS] = EMPTY_HEADER_INDEX_ARRAY; + let mut parsed: [httparse::Header<'_>; MAX_HEADERS] = EMPTY_HEADER_ARRAY; + + let mut req = httparse::Request::new(&mut parsed); + match req.parse(src).unwrap() { + httparse::Status::Complete(_len) => { + HeaderIndex::record(src, req.headers, &mut headers); + req.headers.len() + } + _ => unreachable!(), + } + } +} + +mod _original { + use super::*; + + use std::mem::MaybeUninit; + + pub fn parse_headers(src: &mut BytesMut) -> usize { + let mut headers: [HeaderIndex; MAX_HEADERS] = + unsafe { MaybeUninit::uninit().assume_init() }; + + let mut parsed: [httparse::Header<'_>; MAX_HEADERS] = + unsafe { MaybeUninit::uninit().assume_init() }; + + let mut req = httparse::Request::new(&mut parsed); + match req.parse(src).unwrap() { + httparse::Status::Complete(_len) => { + HeaderIndex::record(src, req.headers, &mut headers); + req.headers.len() + } + _ => unreachable!(), + } + } +} diff --git a/actix-http/src/body.rs b/actix-http/src/body.rs index 0b01aa8ce..8bea8e6b8 100644 --- a/actix-http/src/body.rs +++ b/actix-http/src/body.rs @@ -21,12 +21,7 @@ pub enum BodySize { impl BodySize { pub fn is_eof(&self) -> bool { - match self { - BodySize::None - | BodySize::Empty - | BodySize::Sized(0) => true, - _ => false, - } + matches!(self, BodySize::None | BodySize::Empty | BodySize::Sized(0)) } } @@ -192,14 +187,8 @@ impl MessageBody for Body { impl PartialEq for Body { fn eq(&self, other: &Body) -> bool { match *self { - Body::None => match *other { - Body::None => true, - _ => false, - }, - Body::Empty => match *other { - Body::Empty => true, - _ => false, - }, + Body::None => matches!(*other, Body::None), + Body::Empty => matches!(*other, Body::Empty), Body::Bytes(ref b) => match *other { Body::Bytes(ref b2) => b == b2, _ => false, @@ -476,9 +465,9 @@ where #[cfg(test)] mod tests { use super::*; - use futures_util::stream; use futures_util::future::poll_fn; use futures_util::pin_mut; + use futures_util::stream; impl Body { pub(crate) fn get_ref(&self) -> &[u8] { @@ -612,10 +601,6 @@ mod tests { #[actix_rt::test] async fn test_body_eq() { - assert!(Body::None == Body::None); - assert!(Body::None != Body::Empty); - assert!(Body::Empty == Body::Empty); - assert!(Body::Empty != Body::None); assert!( Body::Bytes(Bytes::from_static(b"1")) == Body::Bytes(Bytes::from_static(b"1")) @@ -627,7 +612,7 @@ mod tests { async fn test_body_debug() { assert!(format!("{:?}", Body::None).contains("Body::None")); assert!(format!("{:?}", Body::Empty).contains("Body::Empty")); - assert!(format!("{:?}", Body::Bytes(Bytes::from_static(b"1"))).contains("1")); + assert!(format!("{:?}", Body::Bytes(Bytes::from_static(b"1"))).contains('1')); } #[actix_rt::test] diff --git a/actix-http/src/client/h2proto.rs b/actix-http/src/client/h2proto.rs index 48ab9fe4a..3f9a981f4 100644 --- a/actix-http/src/client/h2proto.rs +++ b/actix-http/src/client/h2proto.rs @@ -37,10 +37,10 @@ where trace!("Sending client request: {:?} {:?}", head, body.size()); let head_req = head.as_ref().method == Method::HEAD; let length = body.size(); - let eof = match length { - BodySize::None | BodySize::Empty | BodySize::Sized(0) => true, - _ => false, - }; + let eof = matches!( + length, + BodySize::None | BodySize::Empty | BodySize::Sized(0) + ); let mut req = Request::new(()); *req.uri_mut() = head.as_ref().uri.clone(); diff --git a/actix-http/src/client/pool.rs b/actix-http/src/client/pool.rs index 3ce443794..f2c5b0410 100644 --- a/actix-http/src/client/pool.rs +++ b/actix-http/src/client/pool.rs @@ -69,10 +69,7 @@ where inner: Rc::downgrade(&inner_rc), }); - ConnectionPool( - connector_rc, - inner_rc, - ) + ConnectionPool(connector_rc, inner_rc) } } diff --git a/actix-http/src/error.rs b/actix-http/src/error.rs index f0a4b70bc..d0754ce88 100644 --- a/actix-http/src/error.rs +++ b/actix-http/src/error.rs @@ -964,7 +964,6 @@ impl ResponseError for actix::actors::resolver::ResolverError {} mod tests { use super::*; use http::{Error as HttpError, StatusCode}; - use httparse; use std::io; #[test] diff --git a/actix-http/src/extensions.rs b/actix-http/src/extensions.rs index 4e3918537..96e01767b 100644 --- a/actix-http/src/extensions.rs +++ b/actix-http/src/extensions.rs @@ -72,7 +72,7 @@ impl fmt::Debug for Extensions { #[cfg(test)] mod tests { use super::*; - + #[test] fn test_remove() { let mut map = Extensions::new(); diff --git a/actix-http/src/h1/decoder.rs b/actix-http/src/h1/decoder.rs index c9d3edf33..8fdd11be5 100644 --- a/actix-http/src/h1/decoder.rs +++ b/actix-http/src/h1/decoder.rs @@ -1,7 +1,6 @@ use std::convert::TryFrom; use std::io; use std::marker::PhantomData; -use std::mem::MaybeUninit; use std::task::Poll; use actix_codec::Decoder; @@ -46,7 +45,7 @@ impl Decoder for MessageDecoder { pub(crate) enum PayloadLength { Payload(PayloadType), - Upgrade, + UpgradeWebSocket, None, } @@ -65,7 +64,7 @@ pub(crate) trait MessageType: Sized { raw_headers: &[HeaderIndex], ) -> Result { let mut ka = None; - let mut has_upgrade = false; + let mut has_upgrade_websocket = false; let mut expect = false; let mut chunked = false; let mut content_length = None; @@ -77,7 +76,7 @@ pub(crate) trait MessageType: Sized { let name = HeaderName::from_bytes(&slice[idx.name.0..idx.name.1]).unwrap(); - // Unsafe: httparse check header value for valid utf-8 + // SAFETY: httparse checks header value is valid UTF-8 let value = unsafe { HeaderValue::from_maybe_shared_unchecked( slice.slice(idx.value.0..idx.value.1), @@ -124,12 +123,9 @@ pub(crate) trait MessageType: Sized { }; } header::UPGRADE => { - has_upgrade = true; - // check content-length, some clients (dart) - // sends "content-length: 0" with websocket upgrade if let Ok(val) = value.to_str().map(|val| val.trim()) { if val.eq_ignore_ascii_case("websocket") { - content_length = None; + has_upgrade_websocket = true; } } } @@ -156,13 +152,13 @@ pub(crate) trait MessageType: Sized { Ok(PayloadLength::Payload(PayloadType::Payload( PayloadDecoder::chunked(), ))) + } else if has_upgrade_websocket { + Ok(PayloadLength::UpgradeWebSocket) } else if let Some(len) = content_length { // Content-Length Ok(PayloadLength::Payload(PayloadType::Payload( PayloadDecoder::length(len), ))) - } else if has_upgrade { - Ok(PayloadLength::Upgrade) } else { Ok(PayloadLength::None) } @@ -184,16 +180,11 @@ impl MessageType for Request { &mut self.head_mut().headers } - #[allow(clippy::uninit_assumed_init)] fn decode(src: &mut BytesMut) -> Result, ParseError> { - // Unsafe: we read only this data only after httparse parses headers into. - // performance bump for pipeline benchmarks. - let mut headers: [HeaderIndex; MAX_HEADERS] = - unsafe { MaybeUninit::uninit().assume_init() }; + let mut headers: [HeaderIndex; MAX_HEADERS] = EMPTY_HEADER_INDEX_ARRAY; let (len, method, uri, ver, h_len) = { - let mut parsed: [httparse::Header<'_>; MAX_HEADERS] = - unsafe { MaybeUninit::uninit().assume_init() }; + let mut parsed: [httparse::Header<'_>; MAX_HEADERS] = EMPTY_HEADER_ARRAY; let mut req = httparse::Request::new(&mut parsed); match req.parse(src)? { @@ -222,7 +213,7 @@ impl MessageType for Request { // payload decoder let decoder = match length { PayloadLength::Payload(pl) => pl, - PayloadLength::Upgrade => { + PayloadLength::UpgradeWebSocket => { // upgrade(websocket) PayloadType::Stream(PayloadDecoder::eof()) } @@ -260,16 +251,11 @@ impl MessageType for ResponseHead { &mut self.headers } - #[allow(clippy::uninit_assumed_init)] fn decode(src: &mut BytesMut) -> Result, ParseError> { - // Unsafe: we read only this data only after httparse parses headers into. - // performance bump for pipeline benchmarks. - let mut headers: [HeaderIndex; MAX_HEADERS] = - unsafe { MaybeUninit::uninit().assume_init() }; + let mut headers: [HeaderIndex; MAX_HEADERS] = EMPTY_HEADER_INDEX_ARRAY; let (len, ver, status, h_len) = { - let mut parsed: [httparse::Header<'_>; MAX_HEADERS] = - unsafe { MaybeUninit::uninit().assume_init() }; + let mut parsed: [httparse::Header<'_>; MAX_HEADERS] = EMPTY_HEADER_ARRAY; let mut res = httparse::Response::new(&mut parsed); match res.parse(src)? { @@ -324,6 +310,17 @@ pub(crate) struct HeaderIndex { pub(crate) value: (usize, usize), } +pub(crate) const EMPTY_HEADER_INDEX: HeaderIndex = HeaderIndex { + name: (0, 0), + value: (0, 0), +}; + +pub(crate) const EMPTY_HEADER_INDEX_ARRAY: [HeaderIndex; MAX_HEADERS] = + [EMPTY_HEADER_INDEX; MAX_HEADERS]; + +pub(crate) const EMPTY_HEADER_ARRAY: [httparse::Header<'static>; MAX_HEADERS] = + [httparse::EMPTY_HEADER; MAX_HEADERS]; + impl HeaderIndex { pub(crate) fn record( bytes: &[u8], @@ -655,10 +652,7 @@ mod tests { } fn is_unhandled(&self) -> bool { - match self { - PayloadType::Stream(_) => true, - _ => false, - } + matches!(self, PayloadType::Stream(_)) } } @@ -670,10 +664,7 @@ mod tests { } } fn eof(&self) -> bool { - match *self { - PayloadItem::Eof => true, - _ => false, - } + matches!(*self, PayloadItem::Eof) } } @@ -979,7 +970,7 @@ mod tests { unreachable!("Error"); } - // type in chunked + // intentional typo in "chunked" let mut buf = BytesMut::from( "GET /test HTTP/1.1\r\n\ transfer-encoding: chnked\r\n\r\n", @@ -1040,7 +1031,7 @@ mod tests { } #[test] - fn test_http_request_upgrade() { + fn test_http_request_upgrade_websocket() { let mut buf = BytesMut::from( "GET /test HTTP/1.1\r\n\ connection: upgrade\r\n\ @@ -1054,6 +1045,26 @@ mod tests { assert!(pl.is_unhandled()); } + #[test] + fn test_http_request_upgrade_h2c() { + let mut buf = BytesMut::from( + "GET /test HTTP/1.1\r\n\ + connection: upgrade, http2-settings\r\n\ + upgrade: h2c\r\n\ + http2-settings: dummy\r\n\r\n", + ); + let mut reader = MessageDecoder::::default(); + let (req, pl) = reader.decode(&mut buf).unwrap().unwrap(); + // `connection: upgrade, http2-settings` doesn't work properly.. + // see MessageType::set_headers(). + // + // The line below should be: + // assert_eq!(req.head().connection_type(), ConnectionType::Upgrade); + assert_eq!(req.head().connection_type(), ConnectionType::KeepAlive); + assert!(req.upgrade()); + assert!(!pl.is_unhandled()); + } + #[test] fn test_http_request_parser_utf8() { let mut buf = BytesMut::from( diff --git a/actix-http/src/h1/dispatcher.rs b/actix-http/src/h1/dispatcher.rs index 51f107efb..00b36562e 100644 --- a/actix-http/src/h1/dispatcher.rs +++ b/actix-http/src/h1/dispatcher.rs @@ -132,19 +132,11 @@ where B: MessageBody, { fn is_empty(&self) -> bool { - if let State::None = self { - true - } else { - false - } + matches!(self, State::None) } fn is_call(&self) -> bool { - if let State::ServiceCall(_) = self { - true - } else { - false - } + matches!(self, State::ServiceCall(_)) } } enum PollResponse { @@ -156,14 +148,8 @@ enum PollResponse { impl PartialEq for PollResponse { fn eq(&self, other: &PollResponse) -> bool { match self { - PollResponse::DrainWriteBuf => match other { - PollResponse::DrainWriteBuf => true, - _ => false, - }, - PollResponse::DoNothing => match other { - PollResponse::DoNothing => true, - _ => false, - }, + PollResponse::DrainWriteBuf => matches!(other, PollResponse::DrainWriteBuf), + PollResponse::DoNothing => matches!(other, PollResponse::DoNothing), _ => false, } } diff --git a/actix-http/src/h2/dispatcher.rs b/actix-http/src/h2/dispatcher.rs index 33fb3a814..534ce928a 100644 --- a/actix-http/src/h2/dispatcher.rs +++ b/actix-http/src/h2/dispatcher.rs @@ -303,55 +303,59 @@ where } } }, - ServiceResponseStateProj::SendPayload(ref mut stream, ref mut body) => loop { + ServiceResponseStateProj::SendPayload(ref mut stream, ref mut body) => { loop { - if let Some(ref mut buffer) = this.buffer { - match stream.poll_capacity(cx) { - Poll::Pending => return Poll::Pending, - Poll::Ready(None) => return Poll::Ready(()), - Poll::Ready(Some(Ok(cap))) => { - let len = buffer.len(); - let bytes = buffer.split_to(std::cmp::min(cap, len)); + loop { + if let Some(ref mut buffer) = this.buffer { + match stream.poll_capacity(cx) { + Poll::Pending => return Poll::Pending, + Poll::Ready(None) => return Poll::Ready(()), + Poll::Ready(Some(Ok(cap))) => { + let len = buffer.len(); + let bytes = buffer.split_to(std::cmp::min(cap, len)); - if let Err(e) = stream.send_data(bytes, false) { + if let Err(e) = stream.send_data(bytes, false) { + warn!("{:?}", e); + return Poll::Ready(()); + } else if !buffer.is_empty() { + let cap = + std::cmp::min(buffer.len(), CHUNK_SIZE); + stream.reserve_capacity(cap); + } else { + this.buffer.take(); + } + } + Poll::Ready(Some(Err(e))) => { warn!("{:?}", e); return Poll::Ready(()); - } else if !buffer.is_empty() { - let cap = std::cmp::min(buffer.len(), CHUNK_SIZE); - stream.reserve_capacity(cap); - } else { - this.buffer.take(); } } - Poll::Ready(Some(Err(e))) => { - warn!("{:?}", e); - return Poll::Ready(()); - } - } - } else { - match body.as_mut().poll_next(cx) { - Poll::Pending => return Poll::Pending, - Poll::Ready(None) => { - if let Err(e) = stream.send_data(Bytes::new(), true) { - warn!("{:?}", e); + } else { + match body.as_mut().poll_next(cx) { + Poll::Pending => return Poll::Pending, + Poll::Ready(None) => { + if let Err(e) = stream.send_data(Bytes::new(), true) + { + warn!("{:?}", e); + } + return Poll::Ready(()); + } + Poll::Ready(Some(Ok(chunk))) => { + stream.reserve_capacity(std::cmp::min( + chunk.len(), + CHUNK_SIZE, + )); + *this.buffer = Some(chunk); + } + Poll::Ready(Some(Err(e))) => { + error!("Response payload stream error: {:?}", e); + return Poll::Ready(()); } - return Poll::Ready(()); - } - Poll::Ready(Some(Ok(chunk))) => { - stream.reserve_capacity(std::cmp::min( - chunk.len(), - CHUNK_SIZE, - )); - *this.buffer = Some(chunk); - } - Poll::Ready(Some(Err(e))) => { - error!("Response payload stream error: {:?}", e); - return Poll::Ready(()); } } } } - }, + } } } } diff --git a/actix-http/src/header/common/allow.rs b/actix-http/src/header/common/allow.rs index 432cc00d5..88c21763c 100644 --- a/actix-http/src/header/common/allow.rs +++ b/actix-http/src/header/common/allow.rs @@ -1,5 +1,5 @@ -use http::Method; use http::header; +use http::Method; header! { /// `Allow` header, defined in [RFC7231](http://tools.ietf.org/html/rfc7231#section-7.4.1) diff --git a/actix-http/src/header/common/content_disposition.rs b/actix-http/src/header/common/content_disposition.rs index d65933901..051dcfe80 100644 --- a/actix-http/src/header/common/content_disposition.rs +++ b/actix-http/src/header/common/content_disposition.rs @@ -387,26 +387,17 @@ impl ContentDisposition { /// Returns `true` if it is [`Inline`](DispositionType::Inline). pub fn is_inline(&self) -> bool { - match self.disposition { - DispositionType::Inline => true, - _ => false, - } + matches!(self.disposition, DispositionType::Inline) } /// Returns `true` if it is [`Attachment`](DispositionType::Attachment). pub fn is_attachment(&self) -> bool { - match self.disposition { - DispositionType::Attachment => true, - _ => false, - } + matches!(self.disposition, DispositionType::Attachment) } /// Returns `true` if it is [`FormData`](DispositionType::FormData). pub fn is_form_data(&self) -> bool { - match self.disposition { - DispositionType::FormData => true, - _ => false, - } + matches!(self.disposition, DispositionType::FormData) } /// Returns `true` if it is [`Ext`](DispositionType::Ext) and the `disp_type` matches. diff --git a/actix-http/src/header/common/mod.rs b/actix-http/src/header/common/mod.rs index 08950ea8b..83489b864 100644 --- a/actix-http/src/header/common/mod.rs +++ b/actix-http/src/header/common/mod.rs @@ -9,11 +9,13 @@ pub use self::accept_charset::AcceptCharset; //pub use self::accept_encoding::AcceptEncoding; -pub use self::accept_language::AcceptLanguage; pub use self::accept::Accept; +pub use self::accept_language::AcceptLanguage; pub use self::allow::Allow; pub use self::cache_control::{CacheControl, CacheDirective}; -pub use self::content_disposition::{ContentDisposition, DispositionType, DispositionParam}; +pub use self::content_disposition::{ + ContentDisposition, DispositionParam, DispositionType, +}; pub use self::content_language::ContentLanguage; pub use self::content_range::{ContentRange, ContentRangeSpec}; pub use self::content_type::ContentType; @@ -47,7 +49,7 @@ macro_rules! __hyper__deref { &mut self.0 } } - } + }; } #[doc(hidden)] @@ -74,8 +76,8 @@ macro_rules! test_header { ($id:ident, $raw:expr) => { #[test] fn $id() { - use $crate::test; use super::*; + use $crate::test; let raw = $raw; let a: Vec> = raw.iter().map(|x| x.to_vec()).collect(); @@ -118,7 +120,7 @@ macro_rules! test_header { // Test formatting if typed.is_some() { let raw = &($raw)[..]; - let mut iter = raw.iter().map(|b|str::from_utf8(&b[..]).unwrap()); + let mut iter = raw.iter().map(|b| str::from_utf8(&b[..]).unwrap()); let mut joined = String::new(); joined.push_str(iter.next().unwrap()); for s in iter { @@ -128,7 +130,7 @@ macro_rules! test_header { assert_eq!(format!("{}", typed.unwrap()), joined); } } - } + }; } #[macro_export] @@ -330,11 +332,10 @@ macro_rules! header { }; } - mod accept_charset; //mod accept_encoding; -mod accept_language; mod accept; +mod accept_language; mod allow; mod cache_control; mod content_disposition; diff --git a/actix-http/src/header/mod.rs b/actix-http/src/header/mod.rs index 0db26ceb0..46fb31a62 100644 --- a/actix-http/src/header/mod.rs +++ b/actix-http/src/header/mod.rs @@ -148,10 +148,7 @@ impl ContentEncoding { #[inline] /// Is the content compressed? pub fn is_compression(self) -> bool { - match self { - ContentEncoding::Identity | ContentEncoding::Auto => false, - _ => true, - } + matches!(self, ContentEncoding::Identity | ContentEncoding::Auto) } #[inline] diff --git a/actix-http/src/httpmessage.rs b/actix-http/src/httpmessage.rs index e1c4136b0..471fbbcdc 100644 --- a/actix-http/src/httpmessage.rs +++ b/actix-http/src/httpmessage.rs @@ -167,7 +167,6 @@ where mod tests { use bytes::Bytes; use encoding_rs::ISO_8859_2; - use mime; use super::*; use crate::test::TestRequest; diff --git a/actix-http/src/ws/frame.rs b/actix-http/src/ws/frame.rs index 8f7004f18..0598a9b4e 100644 --- a/actix-http/src/ws/frame.rs +++ b/actix-http/src/ws/frame.rs @@ -229,10 +229,7 @@ mod tests { fn is_none( frm: &Result)>, ProtocolError>, ) -> bool { - match *frm { - Ok(None) => true, - _ => false, - } + matches!(*frm, Ok(None)) } fn extract( diff --git a/actix-http/src/ws/mask.rs b/actix-http/src/ws/mask.rs index 7eb5d148f..367fb0212 100644 --- a/actix-http/src/ws/mask.rs +++ b/actix-http/src/ws/mask.rs @@ -139,7 +139,7 @@ mod tests { let mut masked = unmasked.clone(); apply_mask_fallback(&mut masked[1..], &mask); - let mut masked_fast = unmasked.clone(); + let mut masked_fast = unmasked; apply_mask(&mut masked_fast[1..], mask_u32); assert_eq!(masked, masked_fast); diff --git a/actix-http/tests/test_openssl.rs b/actix-http/tests/test_openssl.rs index 3a7bfa409..795deacdc 100644 --- a/actix-http/tests/test_openssl.rs +++ b/actix-http/tests/test_openssl.rs @@ -274,9 +274,7 @@ async fn test_h2_head_empty() { async fn test_h2_head_binary() { let mut srv = test_server(move || { HttpService::build() - .h2(|_| { - ok::<_, ()>(Response::Ok().body(STR)) - }) + .h2(|_| ok::<_, ()>(Response::Ok().body(STR))) .openssl(ssl_acceptor()) .map_err(|_| ()) }) diff --git a/actix-http/tests/test_rustls.rs b/actix-http/tests/test_rustls.rs index 465cba6df..beae359d9 100644 --- a/actix-http/tests/test_rustls.rs +++ b/actix-http/tests/test_rustls.rs @@ -280,9 +280,7 @@ async fn test_h2_head_empty() { async fn test_h2_head_binary() { let mut srv = test_server(move || { HttpService::build() - .h2(|_| { - ok::<_, ()>(Response::Ok().body(STR)) - }) + .h2(|_| ok::<_, ()>(Response::Ok().body(STR))) .rustls(ssl_acceptor()) }) .await; diff --git a/actix-http/tests/test_server.rs b/actix-http/tests/test_server.rs index bee5ebef2..0375b6f66 100644 --- a/actix-http/tests/test_server.rs +++ b/actix-http/tests/test_server.rs @@ -489,9 +489,7 @@ async fn test_h1_head_empty() { async fn test_h1_head_binary() { let mut srv = test_server(|| { HttpService::build() - .h1(|_| { - ok::<_, ()>(Response::Ok().body(STR)) - }) + .h1(|_| ok::<_, ()>(Response::Ok().body(STR))) .tcp() }) .await; diff --git a/actix-multipart/CHANGES.md b/actix-multipart/CHANGES.md index df3cecf71..261836223 100644 --- a/actix-multipart/CHANGES.md +++ b/actix-multipart/CHANGES.md @@ -1,15 +1,17 @@ # Changes -## [0.3.0-alpha.1] - 2020-05-25 +## Unreleased - 2020-xx-xx + +## 0.3.0-beta.1 - 2020-07-15 +* Update `actix-web` to 3.0.0-beta.1 + + +## 0.3.0-alpha.1 - 2020-05-25 * Update `actix-web` to 3.0.0-alpha.3 - * Bump minimum supported Rust version to 1.40 - * Minimize `futures` dependencies - * Remove the unused `time` dependency - * Fix missing `std::error::Error` implement for `MultipartError`. ## [0.2.0] - 2019-12-20 diff --git a/actix-multipart/Cargo.toml b/actix-multipart/Cargo.toml index f6d6a37a1..7e81035cc 100644 --- a/actix-multipart/Cargo.toml +++ b/actix-multipart/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "actix-multipart" -version = "0.3.0-alpha.1" +version = "0.3.0-beta.1" authors = ["Nikolay Kim "] description = "Multipart support for actix web framework." readme = "README.md" @@ -16,7 +16,7 @@ name = "actix_multipart" path = "src/lib.rs" [dependencies] -actix-web = { version = "3.0.0-alpha.3", default-features = false } +actix-web = { version = "3.0.0-beta.1", default-features = false } actix-service = "1.0.1" actix-utils = "1.0.3" bytes = "0.5.3" @@ -29,4 +29,4 @@ twoway = "0.2" [dev-dependencies] actix-rt = "1.0.0" -actix-http = "2.0.0-alpha.4" +actix-http = "2.0.0-beta.1" diff --git a/actix-multipart/src/server.rs b/actix-multipart/src/server.rs index f96a7821a..449c7da29 100644 --- a/actix-multipart/src/server.rs +++ b/actix-multipart/src/server.rs @@ -9,8 +9,6 @@ use std::{cmp, fmt}; use bytes::{Bytes, BytesMut}; use futures_util::stream::{LocalBoxStream, Stream, StreamExt}; -use httparse; -use mime; use actix_utils::task::LocalWaker; use actix_web::error::{ParseError, PayloadError}; @@ -876,11 +874,11 @@ mod tests { impl SlowStream { fn new(bytes: Bytes) -> SlowStream { - return SlowStream { - bytes: bytes, + SlowStream { + bytes, pos: 0, ready: false, - }; + } } } diff --git a/actix-web-actors/CHANGES.md b/actix-web-actors/CHANGES.md index 8fd48f77c..868f17e2a 100644 --- a/actix-web-actors/CHANGES.md +++ b/actix-web-actors/CHANGES.md @@ -2,10 +2,13 @@ ## [Unreleased] - 2020-xx-xx + +## [3.0.0-beta.1] - 2020-xx-xx +* Update `actix-web` & `actix-http` dependencies to beta.1 * Bump minimum supported Rust version to 1.40 -## [3.0.0-alpha.1] - 2020-05-08 +## [3.0.0-alpha.1] - 2020-05-08 * Update the actix-web dependency to 3.0.0-alpha.1 * Update the actix dependency to 0.10.0-alpha.2 * Update the actix-http dependency to 2.0.0-alpha.3 diff --git a/actix-web-actors/Cargo.toml b/actix-web-actors/Cargo.toml index 7383b2032..8de679da5 100644 --- a/actix-web-actors/Cargo.toml +++ b/actix-web-actors/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "actix-web-actors" -version = "3.0.0-alpha.1" +version = "3.0.0-beta.1" authors = ["Nikolay Kim "] description = "Actix actors support for actix web framework." readme = "README.md" @@ -17,8 +17,8 @@ path = "src/lib.rs" [dependencies] actix = "0.10.0-alpha.2" -actix-web = { version = "3.0.0-alpha.3", default-features = false } -actix-http = "2.0.0-alpha.4" +actix-web = { version = "3.0.0-beta.1", default-features = false } +actix-http = "2.0.0-beta.1" actix-codec = "0.2.0" bytes = "0.5.2" futures-channel = { version = "0.3.5", default-features = false } diff --git a/actix-web-actors/tests/test_ws.rs b/actix-web-actors/tests/test_ws.rs index 25977c2c2..dda9f6f0b 100644 --- a/actix-web-actors/tests/test_ws.rs +++ b/actix-web-actors/tests/test_ws.rs @@ -30,8 +30,8 @@ impl StreamHandler> for Ws { async fn test_simple() { let mut srv = test::start(|| { App::new().service(web::resource("/").to( - |req: HttpRequest, stream: web::Payload| { - async move { ws::start(Ws, &req, stream) } + |req: HttpRequest, stream: web::Payload| async move { + ws::start(Ws, &req, stream) }, )) }); @@ -51,7 +51,7 @@ async fn test_simple() { .await .unwrap(); let item = framed.next().await.unwrap().unwrap(); - assert_eq!(item, ws::Frame::Binary(Bytes::from_static(b"text").into())); + assert_eq!(item, ws::Frame::Binary(Bytes::from_static(b"text"))); framed.send(ws::Message::Ping("text".into())).await.unwrap(); let item = framed.next().await.unwrap().unwrap(); diff --git a/actix-web-codegen/src/route.rs b/actix-web-codegen/src/route.rs index 7e3d43f1d..676e75e07 100644 --- a/actix-web-codegen/src/route.rs +++ b/actix-web-codegen/src/route.rs @@ -3,7 +3,7 @@ extern crate proc_macro; use proc_macro::TokenStream; use proc_macro2::{Span, TokenStream as TokenStream2}; use quote::{format_ident, quote, ToTokens, TokenStreamExt}; -use syn::{AttributeArgs, Ident, NestedMeta, parse_macro_input}; +use syn::{parse_macro_input, AttributeArgs, Ident, NestedMeta}; enum ResourceType { Async, @@ -196,7 +196,12 @@ impl ToTokens for Route { name, guard, ast, - args: Args { path, guards, wrappers }, + args: + Args { + path, + guards, + wrappers, + }, resource_type, } = self; let resource_name = name.to_string(); diff --git a/actix-web-codegen/tests/test_macro.rs b/actix-web-codegen/tests/test_macro.rs index 0ef7e1c75..13e9120f6 100644 --- a/actix-web-codegen/tests/test_macro.rs +++ b/actix-web-codegen/tests/test_macro.rs @@ -2,11 +2,11 @@ use std::future::Future; use std::pin::Pin; use std::task::{Context, Poll}; -use actix_web::{http, test, web::Path, App, HttpResponse, Responder, Error}; -use actix_web::dev::{Service, Transform, ServiceRequest, ServiceResponse}; +use actix_web::dev::{Service, ServiceRequest, ServiceResponse, Transform}; +use actix_web::http::header::{HeaderName, HeaderValue}; +use actix_web::{http, test, web::Path, App, Error, HttpResponse, Responder}; use actix_web_codegen::{connect, delete, get, head, options, patch, post, put, trace}; use futures_util::future; -use actix_web::http::header::{HeaderName, HeaderValue}; // Make sure that we can name function as 'config' #[get("/config")] @@ -112,6 +112,7 @@ where type Request = ServiceRequest; type Response = ServiceResponse; type Error = Error; + #[allow(clippy::type_complexity)] type Future = Pin>>>; fn poll_ready(&mut self, cx: &mut Context<'_>) -> Poll> { @@ -119,7 +120,6 @@ where } fn call(&mut self, req: ServiceRequest) -> Self::Future { - let fut = self.service.call(req); Box::pin(async move { @@ -223,10 +223,7 @@ async fn test_auto_async() { #[actix_rt::test] async fn test_wrap() { - let srv = test::start(|| { - App::new() - .service(get_wrap) - }); + let srv = test::start(|| App::new().service(get_wrap)); let request = srv.request(http::Method::GET, srv.url("/test/wrap")); let response = request.send().await.unwrap(); diff --git a/awc/CHANGES.md b/awc/CHANGES.md index 19154d35d..3d2806a9f 100644 --- a/awc/CHANGES.md +++ b/awc/CHANGES.md @@ -1,5 +1,13 @@ # Changes +## Unreleased - 2020-xx-xx + + +## 2.0.0-beta.2 - 2020-07-21 +### Changed +* Update `actix-http` dependency to 2.0.0-beta.2 + + ## [2.0.0-beta.1] - 2020-07-14 ### Changed * Update `actix-http` dependency to 2.0.0-beta.1 diff --git a/awc/Cargo.toml b/awc/Cargo.toml index 73dc6e0e5..54db46901 100644 --- a/awc/Cargo.toml +++ b/awc/Cargo.toml @@ -1,16 +1,19 @@ [package] name = "awc" -version = "2.0.0-beta.1" +version = "2.0.0-beta.2" authors = ["Nikolay Kim "] -description = "Actix http client." +description = "Async HTTP client library that uses the Actix runtime." readme = "README.md" keywords = ["actix", "http", "framework", "async", "web"] homepage = "https://actix.rs" repository = "https://github.com/actix/actix-web.git" documentation = "https://docs.rs/awc/" -categories = ["network-programming", "asynchronous", - "web-programming::http-client", - "web-programming::websocket"] +categories = [ + "network-programming", + "asynchronous", + "web-programming::http-client", + "web-programming::websocket", +] license = "MIT OR Apache-2.0" edition = "2018" @@ -36,7 +39,7 @@ compress = ["actix-http/compress"] [dependencies] actix-codec = "0.2.0" actix-service = "1.0.1" -actix-http = "2.0.0-beta.1" +actix-http = "2.0.0-beta.2" actix-rt = "1.0.0" base64 = "0.12" @@ -56,7 +59,7 @@ rust-tls = { version = "0.17.0", package = "rustls", optional = true, features = [dev-dependencies] actix-connect = { version = "2.0.0-alpha.2", features = ["openssl"] } actix-web = { version = "3.0.0-alpha.3", features = ["openssl"] } -actix-http = { version = "2.0.0-beta.1", features = ["openssl"] } +actix-http = { version = "2.0.0-beta.2", features = ["openssl"] } actix-http-test = { version = "2.0.0-alpha.1", features = ["openssl"] } actix-utils = "1.0.3" actix-server = "1.0.0" diff --git a/awc/src/response.rs b/awc/src/response.rs index ffc8c5408..8364aa556 100644 --- a/awc/src/response.rs +++ b/awc/src/response.rs @@ -402,14 +402,12 @@ mod tests { fn json_eq(err: JsonPayloadError, other: JsonPayloadError) -> bool { match err { - JsonPayloadError::Payload(PayloadError::Overflow) => match other { - JsonPayloadError::Payload(PayloadError::Overflow) => true, - _ => false, - }, - JsonPayloadError::ContentType => match other { - JsonPayloadError::ContentType => true, - _ => false, - }, + JsonPayloadError::Payload(PayloadError::Overflow) => { + matches!(other, JsonPayloadError::Payload(PayloadError::Overflow)) + } + JsonPayloadError::ContentType => { + matches!(other, JsonPayloadError::ContentType) + } _ => false, } } diff --git a/awc/tests/test_client.rs b/awc/tests/test_client.rs index cc61f1006..21be155e8 100644 --- a/awc/tests/test_client.rs +++ b/awc/tests/test_client.rs @@ -167,8 +167,7 @@ async fn test_connection_reuse() { }) .and_then( HttpService::new(map_config( - App::new() - .service(web::resource("/").route(web::to(|| HttpResponse::Ok()))), + App::new().service(web::resource("/").route(web::to(HttpResponse::Ok))), |_| AppConfig::default(), )) .tcp(), @@ -205,8 +204,7 @@ async fn test_connection_force_close() { }) .and_then( HttpService::new(map_config( - App::new() - .service(web::resource("/").route(web::to(|| HttpResponse::Ok()))), + App::new().service(web::resource("/").route(web::to(HttpResponse::Ok))), |_| AppConfig::default(), )) .tcp(), diff --git a/awc/tests/test_connector.rs b/awc/tests/test_connector.rs index b352eaab4..633ac2d50 100644 --- a/awc/tests/test_connector.rs +++ b/awc/tests/test_connector.rs @@ -32,8 +32,7 @@ async fn test_connection_window_size() { let srv = test_server(move || { HttpService::build() .h2(map_config( - App::new() - .service(web::resource("/").route(web::to(|| HttpResponse::Ok()))), + App::new().service(web::resource("/").route(web::to(HttpResponse::Ok))), |_| AppConfig::default(), )) .openssl(ssl_acceptor()) diff --git a/awc/tests/test_rustls_client.rs b/awc/tests/test_rustls_client.rs index 0c6be76d4..8fb43c439 100644 --- a/awc/tests/test_rustls_client.rs +++ b/awc/tests/test_rustls_client.rs @@ -64,9 +64,8 @@ async fn _test_connection_reuse_h2() { .and_then( HttpService::build() .h2(map_config( - App::new().service( - web::resource("/").route(web::to(|| HttpResponse::Ok())), - ), + App::new() + .service(web::resource("/").route(web::to(HttpResponse::Ok))), |_| AppConfig::default(), )) .openssl(ssl_acceptor()) diff --git a/awc/tests/test_ssl_client.rs b/awc/tests/test_ssl_client.rs index b2a2e1785..ca65fb248 100644 --- a/awc/tests/test_ssl_client.rs +++ b/awc/tests/test_ssl_client.rs @@ -45,9 +45,8 @@ async fn test_connection_reuse_h2() { .and_then( HttpService::build() .h2(map_config( - App::new().service( - web::resource("/").route(web::to(|| HttpResponse::Ok())), - ), + App::new() + .service(web::resource("/").route(web::to(HttpResponse::Ok))), |_| AppConfig::default(), )) .openssl(ssl_acceptor()) diff --git a/rust-toolchain b/rust-toolchain index f86fb9cbc..a50908ca3 100644 --- a/rust-toolchain +++ b/rust-toolchain @@ -1 +1 @@ -1.41.1 +1.42.0 diff --git a/src/app.rs b/src/app.rs index ae3d9fdf0..fdedb0a75 100644 --- a/src/app.rs +++ b/src/app.rs @@ -489,7 +489,7 @@ mod tests { #[actix_rt::test] async fn test_default_resource() { let mut srv = init_service( - App::new().service(web::resource("/test").to(|| HttpResponse::Ok())), + App::new().service(web::resource("/test").to(HttpResponse::Ok)), ) .await; let req = TestRequest::with_uri("/test").to_request(); @@ -502,13 +502,13 @@ mod tests { let mut srv = init_service( App::new() - .service(web::resource("/test").to(|| HttpResponse::Ok())) + .service(web::resource("/test").to(HttpResponse::Ok)) .service( web::resource("/test2") .default_service(|r: ServiceRequest| { ok(r.into_response(HttpResponse::Created())) }) - .route(web::get().to(|| HttpResponse::Ok())), + .route(web::get().to(HttpResponse::Ok)), ) .default_service(|r: ServiceRequest| { ok(r.into_response(HttpResponse::MethodNotAllowed())) @@ -585,7 +585,7 @@ mod tests { DefaultHeaders::new() .header(header::CONTENT_TYPE, HeaderValue::from_static("0001")), ) - .route("/test", web::get().to(|| HttpResponse::Ok())), + .route("/test", web::get().to(HttpResponse::Ok)), ) .await; let req = TestRequest::with_uri("/test").to_request(); @@ -601,7 +601,7 @@ mod tests { async fn test_router_wrap() { let mut srv = init_service( App::new() - .route("/test", web::get().to(|| HttpResponse::Ok())) + .route("/test", web::get().to(HttpResponse::Ok)) .wrap( DefaultHeaders::new() .header(header::CONTENT_TYPE, HeaderValue::from_static("0001")), @@ -632,7 +632,7 @@ mod tests { Ok(res) } }) - .service(web::resource("/test").to(|| HttpResponse::Ok())), + .service(web::resource("/test").to(HttpResponse::Ok)), ) .await; let req = TestRequest::with_uri("/test").to_request(); @@ -648,7 +648,7 @@ mod tests { async fn test_router_wrap_fn() { let mut srv = init_service( App::new() - .route("/test", web::get().to(|| HttpResponse::Ok())) + .route("/test", web::get().to(HttpResponse::Ok)) .wrap_fn(|req, srv| { let fut = srv.call(req); async { @@ -679,10 +679,9 @@ mod tests { .route( "/test", web::get().to(|req: HttpRequest| { - HttpResponse::Ok().body(format!( - "{}", - req.url_for("youtube", &["12345"]).unwrap() - )) + HttpResponse::Ok().body( + req.url_for("youtube", &["12345"]).unwrap().to_string(), + ) }), ), ) diff --git a/src/app_service.rs b/src/app_service.rs index 233cfc0d5..98d8c8a8d 100644 --- a/src/app_service.rs +++ b/src/app_service.rs @@ -10,6 +10,7 @@ use actix_router::{Path, ResourceDef, ResourceInfo, Router, Url}; use actix_service::boxed::{self, BoxService, BoxServiceFactory}; use actix_service::{fn_service, Service, ServiceFactory}; use futures_util::future::{join_all, ok, FutureExt, LocalBoxFuture}; +use tinyvec::tiny_vec; use crate::config::{AppConfig, AppService}; use crate::data::{DataFactory, FnDataFactory}; @@ -245,7 +246,7 @@ where inner.path.reset(); inner.head = head; inner.payload = payload; - inner.app_data.push(self.data.clone()); + inner.app_data = tiny_vec![self.data.clone()]; req } else { HttpRequest::new( @@ -474,7 +475,7 @@ mod tests { let mut app = init_service( App::new() .data(DropData(data.clone())) - .service(web::resource("/test").to(|| HttpResponse::Ok())), + .service(web::resource("/test").to(HttpResponse::Ok)), ) .await; let req = TestRequest::with_uri("/test").to_request(); diff --git a/src/config.rs b/src/config.rs index 19a5ccc7b..0f49288ec 100644 --- a/src/config.rs +++ b/src/config.rs @@ -311,10 +311,9 @@ mod tests { .route( "/test", web::get().to(|req: HttpRequest| { - HttpResponse::Ok().body(format!( - "{}", - req.url_for("youtube", &["12345"]).unwrap() - )) + HttpResponse::Ok().body( + req.url_for("youtube", &["12345"]).unwrap().to_string(), + ) }), ), ) @@ -330,9 +329,9 @@ mod tests { async fn test_service() { let mut srv = init_service(App::new().configure(|cfg| { cfg.service( - web::resource("/test").route(web::get().to(|| HttpResponse::Created())), + web::resource("/test").route(web::get().to(HttpResponse::Created)), ) - .route("/index.html", web::get().to(|| HttpResponse::Ok())); + .route("/index.html", web::get().to(HttpResponse::Ok)); })) .await; diff --git a/src/data.rs b/src/data.rs index 34ada863d..c50b30328 100644 --- a/src/data.rs +++ b/src/data.rs @@ -200,14 +200,14 @@ mod tests { #[actix_rt::test] async fn test_route_data_extractor() { - let mut srv = - init_service(App::new().service(web::resource("/").data(10usize).route( - web::get().to(|data: web::Data| { - let _ = data.clone(); - HttpResponse::Ok() - }), - ))) - .await; + let mut srv = init_service( + App::new().service( + web::resource("/") + .data(10usize) + .route(web::get().to(|_data: web::Data| HttpResponse::Ok())), + ), + ) + .await; let req = TestRequest::default().to_request(); let resp = srv.call(req).await.unwrap(); @@ -233,7 +233,6 @@ mod tests { web::resource("/").data(10usize).route(web::get().to( |data: web::Data| { assert_eq!(**data, 10); - let _ = data.clone(); HttpResponse::Ok() }, )), diff --git a/src/lib.rs b/src/lib.rs index eb46af664..8776a62b5 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -9,8 +9,8 @@ //! use actix_web::{get, web, App, HttpServer, Responder}; //! //! #[get("/{id}/{name}/index.html")] -//! async fn index(info: web::Path<(u32, String)>) -> impl Responder { -//! format!("Hello {}! id:{}", info.1, info.0) +//! async fn index(web::Path((id, name)): web::Path<(u32, String)>) -> impl Responder { +//! format!("Hello {}! id:{}", name, id) //! } //! //! #[actix_web::main] diff --git a/src/middleware/logger.rs b/src/middleware/logger.rs index 57b640bdd..dc6468ab3 100644 --- a/src/middleware/logger.rs +++ b/src/middleware/logger.rs @@ -626,7 +626,7 @@ mod tests { Ok(()) }; let s = format!("{}", FormatDisplay(&render)); - assert!(s.contains(&format!("{}", now.format("%Y-%m-%dT%H:%M:%S")))); + assert!(s.contains(&now.format("%Y-%m-%dT%H:%M:%S"))); } #[actix_rt::test] diff --git a/src/middleware/normalize.rs b/src/middleware/normalize.rs index 3d5d30c80..a1021ed14 100644 --- a/src/middleware/normalize.rs +++ b/src/middleware/normalize.rs @@ -91,9 +91,9 @@ where // That approach fails when a trailing slash is added, // and a duplicate slash is removed, // since the length of the strings remains the same - // + // // For example, the path "/v1//s" will be normalized to "/v1/s/" - // Both of the paths have the same length, + // Both of the paths have the same length, // so the change can not be deduced from the length comparison if path != original_path { let mut parts = head.uri.clone().into_parts(); @@ -129,7 +129,7 @@ mod tests { let mut app = init_service( App::new() .wrap(NormalizePath::default()) - .service(web::resource("/v1/something/").to(|| HttpResponse::Ok())), + .service(web::resource("/v1/something/").to(HttpResponse::Ok)), ) .await; diff --git a/src/request.rs b/src/request.rs index 85f409016..a1b42f926 100644 --- a/src/request.rs +++ b/src/request.rs @@ -276,6 +276,7 @@ impl HttpMessage for HttpRequest { impl Drop for HttpRequest { fn drop(&mut self) { + // if possible, contribute to current worker's HttpRequest allocation pool if Rc::strong_count(&self.0) == 1 { let v = &mut self.0.pool.0.borrow_mut(); if v.len() < 128 { @@ -340,25 +341,32 @@ impl fmt::Debug for HttpRequest { } } -/// Request's objects pool +/// Slab-allocated `HttpRequest` Pool +/// +/// Since request processing may yield for asynchronous events to complete, a worker may have many +/// requests in-flight at any time. Pooling requests like this amortizes the performance and memory +/// costs of allocating and de-allocating HttpRequest objects as frequently as they otherwise would. +/// +/// Request objects are added when they are dropped (see `::drop`) and re-used +/// in `::call` when there are available objects in the list. +/// +/// The pool's initial capacity is 128 items. pub(crate) struct HttpRequestPool(RefCell>>); impl HttpRequestPool { + /// Allocates a slab of memory for pool use. pub(crate) fn create() -> &'static HttpRequestPool { let pool = HttpRequestPool(RefCell::new(Vec::with_capacity(128))); Box::leak(Box::new(pool)) } - /// Get message from the pool + /// Re-use a previously allocated (but now completed/discarded) HttpRequest object. #[inline] pub(crate) fn get_request(&self) -> Option { - if let Some(inner) = self.0.borrow_mut().pop() { - Some(HttpRequest(inner)) - } else { - None - } + self.0.borrow_mut().pop().map(HttpRequest) } + /// Clears all allocated HttpRequest objects. pub(crate) fn clear(&self) { self.0.borrow_mut().clear() } diff --git a/src/resource.rs b/src/resource.rs index 5da1de62f..dd9b23012 100644 --- a/src/resource.rs +++ b/src/resource.rs @@ -607,7 +607,7 @@ mod tests { header::CONTENT_TYPE, HeaderValue::from_static("0001"), )) - .route(web::get().to(|| HttpResponse::Ok())), + .route(web::get().to(HttpResponse::Ok)), ), ) .await; @@ -637,7 +637,7 @@ mod tests { }) } }) - .route(web::get().to(|| HttpResponse::Ok())), + .route(web::get().to(HttpResponse::Ok)), ), ) .await; @@ -684,9 +684,7 @@ mod tests { async fn test_default_resource() { let mut srv = init_service( App::new() - .service( - web::resource("/test").route(web::get().to(|| HttpResponse::Ok())), - ) + .service(web::resource("/test").route(web::get().to(HttpResponse::Ok))) .default_service(|r: ServiceRequest| { ok(r.into_response(HttpResponse::BadRequest())) }), @@ -705,7 +703,7 @@ mod tests { let mut srv = init_service( App::new().service( web::resource("/test") - .route(web::get().to(|| HttpResponse::Ok())) + .route(web::get().to(HttpResponse::Ok)) .default_service(|r: ServiceRequest| { ok(r.into_response(HttpResponse::BadRequest())) }), @@ -731,17 +729,17 @@ mod tests { .service( web::resource("/test/{p}") .guard(guard::Get()) - .to(|| HttpResponse::Ok()), + .to(HttpResponse::Ok), ) .service( web::resource("/test/{p}") .guard(guard::Put()) - .to(|| HttpResponse::Created()), + .to(HttpResponse::Created), ) .service( web::resource("/test/{p}") .guard(guard::Delete()) - .to(|| HttpResponse::NoContent()), + .to(HttpResponse::NoContent), ), ) .await; @@ -783,7 +781,8 @@ mod tests { data3: web::Data| { assert_eq!(**data1, 10); assert_eq!(**data2, '*'); - assert_eq!(**data3, 1.0); + let error = std::f64::EPSILON; + assert!((**data3 - 1.0).abs() < error); HttpResponse::Ok() }, ), diff --git a/src/responder.rs b/src/responder.rs index e102d23e1..fc80831b8 100644 --- a/src/responder.rs +++ b/src/responder.rs @@ -480,7 +480,7 @@ pub(crate) mod tests { assert_eq!(resp.status(), StatusCode::OK); match resp.response().body() { ResponseBody::Body(Body::Bytes(ref b)) => { - let bytes: Bytes = b.clone().into(); + let bytes = b.clone(); assert_eq!(bytes, Bytes::from_static(b"some")); } _ => panic!(), diff --git a/src/route.rs b/src/route.rs index b17fa9b06..3a833bdf3 100644 --- a/src/route.rs +++ b/src/route.rs @@ -362,7 +362,7 @@ mod tests { App::new() .service( web::resource("/test") - .route(web::get().to(|| HttpResponse::Ok())) + .route(web::get().to(HttpResponse::Ok)) .route(web::put().to(|| async { Err::(error::ErrorBadRequest("err")) })) diff --git a/src/scope.rs b/src/scope.rs index b9166a4bf..c4b01d266 100644 --- a/src/scope.rs +++ b/src/scope.rs @@ -678,12 +678,9 @@ mod tests { #[actix_rt::test] async fn test_scope() { - let mut srv = init_service( - App::new().service( - web::scope("/app") - .service(web::resource("/path1").to(|| HttpResponse::Ok())), - ), - ) + let mut srv = init_service(App::new().service( + web::scope("/app").service(web::resource("/path1").to(HttpResponse::Ok)), + )) .await; let req = TestRequest::with_uri("/app/path1").to_request(); @@ -696,8 +693,8 @@ mod tests { let mut srv = init_service( App::new().service( web::scope("/app") - .service(web::resource("").to(|| HttpResponse::Ok())) - .service(web::resource("/").to(|| HttpResponse::Created())), + .service(web::resource("").to(HttpResponse::Ok)) + .service(web::resource("/").to(HttpResponse::Created)), ), ) .await; @@ -714,7 +711,7 @@ mod tests { #[actix_rt::test] async fn test_scope_root2() { let mut srv = init_service(App::new().service( - web::scope("/app/").service(web::resource("").to(|| HttpResponse::Ok())), + web::scope("/app/").service(web::resource("").to(HttpResponse::Ok)), )) .await; @@ -730,7 +727,7 @@ mod tests { #[actix_rt::test] async fn test_scope_root3() { let mut srv = init_service(App::new().service( - web::scope("/app/").service(web::resource("/").to(|| HttpResponse::Ok())), + web::scope("/app/").service(web::resource("/").to(HttpResponse::Ok)), )) .await; @@ -748,8 +745,8 @@ mod tests { let mut srv = init_service( App::new().service( web::scope("app") - .route("/path1", web::get().to(|| HttpResponse::Ok())) - .route("/path1", web::delete().to(|| HttpResponse::Ok())), + .route("/path1", web::get().to(HttpResponse::Ok)) + .route("/path1", web::delete().to(HttpResponse::Ok)), ), ) .await; @@ -777,8 +774,8 @@ mod tests { App::new().service( web::scope("app").service( web::resource("path1") - .route(web::get().to(|| HttpResponse::Ok())) - .route(web::delete().to(|| HttpResponse::Ok())), + .route(web::get().to(HttpResponse::Ok)) + .route(web::delete().to(HttpResponse::Ok)), ), ), ) @@ -807,7 +804,7 @@ mod tests { App::new().service( web::scope("/app") .guard(guard::Get()) - .service(web::resource("/path1").to(|| HttpResponse::Ok())), + .service(web::resource("/path1").to(HttpResponse::Ok)), ), ) .await; @@ -842,7 +839,7 @@ mod tests { match resp.response().body() { ResponseBody::Body(Body::Bytes(ref b)) => { - let bytes: Bytes = b.clone().into(); + let bytes = b.clone(); assert_eq!(bytes, Bytes::from_static(b"project: project1")); } _ => panic!(), @@ -855,14 +852,9 @@ mod tests { #[actix_rt::test] async fn test_nested_scope() { - let mut srv = init_service( - App::new().service( - web::scope("/app") - .service(web::scope("/t1").service( - web::resource("/path1").to(|| HttpResponse::Created()), - )), - ), - ) + let mut srv = init_service(App::new().service(web::scope("/app").service( + web::scope("/t1").service(web::resource("/path1").to(HttpResponse::Created)), + ))) .await; let req = TestRequest::with_uri("/app/t1/path1").to_request(); @@ -872,14 +864,9 @@ mod tests { #[actix_rt::test] async fn test_nested_scope_no_slash() { - let mut srv = init_service( - App::new().service( - web::scope("/app") - .service(web::scope("t1").service( - web::resource("/path1").to(|| HttpResponse::Created()), - )), - ), - ) + let mut srv = init_service(App::new().service(web::scope("/app").service( + web::scope("t1").service(web::resource("/path1").to(HttpResponse::Created)), + ))) .await; let req = TestRequest::with_uri("/app/t1/path1").to_request(); @@ -893,8 +880,8 @@ mod tests { App::new().service( web::scope("/app").service( web::scope("/t1") - .service(web::resource("").to(|| HttpResponse::Ok())) - .service(web::resource("/").to(|| HttpResponse::Created())), + .service(web::resource("").to(HttpResponse::Ok)) + .service(web::resource("/").to(HttpResponse::Created)), ), ), ) @@ -916,7 +903,7 @@ mod tests { web::scope("/app").service( web::scope("/t1") .guard(guard::Get()) - .service(web::resource("/path1").to(|| HttpResponse::Ok())), + .service(web::resource("/path1").to(HttpResponse::Ok)), ), ), ) @@ -953,7 +940,7 @@ mod tests { match resp.response().body() { ResponseBody::Body(Body::Bytes(ref b)) => { - let bytes: Bytes = b.clone().into(); + let bytes = b.clone(); assert_eq!(bytes, Bytes::from_static(b"project: project_1")); } _ => panic!(), @@ -981,7 +968,7 @@ mod tests { match resp.response().body() { ResponseBody::Body(Body::Bytes(ref b)) => { - let bytes: Bytes = b.clone().into(); + let bytes = b.clone(); assert_eq!(bytes, Bytes::from_static(b"project: test - 1")); } _ => panic!(), @@ -997,7 +984,7 @@ mod tests { let mut srv = init_service( App::new().service( web::scope("/app") - .service(web::resource("/path1").to(|| HttpResponse::Ok())) + .service(web::resource("/path1").to(HttpResponse::Ok)) .default_service(|r: ServiceRequest| { ok(r.into_response(HttpResponse::BadRequest())) }), @@ -1018,9 +1005,10 @@ mod tests { async fn test_default_resource_propagation() { let mut srv = init_service( App::new() - .service(web::scope("/app1").default_service( - web::resource("").to(|| HttpResponse::BadRequest()), - )) + .service( + web::scope("/app1") + .default_service(web::resource("").to(HttpResponse::BadRequest)), + ) .service(web::scope("/app2")) .default_service(|r: ServiceRequest| { ok(r.into_response(HttpResponse::MethodNotAllowed())) @@ -1043,21 +1031,21 @@ mod tests { #[actix_rt::test] async fn test_middleware() { - let mut srv = - init_service( - App::new().service( - web::scope("app") - .wrap(DefaultHeaders::new().header( + let mut srv = init_service( + App::new().service( + web::scope("app") + .wrap( + DefaultHeaders::new().header( header::CONTENT_TYPE, HeaderValue::from_static("0001"), - )) - .service( - web::resource("/test") - .route(web::get().to(|| HttpResponse::Ok())), ), - ), - ) - .await; + ) + .service( + web::resource("/test").route(web::get().to(HttpResponse::Ok)), + ), + ), + ) + .await; let req = TestRequest::with_uri("/app/test").to_request(); let resp = call_service(&mut srv, req).await; @@ -1084,7 +1072,7 @@ mod tests { Ok(res) } }) - .route("/test", web::get().to(|| HttpResponse::Ok())), + .route("/test", web::get().to(HttpResponse::Ok)), ), ) .await; @@ -1105,7 +1093,6 @@ mod tests { "/t", web::get().to(|data: web::Data| { assert_eq!(**data, 10); - let _ = data.clone(); HttpResponse::Ok() }), ), @@ -1141,7 +1128,6 @@ mod tests { "/t", web::get().to(|data: web::Data| { assert_eq!(**data, 10); - let _ = data.clone(); HttpResponse::Ok() }), ), @@ -1157,7 +1143,7 @@ mod tests { async fn test_scope_config() { let mut srv = init_service(App::new().service(web::scope("/app").configure(|s| { - s.route("/path1", web::get().to(|| HttpResponse::Ok())); + s.route("/path1", web::get().to(HttpResponse::Ok)); }))) .await; @@ -1171,7 +1157,7 @@ mod tests { let mut srv = init_service(App::new().service(web::scope("/app").configure(|s| { s.service(web::scope("/v1").configure(|s| { - s.route("/", web::get().to(|| HttpResponse::Ok())); + s.route("/", web::get().to(HttpResponse::Ok)); })); }))) .await; @@ -1193,10 +1179,9 @@ mod tests { s.route( "/", web::get().to(|req: HttpRequest| async move { - HttpResponse::Ok().body(format!( - "{}", - req.url_for("youtube", &["xxxxxx"]).unwrap().as_str() - )) + HttpResponse::Ok().body( + req.url_for("youtube", &["xxxxxx"]).unwrap().to_string(), + ) }), ); })); diff --git a/src/test.rs b/src/test.rs index a64ec3a73..49c5cc214 100644 --- a/src/test.rs +++ b/src/test.rs @@ -23,7 +23,6 @@ use futures_util::future::ok; use futures_util::StreamExt; use serde::de::DeserializeOwned; use serde::Serialize; -use serde_json; use socket2::{Domain, Protocol, Socket, Type}; pub use actix_http::test::TestBuffer; diff --git a/src/types/form.rs b/src/types/form.rs index c10ed4649..ea061d553 100644 --- a/src/types/form.rs +++ b/src/types/form.rs @@ -407,18 +407,15 @@ mod tests { fn eq(err: UrlencodedError, other: UrlencodedError) -> bool { match err { - UrlencodedError::Overflow { .. } => match other { - UrlencodedError::Overflow { .. } => true, - _ => false, - }, - UrlencodedError::UnknownLength => match other { - UrlencodedError::UnknownLength => true, - _ => false, - }, - UrlencodedError::ContentType => match other { - UrlencodedError::ContentType => true, - _ => false, - }, + UrlencodedError::Overflow { .. } => { + matches!(other, UrlencodedError::Overflow { .. }) + } + UrlencodedError::UnknownLength => { + matches!(other, UrlencodedError::UnknownLength) + } + UrlencodedError::ContentType => { + matches!(other, UrlencodedError::ContentType) + } _ => false, } } diff --git a/src/types/json.rs b/src/types/json.rs index 6de9e0d86..527b4b611 100644 --- a/src/types/json.rs +++ b/src/types/json.rs @@ -433,14 +433,10 @@ mod tests { fn json_eq(err: JsonPayloadError, other: JsonPayloadError) -> bool { match err { - JsonPayloadError::Overflow => match other { - JsonPayloadError::Overflow => true, - _ => false, - }, - JsonPayloadError::ContentType => match other { - JsonPayloadError::ContentType => true, - _ => false, - }, + JsonPayloadError::Overflow => matches!(other, JsonPayloadError::Overflow), + JsonPayloadError::ContentType => { + matches!(other, JsonPayloadError::ContentType) + } _ => false, } } @@ -486,7 +482,7 @@ mod tests { .to_http_parts(); let s = Json::::from_request(&req, &mut pl).await; - let mut resp = Response::from_error(s.err().unwrap().into()); + let mut resp = Response::from_error(s.err().unwrap()); assert_eq!(resp.status(), StatusCode::BAD_REQUEST); let body = load_stream(resp.take_body()).await.unwrap(); diff --git a/src/types/path.rs b/src/types/path.rs index 82050171c..dbb5f3ee0 100644 --- a/src/types/path.rs +++ b/src/types/path.rs @@ -25,8 +25,8 @@ use crate::FromRequest; /// /// extract path info from "/{username}/{count}/index.html" url /// /// {username} - deserializes to a String /// /// {count} - - deserializes to a u32 -/// async fn index(info: web::Path<(String, u32)>) -> String { -/// format!("Welcome {}! {}", info.0, info.1) +/// async fn index(web::Path((username, count)): web::Path<(String, u32)>) -> String { +/// format!("Welcome {}! {}", username, count) /// } /// /// fn main() { @@ -61,20 +61,18 @@ use crate::FromRequest; /// ); /// } /// ``` -pub struct Path { - inner: T, -} +pub struct Path(pub T); impl Path { /// Deconstruct to an inner value pub fn into_inner(self) -> T { - self.inner + self.0 } } impl AsRef for Path { fn as_ref(&self) -> &T { - &self.inner + &self.0 } } @@ -82,31 +80,31 @@ impl ops::Deref for Path { type Target = T; fn deref(&self) -> &T { - &self.inner + &self.0 } } impl ops::DerefMut for Path { fn deref_mut(&mut self) -> &mut T { - &mut self.inner + &mut self.0 } } impl From for Path { fn from(inner: T) -> Path { - Path { inner } + Path(inner) } } impl fmt::Debug for Path { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - self.inner.fmt(f) + self.0.fmt(f) } } impl fmt::Display for Path { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - self.inner.fmt(f) + self.0.fmt(f) } } @@ -120,8 +118,8 @@ impl fmt::Display for Path { /// /// extract path info from "/{username}/{count}/index.html" url /// /// {username} - deserializes to a String /// /// {count} - - deserializes to a u32 -/// async fn index(info: web::Path<(String, u32)>) -> String { -/// format!("Welcome {}! {}", info.0, info.1) +/// async fn index(web::Path((username, count)): web::Path<(String, u32)>) -> String { +/// format!("Welcome {}! {}", username, count) /// } /// /// fn main() { @@ -173,7 +171,7 @@ where ready( de::Deserialize::deserialize(PathDeserializer::new(req.match_info())) - .map(|inner| Path { inner }) + .map(Path) .map_err(move |e| { log::debug!( "Failed during Path extractor deserialization. \ @@ -290,21 +288,22 @@ mod tests { resource.match_path(req.match_info_mut()); let (req, mut pl) = req.into_parts(); - let res = <(Path<(String, String)>,)>::from_request(&req, &mut pl) + let (Path(res),) = <(Path<(String, String)>,)>::from_request(&req, &mut pl) .await .unwrap(); - assert_eq!((res.0).0, "name"); - assert_eq!((res.0).1, "user1"); + assert_eq!(res.0, "name"); + assert_eq!(res.1, "user1"); - let res = <(Path<(String, String)>, Path<(String, String)>)>::from_request( - &req, &mut pl, - ) - .await - .unwrap(); - assert_eq!((res.0).0, "name"); - assert_eq!((res.0).1, "user1"); - assert_eq!((res.1).0, "name"); - assert_eq!((res.1).1, "user1"); + let (Path(a), Path(b)) = + <(Path<(String, String)>, Path<(String, String)>)>::from_request( + &req, &mut pl, + ) + .await + .unwrap(); + assert_eq!(a.0, "name"); + assert_eq!(a.1, "user1"); + assert_eq!(b.0, "name"); + assert_eq!(b.1, "user1"); let () = <()>::from_request(&req, &mut pl).await.unwrap(); } @@ -329,7 +328,7 @@ mod tests { let s = s.into_inner(); assert_eq!(s.value, "user2"); - let s = Path::<(String, String)>::from_request(&req, &mut pl) + let Path(s) = Path::<(String, String)>::from_request(&req, &mut pl) .await .unwrap(); assert_eq!(s.0, "name"); @@ -344,7 +343,7 @@ mod tests { assert_eq!(s.as_ref().key, "name"); assert_eq!(s.value, 32); - let s = Path::<(String, u8)>::from_request(&req, &mut pl) + let Path(s) = Path::<(String, u8)>::from_request(&req, &mut pl) .await .unwrap(); assert_eq!(s.0, "name"); diff --git a/src/types/payload.rs b/src/types/payload.rs index 0efdc2c09..653abf089 100644 --- a/src/types/payload.rs +++ b/src/types/payload.rs @@ -13,10 +13,10 @@ use futures_util::future::{err, ok, Either, FutureExt, LocalBoxFuture, Ready}; use futures_util::StreamExt; use mime::Mime; -use crate::dev; use crate::extract::FromRequest; use crate::http::header; use crate::request::HttpRequest; +use crate::{dev, web}; /// Payload extractor returns request 's payload stream. /// @@ -142,13 +142,8 @@ impl FromRequest for Bytes { #[inline] fn from_request(req: &HttpRequest, payload: &mut dev::Payload) -> Self::Future { - let tmp; - let cfg = if let Some(cfg) = req.app_data::() { - cfg - } else { - tmp = PayloadConfig::default(); - &tmp - }; + // allow both Config and Data + let cfg = PayloadConfig::from_req(req); if let Err(e) = cfg.check_mimetype(req) { return Either::Right(err(e)); @@ -197,13 +192,7 @@ impl FromRequest for String { #[inline] fn from_request(req: &HttpRequest, payload: &mut dev::Payload) -> Self::Future { - let tmp; - let cfg = if let Some(cfg) = req.app_data::() { - cfg - } else { - tmp = PayloadConfig::default(); - &tmp - }; + let cfg = PayloadConfig::from_req(req); // check content-type if let Err(e) = cfg.check_mimetype(req) { @@ -237,7 +226,12 @@ impl FromRequest for String { ) } } -/// Payload configuration for request's payload. + +/// Configuration for request's payload. +/// +/// Applies to the built-in `Bytes` and `String` extractors. Note that the Payload extractor does +/// not automatically check conformance with this configuration to allow more flexibility when +/// building extractors on top of `Payload`. #[derive(Clone)] pub struct PayloadConfig { limit: usize, @@ -284,14 +278,28 @@ impl PayloadConfig { } Ok(()) } + + /// Allow payload config extraction from app data checking both `T` and `Data`, in that + /// order, and falling back to the default payload config. + fn from_req(req: &HttpRequest) -> &PayloadConfig { + req.app_data::() + .or_else(|| { + req.app_data::>() + .map(|d| d.as_ref()) + }) + .unwrap_or_else(|| &DEFAULT_PAYLOAD_CONFIG) + } } +// Allow shared refs to default. +static DEFAULT_PAYLOAD_CONFIG: PayloadConfig = PayloadConfig { + limit: 262_144, // 2^18 bytes (~256kB) + mimetype: None, +}; + impl Default for PayloadConfig { fn default() -> Self { - PayloadConfig { - limit: 262_144, - mimetype: None, - } + DEFAULT_PAYLOAD_CONFIG.clone() } } @@ -407,8 +415,9 @@ mod tests { use bytes::Bytes; use super::*; - use crate::http::header; - use crate::test::TestRequest; + use crate::http::{header, StatusCode}; + use crate::test::{call_service, init_service, TestRequest}; + use crate::{web, App, Responder}; #[actix_rt::test] async fn test_payload_config() { @@ -428,6 +437,86 @@ mod tests { assert!(cfg.check_mimetype(&req).is_ok()); } + #[actix_rt::test] + async fn test_config_recall_locations() { + async fn bytes_handler(_: Bytes) -> impl Responder { + "payload is probably json bytes" + } + + async fn string_handler(_: String) -> impl Responder { + "payload is probably json string" + } + + let mut srv = init_service( + App::new() + .service( + web::resource("/bytes-app-data") + .app_data( + PayloadConfig::default().mimetype(mime::APPLICATION_JSON), + ) + .route(web::get().to(bytes_handler)), + ) + .service( + web::resource("/bytes-data") + .data(PayloadConfig::default().mimetype(mime::APPLICATION_JSON)) + .route(web::get().to(bytes_handler)), + ) + .service( + web::resource("/string-app-data") + .app_data( + PayloadConfig::default().mimetype(mime::APPLICATION_JSON), + ) + .route(web::get().to(string_handler)), + ) + .service( + web::resource("/string-data") + .data(PayloadConfig::default().mimetype(mime::APPLICATION_JSON)) + .route(web::get().to(string_handler)), + ), + ) + .await; + + let req = TestRequest::with_uri("/bytes-app-data").to_request(); + let resp = call_service(&mut srv, req).await; + assert_eq!(resp.status(), StatusCode::BAD_REQUEST); + + let req = TestRequest::with_uri("/bytes-data").to_request(); + let resp = call_service(&mut srv, req).await; + assert_eq!(resp.status(), StatusCode::BAD_REQUEST); + + let req = TestRequest::with_uri("/string-app-data").to_request(); + let resp = call_service(&mut srv, req).await; + assert_eq!(resp.status(), StatusCode::BAD_REQUEST); + + let req = TestRequest::with_uri("/string-data").to_request(); + let resp = call_service(&mut srv, req).await; + assert_eq!(resp.status(), StatusCode::BAD_REQUEST); + + let req = TestRequest::with_uri("/bytes-app-data") + .header(header::CONTENT_TYPE, mime::APPLICATION_JSON) + .to_request(); + let resp = call_service(&mut srv, req).await; + assert_eq!(resp.status(), StatusCode::OK); + + let req = TestRequest::with_uri("/bytes-data") + .header(header::CONTENT_TYPE, mime::APPLICATION_JSON) + .to_request(); + let resp = call_service(&mut srv, req).await; + assert_eq!(resp.status(), StatusCode::OK); + + let req = TestRequest::with_uri("/string-app-data") + .header(header::CONTENT_TYPE, mime::APPLICATION_JSON) + .to_request(); + let resp = call_service(&mut srv, req).await; + assert_eq!(resp.status(), StatusCode::OK); + + let req = TestRequest::with_uri("/string-data") + .header(header::CONTENT_TYPE, mime::APPLICATION_JSON) + .to_request(); + let resp = call_service(&mut srv, req).await; + assert_eq!(resp.status(), StatusCode::OK); + } + #[actix_rt::test] async fn test_bytes() { let (req, mut pl) = TestRequest::with_header(header::CONTENT_LENGTH, "11") diff --git a/tests/test_server.rs b/tests/test_server.rs index 0ac4b0232..fa8a93f06 100644 --- a/tests/test_server.rs +++ b/tests/test_server.rs @@ -851,7 +851,7 @@ async fn test_slow_request() { use std::net; let srv = test::start_with(test::config().client_timeout(200), || { - App::new().service(web::resource("/").route(web::to(|| HttpResponse::Ok()))) + App::new().service(web::resource("/").route(web::to(HttpResponse::Ok))) }); let mut stream = net::TcpStream::connect(srv.addr()).unwrap();