Problem/Motivation

After #3537962: EntityFieldManager::getFieldDefinitions() per-bundle caching can be expensive, moderation state field definitions may be built with an incorrect target bundle. This change exposed a latent bug in ContentModerationHooks::entityBundleFieldInfo() in which a base field definition is returned without first being cloned, allowing the target bundle to be mutated through a shared reference.

Steps to reproduce

  1. Create two moderated bundles.
  2. Inspect the field definitions for both bundles.
  3. One of the moderation_state definitions will have the wrong target bundle.

Proposed resolution

Return a cloned base field definition in ContentModerationHooks::entityBundleFieldInfo().

Remarks

  • It's more performant to fix this in each hook implementation, albeit less reliable than deep cloning $base_field_definitions in the entity field manager prior to each invocation.
  • Since the entity field manager already sets the target bundle for each definition, that same step can be omitted in the content moderation module's hook implementation.
  • I chose Major priority for this issue since it's a "significant [...] developer-facing bug with no workaround."

Issue fork drupal-3611529

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

clayfreeman created an issue. See original summary.

clayfreeman’s picture

Issue summary: View changes

clayfreeman’s picture

Assigned: clayfreeman » Unassigned
Status: Active » Needs review

Ready for review.

acbramley’s picture

Couple small comments but otherwise this looks good to go.

clayfreeman’s picture

Made the requested test change; ready for review again.

acbramley’s picture

Status: Needs review » Reviewed & tested by the community

Thanks! Good to go now

longwave’s picture

Status: Reviewed & tested by the community » Fixed

Thanks for the explanation of how the bug occurs, that makes sense to me and was helpful when reviewing.

Committed and pushed 28d2798c5cd to main and e179aaeca3b to 11.x and bced52eee4d to 11.4.x. Thanks!

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

  • longwave committed bced52ee on 11.4.x
    fix: #3611529 Moderation state field definitions can have an incorrect...

  • longwave committed e179aaec on 11.x
    fix: #3611529 Moderation state field definitions can have an incorrect...

  • longwave committed 28d2798c on main
    fix: #3611529 Moderation state field definitions can have an incorrect...

Status: Fixed » Closed (fixed)

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