Comments (18)
Wow, time flies. I actually got the code for this done pretty quick, but ran out of gas re-working all the tests. Plugging away...
from cors.
Yes, if this is operating contary to the CORS spec, then it should be changed. If the CORS spec says the way this works now it should not change. If the CORS spec does not say either way, add it as an optional behavior change.
I haven't really studied how this module is working specifically, which is what I was going to look into. Just back up the pull request with a reference to what the spec says is all.
I hope that makes sense.
from cors.
Thanks for the bump y'all. Will revisit this week. Again, the delay was in getting all the tests in order.
from cors.
Hi @sgress454 thanks for this long post :) ! I am helping out on this module now and going through the issues. I need to get more familiar with the implementation here to provide insight, but the general answer is any deviation from the W3C CORS spec is a bug :)
from cors.
@dougwilson any update on this?
from cors.
I have not had time to dig in much. If you have any insight, that would be awesome!
from cors.
Hey @dougwilson sure, we noticed line 67 of lib/index.js (master) has a conditional to check if the options.origin parameter is a string. However the isOriginAllowed method seemingly does the same check (line 27). ideally as I understand the spec indicates not to return the origin header at all. which you do by setting it false. However it would require verification.
else if (isString(options.origin)) {
// fixed origin
headers.push([{
key: 'Access-Control-Allow-Origin',
value: options.origin
}]);
headers.push([{
key: 'Vary',
value: 'Origin'
}]);
} else {
isAllowed = isOriginAllowed(requestOrigin, options.origin);
console.log('isAllowed', isAllowed);
// reflect origin
headers.push([{
key: 'Access-Control-Allow-Origin',
value: isAllowed ? requestOrigin : false
}]);
headers.push([{
key: 'Vary',
value: 'Origin'
}]);
}
function isOriginAllowed(origin, allowedOrigin) {
if (Array.isArray(allowedOrigin)) {
for (var i = 0; i < allowedOrigin.length; ++i) {
if (isOriginAllowed(origin, allowedOrigin[i])) {
return true;
}
}
return false;
} else if (isString(allowedOrigin)) {
return origin === allowedOrigin;
} else if (allowedOrigin instanceof RegExp) {
return allowedOrigin.test(origin);
} else {
return !!allowedOrigin;
}
}
from cors.
which you do by setting it false
Sorry, I'm not sure if you really followed the thread above; I didn't write the original code, so replying to me with "you do ..." when you're taking about the code makes it sound like (a) I did and (b) I am fully aware of how it works. The reason I'm asking for help is because I haven't fully looked over the code as I said.
I would suggest maybe helping by putting together a pull request. I'm not really following what you're saying, perhaps because there is an assumption here that I understand the code within the module fully, which is why I'm just trying to re-iterate that I don't :) I can try to put together a fix if you can be more specific on what needs to change where in the code. I see you pointed to a line, but then I don't really follow the rest of what you're saying needs to be changed if you can help out there.
from cors.
@dougwilson I'll make a PR out of the patched version we've been using in Sails; the massive post above was meant to elicit feedback from the community as to whether my expectations for CORS headers were correct. It sounds like at least one person thinks so :P
from cors.
I noticed there are a few more problems such as returning the access-control-allow-credentials even when the request is not a valid origin. Also not case sensitive.
from cors.
@dougwilson Actually, not sure how I missed this in the CORS doc before (other than my eyes glazing over every time I look at it), but this section clearly states that if no origin
header is present in the request, or if the header doesn't match the whitelist, then the server should not set any additional headers in the response. Same goes for failed checks in the preflight. I'll link to that section in the PR.
from cors.
Yea, @sgress454 , that was reported in #114 though the user declined to make a pull request.
from cors.
Alrighty, working on it.
from cors.
I've come across these findings as well. Any updates @sgress454 ?
from cors.
I have come across it as well, would be great to have an update @sgress454
from cors.
I'm about to choose a cors library. This seems to be a go-to lib. But I'm a little weary due to this issue being open for three years.
Is this "leak" a verified problem?
Is is a very bad thing?
If it were very bad I'd hope more people would have chimed in at this issue.
from cors.
I don't see the rationale for exposing the criteria for acceptance to a requestor that doesn't meet those criteria. For example, if you had an admin app on some subdomain ("xyzadmin.myapp.com") that you didn't want to be made public, and you have your main app ("myapp.com") accepting cross-origin requests only from that domain, it doesn't seem right that by making an AJAX request to myapp.com from any random domain you could get the Access-Control-Allow-Origin header with the value xyzadmin.myapp.com and thus learn about that existence of the admin server.
The current spec for CORS can be found in the Fetch standard, which is less prescriptive than the original W3C standard was about how servers ought to implement CORS.
The last paragraph in https://fetch.spec.whatwg.org/#cors-protocol-and-http-caches gives one use case for unconditionally including Access-Control-Allow-Origin
in responses:
if
Access-Control-Allow-Origin
is set to*
or a static origin for a particular resource, then configure the server to always sendAccess-Control-Allow-Origin
in responses for the resource — for non-CORS requests as well as CORS requests — and do not useVary
.
from cors.
Related: whatwg/fetch#1588
from cors.
Related Issues (20)
- No Configuration Options for Access-Control-Allow-Private-Network HOT 1
- CORS Error only on Mac HOT 2
- Cors origin RegExp issues HOT 10
- Option preflightContinue not working with origin function
- Array - set origin -Not working HOT 3
- Incorrect response when option origin is true and requestOrigin is undefined HOT 2
- "origin" is undefined when requests are received from the same server AND when malicious requests are sent from a program HOT 1
- Undefined origin should be treated as not allowed - discusson HOT 4
- Configure Allowed Headers as Array of RegExp
- DEMO is broken HOT 1
- Invalid Vary header in Access-Control-Allow-Headers HOT 2
- `OPTIONS` request handler missing `Allow` header HOT 13
- cors is hanging HOT 2
- CORS error when fonts
- Add support for having specified domain instead of wildcard HOT 3
- Request: callback for failed CORS HOT 5
- Cors error when connecting through ssh tunnel HOT 1
- I have random 'Access-Control-Allow-Origin' errors, even if i set origin: '*', is my usage correct ? HOT 3
- Add ability to omit `Vary: Origin` header HOT 3
- Improve testing robustness HOT 2
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from cors.