Merge branch 'master' into less-pin-unchecked

.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
+<!--- If you're describing a bug, tell us what should happen -->
+<!--- If you're suggesting a change/improvement, tell us how it should work -->
+
+## Current Behavior
+<!--- If describing a bug, tell us what happens instead of the expected behavior -->
+<!--- If suggesting a change/improvement, explain the difference from current behavior -->
+
+## Possible Solution
+<!--- Not obligatory, but suggest a fix/reason for the bug, -->
+<!--- or ideas how to implement the addition or change -->
+
+## Steps to Reproduce (for bugs)
+<!--- Provide a link to a live example, or an unambiguous set of steps to -->
+<!--- reproduce this bug. Include code to reproduce, if relevant -->
+1.
+2.
+3.
+4.
+
+## Context
+<!--- How has this issue affected you? What are you trying to accomplish? -->
+<!--- Providing context helps us come up with a solution that is most useful in the real world -->
+
+## Your Environment
+<!--- Include as many relevant details about the environment you experienced the bug in -->
+
+* Rust Version (I.e, output of `rustc -V`):
+* Actix Web Version:

.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

.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

.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') }}
+          key: ${{ matrix.version }}-x86_64-pc-windows-msvc-cargo-build-trimmed-${{ hashFiles('**/Cargo.lock') }}
-      - name: Cache vcpkg package
-        uses: actions/cache@v1
-        id: cache-vcpkg
-        with:
-          path: C:\vcpkg
-          key: windows_x64-${{ matrix.version }}-vcpkg
 
       - 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

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

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

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

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<T: Service>(Rc<UnsafeCell<T>>);
+///
+/// # 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<T: Service>(Rc<RefCell<T>>);
 
 impl<T: Service> CloneableService<T> {
     pub(crate) fn new(service: T) -> Self {
-        Self(Rc::new(UnsafeCell::new(service)))
+        Self(Rc::new(RefCell::new(service)))
     }
 }
 
@@ -27,10 +31,10 @@ impl<T: Service> Service for CloneableService<T> {
     type Future = T::Future;
 
     fn poll_ready(&mut self, cx: &mut Context<'_>) -> Poll<Result<(), Self::Error>> {
-        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)
     }
 }

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
+/// If the `SameSite` attribute is not present then the cookie will be sent as
-/// `SameSite::None` variant), then the cookie will be sent as normal.
+/// 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"),
         }
     }
 }

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]

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

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)
                 }
             }
         };

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()

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);

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<S>(c: &mut Criterion, srv: S, name: &str)
+where
+    S: Service<Request = ServiceRequest, Response = ServiceResponse, Error = Error>
+        + '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<ServiceResponse, Error> {
+    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);

codecov.yml

@@ -0,0 +1,5 @@
+ignore: # ignore codecoverage on following paths
+  - "**/tests"
+  - "test-server"
+  - "**/benches"
+  - "**/examples"