Problem/Motivation

Context mapping should happen before $block->access()

For \Drupal\ctools_block\Plugin\Block\EntityField->blockAccess() this causes WSOD.

Steps to reproduce

Add an ctools EntityField to a context reaction - see WSOD.

Proposed resolution

Move context mapping before $block->access().

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#3 context-mapping-should-happen-before-block-access-3.patch1.56 KBsilverham
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

silverham created an issue. See original summary.

silverham’s picture

silverham CreditAttribution: silverham at EY Digital commented
Title: Context mapping should happen before $block->access() - avoid WSOD for EntityField->blockAccess( » Context mapping should happen before $block->access() - avoid WSOD for EntityField->blockAccess()
silverham’s picture

silverham CreditAttribution: silverham at EY Digital commented
Status: Active » Needs review
FileSize
context-mapping-should-happen-before-block-access-3.patch1.56 KB

Status: Needs review » Needs work

The last submitted patch, 3: context-mapping-should-happen-before-block-access-3.patch, failed testing. View results

paulocs’s picture

paulocs
Primary language Portuguese, Brazil
Location Belo Horizonte
CreditAttribution: paulocs at CI&T commented

Hey @silverham,
Can you provide the steps to add the EntityField block to a reaction?

I installed the Chaos Tools Blocks module but I don't know exactly how it works. I see that module prevents the EntityField block to be displayed on viewable block types so I don't know how to use it and what it does exactly.

DeaOm’s picture

DeaOm CreditAttribution: DeaOm at Agiledrop - Your Trusted Drupal Teammates commented
Status: Needs work » Needs review

This is only changing the order of the code execution, so if access for blocks is still working correctly, I would say commit the patch but without all the unnecessary comments added.

silverham’s picture

silverham CreditAttribution: silverham at EY Digital commented

Hi @paulocs

How to replicate:

1. Install Drupal 8
2. Install ctools & submodule ctools_block (latest stable - 8.x-3.2)
3. Install context module (latest dev - issue appears in stable as well)
4. Create a context - any condition e.g. path NOT admin/* - with reaction block -> (Block: "ID", Category: "Content")
5. Go to the page which will trigger happen (e.g. homepage or any node page) -> see Error of death

Note that the $block->access() is cached when fixed, so you must clear cache and then go the page with fix removed to see the reaction cause the WSOD again. (if testing a few times)

paulocs’s picture

paulocs
Primary language Portuguese, Brazil
Location Belo Horizonte
CreditAttribution: paulocs at CI&T commented
Status: Needs review » Needs work

Hello @silverham,

after I applied the patch, I got the error:
Drupal\Component\Plugin\Exception\MissingValueContextException: Required contexts without a value: entity in Drupal\Core\Plugin\Context\ContextHandler->applyContextMapping() (line 155 of core/lib/Drupal/Core/Plugin/Context/ContextHandler.php).

I understand the problem now, but I'm not sure yet what the best solution is. If you have any suggestion, please let me know.
Maybe we should disable the context or the block that throws the exception and tell the user (or not) that is not possible to use the reaction in this context.

I'm looking if the core has already any solution for it so we can do something similar.

silverham’s picture

silverham CreditAttribution: silverham at EY Digital commented
Status: Needs work » Needs review

I double checked the patch does actually work on node pages.

But am wrong however about working on homepages - it still crashes there - but that's out of scope.

Detailed steps to reproduce:

# Install Drupal 8
composer create-project drupal/recommended-project:8.x drupal_8_2020_10_20__context
cd drupal_8_2020_10_20__context/
# [ set database settings in settings.php ]
drush si
# Add ctools and context
composer require drupal/ctools drupal/context:4.x-dev
# install modules
drush en ctools ctools_block context context_ui
# confirm context module version
cd web/modules/contrib/context/
git log | head
# commit 0924f0976f3002a8a4debc5e741c8d6745fab9c4
# Author: silverham <silverham@3499769.no-reply.drupal.org>
# Date:   Thu Oct 15 11:20:04 2020 -0300
# Issue #3177007 by paulocs, silverham: Sitewide context broken due to #3045666
#
# TEST ISSUE issue:
# Create a context - any condition e.g. path NOT /admin/* - with reaction block -> (Block: "ID", Category: "Content")
# Go to a page that error should NOT happen (e.g.  any node page) -> see Error of death
# Error of death:
# Uncaught PHP Exception Drupal\\Component\\Plugin\\Exception\\ContextException: "The 'entity:node' context is required and not present." at [...]\\web\\core\\lib\\Drupal\\Core\\Plugin\\Context\\Context.php line 73, referer: http://drupal_8_2020_10_20__context.build.localtest.me:8080/node/add/page
# [Copy patch file into module folder & apply ]
patch -p1 < context-mapping-should-happen-before-block-access-3.patch
# Refresh node page - should immediately.
# To reproduce issue again::
# Remove patch:
patch -R -p1 < context-mapping-should-happen-before-block-access-3.patch
# Clear cache
http://drupal_8_2020_10_20__context.build.localtest.me:8080/admin/config/development/performance
# Visit a node page -> see crash again.
paulocs’s picture

paulocs
Primary language Portuguese, Brazil
Location Belo Horizonte
CreditAttribution: paulocs at CI&T commented
Status: Needs review » Reviewed & tested by the community

Thanks @silverham!

  • paulocs committed 7064385 on 8.x-4.x authored by silverham 
    Issue #3177008 by silverham, paulocs, DeaOm: Context mapping should...
paulocs’s picture

paulocs
Primary language Portuguese, Brazil
Location Belo Horizonte
CreditAttribution: paulocs at CI&T commented
Status: Reviewed & tested by the community » Fixed

Fixed in dev branch.

Thanks @all.

silverham’s picture

silverham CreditAttribution: silverham at EY Digital commented

Great - thanks. :-)

Status: Fixed » Closed (fixed)

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