-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
fix: duplicate Cache-Control header with Session #8601
fix: duplicate Cache-Control header with Session #8601
Conversation
Co-authored-by: John Paul E. Balandan, CPA <[email protected]>
@kenjis Excuse my ignorance, but wouldn't it be better not to create this 'Cache-Control' header in the first place instead of creating it and removing it afterwards and thus adding a (maybe) unnecessary overhead to the framework? The ResponseTrait has $this->removeHeader('Cache-Control');
$this->setHeader('Cache-Control', ['no-store', 'max-age=0', 'no-cache']); It seems to me that the first line does not remove anything and the second line is adding the headers we want to remove while using sessions... What do you think? |
I don't know why, but the behavior is intentional. It is not a bug.
So we cannot change it as a bug fix. Do you propose to remove the default cache control header? CodeIgniter4/system/HTTP/ResponseTrait.php Lines 344 to 350 in a99787c
It is the code in the public
I think it is better to remove the default cache control header. That is, to remove these lines: CodeIgniter4/system/HTTP/Response.php Lines 152 to 154 in a99787c
Because I think it is better that devs have full control to cache control header, and it is simpler. But it is a breaking change, so we need more consideration, and need to do it in 4.5 branch, if we do it.
|
@kenjis The issue here is marked as fixed but I am still getting duplicate header when the session enabled globally and I have been trying to cache response of an API request, if I call this function Should I extend different controller for the API requests which doesn't start the session? What should be the HTTP response code if the request cached? I believe it should be Framework version - 4.5.3 |
@vikaskhunteta What do you mean by "when the session enabled globally"? Read the code: https://github.com/codeigniter4/CodeIgniter4/pull/8601/files |
@kenjis I mean I have enabled the session through You can clearly see in the previous comment screenshot that I have been getting two |
If you use Shield, Shield may enable Session before your controller. |
@kenjis But there should be no session enable before the login, btw where should I look this? Is there any impact on the headers due to site environment? |
We use GitHub issues to track BUGS and to track approved DEVELOPMENT work packages. We use our forum to provide SUPPORT and to discuss FEATURE REQUESTS. |
Description
Fixes #7266
Cache-Control
header before the Session starts.Before:
After:
Checklist: