Update HTTP headers #1644
Merged
Update HTTP headers #1644
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
nbulaj
approved these changes
Mar 14, 2023
There was a problem hiding this comment.
LGTM 👍
For the record: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Pragma
|
@marjanovic93 could you please add a changelog entry? |
|
@nbulaj sure, updated! Back to you 🏓 |
|
I see some tests are failling, could you please check? And also squash all the commints into a single one later 🙏 Thanks! |
Pragma is a deprecated HTTP header. It changes from Cache-Control: no-store to Cache-Control: no-store, no-cache to Doorkeeper.
|
Sure @nbulaj 🚀 It should be fixed, your turn! 🏓 |
zubin
reviewed
Apr 23, 2023
| @@ -15,8 +15,7 @@ | |||
| it "respond with correct headers" do | |||
| post token_endpoint_url(code: @authorization.token, client: @client) | |||
|
|
|||
| expect(headers["Pragma"]).to eq("no-cache") | |||
| expect(headers["Cache-Control"]).to be_in(["no-store", "private, no-store"]) | |||
| expect(headers["Cache-Control"]).to be_in(["no-store", "no-cache, no-store", "private, no-store"]) | |||
There was a problem hiding this comment.
Bit late here but, given that Pragma: no-cache is always present here, would the equivalent not be:
expect(headers["Cache-Control"]).to be_in(["no-store, no-cache", "private, no-store, no-cache"])
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Pragma is a deprecated HTTP header. It changes from Cache-Control: no-store to Cache-Control: no-store, no-cache to Doorkeeper.
Summary
Those headers are set by Doorkeeper. We have:
which are set in https://github.com/doorkeeper-gem/doorkeeper/search?q=no-cache
Pragma is a deprecated HTTP header and Atlassian's suggestion is what is favored now so we should contribute the simple change from
Cache-Control: no-storetoCache-Control: no-store, no-cacheto Doorkeeper and pick that up in a gem update.Other Information
If there's anything else that's important and relevant to your pull
request, mention that information here. This could include
benchmarks, or other information.
If you are updating CHANGELOG.md file or are asked to update it by reviewers,
please add the changelog entry at the top of the file.
Thanks for contributing to Doorkeeper project!