added support for CORS HTTP headers #166
Conversation
Sets the Cross-Origin Resource Sharing (CORS) headers in a global express middleware. This option is configurable and disabled by default. To enable CORS use the following config entry: [http] cors: true
|
I think the travis error is related to this: https://twitter.com/travisci/status/637183387669778432 - so should be fixed when travis solved the ios problem |
|
@bumi Yeah, iojs just broke too many things with 3.0. It's actually a change that breaks compatibility with It looks like they just recently fixed the issue, however, so bumping the hiredis version in |
|
Odd that it's still erroring, I mentioned it to the hiredis team. I will review your PR ASAP and get back to you on the rest. |
| @@ -41,7 +41,7 @@ | |||
| "bottleneck": "1.5.x", | |||
| "event-stream": "3.2.2", | |||
| "express": "4.11.2", | |||
| "hiredis": "0.4.0", | |||
| "hiredis": "~0.4.1", | |||
There was a problem hiding this comment.
This should not include the tilde. We've had issues where DNSChain failed to work on future installs because of an updated library, so we want to stick to precise version numbers.
There was a problem hiding this comment.
OK, do you still want that change in this PR?
saw the hiredis-node comment on gcc.
There was a problem hiding this comment.
Yes, please remove the tilde, it seems like they did fix it and we just need to update our .travis.yml to tell Travis to install g++-4.8. If you want to add these lines yourself as part of the PR you're welcome to, otherwise I'll do later on when I get a chance.
Also, don't forget to update the warning message (see comment below).
There was a problem hiding this comment.
(Oh, and if you decide to update the .travis.yml file, you should also add these lines as well to force the compiler version.)
|
hmmm. ja, will try to debug the build error tomorrow. it's not really obvious to me what's going on. |
You don't need to do anything about that, don't worry. The build works on nodejs 0.10 and 0.12. That's good enough for me. This is a bug in the hiredis library due to broken compatibility with iojs 3. I'm done with my review. You should also update the README to add your name to the list of Contributors. 😄 |
this is a requirement by the hiredis-node dependency which otherwise fails on iojs. also fixed hiredis version to the latest 0.4.1 release
as per request :)
|
OK, update the PR with your latest comments. thanks for the support. |
|
Great job @bumi! It's so funny, we've fixed iojs and broke the other two builds. I can't wait until the two projects merge, then I can have Travis just target one node platform. (Maybe they already have, I haven't been keeping up.) I'm going to merge this and deal with Travis later. Of course, you or anyone else is welcome to help with that, as at the moment my attention is somewhat divided and focused in other areas. But I'm adding releasing a minor point update to my near-term TODO. Thanks so much for your great work with this! 😄 👍 |
added support for CORS HTTP headers
Weird! After I merged it Travis appears fixed! No idea why it showed your PR as broken but worked after merge, but am glad that's the case, one less thing to fix. :) |
|
yay, thanks a lot! |
Sets the Cross-Origin Resource Sharing (CORS) headers in a global express middleware.
This option is configurable and disabled by default.
To enable CORS use the following config entry:
See #162 for original PR discussion about enabling CORS. This PR makes CORS configurable and disables CORS by default.
I have a test that checks if CORS is disabled (having only the default configuration). But I am not sure how to change a global option on the fly within a test that affects the server bootup. - if needed.