Skip to content

Conversation

dranikpg
Copy link
Member

@dranikpg dranikpg commented Feb 20, 2022

Hi! I've implemented middleware for supporting CORS headers like discussed here.

What it looks like

CORSHandler is responsible for setting headers in after_handle. By default, it already enables CORS after being included as a middleware. The global rules and rules for certain prefixes can be changed as follows:

    crow::App<crow::CORSHandler> app;

    auto& cors = app.get_middleware<crow::CORSHandler>();

    cors
      .global()
        .methods("POST"_method, "GET"_method)
      .prefix("/cors")
        .origin("example.com")
        .headers("X-Custom-Header")
      .blueprint(bp)
        .ignore();

It resembles Flasks resource specific CORS to some extent. But with a builder instead of dicts 🙂

What I've changed

middlewares/cors.h now contains the CORSHandler with CORSRules.

examples/middlewares/cors.h has a simple example for setting cors rules.

I've also added a simple test.

Issues

  1. Middleware is initialized by the app. This means that an additional
auto& cors = app.get_middleware<crow::CORSHandler>();

is required for setting specific rules.

I also though of implementing a custom cors handler, which extends the default one

class MyCORS : public CORSHandler 
{
  MyCORS() 
  {
    // customize here
  }
}

This makes the code less imperative, but it makes it more difficult to customize it's behaviour dynamically (i.e. use variables from the main scope). This would also allow overwriting rules for handlers with local middleware.

  1. Only one origin can be specified (The origin header can return only one). But at least some web frameworks allow specifying multiple, from which the one is automatically selected (if its the same origin)

  2. There is no inheritance from global to prefix rules, i.e. rules are applied either from a prefix or the global handler. Should global rules be applied always? Or should they be independent from prefix rules?

@The-EDev The-EDev linked an issue Feb 20, 2022 that may be closed by this pull request
@crow-clang-format
Copy link

--- tests/unittest.cpp	(before formatting)
+++ tests/unittest.cpp	(after formatting)
@@ -1531,7 +1531,7 @@
 
     auto& cors = app.get_middleware<crow::CORSHandler>();
     cors.prefix("/origin")
-        .origin("test.test");
+      .origin("test.test");
 
     CROW_ROUTE(app, "/")
     ([&](const request& req) {

void set_header(const std::string& key, const std::string& value, crow::response& res)
{
if (value.size() == 0) return;
if (!get_header_value(res.headers, key).empty()) return;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Headers are not overwritten, this means that a handler can change the CORS behaviour if it wants to

@dranikpg dranikpg marked this pull request as ready for review February 22, 2022 15:00
@dranikpg dranikpg changed the title WIP CORS Middleware CORS Middleware Feb 22, 2022
Copy link
Member

@The-EDev The-EDev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems all good, merging now.

@The-EDev The-EDev merged commit 81a1b49 into CrowCpp:master Mar 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Requests by Javascript client are blocked by CORS policy
2 participants