CORS on rejected requests

[bertptrs]

The CORS filter automatically adds the proper headers to responses and handles pre-flight requests, but if a route is ever rejected, the headers are not added, and so a browser will reject the response. This makes it hard to determine the failure from the client side, since the response never arrives there.

I've created a small demo to show this problem. Based on the example in the readme, we have the following server:

use warp::Filter;

#[tokio::main]
async fn main() {
    // GET / with header X-Username -> Hello, X-Username
    let hello = warp::filters::header::header("X-Username")
        .map(|name: String| format!("Hello, {}!", name));

    // Add CORS to that route
    let hello = hello.with(warp::cors().allow_any_origin());

    warp::serve(hello)
        .run(([127, 0, 0, 1], 3030))
        .await;
}

A valid request is handled correctly, and the allowed origin header will show up:

$ curl localhost:3030 -H "Origin: example.org" -v -H "X-Username: Bert"
*   Trying 127.0.0.1:3030...
* Connected to localhost (127.0.0.1) port 3030 (#0)
> GET / HTTP/1.1
> Host: localhost:3030
> User-Agent: curl/7.69.1
> Accept: */*
> Origin: example.org
> X-Username: Bert
> 
* Mark bundle as not supporting multiuse
< HTTP/1.1 200 OK
< content-type: text/plain; charset=utf-8
< access-control-allow-origin: example.org
< content-length: 12
< date: Fri, 03 Apr 2020 08:29:43 GMT
< 
* Connection #0 to host localhost left intact
Hello, Bert!

If we are missing the required header, we get a Bad Request response as expected, but the access-control-allow-origin-header will be missing:

$ curl localhost:3030 -H "Origin: example.org" -v
*   Trying 127.0.0.1:3030...
* Connected to localhost (127.0.0.1) port 3030 (#0)
> GET / HTTP/1.1
> Host: localhost:3030
> User-Agent: curl/7.69.1
> Accept: */*
> Origin: example.org
> 
* Mark bundle as not supporting multiuse
< HTTP/1.1 400 Bad Request
< content-type: text/plain; charset=utf-8
< content-length: 35
< date: Fri, 03 Apr 2020 08:35:37 GMT
< 
* Connection #0 to host localhost left intact
Missing request header "X-Username"

It's possible to work around this issue by having a recover at the end to catch all possible errors at the end. This is inconvenient because in general, the default error messages are pretty good and don't need replacement, since you can clearly see what's wrong here.

[D1plo1d]

Having the same issue. Could you share your recover workaround?

[bertptrs]

I'm not sure whether I'm at liberty to share it, but the basic idea is just the normal use of the recover system. Just add a check for every Rejection that might happen in your route and handle them manually. That way, the route ends "successfully" and the CORS handler works as normal.

Take a look at the rejections example.

[khvorov45]

The recover "workaround" would look like this

use warp::Filter;

#[tokio::main]
async fn main() {
    // GET / with header X-Username -> Hello, X-Username
    let hello = warp::filters::header::header("X-Username")
        .map(|name: String| format!("Hello, {}!", name));

    // Add CORS to that route
    let hello = hello
        .recover(handle_rejection)
        .with(warp::cors().allow_any_origin());

    warp::serve(hello).run(([127, 0, 0, 1], 3030)).await;
}

async fn handle_rejection(
    err: warp::Rejection,
) -> Result<impl warp::Reply, std::convert::Infallible> {
    Ok(warp::reply::json(&format!("{:?}", err)))
}

You would presumably elaborate on handle_rejection

[sinking-point]

My view is that the current behaviour is already semantically correct, and that the 'workaround' is in fact valid usage. recover should be applied at the path level. I can see no reason why you would want to try other paths after one has matched and resulted in a rejection. So applying recover at the path level, followed by with(cors), makes perfect sense.

[sinking-point]

Perhaps the rejections example could be extended to include a second path with recover called individually on each path.

[sinking-point]

To clarify, this will NOT work:

pub fn all_routes() -> impl Filter<Extract = impl warp::Reply, Error = Infallible> + Clone {
    let hello = warp::path("hello")
        .and(hello_filter())
        .recover(handle_rejection);

    let goodbye = warp::path("goodbye")
        .and(goodbye_filter())
        .recover(handle_rejection);

    hello
        .or(goodbye)
}

If /goodbye is requested, the first hello path filter will reject, and then the rejection is turned into a reply by the recover before the goodbye path is tried.

You have to do it like this:

pub fn all_routes() -> impl Filter<Extract = impl warp::Reply, Error = Infallible> + Clone {
    let hello = warp::path("hello")
        .and(hello_filter().recover(handle_rejection));

    let goodbye = warp::path("goodbye")
        .and(goodbye_filter().recover(handle_rejection));

    hello
        .or(goodbye)
}

[tiptenbrink]

Unfortunately even the workarounds do not work in all cases. It does work when using custom rejections or with the default 404. However, the workaround does not work in the case of e.g. using a CORS-disallowed method.

Here, if you run below as a test:

async fn handle_rejection(
    err: warp::Rejection,
) -> Result<impl warp::Reply, std::convert::Infallible> {
    println!("hi");    
    Ok(warp::reply::json(&format!("{:?}", err)))
}

let cors = warp::cors()
    .allow_headers(vec!["content-type"])
    .allow_methods(vec!["POST", "GET"])
    .allow_any_origin();

let test_path = warp::path("test")
    .map(|| "hello");

let route = warp::any()
    .and(test_path)
    .recover(handle_rejection)
    .with(cors);

let res = warp::test::request()
    .path("/test")
    .method("OPTIONS")
    .header("origin", "http://localhost:8898")
    .reply(&route)
    .await;

println!("{}", res.status());
println!("{:?}", res.headers());

You will see the headers do not include access-control-allow-origin, you'll just get:

403 Forbidden
{"content-type": "text/plain; charset=utf-8"}

This looks like a bug, but unfortunately I can't really read the source code well-enough to find out exactly why this Rejection is not being recovered. As you can see from the output, the recover filter is not being run at all as there is no "hi" from the println. If anybody knows a solution to this, I'd very much like to hear about it.