Worse Than Failure. Approximates the unpleasant remark made by Drupal developers when they first encounter a particular (mis)feature.
⚡️ Live updates comments, jobs, and issues, tagged with #DrupalWTF will update issues and activities on this page.

Issues

The last 100 updated issues.

Activities

The last 7 days of comments and CI jobs.

  • 🇪🇸Spain vidorado Logroño (La Rioja)

    Oh, my bad—I mixed things up again...

    Now it’s clear to me: HTTP 1.0 proxies must always be skipped using a past Expires header. So, there’s nothing to fix. Thanks for rephrasing the explanation—it helped me finally grasp the full picture.

    Since the third point is covered by 🐛 [pp-3] Bubbling of elements' max-age to the page's headers and the page cache Postponed , there's nothing else to address. I agree on closing this as "Works as designed" and leaving it as is. 🙂

  • 🇨🇭Switzerland znerol

    I think that the only change this issue fixes, then, is setting an Expires header which honors system_performance.cache.page.max_age, which would be possibly beneficial for HTTP 1.0 proxies down the line of a response.

    No, that wouldn't be desirable. If Drupal did this, then that would lead to potential information leak via HTTP/1.0 caches. This is because HTTP/1.0 caches would store/serve every page, regardless of whether there were cookies on the request or not. HTTP/1.0 caches are unable to differentiate between authenticated requests and anonymous ones simply because they do not know anything about cookies.

    Bring the discussion of max_age config parameter honoring to 🐛 [pp-3] Bubbling of elements' max-age to the page's headers and the page cache Postponed

    Depends. If you mean that no page should be cached in the page cache longer than system_performance.cache.page.max_age, then no. If you mean that page cache should honor the max-age directive collected from render metadata, then yes, that is the correct issue.

    Create another issue to fix the mentioned remaining problem with PageCache inspectiong Expires header instead of Cache-Control, so this one don't get "noisy"?

    No need for that, this is covered by 🐛 [pp-3] Bubbling of elements' max-age to the page's headers and the page cache Postponed .

  • 🇪🇸Spain vidorado Logroño (La Rioja)

    Thanks @znerol for a detailed response. It makes me feel better after have worked in vain, due to misunderstanding the Page Cache role in the whole picture.

    I think that the only change this issue fixes, then, is setting an Expires header which honors system_performance.cache.page.max_age, which would be possibly beneficial for HTTP 1.0 proxies down the line of a response.

    Before:

     protected function setResponseCacheable(Response $response, Request $request) {
        // HTTP/1.0 proxies do not support the Vary header, so prevent any caching
        // by sending an Expires date in the past. HTTP/1.1 clients ignore the
        // Expires header if a Cache-Control: max-age directive is specified (see
        // RFC 2616, section 14.9.3).
        if (!$response->headers->has('Expires')) {
          $this->setExpiresNoCache($response);
        }
    
        $max_age = $this->config->get('cache.page.max_age');
        $response->headers->set('Cache-Control', 'public, max-age=' . $max_age);
    

    After:

     protected function setResponseCacheable(Response $response, Request $request) {
        $max_age = $this->config->get('cache.page.max_age');
        $response->headers->set('Cache-Control', 'public, max-age=' . $max_age);
    
        // If the response does not already have an Expires header, set one.
        if (is_int($max_age) && !$response->headers->has('Expires')) {
          $response->setExpires((new \DateTime())->setTimestamp($this->time->getRequestTime() + $max_age));
        }
    

    Which I believe is not in the scope of the original issue and could fit better as an additional task to 🐛 [pp-3] Bubbling of elements' max-age to the page's headers and the page cache Postponed .

    But, as you said, a problem remains unsolved:

    PageCache determines which responses to cache by inspecting the Expires header. (This should be updated to use the Cache-Control header instead, but that's out of scope here.)

    So, what's next? I'm not sure at all...

    1. Bring the discussion of max_age config parameter honoring to 🐛 [pp-3] Bubbling of elements' max-age to the page's headers and the page cache Postponed
    2. Close this issue as "Works as designed" or leave only the fix that fixes the Expires header and go on to Fixed? I'm not sure if it's beneficial for HTTP 1.0 proxies or if they really exist yet nowadays.
    3. Create another issue to fix the mentioned remaining problem with PageCache inspectiong Expires header instead of Cache-Control, so this one don't get "noisy"?

    Am I right? :)

  • 🇨🇭Switzerland znerol

    Sorry for the late answer, commenting on the issue summary and the two points raised by @Wim Leers:

    1. Drupal 8 intentionally disables HTTP/1.0 proxies

    This has been the case since Drupal 7 (and also Pressflow - Drupal 6 optimized for high performance content oriented websites). It has been necessary for any web application which uses cookies to disable HTTP/1.0 caches. There is no way in HTTP/1.0 to maintain different page variations under the same URL. Everything which influences the result (like desired language, session and login state) has to be encoded in the URL. Yes, session identifiers were passed via request parameters back then.

    When HTTP/1.1 introduced cookies and cache control, it was specified that the Cache-Control: max-age directive takes precedence over the HTTP/1.0 Expires header.

    "Modern" web applications which serve multiple variants of a page under the same URL (depending on the application state maintained in the cookie header) can be made compatible with both HTTP protocol versions by serving a suitable Cache-Control and Expires header. The way to do this is to serve an appropriate Cache-Control header to permit storage in HTTP/1.1 caches (if appropriate) and serve an Expires header which is set into the past in order to disable caching in HTTP/1.0 caches. Remember, HTTP/1.0 caches assume that the only thing which can influence the response is the URL.

    This pattern is not specific to Drupal. Everybody does this.

    Drupal 8 ships with a HTTP/1.0 reverse proxy that is breaking the spec: Page Cache

    Responses that have Expires: Sun, 19 Nov 1978 05:00:00 GMT are meant to not be cached, because it's an expiration date in the past. Yet PageCache caches them permanently:

    The page cache module is not a HTTP/1.0 reverse proxy. It is neither a HTTP/1.1 reverse proxy. The caching logic has its roots in Pressflow and is tailored to content oriented websites.

    In unmodified HTTP/1.1+ reverse proxies and browser caches, the only way to get rid of cached pages is to limit their validity to a certain time frame (i.e., max-age or s-max-age). The system.performance.cache.page.max_age config value is the way to specify that value for those downstream caches which are otherwise out-of-reach for Drupal.

    With the built-in cache the situation is different. It is possible to remove cached pages whenever a change is made on the website. Stock Drupal 7 did nuke the whole page cache whenever any content was changed. Some contrib modules rectified that heavy handed approach and made it possible to define rules which pages were purged depending on which content was changed.

    D8+ introduced cache tags and that removed all the guess work. No site builder has to think about which pages to purge from the cache nowadays.

    For this reason the page cache module keeps rendered pages in the cache for an unspecified amount of time. This has been the way it worked since Pressflow.

    The following remark in the issue summary is indeed pointing to a problem point in the page caching module:

    PageCache determines which responses to cache by inspecting the Expires header. (This should be updated to use the Cache-Control header instead, but that's out of scope here.)

    The page cache module really shouldn't inspect the Expires header. That logic needs to be deprecated indeed.

    Updating issue status to Postponed (maintainer needs more info). I will wait a couple of weeks and then set it to Closed (works as designed) unless there is significant new (and valid) information added to the issue summary.

Production build 0.71.5 2024