Problem/Motivation

Non-core service definitions are loaded after core's, which will cause module event subscribers with the same priority to execute later. As a result, core will:
(1) In normal cases likely never execute the line of code to append to an existing Permissions-Policy header
(2) Always have its header value overwritten by any other implementation

The results in a section of code that is normally unreachable, but with the potential for unexpected behaviour in non-typical circumstances.

Steps to reproduce

- Add the Permissions Policy module to a site
- In the module's configuration, enable the 'Enforced' policy and a directive (e.g. set geolocation to 'empty')
- Inspect the headers of a response

Because the module's subscriber has the same priority as core's subscriber (both use the default of 0), its value completely replaces the core value.

Proposed resolution

Core should only set a default value if a Permissions-Policy header is not already set, and not attempt to modify any existing value.

Remaining tasks

None

User interface changes

None

API changes

None

Data model changes

None

Release notes snippet

Issue fork drupal-3218139

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:

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

gapple created an issue. See original summary.

gapple’s picture

Status: Active » Needs review
gapple’s picture

Issue summary: View changes
longwave’s picture

Status: Needs review » Reviewed & tested by the community

The justification for this issue is good, and this simplifies the code/makes it less surprising to users who might use Permissions-Policy elsewhere.

  • catch committed d0e404c on 9.3.x
    Issue #3218139 by gapple, longwave: Stop altering existing Permissions-...

  • catch committed 2e402f8 on 9.2.x
    Issue #3218139 by gapple, longwave: Stop altering existing Permissions-...
catch’s picture

Status: Reviewed & tested by the community » Fixed

This makes sense. Committed/pushed to 9.3.x and cherry-picked to 9.2.x, thanks!

Status: Fixed » Closed (fixed)

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