-
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: DownloadResponse
cache headers
#9237
base: develop
Are you sure you want to change the base?
Conversation
Good. There is a misunderstanding. ["bodyFormat":protected]=>
string(4) "html"
["filename":"CodeIgniter\HTTP\DownloadResponse":private]=>
string(13) "unit-test.php"
["file":"CodeIgniter\HTTP\DownloadResponse":private]=>
object(CodeIgniter\Files\File)#78353 (4) {
["size":protected]=>
int(12919)
["originalMimeType":protected]=>
NULL
["pathName":"SplFileInfo":private]=>
string(86) "/home/aleksandr/www/codeigniter/development/tests/system/HTTP/DownloadResponseTest.php"
["fileName":"SplFileInfo":private]=>
string(24) "DownloadResponseTest.php"
}
["setMime":"CodeIgniter\HTTP\DownloadResponse":private]=>
bool(false)
["binary":"CodeIgniter\HTTP\DownloadResponse":private]=>
NULL
["charset":"CodeIgniter\HTTP\DownloadResponse":private]=>
string(5) "UTF-8" CodeIgniter4/system/HTTP/ResponseTrait.php Lines 241 to 253 in caf4d53
|
👋 Hi, @michalsn! |
151f622
to
242a300
Compare
But if you're referring to the scenario like this: return $this->response->download($filename, null)->setJSON(['foo' => 'bar']); Which doesn't make much sense, then yes. The Content-Type will be changed to |
@michalsn rebase is needed |
242a300
to
a128dc5
Compare
@samsonasik Thanks, done. |
Description
This PR fixes a bug when we cannot update the
Cache-Control
header for aDownloadResponse
class.By default, the cache is disabled by calling the
noCache()
method in the constructor of the parent class. But now we are able to override these settings simply by using thesetHeader()
orsetCache()
methods.I don't know why previously it was decided that
setCache()
would throw an exception since setting the cache should be possible. In many cases, we probably don't want to do that, but still... it should be possible.Last but not least, the header
Expire-Disposition
(this is the first time I have seen it) was removed since I don't think it's a valid one and has no effect.Ref: #9234
Checklist: