Open kurayama opened 7 years ago
Merging #34 into v2.x will not change coverage. The diff coverage is
100%
.
@@ Coverage Diff @@
## v2.x #34 +/- ##
===================================
Coverage 100% 100%
===================================
Files 1 1
Lines 54 54
Branches 19 19
===================================
Hits 54 54
Impacted Files | Coverage Δ | |
---|---|---|
index.js | 100% <100%> (ø) |
:arrow_up: |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact)
,ø = not affected
,? = missing data
Powered by Codecov. Last update a396989...11abb36. Read the comment docs.
@fengmk2, unless I'm missing something, this seems like a security concern as the request will be processed even in the case of CORS not being allowed. Either everyone is aware that there has to be another middleware which tests for the existence of the header and skips route processing or independently of the origin validation the route logic will still execute.
ping @dead-horse
For what it's worth, I think it would more correct for this feature to be optional and to use status 403 instead of 404.
The CORS protocol standard says (emphasis mine):
In case a server does not wish to participate in the CORS protocol, its HTTP response to the CORS or CORS-preflight request must not include any of the above headers. The server is encouraged to use the 403 status in such HTTP responses.
The first part describes the current behavior. It seems that this PR address the second par (in bold). The wording "is encouraged" suggest that it's optional, i.e. not required for the CORS protocol to function correctly.
Then again I'm not really sure "Skip request if origin is not allowed" relates to "In case a server does not wish to participate in the CORS protocol".
@ruimarinho
unless I'm missing something, this seems like a security concern as the request will be processed even in the case of CORS not being allowed.
Actually a browser that uses the CORS protocol will always use a CORS-preflight request (OPTIONS) before attempting a non GET/HEAD request (i.e. requests with side effects), and will abort if the response headers don't allow it to proceed (in which case the POST/PUT/DELETE request is not sent). For GET and HEAD, the actual request is sent without any prior CORS-preflight request because they should only retrieve data and have no side effects; the browser will still block the response content from being accessed by the requester if the server doesn't allow it.
So in both case it is safe even without skipping anything. As long as the server respects HTTP methods semantics.
Ultimately, CORS relies on browser compliance and isn't meant to replace request authorisation; it's just a safe and standard way to break free of the strict same-origin policy implemented by browsers.
So in both case it is safe even without skipping anything. As long as the server respects HTTP methods semantics.
It's still a waste of resources processing GET/HEAD requests if the origin is invalid and the browser won't be displaying the response anyway.
Requests are currently going through even if the origin is not allowed.