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

disable php cache control #30178

Closed
wants to merge 1 commit into from
Closed

disable php cache control #30178

wants to merge 1 commit into from

Conversation

butonic
Copy link
Member

@butonic butonic commented Jan 17, 2018

Had to prevent PHP from sending cache control headers. Otherwise the dav previews from #29319 always had to reload the previews because Chrome respects the Pragma:nocache header that is sent by default.

Related: #26922 (comment)

Had to prevent PHP from sending cache control headers. Otherwise the dav previews from #29319 always had to reload the previews because Chrome respects the Pragma:nocache header that is sent by default.

using 0 instead of emptystring, otherwise we get a `.htaccess: php_value takes two arguments, PHP Value Modifier` error.
@@ -43,6 +43,7 @@
php_value mbstring.func_overload 0
php_value default_charset 'UTF-8'
php_value output_buffering 0
php_value session.cache_limiter 0
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes but php_value session.cache_limiter '' leads to a 500 status and .htaccess: php_value takes two arguments, PHP Value Modifier in the apache error log. 0 seems to work

@DeepDiver1975
Copy link
Member

what about environments where htaccess is not in place?
should we add this to base.php?

@butonic butonic mentioned this pull request Jan 17, 2018
13 tasks
@butonic
Copy link
Member Author

butonic commented Jan 17, 2018

maybe base.php is a better place ... all app framework routes already handle cache control headers

@mmattel
Copy link
Contributor

mmattel commented Jun 11, 2018

@butonic any update on this ?

@PVince81
Copy link
Contributor

PVince81 commented Feb 8, 2019

=> base.php or Webdav plugin layer ? @DeepDiver1975 @butonic

decide and then we can take over and reassign

@PVince81 PVince81 added this to the backlog milestone Feb 8, 2019
@butonic
Copy link
Member Author

butonic commented Feb 11, 2019

To clarify, the php docs on session_cache_limiter say

Setting the cache limiter to '' will turn off automatic sending of cache headers entirely.

In general, we are managing the cache control headers ourself in the app framework Response https://github.com/owncloud/core/blob/v10.1.0/lib/public/AppFramework/Http/Response.php#L49 or any derived class. So disabling cache management in php is the right thing to do, because it might otherwise cause unwanted caching.

However, setting it to '' in the .htaccess produces a 500 error, so I tried 0 and it worked. It seems Drupal is using none to do the same thing. Maybe ^$ works as expected but might be even more cryptic.

I vote to disable php cache control in the base.php. It least we can then properly set it to ''.

@PVince81
Copy link
Contributor

raised as #34441

if there is no particular criticality to this, the ticket will remain in backlog for later

@PVince81 PVince81 closed this Feb 11, 2019
@PVince81 PVince81 deleted the disable-php-cache-control branch February 11, 2019 14:41
@lock lock bot locked as resolved and limited conversation to collaborators Feb 12, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants