Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added CORS support for both HTTP and HTTPS servers. #111

Closed
wants to merge 2 commits into from
Closed

Added CORS support for both HTTP and HTTPS servers. #111

wants to merge 2 commits into from

Conversation

OSSDeveloper
Copy link

@OSSDeveloper OSSDeveloper commented Jun 23, 2017

while creating http server/ https server, opts / sslOpts are already sent as parameters.

add opts['cors'] object to the opts / sslOpts.

opts['cors'] needs the key opts['cors']['allow'] = true

If opts['cors']['allow'] is not sent in specified format, it will just ignore it and work as if no CORS is added.

For now, all requests originating from any source would be allowed.

Ability to filter the requests can be added in future.

while creating http server/ https server, opts / sslOpts are already sent as parameters.

add opts['cors'] object to the opts / sslOpts.

opts['cors'] needs the key opts['cors']['allow'] = true

If opts['cors']['allow'] is not sent in specified format, it will just ignore it and work as if no CORS is added.

For now, all requests originating from any source would be allowed.

Ability to filter the requests can be added in future.
lib/proxy.js Outdated
function allowCORS(objServer, opts){
if(_.isEmpty(opts['cors']) === false && opts['cors']['allow'] && opts['cors']['allow'] === true){
objServer.use(function(req, res, next) {
res.header("Access-Control-Allow-Origin", "*");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

redbird is not implemented using express, so .use is not available. This would need to be implemented directly on nodejs http object.

@manast
Copy link
Member

manast commented Jul 7, 2017

Sorry for the late response. Please check the comment.

@OSSDeveloper
Copy link
Author

my bad... shall do it as required.

while creating http server/ https server, opts / sslOpts are already sent as parameters.

add opts['cors'] object to the opts / sslOpts.

opts['cors'] needs the key opts['cors']['allow'] = true

If opts['cors']['allow'] is not sent in specified format, it will just ignore it and work as if no CORS is added.

For now, all requests originating from any source would be allowed.

Ability to filter the requests can be added in future.
lib/proxy.js Outdated
@@ -781,15 +785,14 @@ function setHttp(link) {

// To allow CORS

function allowCORS(objServer, opts){
function allowCORS(res, opts){
if(_.isEmpty(opts['cors']) === false && opts['cors']['allow'] && opts['cors']['allow'] === true){
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wouldn't be the same to just check if opts.cors is defined? like:

if(opts.cors){
...
}

// To allow CORS

function allowCORS(res, opts){
if(_.isEmpty(opts['cors']) === false && opts['cors']['allow'] && opts['cors']['allow'] === true){
Copy link
Member

@manast manast Jul 10, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Simplfy this expression to just if(opts.cors)

@manast
Copy link
Member

manast commented Aug 18, 2017

@OSSDeveloper yo!. We are very close to merge this one. Would be nice if you could address the feedback :).

@mhemrg
Copy link
Contributor

mhemrg commented Sep 28, 2017

Why not allow each domain to has it's own preferred CORS rules?

@manast
Copy link
Member

manast commented Mar 21, 2020

closing due to lack of response

@manast manast closed this Mar 21, 2020
@dmidlo
Copy link

dmidlo commented Apr 8, 2020

Closed on an afk? sad.

+1 for this.

I'm starting up a project. I'm using redbird. ...and I will use cors, but it isn't a top priority just yet. If this PR isn't picked back up; I might just. :)

Thanks for your work here @manast

@manast
Copy link
Member

manast commented Apr 9, 2020

afk for 2 years is not enough to close a PR?

@sykire
Copy link

sykire commented Jun 12, 2020

bump

@sykire
Copy link

sykire commented Jun 12, 2020

well, we could always fork

ycp424c pushed a commit to ycp424c/redbird that referenced this pull request Jul 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants