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

Do not store CORS headers in output cache #896

Merged
merged 1 commit into from
Oct 18, 2024

Conversation

gajdusek
Copy link
Contributor

Ensure that that dynamic Access-Control-Allow-Origin header is not cached.

  • Remove Access-Control-Allow-Origin header before saving to cache
  • Add Access-Control-Allow-Origin dynamically when serving cached responses based on the incoming request's Origin

This resolves issue (#895) with CORS headers being cached alongside GraphQL responses, which could cause incorrect Access-Control-Allow-Origin values for clients with different origins.

Copy link

github-actions bot commented Oct 17, 2024

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

@gajdusek
Copy link
Contributor Author

This does not fix the issue entirely. I am experiencing responses with missing Access-Control-Allow-Origin. Going to look into it.

@gajdusek gajdusek force-pushed the fix/output-cache-cors branch from c130c8c to 606d7c8 Compare October 17, 2024 15:54
Ensure that that dynamic Access-Control-Allow-Origin header is not cached.

- Remove Access-Control-Allow-Origin header before saving to cache
- Add Access-Control-Allow-Origin dynamically when serving cached responses
  based on the incoming request's Origin

This resolves issues with CORS headers being cached alongside GraphQL
responses, which could cause incorrect Access-Control-Allow-Origin values
for clients with different origins.
@gajdusek gajdusek force-pushed the fix/output-cache-cors branch from 606d7c8 to 7fb17e2 Compare October 17, 2024 16:27
Copy link

@gajdusek
Copy link
Contributor Author

This does not fix the issue entirely. I am experiencing responses with missing Access-Control-Allow-Origin. Going to look into it.

It's fixed.

@gajdusek
Copy link
Contributor Author

I have read the CLA Document and I hereby sign the CLA

@gajdusek gajdusek changed the title Fix CORS header caching in GraphQL responses Prevent CORS header caching in GraphQL responses Oct 17, 2024
@gajdusek gajdusek changed the title Prevent CORS header caching in GraphQL responses Do not store CORS headers in output cache Oct 17, 2024
@mcop1 mcop1 self-assigned this Oct 18, 2024
@mcop1 mcop1 added the Bug label Oct 18, 2024
@mcop1 mcop1 changed the base branch from 1.x to 1.8 October 18, 2024 10:03
@mcop1 mcop1 changed the base branch from 1.8 to 1.x October 18, 2024 10:03
@mcop1
Copy link
Contributor

mcop1 commented Oct 18, 2024

LGTM, thank you!

@mcop1 mcop1 merged commit 029f58c into pimcore:1.x Oct 18, 2024
10 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Oct 18, 2024
@mcop1 mcop1 added this to the 1.8.3 milestone Oct 18, 2024
@mcop1
Copy link
Contributor

mcop1 commented Oct 18, 2024

Cherry picked to 1.8 -> b71636a

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
2 participants