Neoteroi / BlackSheep

Fast ASGI web framework for Python
https://www.neoteroi.dev/blacksheep/
MIT License
1.8k stars 75 forks source link

CORS middleware sets invalid `Access-Control-Allow-Origin` header value when multiple `allow_origins` options are set #364

Closed waweber closed 1 year ago

waweber commented 1 year ago

Set up an app with settings similar to:

app.use_cors(
    allow_methods=("GET", "POST", "PUT", "DELETE"),
    allow_origins=("http://localhost:8000", "http://localhost:9000"),
)

Then, try a request from one of the allowed origins. You may see the request fail CORS authorization due to the presence of multiple values for the Access-Control-Allow-Origin header.

Documentation for this header states:

Specifies an origin. Only a single origin can be specified. If the server supports clients from multiple origins, it must return the origin for the specific client making the request.

Meaning, the server must not set each value provided in the allow_origins argument. See blacksheep/server/cors.py where the values are joined together with ", ".

The correct behavior would be to inspect the value of the Origin header sent in the request, and compare it to the values in the allow_origins setting. If there is a match, set the Access-Control-Allow-Origin response header to the matched value. If there is no match, omit the header.

Relatedly, the port of an origin is significant, but may be omitted, in which case the default port for the scheme is used. For example http://example.foo is implied to be http://example.foo:80. The current code for matching the origin doesn't seem to take this into account. If you specify an allowed origin of http://example.foo:80 the CORS middleware will not match requests from http://example.foo

RobertoPrevato commented 1 year ago

Hi @waweber Thank You for the heads up! I will take a look at this, and try to fix ASAP.

RobertoPrevato commented 1 year ago

Thank You again @waweber This has been fixed