-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
Some headers were missing when serving from cache #6955
Conversation
hello @Chraneco Thank you for your contribution. Please provide clear test instructions to be able to test / reproduce this issue. Thanks for understanding! This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/6955. |
Yes, I added instructions above. Unfortunately, it doesn't seem to be possible to add extensions archives as attachments here, so I added an issue on Joomlacode and attached the test component there: This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/6955. |
@Chraneco I tried the component but initially the image wouldn't show. I do get the link but when I click it I get an empty page... Hope you can fix the component. You can mail me component directly: rick@r2h.nl |
@RickR2H Sorry, the sample image in Joomla I was using was apparently deleted in one of the latest versions. I updated the component with a path to another image and sent it to your email address. If it still doesn't work please check whether the image Thanks for testing! |
Patch works! Thanks @Chraneco for putting all the effort in making a component to test this PR. This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/6955. |
Works here! Thanks. This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/6955. |
RTC based on tests by @RickR2H and @RichardR2H. This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/6955. |
Some headers were missing when serving from cache
The setting of headers like 'Content-Type' is currently happening in method
respond
of classJApplicationWeb
.Unfortunately, the system cache plugin stores contents at event
onAfterRender
which is triggered beforerespond
is executed. Therefore, certain headers are missing when serving from the cache.This creates problems especially if we are serving content that is not of Content-Type
text/html
.This pull request changes the plugin so that the cache is created only after these headers are set by using event
onAfterRespond
instead ofonAfterRender
.Since gzip compression already happened at this point, the compressed contents are stored in the cache and therefore no further compression is necessary when serving cached contents. Thus, the second change in the plugin file.
For testing please make sure that everything works as before and check that the
Content-Type
header is also cached now.Testing Instructions