diff --git a/.github/ISSUE_TEMPLATE/bug_report.md b/.github/ISSUE_TEMPLATE/bug_report.md new file mode 100644 index 00000000..128f51ff --- /dev/null +++ b/.github/ISSUE_TEMPLATE/bug_report.md @@ -0,0 +1,37 @@ +--- +name: bug report +about: create a bug report +--- + +Your issue may already be reported! +Please search on the [Actix Web issue tracker](https://github.com/actix/actix-web/issues) before creating one. + +## Expected Behavior + + + +## Current Behavior + + + +## Possible Solution + + + +## Steps to Reproduce (for bugs) + + +1. +2. +3. +4. + +## Context + + + +## Your Environment + + +* Rust Version (I.e, output of `rustc -V`): +* Actix Web Version: diff --git a/.github/workflows/bench.yml b/.github/workflows/bench.yml new file mode 100644 index 00000000..08bb81d4 --- /dev/null +++ b/.github/workflows/bench.yml @@ -0,0 +1,47 @@ +name: Benchmark (Linux) + +on: [push, pull_request] + +jobs: + check_benchmark: + runs-on: ubuntu-latest + + steps: + - uses: actions/checkout@master + + - name: Install Rust + uses: actions-rs/toolchain@v1 + with: + toolchain: nightly + profile: minimal + override: true + + - name: Generate Cargo.lock + uses: actions-rs/cargo@v1 + with: + command: generate-lockfile + - name: Cache cargo registry + uses: actions/cache@v1 + with: + path: ~/.cargo/registry + key: ${{ matrix.version }}-x86_64-unknown-linux-gnu-registry-trimmed-${{ hashFiles('**/Cargo.lock') }} + - name: Cache cargo index + uses: actions/cache@v1 + with: + path: ~/.cargo/git + key: ${{ matrix.version }}-x86_64-unknown-linux-gnu-cargo-index-trimmed-${{ hashFiles('**/Cargo.lock') }} + - name: Cache cargo build + uses: actions/cache@v1 + with: + path: target + key: ${{ matrix.version }}-x86_64-unknown-linux-gnu-cargo-build-trimmed-${{ hashFiles('**/Cargo.lock') }} + + - name: Check benchmark + uses: actions-rs/cargo@v1 + with: + command: bench + + - name: Clear the cargo caches + run: | + cargo install cargo-cache --no-default-features --features ci-autoclean + cargo-cache diff --git a/.github/workflows/macos.yml b/.github/workflows/macos.yml index f50ae2f0..397236a2 100644 --- a/.github/workflows/macos.yml +++ b/.github/workflows/macos.yml @@ -27,22 +27,22 @@ jobs: - name: Generate Cargo.lock uses: actions-rs/cargo@v1 with: - command: update + command: generate-lockfile - name: Cache cargo registry uses: actions/cache@v1 with: path: ~/.cargo/registry - key: ${{ matrix.version }}-x86_64-apple-darwin-cargo-registry-${{ hashFiles('**/Cargo.lock') }} + key: ${{ matrix.version }}-x86_64-apple-darwin-cargo-registry-trimmed-${{ hashFiles('**/Cargo.lock') }} - name: Cache cargo index uses: actions/cache@v1 with: path: ~/.cargo/git - key: ${{ matrix.version }}-x86_64-apple-darwin-cargo-index-${{ hashFiles('**/Cargo.lock') }} + key: ${{ matrix.version }}-x86_64-apple-darwin-cargo-index-trimmed-${{ hashFiles('**/Cargo.lock') }} - name: Cache cargo build uses: actions/cache@v1 with: path: target - key: ${{ matrix.version }}-x86_64-apple-darwin-cargo-build-${{ hashFiles('**/Cargo.lock') }} + key: ${{ matrix.version }}-x86_64-apple-darwin-cargo-build-trimmed-${{ hashFiles('**/Cargo.lock') }} - name: check build uses: actions-rs/cargo@v1 @@ -57,3 +57,8 @@ jobs: args: --all --all-features --no-fail-fast -- --nocapture --skip=test_h2_content_length --skip=test_reading_deflate_encoding_large_random_rustls + + - name: Clear the cargo caches + run: | + cargo install cargo-cache --no-default-features --features ci-autoclean + cargo-cache diff --git a/.github/workflows/windows.yml b/.github/workflows/windows.yml index 9aa3d3ba..fed4ce03 100644 --- a/.github/workflows/windows.yml +++ b/.github/workflows/windows.yml @@ -30,34 +30,31 @@ jobs: - name: Generate Cargo.lock uses: actions-rs/cargo@v1 with: - command: update + command: generate-lockfile - name: Cache cargo registry uses: actions/cache@v1 with: path: ~/.cargo/registry - key: ${{ matrix.version }}-x86_64-pc-windows-msvc-cargo-registry-${{ hashFiles('**/Cargo.lock') }} + key: ${{ matrix.version }}-x86_64-pc-windows-msvc-cargo-registry-trimmed-${{ hashFiles('**/Cargo.lock') }} - name: Cache cargo index uses: actions/cache@v1 with: path: ~/.cargo/git - key: ${{ matrix.version }}-x86_64-pc-windows-msvc-cargo-index-${{ hashFiles('**/Cargo.lock') }} + key: ${{ matrix.version }}-x86_64-pc-windows-msvc-cargo-index-trimmed-${{ hashFiles('**/Cargo.lock') }} - name: Cache cargo build uses: actions/cache@v1 with: path: target - key: ${{ matrix.version }}-x86_64-pc-windows-msvc-cargo-build-${{ hashFiles('**/Cargo.lock') }} - - name: Cache vcpkg package - uses: actions/cache@v1 - id: cache-vcpkg - with: - path: C:\vcpkg - key: windows_x64-${{ matrix.version }}-vcpkg + key: ${{ matrix.version }}-x86_64-pc-windows-msvc-cargo-build-trimmed-${{ hashFiles('**/Cargo.lock') }} - name: Install OpenSSL - if: steps.cache-vcpkg.outputs.cache-hit != 'true' run: | vcpkg integrate install vcpkg install openssl:x64-windows + Copy-Item C:\vcpkg\installed\x64-windows\bin\libcrypto-1_1-x64.dll C:\vcpkg\installed\x64-windows\bin\libcrypto.dll + Copy-Item C:\vcpkg\installed\x64-windows\bin\libssl-1_1-x64.dll C:\vcpkg\installed\x64-windows\bin\libssl.dll + Get-ChildItem C:\vcpkg\installed\x64-windows\bin + Get-ChildItem C:\vcpkg\installed\x64-windows\lib - name: check build uses: actions-rs/cargo@v1 @@ -76,4 +73,9 @@ jobs: --skip=test_simple --skip=test_expect_continue --skip=test_http10_keepalive - --skip=test_slow_request + --skip=test_slow_request + + - name: Clear the cargo caches + run: | + cargo install cargo-cache --no-default-features --features ci-autoclean + cargo-cache diff --git a/Cargo.toml b/Cargo.toml index 9e1b559e..9f0748e0 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -99,6 +99,7 @@ env_logger = "0.6" serde_derive = "1.0" brotli2 = "0.3.2" flate2 = "1.0.13" +criterion = "0.3" [profile.release] lto = true @@ -116,3 +117,11 @@ actix-session = { path = "actix-session" } actix-files = { path = "actix-files" } actix-multipart = { path = "actix-multipart" } awc = { path = "awc" } + +[[bench]] +name = "server" +harness = false + +[[bench]] +name = "service" +harness = false diff --git a/MIGRATION.md b/MIGRATION.md index 529f9714..aef382a2 100644 --- a/MIGRATION.md +++ b/MIGRATION.md @@ -1,3 +1,9 @@ +## Unreleased + +* Setting a cookie's SameSite property, explicitly, to `SameSite::None` will now + result in `SameSite=None` being sent with the response Set-Cookie header. + To create a cookie without a SameSite attribute, remove any calls setting same_site. + ## 2.0.0 * `HttpServer::start()` renamed to `HttpServer::run()`. It also possible to diff --git a/actix-http/CHANGES.md b/actix-http/CHANGES.md index 1c8e4f05..ee3dae5d 100644 --- a/actix-http/CHANGES.md +++ b/actix-http/CHANGES.md @@ -1,5 +1,11 @@ # Changes +# [Unreleased] + +### Fixed + +* Allow `SameSite=None` cookies to be sent in a response. + ## [1.0.1] - 2019-12-20 ### Fixed diff --git a/actix-http/src/cloneable.rs b/actix-http/src/cloneable.rs index 65c6bec2..b64c299f 100644 --- a/actix-http/src/cloneable.rs +++ b/actix-http/src/cloneable.rs @@ -1,4 +1,4 @@ -use std::cell::UnsafeCell; +use std::cell::RefCell; use std::rc::Rc; use std::task::{Context, Poll}; @@ -6,11 +6,15 @@ use actix_service::Service; #[doc(hidden)] /// Service that allows to turn non-clone service to a service with `Clone` impl -pub(crate) struct CloneableService(Rc>); +/// +/// # Panics +/// CloneableService might panic with some creative use of thread local storage. +/// See https://github.com/actix/actix-web/issues/1295 for example +pub(crate) struct CloneableService(Rc>); impl CloneableService { pub(crate) fn new(service: T) -> Self { - Self(Rc::new(UnsafeCell::new(service))) + Self(Rc::new(RefCell::new(service))) } } @@ -27,10 +31,10 @@ impl Service for CloneableService { type Future = T::Future; fn poll_ready(&mut self, cx: &mut Context<'_>) -> Poll> { - unsafe { &mut *self.0.as_ref().get() }.poll_ready(cx) + self.0.borrow_mut().poll_ready(cx) } fn call(&mut self, req: T::Request) -> Self::Future { - unsafe { &mut *self.0.as_ref().get() }.call(req) + self.0.borrow_mut().call(req) } } diff --git a/actix-http/src/cookie/draft.rs b/actix-http/src/cookie/draft.rs index a2b03912..a6525a60 100644 --- a/actix-http/src/cookie/draft.rs +++ b/actix-http/src/cookie/draft.rs @@ -10,18 +10,26 @@ use std::fmt; /// attribute is "Strict", then the cookie is never sent in cross-site requests. /// If the `SameSite` attribute is "Lax", the cookie is only sent in cross-site /// requests with "safe" HTTP methods, i.e, `GET`, `HEAD`, `OPTIONS`, `TRACE`. -/// If the `SameSite` attribute is not present (made explicit via the -/// `SameSite::None` variant), then the cookie will be sent as normal. +/// If the `SameSite` attribute is not present then the cookie will be sent as +/// normal. In some browsers, this will implicitly handle the cookie as if "Lax" +/// and in others, "None". It's best to explicitly set the `SameSite` attribute +/// to avoid inconsistent behavior. +/// +/// **Note:** Depending on browser, the `Secure` attribute may be required for +/// `SameSite` "None" cookies to be accepted. /// /// **Note:** This cookie attribute is an HTTP draft! Its meaning and definition /// are subject to change. +/// +/// More info about these draft changes can be found in the draft spec: +/// - https://tools.ietf.org/html/draft-west-cookie-incrementalism-00 #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] pub enum SameSite { /// The "Strict" `SameSite` attribute. Strict, /// The "Lax" `SameSite` attribute. Lax, - /// No `SameSite` attribute. + /// The "None" `SameSite` attribute. None, } @@ -92,7 +100,7 @@ impl fmt::Display for SameSite { match *self { SameSite::Strict => write!(f, "Strict"), SameSite::Lax => write!(f, "Lax"), - SameSite::None => Ok(()), + SameSite::None => write!(f, "None"), } } } diff --git a/actix-http/src/cookie/mod.rs b/actix-http/src/cookie/mod.rs index 13fd5cf4..d9db600e 100644 --- a/actix-http/src/cookie/mod.rs +++ b/actix-http/src/cookie/mod.rs @@ -746,9 +746,7 @@ impl<'c> Cookie<'c> { } if let Some(same_site) = self.same_site() { - if !same_site.is_none() { - write!(f, "; SameSite={}", same_site)?; - } + write!(f, "; SameSite={}", same_site)?; } if let Some(path) = self.path() { @@ -1037,7 +1035,7 @@ mod tests { let cookie = Cookie::build("foo", "bar") .same_site(SameSite::None) .finish(); - assert_eq!(&cookie.to_string(), "foo=bar"); + assert_eq!(&cookie.to_string(), "foo=bar; SameSite=None"); } #[test] diff --git a/actix-web-codegen/CHANGES.md b/actix-web-codegen/CHANGES.md index de676f68..95696abd 100644 --- a/actix-web-codegen/CHANGES.md +++ b/actix-web-codegen/CHANGES.md @@ -1,5 +1,9 @@ # Changes +## [0.2.NEXT] - 2020-xx-xx + +* Allow the handler function to be named as `config` #1290 + ## [0.2.0] - 2019-12-13 * Generate code for actix-web 2.0 diff --git a/actix-web-codegen/src/route.rs b/actix-web-codegen/src/route.rs index 16d3e815..d4819848 100644 --- a/actix-web-codegen/src/route.rs +++ b/actix-web-codegen/src/route.rs @@ -195,15 +195,15 @@ impl Route { pub struct #name; impl actix_web::dev::HttpServiceFactory for #name { - fn register(self, config: &mut actix_web::dev::AppService) { + fn register(self, __config: &mut actix_web::dev::AppService) { #ast - let resource = actix_web::Resource::new(#path) + let __resource = actix_web::Resource::new(#path) .name(#resource_name) .guard(actix_web::guard::#guard()) #(.guard(actix_web::guard::fn_guard(#extra_guards)))* .#resource_type(#name); - actix_web::dev::HttpServiceFactory::register(resource, config) + actix_web::dev::HttpServiceFactory::register(__resource, __config) } } }; diff --git a/actix-web-codegen/tests/test_macro.rs b/actix-web-codegen/tests/test_macro.rs index 4ac1a802..ffb50c11 100644 --- a/actix-web-codegen/tests/test_macro.rs +++ b/actix-web-codegen/tests/test_macro.rs @@ -2,6 +2,12 @@ use actix_web::{http, test, web::Path, App, HttpResponse, Responder}; use actix_web_codegen::{connect, delete, get, head, options, patch, post, put, trace}; use futures::{future, Future}; +// Make sure that we can name function as 'config' +#[get("/config")] +async fn config() -> impl Responder { + HttpResponse::Ok() +} + #[get("/test")] async fn test_handler() -> impl Responder { HttpResponse::Ok() diff --git a/benches/server.rs b/benches/server.rs new file mode 100644 index 00000000..93079a22 --- /dev/null +++ b/benches/server.rs @@ -0,0 +1,64 @@ +use actix_web::{test, web, App, HttpResponse}; +use awc::Client; +use criterion::{criterion_group, criterion_main, Criterion}; +use futures::future::join_all; + +const STR: &str = "Hello World Hello World Hello World Hello World Hello World \ + Hello World Hello World Hello World Hello World Hello World \ + Hello World Hello World Hello World Hello World Hello World \ + Hello World Hello World Hello World Hello World Hello World \ + Hello World Hello World Hello World Hello World Hello World \ + Hello World Hello World Hello World Hello World Hello World \ + Hello World Hello World Hello World Hello World Hello World \ + Hello World Hello World Hello World Hello World Hello World \ + Hello World Hello World Hello World Hello World Hello World \ + Hello World Hello World Hello World Hello World Hello World \ + Hello World Hello World Hello World Hello World Hello World \ + Hello World Hello World Hello World Hello World Hello World \ + Hello World Hello World Hello World Hello World Hello World \ + Hello World Hello World Hello World Hello World Hello World \ + Hello World Hello World Hello World Hello World Hello World \ + Hello World Hello World Hello World Hello World Hello World \ + Hello World Hello World Hello World Hello World Hello World \ + Hello World Hello World Hello World Hello World Hello World \ + Hello World Hello World Hello World Hello World Hello World \ + Hello World Hello World Hello World Hello World Hello World \ + Hello World Hello World Hello World Hello World Hello World"; + +// benchmark sending all requests at the same time +fn bench_async_burst(c: &mut Criterion) { + let srv = test::start(|| { + App::new() + .service(web::resource("/").route(web::to(|| HttpResponse::Ok().body(STR)))) + }); + + // We are using System here, since Runtime requires preinitialized tokio + // Maybe add to actix_rt docs + let url = srv.url("/"); + let mut rt = actix_rt::System::new("test"); + + c.bench_function("get_body_async_burst", move |b| { + b.iter_custom(|iters| { + let client = Client::new().get(url.clone()).freeze().unwrap(); + + let start = std::time::Instant::now(); + // benchmark body + let resps = rt.block_on(async move { + let burst = (0..iters).map(|_| client.send()); + join_all(burst).await + }); + let elapsed = start.elapsed(); + + // if there are failed requests that might be an issue + let failed = resps.iter().filter(|r| r.is_err()).count(); + if failed > 0 { + eprintln!("failed {} requests (might be bench timeout)", failed); + }; + + elapsed + }) + }); +} + +criterion_group!(server_benches, bench_async_burst); +criterion_main!(server_benches); diff --git a/benches/service.rs b/benches/service.rs new file mode 100644 index 00000000..8adbc8a0 --- /dev/null +++ b/benches/service.rs @@ -0,0 +1,108 @@ +use actix_service::Service; +use actix_web::dev::{ServiceRequest, ServiceResponse}; +use actix_web::{web, App, Error, HttpResponse}; +use criterion::{criterion_main, Criterion}; +use std::cell::RefCell; +use std::rc::Rc; + +use actix_web::test::{init_service, ok_service, TestRequest}; + +/// Criterion Benchmark for async Service +/// Should be used from within criterion group: +/// ```rust,ignore +/// let mut criterion: ::criterion::Criterion<_> = +/// ::criterion::Criterion::default().configure_from_args(); +/// bench_async_service(&mut criterion, ok_service(), "async_service_direct"); +/// ``` +/// +/// Usable for benching Service wrappers: +/// Using minimum service code implementation we first measure +/// time to run minimum service, then measure time with wrapper. +/// +/// Sample output +/// async_service_direct time: [1.0908 us 1.1656 us 1.2613 us] +pub fn bench_async_service(c: &mut Criterion, srv: S, name: &str) +where + S: Service + + 'static, +{ + let mut rt = actix_rt::System::new("test"); + let srv = Rc::new(RefCell::new(srv)); + + let req = TestRequest::default().to_srv_request(); + assert!(rt + .block_on(srv.borrow_mut().call(req)) + .unwrap() + .status() + .is_success()); + + // start benchmark loops + c.bench_function(name, move |b| { + b.iter_custom(|iters| { + let srv = srv.clone(); + // exclude request generation, it appears it takes significant time vs call (3us vs 1us) + let reqs: Vec<_> = (0..iters) + .map(|_| TestRequest::default().to_srv_request()) + .collect(); + let start = std::time::Instant::now(); + // benchmark body + rt.block_on(async move { + for req in reqs { + srv.borrow_mut().call(req).await.unwrap(); + } + }); + let elapsed = start.elapsed(); + // check that at least first request succeeded + elapsed + }) + }); +} + +async fn index(req: ServiceRequest) -> Result { + Ok(req.into_response(HttpResponse::Ok().finish())) +} + +// Benchmark basic WebService directly +// this approach is usable for benching WebService, though it adds some time to direct service call: +// Sample results on MacBook Pro '14 +// time: [2.0724 us 2.1345 us 2.2074 us] +fn async_web_service(c: &mut Criterion) { + let mut rt = actix_rt::System::new("test"); + let srv = Rc::new(RefCell::new(rt.block_on(init_service( + App::new().service(web::service("/").finish(index)), + )))); + + let req = TestRequest::get().uri("/").to_request(); + assert!(rt + .block_on(srv.borrow_mut().call(req)) + .unwrap() + .status() + .is_success()); + + // start benchmark loops + c.bench_function("async_web_service_direct", move |b| { + b.iter_custom(|iters| { + let srv = srv.clone(); + let reqs = (0..iters).map(|_| TestRequest::get().uri("/").to_request()); + + let start = std::time::Instant::now(); + // benchmark body + rt.block_on(async move { + for req in reqs { + srv.borrow_mut().call(req).await.unwrap(); + } + }); + let elapsed = start.elapsed(); + // check that at least first request succeeded + elapsed + }) + }); +} + +pub fn service_benches() { + let mut criterion: ::criterion::Criterion<_> = + ::criterion::Criterion::default().configure_from_args(); + bench_async_service(&mut criterion, ok_service(), "async_service_direct"); + async_web_service(&mut criterion); +} +criterion_main!(service_benches); diff --git a/codecov.yml b/codecov.yml new file mode 100644 index 00000000..90cdfab4 --- /dev/null +++ b/codecov.yml @@ -0,0 +1,5 @@ +ignore: # ignore codecoverage on following paths + - "**/tests" + - "test-server" + - "**/benches" + - "**/examples"