Problem/Motivation

I am having some issues with a site that migrated to Drupal. The previous site used query parameters for site navigation in a way that quite confuses search spiders. Currently the Baidu spider is aggresively (re)indexing millions of pages that no longer exist.

When the page_cache module is enabled and thousands of requests are done to unqique, non-existing pages the cache_render table fills up dramatically to tens of gigabytes per day with 4xx-response records.

I made this issue critical as I suspect this will make it possible to easily take down sites where storage is limited.

Proposed resolution

Not sure

Remaining tasks

(reviews needed, tests to be written or run, documentation to be written, etc.)

User interface changes

(New or changed features/functionality in the user interface, modules added or removed, changes to URL paths, changes to user interface text.)

API changes

(API changes/additions that would affect module, install profile, and theme developers, including examples of before/after code if appropriate.)

Data model changes

(Database or configuration data changes that would make stored data on an existing site incompatible with the site's updated codebase, including changes to hook_schema(), configuration schema or keys, or the expected format of stored data, etc.)

Comments

casey created an issue. See original summary.

casey’s picture

casey commented
Issue tags: +D8 cacheability
catch’s picture

catch
he/him
Primary language English
commented
Issue summary: View changes

Can you confirm whether the table size really is all from page cache, or whether other render caching is kicking in on the 404/403 pages as well? If it's other render caching that's the main problem, then this is a duplicate of #2362999: Render cache the 403/404 HTML response which would dramatically reduce the storage needed for other render cached things on 404s.

Also can you confirm what cache.page.max_age is set to? If you have it set to permanent, then those pages won't purge unless something changes, but it can easily be changed to a few minutes in the UI as a mitigation. I agree our default settings shouldn't let disk fill up unbounded, but #2526150: Database cache bins allow unlimited growth: cache DB tables of gigabytes! is open for that which would allow them to get purged eventually.

If it's neither of those, need more information here.

casey’s picture

casey commented

cache.page.max_age is set to 60. As far as I understand those 4xx-response entries will never be removed.

The cache tags for these cache entries look normal; they actually are the same for all 4xx-response entries. In my case:

4xx-response config:block.block.breadcrumb config:block.block.footernavigation config:block.block.local_actions config:block.block.local_tasks config:block.block.mailingsubscribe config:block.block.mainnavigation config:block.block.messages config:block.block.page_title config:block.block.searchform config:block.block.sitebranding config:block.block.subnavigation config:block.block.topnavigation config:block_list config:search.settings config:system.menu.footer config:system.menu.main config:system.menu.top config:system.site config:user.role.anonymous node:1 node:17 node:22 node:3 node:31 node:32 node:33 node:34 node:35 node:36 node:37 node:38 rendered

In my specific case its about urls like /0/d8ba5d9bad8c81b4c12573be00361c77!OpenDocument&ExpandSection=4,2,5,8,3,9,7 so you might understand what the Baiduspider is doing.

The cache cid's are also very flat (no extra cache contexts included, or is that always the cache for page_cache), e.g. http://.../0/d8ba5d9bad8c81b4c12573be00361c77!OpenDocument&ExpandSection=4,2,7,10,6,9,3:html

In my case I am probably going to block the Baiduspider, but the issue seems critical to me.

casey’s picture

casey commented

Oh, this is also happening for cache_menu

Example entry (for a path that returns a 404):

cid: active-trail:path:0/d8ba5d9bad8c81b4c12573be00361c77!OpenDocument&ExpandSection=5,10,6,2,8,9,7
tags: config:system.menu.footer config:system.menu.main config:system.menu.top

dawehner’s picture

dawehner
Primary language German
commented

@casey From a technical point of view this happens because all those blocks seems to have some output which changes per URL, at least for breadcrumb local_actions etc. this makes sense.

In your case I would write a custom module which adds a event subscriber, that listens to KernelEvents::EXCEPTION, detects whether the request was weird and set directly an empty 4xx response, so no actual rendering/caching is done.

In general Drupal could always be easily flooded with query parameters.

catch’s picture

catch
he/him
Primary language English
commented

Yes you're right on the page cache, we don't use max_age for the page cache ttl. #2526150: Database cache bins allow unlimited growth: cache DB tables of gigabytes! would be the issue to allow those to be purged then. And just to confirm there's no cache contexts used in the internal page cache, just URL only is right.

I'm confused by that active_trail cid - I thought we cached that by route:

https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Menu%21Me...

dawehner’s picture

dawehner
Primary language German
commented

$route_parameters = $this->routeMatch->getRawParameters()->all();
ksort($route_parameters);

Do the query parameters somehow end up in there?

znerol’s picture

znerol commented

Whether or not to cache error pages is a subject which seems to crop up again and again. Regrettably there is not one single simple answer to that problem.

High traffic sites risk to accidentally DOS themselves with links pointing at black-holes if error pages are not cached (CPU/SQL bottleneck). I think that this problem is more severe for sites behind a caching reverse proxy, since in that case the backend machines are not prepared to handle a sudden traffic explosions. That's why there is a reverse proxy in the first place.

On the other hand there is a DOS risk if we cache error pages indefinitely by default (Cache bloat), as demonstrated by the issue here.

So I guess we might be needing time based expiration (in addition to cache-tags) for certain responses.

Maybe we could mitigate the problem by just excluding error pages from the internal cache, but continue to add cache control headers in order to allow time based caching on the edge servers / in the browser.

catch’s picture

catch
he/him
Primary language English
commented

@casey From a technical point of view this happens because all those blocks seems to have some output which changes per URL, at least for breadcrumb local_actions etc. this makes sense.

Most cases we should be caching by route rather + params rather than URL, in which case the 404 page counts as a single route + params combination which should not result in cache filling. This is why I really want to know how casey is getting an active-trail:path: cid.

Breadcrumbs is an exception to that, we might want to revisit the cache context, or make it behave differently when the request path doesn't resolve to a route at all.

casey’s picture

casey commented

@dawehner, thanks for the suggestion.

@catch, my bad, the active-trail:path: cid is being added by a custom module (in which I am overriding the menu.active_trail service).

casey’s picture

casey commented

As a temporary solution for my case I have added a custom Drupal\Core\PageCache\ResponsePolicyInterface implementation that just denies caching 404 responses. All blocks are already render cached (mostly by route, I also changed the custom menu.active_trail service implementation) separately so the impact is acceptable.

  public function check(Response $response, Request $request) {
    $exception = $this->requestStack->getCurrentRequest()->attributes->get('exception');
    if ($exception instanceof NotFoundHttpException) {
      return static::DENY;
    }
  }

Maybe this is what the page_cache module should also do; never cache 404 responses.

casey’s picture

casey commented

@catch, PageCache doesn't use the cache contexts for its cid. It uses the requests uri and response format: https://api.drupal.org/api/drupal/core!modules!page_cache!src!StackMiddl...

catch’s picture

catch
he/him
Primary language English
commented
Category: Bug report » Plan

I think we could set a shorter TTL for 404 responses in page_cache, but don't think we should completely prevent caching for the reason @znerol mentions of bad links. Opened #2699613: Set a shorter TTL for 404 responses in page_cache module for that.

Also opened #2699627: url.path cache context for breadcrumbs is unnecessarily granular for breadcrumbs.

Making this a plan since there's not a single solution to this.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

my bad, the active-trail:path: cid is being added by a custom module (in which I am overriding the menu.active_trail service).

Does this then mean this should not be critical?

catch’s picture

catch
he/him
Primary language English
commented

@Wim so it depends whether we think the database cache being filled up by 404 requests (or more generally) is critical. Disk full can be very serious for a site, then it's a question of how easy it is for that to happen and which of the three child issues that would help address it are must-haves if we do.

I don't have a strong opinion on the criticality of this issue - I do think the three child issues are at least major.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Title: cache_render overflows with 4xx-response records » Page cache caches a different HTML response for every 404 URL
Issue tags: +Needs backport to D7

I just verified the same problem exists in Drupal 7. Hasn't this been known to be a problem (and in fact also a DDoS attack vector) for many years? I'd swear there is a pre-existing issue for exactly this problem somewhere.

Clarifying title in the mean time.

znerol’s picture

znerol commented

D7 is less of a problem because by default the page cache is purged on every cron run.

Please let's not confuse terms here, this is not really about DDoS.

catch’s picture

catch
he/him
Primary language English
commented
Title: Page cache caches a different HTML response for every 404 URL » Database cache can fill the disk, especially via 404s

Re-titling again for what I think we should try to fix here.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

D7 is less of a problem because by default the page cache is purged on every cron run.

Wow, I'd totally forgotten that. Yet another reason why the But D7 also has page cache! argument makes very little sense. Thanks for the reminder.

effulgentsia’s picture

effulgentsia commented
Issue tags: +Triaged D8 critical

Discussed with @xjm, @alexpott, and @Cottser, and we agreed this is Critical for 8.x due to the lack of automatic purging per #18.

catch’s picture

catch
he/him
Primary language English
commented
Title: Database cache can fill the disk, especially via 404s » [meta] Database cache can fill the disk, especially via 404s
Component: page_cache.module » cache system
catch’s picture

catch
he/him
Primary language English
commented
Issue tags: -Needs backport to D7
cilefen’s picture

cilefen commented
Title: [meta] Database cache can fill the disk, especially via 404s » [meta] Cache storage in database can fill the disk, especially via 404s

Clearing up the title, because database servers themselves have caches, which is not the subject of this issue.

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

catch’s picture

catch
he/him
Primary language English
commented

Patches up at #2699613: Set a shorter TTL for 404 responses in page_cache module and #2526150: Database cache bins allow unlimited growth: cache DB tables of gigabytes!.

catch’s picture

catch
he/him
Primary language English
commented
Priority: Critical » Major

With the three sub-issues all having patches, and breadcrumbs which is IMO the worst one close to RTBC, I think we can demote this meta issue to major. I've also reviewed our use of cache contexts in general and we don't have anything quite equivalent to breadcrumbs.

Leaving open for tracking the results of those issues on real sites, and in case anything else comes up.

catch’s picture

catch
he/him
Primary language English
commented
Status: Active » Fixed

Both those issues are in, the CACHE_PERMANENT one is postponed. I think we can mark this fixed. Adding commit credit since there was a lot of investigative/planning work done here that didn't necessarily make it into patch issue credits, which seems OK for a plan issue

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.