Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lack of handling for CORS/OPTIONS #25

Open
LLBlumire opened this issue Dec 23, 2016 · 21 comments

Comments

@LLBlumire
Copy link

@LLBlumire LLBlumire commented Dec 23, 2016

In order for a server to provide an external API it needs to be able to deal with Cross Origin Resource Sharing. These are done by web browsers sending a preflight OPTIONS request asking what resources it is allowed to access on the server, then after being given a resonse containing certain information in it's header a browser will send the actual GET/PUT/POST/etc query to the server, with no response it will not do this and cause Cross Site Scripting errors to be reported.

There are two obvious ways this can be handled, the first and simplest is to add an "OPTIONS" annotation of the same ilk as the existing HTTP Methods, this will allow users of Rocket to manually implement the correct preflight request handling for api endpoints that need to provide CORS.

The other is to provide some form of automatic CORS handling, this can be seen in a number of other libraries used for web server development (outside the realm of rust) such as spring.io (http://docs.spring.io/spring/docs/current/spring-framework-reference/html/cors.html) or flasks (https://pypi.python.org/pypi/Flask-Cors).

W3 Specification on CORS: https://www.w3.org/TR/cors/

@fabricedesre

This comment has been minimized.

Copy link
Contributor

@fabricedesre fabricedesre commented Dec 23, 2016

Having a "native" built in support would be better. We did something for iron, which was quite simple (https://github.com/fxbox/iron-cors/).

@kybishop

This comment has been minimized.

Copy link

@kybishop kybishop commented Dec 29, 2016

Info a little more readable than the w3 spec 😉: http://enable-cors.org/server.html

@SergioBenitez

This comment has been minimized.

Copy link
Owner

@SergioBenitez SergioBenitez commented Dec 30, 2016

I agree that Rocket should have a nicer way to handle this, and it's on my list of things to determine how to do better. For now, I've added the ability to use #[route(OPTIONS, "/", ...)] to manually handle OPTIONS requests in 2de006d. As soon as #83 is resolved, you'll be able to use options as a decorator directly.

@flosse

This comment has been minimized.

Copy link

@flosse flosse commented Jan 5, 2017

I'd love to see a full CORS example :) Did someone already made experiences?

@sebasmagri

This comment has been minimized.

Copy link

@sebasmagri sebasmagri commented Jan 7, 2017

This is what I'm currently doing to handle the preflight request:

#[route(OPTIONS, "/endpoint/")]
fn options_handler<'a>() -> Response<'a> {
    Response::build()
        .raw_header("Access-Control-Allow-Origin", "http://host.tld")
        .raw_header("Access-Control-Allow-Methods", "OPTIONS, POST")
        .raw_header("Access-Control-Allow-Headers", "Content-Type")
        .finalize()
}

However, for a handler returning data, I can't still find a way to wrap the existing response in a Response to add the headers. The existing handler looks like this:

#[post("/endpoint/", format = "application/json", data = "<config>")]
fn post_handler(config: JSON<Config>) -> Option<JSON<Wrapper>> {
    let wrapper = factory(report_id);
    match wrapper {
        Some(f) => Some(JSON(f(&(config.unwrap())))),
        None => None
    }
}

Is this a correct approach? How could I add headers to the JSON response?

@kybishop

This comment has been minimized.

Copy link

@kybishop kybishop commented Jan 7, 2017

@sebasmagri JSON impliments the Responder trait. Something like this should work:

#[post("/endpoint/", format = "application/json", data = "<config>")]
fn post_handler<'request>(config: JSON<Config>)
                          -> Option<Result<Response<'request>, &'str>> {
    let wrapper = factory(report_id);
    match wrapper {
        Some(f) => {
            let response = Response::build_from(JSON(f(&(config.unwrap()))).respond().unwrap());

            response.header(hyper::header::AccessControlAllowOrigin::Any)

            Some(response.ok())
        },
        None => None
    }
}
@SergioBenitez

This comment has been minimized.

Copy link
Owner

@SergioBenitez SergioBenitez commented Jan 7, 2017

@kybishop's got the right idea, but you should rarely, if ever, be constructing a Response object in your handler. Instead, create a new type and implement Responder for it. You can then return it directly from your handler. See the wrapping Responder section of the guide for more info.

For this problem in particular, I would create a new CORS type and implement Responder for it. I implemented a small one, and I copy the code below. It's incomplete, inefficient, and could be made less error-prone, but it works, and using it is clean.

use std::collections::HashSet;
use rocket::response::{self, Response, Responder};
use rocket::http::Method;

struct CORS<R> {
    responder: R,
    allow_origin: &'static str,
    expose_headers: HashSet<&'static str>,
    allow_credentials: bool,
    allow_headers: HashSet<&'static str>,
    allow_methods: HashSet<Method>,
    max_age: Option<usize>
}

type PreflightCORS = CORS<()>;

impl PreflightCORS {
    pub fn preflight(origin: &'static str) -> PreflightCORS {
        CORS::origin((), origin)
    }
}

impl<'r, R: Responder<'r>> CORS<R> {
    pub fn origin(responder: R, origin: &'static str) -> CORS<R> {
        CORS {
            responder: responder,
            allow_origin: origin,
            expose_headers: HashSet::new(),
            allow_credentials: false,
            allow_headers: HashSet::new(),
            allow_methods: HashSet::new(),
            max_age: None
        }
    }

    pub fn any(responder: R) -> CORS<R> {
        CORS::origin(responder, "*")
    }

    pub fn credentials(mut self, value: bool) -> CORS<R> {
        self.allow_credentials = value;
        self
    }

    pub fn methods(mut self, methods: Vec<Method>) -> CORS<R> {
        for method in methods {
            self.allow_methods.insert(method);
        }

        self
    }

    pub fn headers(mut self, headers: Vec<&'static str>) -> CORS<R> {
        for header in headers {
            self.allow_headers.insert(header);
        }

        self
    }

    // TODO: Add more builder methods to set the rest of the fields.
}

impl<'r, R: Responder<'r>> Responder<'r> for CORS<R> {
    fn respond(self) -> response::Result<'r> {
        let mut response = Response::build_from(self.responder.respond()?)
            .raw_header("Access-Control-Allow-Origin", self.allow_origin)
            .finalize();

        match self.allow_credentials {
            true => response.set_raw_header("Access-Control-Allow-Credentials", "true"),
            false => response.set_raw_header("Access-Control-Allow-Credentials", "false")
        };

        if !self.allow_methods.is_empty() {
            let mut methods = String::with_capacity(self.allow_methods.len() * 7);
            for (i, method) in self.allow_methods.iter().enumerate() {
                if i != 0 { methods.push_str(", ") }
                methods.push_str(method.as_str());
            }

            response.set_raw_header("Access-Control-Allow-Methods", methods);
        }

        // FIXME: Get rid of this dupe.
        if !self.allow_headers.is_empty() {
            let mut headers = String::with_capacity(self.allow_headers.len() * 15);
            for (i, header) in self.allow_headers.iter().enumerate() {
                if i != 0 { headers.push_str(", ") }
                headers.push_str(header);
            }

            response.set_raw_header("Access-Control-Allow-Headers", headers);
        }

        // TODO: Inspect and set the rest of the fields.

        Ok(response)
    }

}

You would then use it in routes like:

#[route(OPTIONS, "/item")]
fn cors_preflight() -> PreflightCORS {
    CORS::preflight("http://host.tld")
        .methods(vec![Method::Options, Method::Post])
        .headers(vec!["Content-Type"])
}

#[post("/item")]
fn cors_demo() -> CORS<&'static str> {
    CORS::any("This is the response.")
}

In the particular POST route above, you could do:

#[post("/item")]
fn cors_demo() -> CORS<Option<JSON<Wrapper>>> {
    CORS::any(option_json_wrapper)
}
@sebasmagri

This comment has been minimized.

Copy link

@sebasmagri sebasmagri commented Jan 7, 2017

I believe having this type into the framework would be really good. Thanks @SergioBenitez

@rofrol

This comment has been minimized.

Copy link
Contributor

@rofrol rofrol commented Apr 20, 2017

Regarding the code from #25 (comment), I have found this:

The only valid value for this header is true (case-sensitive). If you don't need credentials, omit this header entirely (rather than setting its value to false).

https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Access-Control-Allow-Credentials

@SergioBenitez

This comment has been minimized.

Copy link
Owner

@SergioBenitez SergioBenitez commented Apr 21, 2017

Now that #55 has landed, we can add proper support for CORS to Rocket! I'm happy to field any design ideas and/or mentor anyone who might be willing to implement some CORS related fairings and structures for contrib.

@Arzte

This comment has been minimized.

Copy link

@Arzte Arzte commented Apr 21, 2017

This could be a fun way to use fairings for the first time & contribute to rocket something that would be super helpful for a project of mine. I'm going to get started trying to implement this into contrib via. fairings. @SergioBenitez I'll take some of the code you provided for a CORS type to make this come in faster. (I realize some work is to be done on that example, just part of the fun) I'll send in a pr today or tomorrow with a basic implementation for it.

EDIT: I'm waiting on 0.3.0 as there's more work to be done on fairings, I'm still working on it, but I'm planning to really get a pr in after 0.3 is out.

@SergioBenitez

This comment has been minimized.

Copy link
Owner

@SergioBenitez SergioBenitez commented May 19, 2017

@SirDoctors Just a heads up: the fairings implementation that just landed in master is very likely to be the version that ships in 0.3.

@nicholasday

This comment has been minimized.

Copy link

@nicholasday nicholasday commented Jul 9, 2017

Just for future reference, here is the fairing I wrote for easy CORS:

use rocket::{Request, Response};
use rocket::fairing::{Fairing, Info, Kind};
use rocket::http::{Header, ContentType, Method};
use std::io::Cursor;

pub struct CORS();

impl Fairing for CORS {
    fn info(&self) -> Info {
        Info {
            name: "Add CORS headers to requests",
            kind: Kind::Response
        }
    }

    fn on_response(&self, request: &Request, response: &mut Response) {
        if request.method() == Method::Options || response.content_type() == Some(ContentType::JSON) {
            response.set_header(Header::new("Access-Control-Allow-Origin", "http://localhost:9000"));
            response.set_header(Header::new("Access-Control-Allow-Methods", "POST, GET, OPTIONS"));
            response.set_header(Header::new("Access-Control-Allow-Headers", "Content-Type"));
            response.set_header(Header::new("Access-Control-Allow-Credentials", "true"));
        }

        if request.method() == Method::Options {
            response.set_header(ContentType::Plain);
            response.set_sized_body(Cursor::new(""));
        }
    }
}

You just have to attach it like this:

rocket::ignite()
    .attach(CORS())
@flosse flosse mentioned this issue Jul 9, 2017
@dlight

This comment has been minimized.

Copy link

@dlight dlight commented Jul 18, 2017

@nicholasday perhaps this fairing could be added to contrib?

@mehcode

This comment has been minimized.

Copy link
Contributor

@mehcode mehcode commented Jul 18, 2017

Now that Rocket v0.3 is here a formalized ( and configurable, see https://echo.labstack.com/middleware/cors for an idea ) should definitely be added to contrib.

@lawliet89

This comment has been minimized.

Copy link
Contributor

@lawliet89 lawliet89 commented Jul 18, 2017

I have implemented CORS both as Fairing and for ad-hoc route based usage in a rocket_cors crate. Hopefully the documentation is sufficient. The options are configurable.

The problem with @nicholasday's implementation is that if the routes have any side effects, it will be too late to reverse them if the CORS check fails in on_response.

@tkondrashov

This comment has been minimized.

Copy link

@tkondrashov tkondrashov commented Jul 3, 2018

I have been using the @nicholasday solution for my project, and it works fine for GET requests. Now I am adding my first POST request and not sure how I can use Rocket for this. Rocket claims success on the request:

POST /image/label/submit application/x-www-form-urlencoded; charset=UTF-8:
    => Matched: POST /image/label/submit (add_label)
    => Outcome: Success
    => Response succeeded.

but Chrome tells a different story:

Failed to load http://localhost:8000/image/label/submit: No 'Access-Control-Allow-Origin' header is present on the requested resource. Origin 'http://localhost:7700' is therefore not allowed access.

Is there a way I can modify that code to work with POST requests? Am I missing something? Not to sound too negative, but I don't think CORS is intuitive or simple enough that everyone who wants to write an API with Rocket should be required to slog through custom solutions like this...

#[post("/image/label/submit", data = "<data>")]
fn add_label(data: String) -> String {
	data
}

This is the API call, in case the mistake is here.

EDIT: Of course, as is standard for asking questions on the internet, I figured it out almost immediately after asking. The problem is that I have the String return type for this POST request, and the fairing only applies to JSON return types. Not that hard to tell if you read the code, but if you can't read like me, my adventure may be helpful! @nicholasday 's solution only works for JSON requests, so if you have issues, make sure your requests return JSON (as I am now going to do), or add a check for ContentType::Plain just like the response.content_type() == Some(ContentType::JSON) in the code.

@derekdreery

This comment has been minimized.

Copy link

@derekdreery derekdreery commented May 13, 2019

@tkondrashov I might be wrong but I think that CORS only applies to json.

@LeviSchuck

This comment has been minimized.

Copy link

@LeviSchuck LeviSchuck commented Aug 19, 2019

Is this considered stalled?

@derekdreery

This comment has been minimized.

Copy link

@derekdreery derekdreery commented Aug 20, 2019

@LeviSchuck I believe this is kinda blocked on fairings being able to return early (because for preflight requests there is no need to go to any route handler - we can just intercept the request early and return whatever parameters are set for Access-Control-Allow-Origin et. al.)

@derekdreery

This comment has been minimized.

Copy link

@derekdreery derekdreery commented Aug 20, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
You can’t perform that action at this time.