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.

Sign up for GitHub

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

Jump to bottom

feat: add disableCORS option to server options #35

Open
wants to merge 6 commits into
base: dita
Choose a base branch
from
Open

feat: add disableCORS option to server options #35

wants to merge 6 commits into from

Conversation

paganellif
Copy link

@paganellif paganellif commented May 16, 2024 •
edited
Loading

This feature allows you to disable CORS via the server options

Closes #34

@paganellif paganellif mentioned this pull request May 16, 2024
Open
@danielweck
Copy link
Contributor

danielweck commented May 16, 2024 •
edited
Loading

Wholesale disabling of CORS may have its uses, but I think it would be a lot more useful if consumers of this package were able to set the "allow origins" (for example) as a wildcard asterisk * typically gets rejected by HTTP clients with "credentials" (for example).

dita-streamer-js/src/http/server.ts

Lines 297 to 311 in ff2a502

public setResponseCORS(res: express.Response) {
res.setHeader("Access-Control-Allow-Origin",
"*");
res.setHeader("Access-Control-Allow-Methods",
"GET, HEAD, OPTIONS"); // POST, DELETE, PUT, PATCH
res.setHeader("Access-Control-Allow-Headers",
// tslint:disable-next-line:max-line-length
"Content-Type, Content-Length, Accept-Ranges, Content-Range, Range, Link, Transfer-Encoding, X-Requested-With, Authorization, Accept, Origin, User-Agent, DNT, Cache-Control, Keep-Alive, If-Modified-Since");
res.setHeader("Access-Control-Expose-Headers",
// tslint:disable-next-line:max-line-length
"Content-Type, Content-Length, Accept-Ranges, Content-Range, Range, Link, Transfer-Encoding, X-Requested-With, Authorization, Accept, Origin, User-Agent, DNT, Cache-Control, Keep-Alive, If-Modified-Since");
}

@paganellif
Copy link
Author

I agree with you, but I think both are a useful feature.
CORS could perhaps be handled better using the CORS package as indicated in the express documentation.
If you agree I can refactor it with these changes.

@paganellif
Copy link
Author

With the latest changes, it works behind an API gateway with CORS enabled and allowedOrigins defined in the serverOptions. I've also updated the custom route methods (i.e. expressUse and expressGet) and the example in the README that shows how to enable CORS in these cases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add enableCORS option to IServerOptions to enable/disable CORS
2 participants
@paganellif @danielweck