okTurtles / dnschain

A blockchain-based DNS + HTTP server that fixes HTTPS security, and more!
https://okturtles.com
Other
1.73k stars 169 forks source link

added support for CORS HTTP headers #166

Closed bumi closed 9 years ago

bumi commented 9 years ago

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

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.

bumi commented 9 years ago

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

taoeffect commented 9 years ago

@bumi Yeah, iojs just broke too many things with 3.0. It's actually a change that breaks compatibility with hiredis that's causing it to fail.

It looks like they just recently fixed the issue, however, so bumping the hiredis version in package.json to 0.4.1 should fix it. Maybe try sending a commit with that change? It should fix the build error.

taoeffect commented 9 years ago

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.

bumi commented 9 years ago

hmmm. ja, will try to debug the build error tomorrow. it's not really obvious to me what's going on.

taoeffect commented 9 years ago

will try to debug the build error tomorrow.

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. :smile:

bumi commented 9 years ago

OK, update the PR with your latest comments. it also includes the changes for travis (which is now actually green) please review this commit I've adopted it from the hiredis travis config.

thanks for the support.

taoeffect commented 9 years ago

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! :smile: :+1:

taoeffect commented 9 years ago

It's so funny, we've fixed iojs and broke the other two builds.

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. :)

bumi commented 9 years ago

yay, thanks a lot! (travis was green here: https://travis-ci.org/bumi/dnschain/builds/78244772)