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

No Cors headers present. #37

Open
BlueSeph28 opened this issue Feb 4, 2020 · 2 comments
Open

No Cors headers present. #37

BlueSeph28 opened this issue Feb 4, 2020 · 2 comments
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed pri-high

Comments

@BlueSeph28
Copy link

Expected behavior:

Serve static Files with cors enabled. No cors headers present.

Actual behavior:

Cors error:

Access to XMLHttpRequest at 'https://[URL]' from origin 'http://localhost:8080' has been blocked by CORS policy: No 'Access-Control-Allow-Origin' header is present on the requested resource.

Steps to reproduce the problem:

Serve the static file:

event.path = file;
Return fileHandler.get(event, context);

Bonus Workaround

Adding the headers it worked but I need to access to the response object.

event.path = file;
let response = await fileHandler.get(event, context)
response.headers = {
   'Content-Type': response.headers["Content-Type"],
   'Access-Control-Allow-Methods': '*',
   'Access-Control-Allow-Origin': '*'
}
return response;

Environment:

OS: Debian 10 Buster

Node Version: 10.15.3

serverless-aws-static-file-handler Version: 2.0.4

@tomchiverton
Copy link
Contributor

@activescott why doesn't the plugin always add these ?

@activescott
Copy link
Owner

@tomchiverton It's a fair question. I don't have a great answer other than I never ran into it. I think I didn't pay much attention to this one honestly since there was a workaround. Now that a second person has inquired, I'm more keen to get something ironed out.

I don't see the harm in doing it by default so as long as it meets the following criteria:

  1. Responds with CORS headers only for GET or OPTIONS requests (as other requests could be state changing by the outer function - unlikely but certainly possible and I noticed the current code never actually checks request method, which kinda stinks now that I think about it).
  2. Access-Control-Allow-Methods only returns the values GET, OPTIONS
  3. There is a way for the calling function to opt out (e.g. another optional constructor param).

Am I missing anything or overthinking it?
If not, you want to send a PR (with tests) or should I prepare one?

Although not technically an API-breaking change, I do wonder if it is a significant enough behavior change (especially since it arguably involves security) to be a MAJOR version bump to prevent inadvertent upgrades. What do you think?

@activescott activescott added enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed pri-high labels Oct 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed pri-high
Projects
None yet
Development

No branches or pull requests

3 participants