Problem/Motivation

Entity usage module implements itself to ANY form object providing a getEntity method. However, it doesn't check whether the method is actually callable.

For instance, the Structured Data module implements a private getEntity() method, which is completely normal.

However this private method would have Entity Usage module to fail:

The website encountered an unexpected error. Please try again later.
Error: Call to private method Drupal\structured_data\Form\PageJsonForm::getEntity() from context '' in entity_usage_form_alter() (line 82 of modules/contrib/entity_usage/entity_usage.module).

Steps to reproduce

  • Enable any module implementing a private or protected getEntity method.
  • Enable Entity Usage module
  • Open any page that Structured Data form is shown (route: /structured_data/page/json/{sd_route_name}/{sd_url}/{sd_bundle}/{sd_entity_id})
  • Observe the error

Proposed resolution

Replace method_exists() function with is_callable()

Error

The website encountered an unexpected error. Please try again later.
Error: Call to private method Drupal\structured_data\Form\PageJsonForm::getEntity() from context '' in entity_usage_form_alter() (line 82 of modules/contrib/entity_usage/entity_usage.module).
entity_usage_form_alter(Array, Object, 'structured_data_page_json_form') (Line: 539)
Drupal\Core\Extension\ModuleHandler->alter('form', Array, Object, 'structured_data_page_json_form') (Line: 835)
Drupal\Core\Form\FormBuilder->prepareForm('structured_data_page_json_form', Array, Object) (Line: 279)
Drupal\Core\Form\FormBuilder->buildForm(Object, Object) (Line: 73)
Drupal\Core\Controller\FormController->getContentResult(Object, Object)
call_user_func_array(Array, Array) (Line: 123)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 564)
Drupal\Core\Render\Renderer->executeInRenderContext(Object, Object) (Line: 124)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->wrapControllerExecutionInRenderContext(Array, Array) (Line: 97)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 158)
Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object, 1) (Line: 80)
Symfony\Component\HttpKernel\HttpKernel->handle(Object, 1, 1) (Line: 58)
Drupal\Core\StackMiddleware\Session->handle(Object, 1, 1) (Line: 48)
Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object, 1, 1) (Line: 106)
Drupal\page_cache\StackMiddleware\PageCache->pass(Object, 1, 1) (Line: 41)
Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object, 1, 1) (Line: 51)
Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object, 1, 1) (Line: 23)
Stack\StackedHttpKernel->handle(Object, 1, 1) (Line: 708)
Drupal\Core\DrupalKernel->handle(Object) (Line: 55)
CommentFileSizeAuthor
#2 use-is_callable-method-3260251-2.patch556 bytesosman
Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

osman created an issue. See original summary.

osman’s picture

Status: Active » Needs review
StatusFileSize
new556 bytes

here's a patch for your review.

donquixote’s picture

The correct solution would be instanceof EntityFormInterface.
Otherwise we call any random function that is called getEntity(), which might not implement the same contract.

silvi.addweb made their first commit to this issue’s fork.

silvi.addweb’s picture

Status: Needs review » Reviewed & tested by the community

I've tested this patch and it's working fine.

marcoscano’s picture

Status: Reviewed & tested by the community » Needs work

Setting NW as per #3

dhruv.mittal’s picture

Assigned: Unassigned » dhruv.mittal
dhruv.mittal’s picture

Assigned: dhruv.mittal » Unassigned
Status: Needs work » Needs review

Please review

alexpott’s picture

@dhruv.mittal you need to update the MR branch with the latest changes from 8.x-2-x... the MR fork is very old.

dhruv.mittal’s picture

Assigned: Unassigned » dhruv.mittal
dhruv.mittal’s picture

Assigned: dhruv.mittal » Unassigned

I have done the requested changes please have a look on it

alexpott’s picture

Status: Needs review » Needs work

Minor fixed needed to pass PHPStan checking...

alexpott’s picture

Version: 8.x-2.0-beta6 » 8.x-2.x-dev
dhruv.mittal’s picture

Status: Needs work » Needs review

Kindly review now

marcoscano’s picture

Status: Needs review » Fixed

Thanks for contributing!

Status: Fixed » Closed (fixed)

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