When Piwik is running behind a reverse proxy (e.g. for load-balancing), hits to
piwik.php may be cached in the proxy.
This has two problems:
This PR adds adds a Cache-Control directive that prevents any caching of the request. For POST requests ~and 204 requests~, caching is already prohibited according to the HTTP spec (unless explicitly permitted by Cache-Control headers), so I do not send the header for these requests in order to save bytes over the wire.
Even though 204 responses are not cacheable without an explicit Cache-Control header, according to RFC 7231, section 6.1, the default Varnish configuration does not respect this, so
Cache-Control: private is also sent for 204 responses.
Thanks for the PR @c960657 :+1:
Before we can merge, could you please add some test(s) in tests/PHPUnit/System/TrackerResponseTest.php to ensure we don't regress this in the future?
I have added some tests.
Instead of using
assertHttpResponseText() that each make an HTTP requests, I used
Http::sendHttpRequest() to make one request that is then used for several assertions using regular
assertHttpResponseText() and the corresponding PHPUnit constraint classes are not used very much in Matomo core, and they don't seem to add much value over just asserting directly on the HTTP response object returned by
Http::sendHttpRequest(), so I have marked them as
<a class='mention' href='https://github.com/deprecated'>@deprecated</a>. I hope this makes sense.
I changed it to use
no-store instead of
no-store seems more appropriate, see e.g. this.
Code looks good to me.
FYI @tsteur this PR deprecates some custom asserts, requires your approval. (Don't think they're actually that useful, but just want to make sure no one else finds them useful.)