CommentFileSizeAuthor
#8 2987636-handle-misconfiguration-2.patch2.11 KBbdragon
#5 2987636-handle-misconfiguration.patch3.33 KBbdragon
D7-d8cache-include_commons_inc.patch465 bytesamcgowanca

Comments

amcgowanca created an issue. See original summary.

fabianx’s picture

fabianx commented
Assigned: amcgowanca » bdragon
Status: Needs review » Needs work

We should update README instead to say that cache_page should never use a D8CacheAttachmentsCollector (and maybe add a code warning too?)

Including common.inc here is not a good idea as it can lead to strange side effects during bootstrap.

Over to Brandon ...

fabianx’s picture

fabianx commented
Component: Code » Documentation

  • edd91c3 committed on 7.x-1.x 
    Issue #2987636 work: Add a note to the README.
bdragon’s picture

bdragon commented
Title: Include includes/common.inc if drupal_process_attached() does not exist » Don't crash if user configures early page cache as D8CacheAttachmentsCollector
Status: Needs work » Needs review
StatusFileSize
new 2987636-handle-misconfiguration.patch3.33 KB

I pushed a change to README.txt to warn about this situation.

Here's a possible fix for making sure things don't crash and the misconfiguration doesn't go unnoticed.

bdragon’s picture

bdragon commented
Assigned: bdragon » fabianx
fabianx’s picture

fabianx commented 
+++ b/d8cache-ac.cache.inc
@@ -95,28 +127,29 @@ class D8CacheAttachmentsCollector extends D8Cache {
   public function set($cid, $data, $expire = Cache::PERMANENT, $tags = array()) {
-    $attachments = array();
+    if ($this->isValid) {
+      $attachments = array();

Can we use early return here to avoid indenting that much? Even if we double the set, that's not a problem.

bdragon’s picture

bdragon commented
StatusFileSize
new 2987636-handle-misconfiguration-2.patch2.11 KB

Makes sense.

fabianx’s picture

fabianx commented
Status: Needs review » Reviewed & tested by the community

RTBC - looks great to me

  • bdragon authored e9f9eab on 7.x-1.x 
    Issue #2987636 by bdragon, amcgowanca, Fabianx: Don't crash if user...
bdragon’s picture

bdragon commented
Status: Reviewed & tested by the community » Fixed

Committed, thanks!

Status: Fixed » Closed (fixed)

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