Skip to content

Conversation

@worace
Copy link

@worace worace commented Oct 14, 2022

No description provided.

@worace worace force-pushed the worace/cors-support branch from d34ed1d to 8a74fcc Compare October 14, 2022 21:17
@worace
Copy link
Author

worace commented Oct 14, 2022

#62

@worace
Copy link
Author

worace commented Oct 14, 2022

Example before/after:

$ curl -I -X OPTIONS http://localhost/user/horace/wof_neighborhoods.fgb
HTTP/1.1 405 Not Allowed
Server: nginx
Date: Fri, 14 Oct 2022 21:27:09 GMT
Content-Type: text/html
Content-Length: 150
Connection: keep-alive
Allow: GET, HEAD

$ curl -I -X OPTIONS http://localhost/user/horace/wof_neighborhoods.fgb
HTTP/1.1 204 No Content
Server: nginx
Date: Fri, 14 Oct 2022 21:27:23 GMT
Connection: keep-alive
Access-Control-Allow-Origin: *
Access-Control-Allow-Methods: GET, POST, OPTIONS
Access-Control-Allow-Headers: DNT,User-Agent,X-Requested-With,If-Modified-Since,Cache-Control,Content-Type,Range
Access-Control-Max-Age: 1728000
Content-Length: 0

@dekobon
Copy link
Collaborator

dekobon commented Oct 17, 2022

Thank you for the PR! I see it is still a draft, so you must be still iterating. All in all this looks, good. I do wonder if it is possible to add an integration test that makes sure that CORS is working as per expectation when enabled.

@worace worace marked this pull request as ready for review October 19, 2022 02:23
@dav-pascual
Copy link
Contributor

dav-pascual commented Nov 25, 2022

These changes look solid to me!
Is there any progress on this review? @dekobon
This would be a very nice feature to have , so using the stubs wouldn't be necessary :)

@dekobon
Copy link
Collaborator

dekobon commented Nov 29, 2022

Hi @dav-pascual and @worace my apologies for the late response. After a lot of thinking, I'd like to implement this in the nginx configuration rather than co-locate it in the AWS/S3 specific JS file.

I've created this PR and I would like your review and testing on it if possible.

@worace
Copy link
Author

worace commented Nov 30, 2022

@dekobon I think that sounds like a great approach; probably more similar to what other users with prior familiarity with nginx would expect

@worace worace closed this Nov 30, 2022
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.

3 participants