Add `Cors` for setting CORS headers #112

davidpdrsn · 2021-07-16T15:32:21Z

Example usage:

use http::{Request, Response, Method, header};
use hyper::Body;
use tower::{ServiceBuilder, ServiceExt, Service};
use tower_http::cors::{CorsLayer, Any};
use std::convert::Infallible;

async fn handle(request: Request<Body>) -> Result<Response<Body>, Infallible> {
    Ok(Response::new(Body::empty()))
}

let cors = CorsLayer::new()
    .allow_methods(vec![Method::GET, Method::POST, Method::OPTIONS])
    .allow_origin(Any)
    .allow_credentials(false);

let mut service = ServiceBuilder::new()
    .layer(cors)
    .service_fn(handle);

let request = Request::builder()
    .header(header::ORIGIN, "https://example.com")
    .body(Body::empty())
    .unwrap();

let response = service
    .ready()
    .await?
    .call(request)
    .await?;

assert_eq!(
    response.headers().get(header::ACCESS_CONTROL_ALLOW_ORIGIN).unwrap(),
    "*",
);

Implementation is based on CorsMiddleware from tide.

jplatte · 2021-08-02T16:50:46Z

I've had a look and one thing I'm missing is a little more flexibility for origins. actix_cors allows you to simply provide a closure to check origins which aren't in the allow list. In the crate docs this is part of their example:

.allowed_origin_fn(|origin, _req_head| {
  origin.as_bytes().ends_with(b".rust-lang.org")
})

and I've previously done a very similar thing with that crate, with an additional origin.starts_with(b"https://") check.

hawkw

I've had a look and one thing I'm missing is a little more flexibility for origins. actix_cors allows you to simply provide a closure to check origins which aren't in the allow list. In the crate docs this is part of their example:
.allowed_origin_fn(|origin, _req_head| {
  origin.as_bytes().ends_with(b".rust-lang.org")
})
and I've previously done a very similar thing with that crate, with an additional origin.starts_with(b"https://") check.

Hmm, yeah, I wonder if what we want is a trait for allowed origin predicates that can be implemented by closures and by T: Into<AnyOr<Origin>>?

examples/tonic-key-value-store/Cargo.toml

tower-http/CHANGELOG.md

tower-http/src/cors.rs

hawkw · 2021-08-02T17:10:42Z

I wonder if it would make sense to implement the CORS logic as an AsyncPredicate for use with tower::filter. The CORS layer could then build filter services with that predicate.

OTTOH, I'm not sure how much value there is from this — most of the meat is in the Future impl, which would stay the same either way.

davidpdrsn · 2021-08-02T17:38:50Z

Hmm, yeah, I wonder if what we want is a trait for allowed origin predicates that can be implemented by closures and by T: Into<AnyOr>?

I like that idea! I'll experiment.

davidpdrsn · 2021-08-03T21:03:07Z

Another thing to consider is that since parts of this code is copied from tide, we have to include their license somewhere.

nkconnor

Adding my comments from Discord so I can add follow-ups given the time

tower-http/src/cors.rs

fourbytes · 2021-08-23T15:46:35Z

Using Axum for a project at the moment and in need of a CORS layer, any idea when this will be ready?

tower-http/src/cors.rs

davidpdrsn · 2021-08-23T16:14:25Z

Using Axum for a project at the moment and in need of a CORS layer, any idea when this will be ready?

I'm gonna look into this once axum 0.2 is released, which will happen this week. I can't give an ETA on merging this however. Until then you can apply the CORS headers manually and add a catch-all route for OPTIONS requests.

fourbytes · 2021-08-25T06:27:03Z

Ended up quickly forking this branch as I needed the ability to allow multiple origins which was difficult to do with manually injected headers.

With these changes it seems to be working nicely.

davidpdrsn · 2021-08-30T19:46:26Z

Believe I've addresses all the comments now. Thanks for the feedback!

Allow-Origin can be now determined via a closure, similar to actix-cors:

use tower_http::cors::{CorsLayer, Any};
use http::{HeaderValue, request::Parts};

let layer = CorsLayer::new().allow_origin(|origin: &HeaderValue, _request_head: &Parts| {
    origin.as_bytes().ends_with(b".rust-lang.org")
});

@jplatte @hawkw @fourbytes @nkconnor Wanna give this another look?

nkconnor · 2021-08-30T21:00:16Z

lg2m - thanks!

fourbytes

Doesn't seem to be working for me yet due to an issue with the valid method check. Other than that though, API looks great.

tower-http/src/cors.rs

jplatte

One thing this still doesn't allow which actix-cors allows is

Cors::ctor()
    .allowed_origin("http://127.0.0.1")
    .allowed_origin("foo.bar.org")
    .allowed_origin_fn(|origin, _| {
        let origin = origin.as_bytes();
        origin.starts_with(b"https://") && origin.ends_with(b".rust-lang.org")
    })

which also means that it's impossible with the current code structure for clients to discover some of the origins that would be allowed to use a route, at least when using a closure (it will look like no origins are allowed when doing the request from a disallowed origin). I think what actix-cors does is send all origins that are specified explicitly regardless of whether they match, and additionally send the request origin back in the header if the function marks it as allowed.

I don't think it would be a problem for me, but I thought I'd still bring it up.

tower-http/src/cors.rs

davidpdrsn · 2021-08-31T12:02:51Z

which also means that it's impossible with the current code structure for clients to discover some of the origins that would be allowed to use a route, at least when using a closure (it will look like no origins are allowed when doing the request from a disallowed origin). I think what actix-cors does is send all origins that are specified explicitly regardless of whether they match, and additionally send the request origin back in the header if the function marks it as allowed.

Hm thats a good point. I was wondering why actix that it setup the way they did. Although I do think its unfortunate to have to store both a list a allowed origins, and a closure to call if nothing else matches 🤔

I don't think it would be a problem for me, but I thought I'd still bring it up.

Would be an issue for me either. I'm leaning towards keeping it as is 🤷

tower-http/src/cors.rs

DoumanAsh

We're interested in having cors for our ongoing migration to axum.
I've put some minor comments

tower-http/src/cors.rs

DoumanAsh

Small suggestions to improve efficiency and performance

tower-http/src/cors.rs

silvioprog · 2021-10-08T16:52:00Z

Hey guys, is there any plan when it will be merged?

davidpdrsn · 2021-10-08T16:57:27Z

I basically just have to clean up the docs and then it should be good to go. Should happen somewhat soon but dunno exactly when. I recommend going with a git dependency on tower-http for now.

- New middleware: Add `Cors` for setting [CORS] headers ([#112]) - New middleware: Add `AsyncRequireAuthorization` ([#118]) - `Compression`: Don't recompress HTTP responses ([#140]) - `Compression` and `Decompression`: Pass configuration from layer into middleware ([#132]) - `ServeDir` and `ServeFile`: Improve performance ([#137]) - `Compression`: Remove needless `ResBody::Error: Into<BoxError>` bounds ([#117]) - `ServeDir`: Percent decode path segments ([#129]) - `ServeDir`: Use correct redirection status ([#130]) - `ServeDir`: Return `404 Not Found` on requests to directories if `append_index_html_on_directories` is set to `false` ([#122]) [#112]: #112 [#118]: #118 [#140]: #140 [#132]: #132 [#137]: #137 [#117]: #117 [#129]: #129 [#130]: #130 [#122]: #122

davidpdrsn added the A-new-middleware Area: new middleware proposals label Jul 16, 2021

hawkw reviewed Aug 2, 2021

View reviewed changes

examples/tonic-key-value-store/Cargo.toml Outdated Show resolved Hide resolved

tower-http/CHANGELOG.md Outdated Show resolved Hide resolved

tower-http/src/cors.rs Outdated Show resolved Hide resolved

nkconnor mentioned this pull request Aug 3, 2021

Support for CORS preflight requests tokio-rs/axum#106

Closed

nkconnor reviewed Aug 4, 2021

View reviewed changes

tower-http/src/cors.rs Show resolved Hide resolved

tower-http/src/cors.rs Outdated Show resolved Hide resolved

tower-http/src/cors.rs Show resolved Hide resolved

fourbytes reviewed Aug 23, 2021

View reviewed changes