search

cyu / rack-cors

Rack Middleware for handling Cross-Origin Resource Sharing (CORS), which makes cross-origin AJAX possible.
MIT License
3.26k stars 263 forks source link

add note on origins w/ default port to README.md #163

Open nbr opened 6 years ago

nbr commented 6 years ago

When http://EXAMPLE:80 is an allowed origin, requests are not allowed from http://EXAMPLE. Since port 80 is the default port for HTTP, browsers will strip it and thus rack-cors never receives a request from http://EXAMPLE.

A similar problem is discussed here: https://github.com/request/request/issues/515

nbr commented 6 years ago

@cyu Is this still being considered? If not, I can close the PR.

cyu commented 6 years ago

@nbr I'm hesitant because of potential unintended side effects of using URI.parse for this purpose. Perhaps use another method to normalize this?

nbr commented 6 years ago

@cyu That makes sense, since it is not an obvious requirement of parse.

Latest commit does not rely on URI#parse to remove the default port. Instead, we use URI#default_port to decide when to strip the port. We could avoid using URI altogether but it simplifies the code.

nbr commented 6 years ago

@cyu Any feedback on the latest commit?

cyu commented 6 years ago

@nbr I left a comment after the last change (about parse errors in URI.parse)

Also, having thought about this more, I’m thinking we should do this on initialization or at least on first eval — we shouldn’t penalize every call with this evaluation. Thoughts?

nbr commented 2 years ago

@cyu The root cause of the issue in requests was fixed (https://github.com/request/request/pull/2904), so I suspect less Ruby apps will run into this. I pivoted this PR to just add a note to the README.md.