Issue #2911747 by stevector: Limit header size to avoid 502

.circleci/config.yml

@@ -4,7 +4,7 @@ jobs:
     # @todo, separate build and test phases.
     build:
         docker:
-            - image: stevector/pantheon_advanced_page_cache
+            - image: quay.io/pantheon-public/build-tools-ci:1.x
         working_directory: ~/pantheon_advanced_page_cache
         environment:
             BASH_ENV: ~/.bashrc
@@ -33,7 +33,7 @@ jobs:
                 name: login-pantheon
                 command: |
                   terminus auth:login -n --machine-token="$TERMINUS_TOKEN"
-                  terminus env:list --field=id  $TERMINUS_SITE | grep -Eo '[0-9]{1,9}' | sort -k1n | sed '1!G;h;$!d' | sed 1,7d | xargs -n1 -I ENV terminus env:delete --yes $TERMINUS_SITE.ENV
+                  terminus env:list --field=id $TERMINUS_SITE | grep -v '[a-z]' | grep -Eo '[0-9]{1,9}' | sort --numeric-sort --reverse | sed 1,7d | xargs -n1 -I ENV terminus env:delete --yes $TERMINUS_SITE.ENV
             - run:
                 name: make-multidev
                 command: |

README.md

@@ -10,7 +10,7 @@ This module has no configuration settings of its own, just enable it and it will
 
 If you want to take finer grain control of how Drupal is handling it's cache data (in ways that will interact with both the Global CDN and internal Drupal caches) consider using [Views Custom Cache Tags](https://www.drupal.org/project/views_custom_cache_tag) and [Cache Control Override](https://www.drupal.org/project/cache_control_override).
 
-### Debugging
+## Debugging
 
 By default, Pantheon's infrastructure strips out the `Surrogate-Key` response header before responses are served to clients. The contents of this header can be viewed as `Surrogate-Key-Raw` by adding on a debugging header to the request.
 
@@ -22,6 +22,12 @@ A direct way of inspecting headers is with `curl -I`. This command will make a r
 
 `curl -IH "Pantheon-Debug:1" https://dev-cache-tags-demo.pantheonsite.io/ | grep -i Surrogate-Key-Raw`
 
-### Feedback and collaboration
+## Limit on header size
+
+Pantheon's nginx configuration limits total header size to 32k.
+This module caps the `Surrogate-Key` at 25,000 bytes to minimize the chances that a very long `Surrogate-Key` header combines with other long headers to trigger a 502 error.
+This limit can be reached if your site renders thousands of entities in a single response.
+You will see warning messages in your log directing you to [the issue queue](https://www.drupal.org/project/pantheon_advanced_page_cache/issues/2973861) if this limit is reached.
+## Feedback and collaboration
 
 For real time discussion of the module find Pantheon developers in our [Power Users Slack channel](https://pantheon.io/docs/power-users/). Bug reports and feature requests should be posted in [the drupal.org issue queue.](https://www.drupal.org/project/issues/pantheon_advanced_page_cache?categories=All) For code changes, please submit pull requests against the [GitHub repository](https://github.com/pantheon-systems/pantheon_advanced_page_cache) rather than posting patches to drupal.org.

composer.json

@@ -9,8 +9,15 @@
     "scripts": {
       "codesniff": [
           "phpcs --report=full --extensions=php,module,inc,theme,info,install --standard=vendor/drupal/coder/coder_sniffer/Drupal src",
-          "phpcs  tests  --standard=PSR2"
+          "phpcs --report=full --extensions=php,module,inc,theme,info,install --standard=vendor/drupal/coder/coder_sniffer/Drupal tests/modules",
+          "phpcs  tests/behat  --standard=PSR2"
       ],
+      "phpcbf": [
+          "phpcbf --report=full --extensions=php,module,inc,theme,info,install --standard=vendor/drupal/coder/coder_sniffer/Drupal src",
+          "phpcbf --report=full --extensions=php,module,inc,theme,info,install --standard=vendor/drupal/coder/coder_sniffer/Drupal tests/modules",
+          "phpcbf  tests/behat  --standard=PSR2"
+      ],
+
       "phpunit": "phpunit tests --colors=always"
     },
     "repositories": {

pantheon_advanced_page_cache.services.yml

@@ -1,9 +1,13 @@
 services:
+  logger.channel.pantheon_advanced_page_cache:
+    parent: logger.channel_base
+    arguments: ['pantheon_advanced_page_cache']
   pantheon_advanced_page_cache.cache_tags.invalidator:
     class: Drupal\pantheon_advanced_page_cache\CacheTagsInvalidator
     tags:
       - { name: cache_tags_invalidator }
   pantheon_advanced_page_cache.cacheable_response_subscriber:
     class: Drupal\pantheon_advanced_page_cache\EventSubscriber\CacheableResponseSubscriber
+    arguments: ['@logger.channel.pantheon_advanced_page_cache']
     tags:
       - { name: event_subscriber }

src/EventSubscriber/CacheableResponseSubscriber.php

@@ -6,12 +6,31 @@
 use Symfony\Component\HttpKernel\Event\FilterResponseEvent;
 use Symfony\Component\HttpKernel\KernelEvents;
 use Symfony\Component\EventDispatcher\EventSubscriberInterface;
+use Psr\Log\LoggerInterface;
+use Drupal\Core\Logger\RfcLogLevel;
 
 /**
  * Adds Surrogate-Key header to cacheable master responses.
  */
 class CacheableResponseSubscriber implements EventSubscriberInterface {
 
+  /**
+   * The logger instance.
+   *
+   * @var \Psr\Log\LoggerInterface
+   */
+  protected $logger;
+
+  /**
+   * Constructs a new DefaultExceptionHtmlSubscriber.
+   *
+   * @param \Psr\Log\LoggerInterface $logger
+   *   The logger service.
+   */
+  public function __construct(LoggerInterface $logger) {
+    $this->logger = $logger;
+  }
+
   /**
    * Adds Surrogate-Key header to cacheable master responses.
    *
@@ -34,7 +53,15 @@ public function onRespond(FilterResponseEvent $event) {
         $tags[$key] = str_replace('_list', '_emit_list', $tag);
       }
 
-      $response->headers->set('Surrogate-Key', implode(' ', $tags));
+      $tags_string = implode(' ', $tags);
+      if (25000 < strlen($tags_string)) {
+        $tags_string = substr($tags_string, 0, 25000);
+        // The string might have cut of in the middle of a tag.
+        // So now find the the last occurence of a space and cut to that length.
+        $tags_string = substr($tags_string, 0, strrpos($tags_string, ' '));
+        $this->logger->log(RfcLogLevel::WARNING, 'More cache tags were present than could be passed in the Surrogate-Key HTTP Header due to length constraints. To avoid a 502 error the list of surrogate keys was trimmed to a maximum length of 25,000 bytes. Since keys beyond the 25,000 maximum were removed this page will not be cleared from the cache when any of the removed keys are cleared (usually by entity save operations) as they have been stripped from the surrogate key header. See https://www.drupal.org/project/pantheon_advanced_page_cache/issues/2973861 for more information about how you can filter out redundant or unnecessary cache metadata.');
+      }
+      $response->headers->set('Surrogate-Key', $tags_string);
     }
   }
 

tests/behat/features/huge_header_warning.feature

@@ -0,0 +1,27 @@
+Feature: Huge Header Warning
+In order to understand the caching behavior of my site
+As an administrator
+I need a notification when my header is truncated.
+
+  @api
+  Scenario: Warning message.
+    Given I run drush "pm-uninstall -y pantheon_advanced_page_cache_test"
+    Given I run drush "en -y pantheon_advanced_page_cache"
+    And I am logged in as a user with the "administrator" role
+
+    And there are 10 article nodes with a huge number of taxonomy terms each
+    When I visit "/node"
+    And I visit "admin/reports/dblog"
+    And I click "More cache tags were present than could be passed in…" in the "pantheon_advanced_page_cache" row
+    Then I should see "More cache tags were present than could be passed in the Surrogate-Key HTTP Header due to length constraints"
+
+    @api
+  Scenario: No warning message after enabling test module.
+    Given I run drush "en -y pantheon_advanced_page_cache_test"
+    And I am logged in as a user with the "administrator" role
+    And I visit "admin/reports/dblog/confirm"
+    And I press "Confirm"
+    And there are 10 article nodes with a huge number of taxonomy terms each
+    When I visit "/node"
+    And I visit "admin/reports/dblog"
+    Then I should not see "pantheon_advanced_page_cache"

tests/behat/helper_classes/Contexts/FeatureContext.php

@@ -125,4 +125,44 @@ protected function getAgeTracker()
         }
         return $this->ageTracker;
     }
+
+    /**
+     * @Given there are :numnber_of_nodes article nodes with a huge number of taxonomy terms each
+     */
+    public function thereAreArticleNodesWithAHugeNumberOfTaxonomyTermsEach($number_of_nodes)
+    {
+        $i = 0;
+        while ($i < $number_of_nodes) {
+            $this->whenIGenerateAnArticleWithLotsOfTerms();
+            $i++;
+        }
+    }
+
+    /**
+     * @When a generate an article with lots of terms
+     */
+    public function whenIGenerateAnArticleWithLotsOfTerms()
+    {
+        $random_node_title = "Random Node Title: " . rand();
+        $this->minkContext->visit('node/add/article');
+        $this->minkContext->fillField('Title', $random_node_title);
+        $this->minkContext->fillField('Tags', $this->generateRandomTaxonomyString());
+        $this->minkContext->pressButton('Save');
+        $this->minkContext->assertTextVisible($random_node_title);
+    }
+
+    /**
+     * Generates a long string of tags used on node add form.
+     */
+    private function generateRandomTaxonomyString()
+    {
+        $letters = explode(' ', 'a b c d e f g h i j k l m n o p q r s t u v w x y z');
+        $i = 0;
+        $random_three_letter_combos = array();
+        while ($i < 250) {
+            $random_three_letter_combos[] = $letters[rand(0, 25)] . $letters[rand(0, 25)] . $letters[rand(0, 25)];
+            $i++;
+        }
+        return implode(",", $random_three_letter_combos);
+    }
 }

tests/behat/run-behat.sh

@@ -8,4 +8,4 @@ echo "\$options['strict'] = 0;" >> ~/.drush/pantheon.aliases.drushrc.php
 TERMINUS_ENV=$CIRCLE_BUILD_NUM
 
 export BEHAT_PARAMS='{"extensions" : {"Behat\\MinkExtension" : {"base_url" : "http://'$TERMINUS_ENV'-'$TERMINUS_SITE'.pantheonsite.io/"}, "Drupal\\DrupalExtension" : {"drush" :   {  "alias":  "@pantheon.'$TERMINUS_SITE'.'$TERMINUS_ENV'" }}}}'
-./vendor/bin/behat --config=tests/behat/behat-pantheon.yml
+./vendor/bin/behat --config=tests/behat/behat-pantheon.yml --strict

tests/modules/pantheon_advanced_page_cache_test/pantheon_advanced_page_cache_test.info.yml

@@ -0,0 +1,5 @@
+name: Pantheon Advanced Page Cache Test
+description: 'Used during automated test of Pantheon Advanced Page Cache'
+package: Testing
+type: module
+core: 8.x

tests/modules/pantheon_advanced_page_cache_test/pantheon_advanced_page_cache_test.services.yml

@@ -0,0 +1,5 @@
+services:
+  pantheon_advanced_page_cache_test.cacheable_response_subscriber:
+    class: Drupal\pantheon_advanced_page_cache_test\EventSubscriber\CacheableResponseSubscriber
+    tags:
+      - { name: event_subscriber }

tests/modules/pantheon_advanced_page_cache_test/src/EventSubscriber/CacheableResponseSubscriber.php

@@ -0,0 +1,58 @@
+<?php
+
+namespace Drupal\pantheon_advanced_page_cache_test\EventSubscriber;
+
+use Drupal\Core\Cache\CacheableResponseInterface;
+use Symfony\Component\HttpKernel\Event\FilterResponseEvent;
+use Symfony\Component\HttpKernel\KernelEvents;
+use Symfony\Component\EventDispatcher\EventSubscriberInterface;
+
+/**
+ * Adds Surrogate-Key header to cacheable master responses.
+ */
+class CacheableResponseSubscriber implements EventSubscriberInterface {
+
+  /**
+   * Adds Surrogate-Key header to cacheable master responses.
+   *
+   * @param \Symfony\Component\HttpKernel\Event\FilterResponseEvent $event
+   *   The event to process.
+   */
+  public function onRespond(FilterResponseEvent $event) {
+    if (!$event->isMasterRequest()) {
+      return;
+    }
+
+    $response = $event->getResponse();
+
+    if ($response instanceof CacheableResponseInterface) {
+      $tags = $response->getCacheableMetadata()->getCacheTags();
+
+      // This is a contrived example of how custom code can be used
+      // to limit a giant list of tags.
+      // In this case, automated Behat tests generate nodes
+      // tagged in 100s of taxonomy terms each. Then when
+      // those nodes are rendered on a view like frontpage
+      // they result in too many total surrogate-keys being set.
+      if (in_array("config:views.view.frontpage", $tags)) {
+        $new_tags = [];
+        foreach ($tags as $tag) {
+          if (strpos($tag, "taxonomy_term:") === FALSE) {
+            $new_tags[] = $tag;
+          }
+        }
+        $response->getCacheableMetadata()->setCacheTags($new_tags);
+      }
+
+    }
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public static function getSubscribedEvents() {
+    $events[KernelEvents::RESPONSE][] = ['onRespond', 100];
+    return $events;
+  }
+
+}