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

Fix dynamic cors setting #63

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

reem
Copy link
Collaborator

@reem reem commented May 4, 2017

Unfortunately I missed some cases in my last PR! I added new tests
to ensure that cors headers are correctly sent in all cases and
refactored the internal options interface to ensure this sort of
error is now impossible.

Also included is a change to npm test to ensure that it always
runs against the latest coffeescript code.

reem added 2 commits May 3, 2017 19:44
…cases

In my previous attempt here I missed settings the 'Access-Control-Allow-Origin'
headers in some cases. I've refactored the options interface to ensure that
it is sent in all cases, and added further tests to ensure it is.
@josephg
Copy link
Owner

josephg commented May 4, 2017

I'm not using this library for anything (I moved my projects to websockets a few years ago and haven't looked back). Do you want commit & npm publish access so you can fix this stuff yourself?

@reem
Copy link
Collaborator Author

reem commented May 4, 2017

Yeah that would be great, we are still using this project.

EDIT: my npm username is also reem

@josephg
Copy link
Owner

josephg commented May 5, 2017

Great. invite sent, and npm owner added. Merge at your leasure.

node-browserchannel josephg$ npm owner add reem
+ reem (browserchannel)

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.

2 participants