Contrib.social

Allow additional keys in #attached

Open on Drupal.org →
Created on 21 May 2024, 26 days ago
Updated 1 June 2024, 15 days ago

Problem/Motivation

The render array #attached key bubbles up and merges information from elements. The keys are checked against an allowed list in HtmlResponseAttachmentsProcessor, and an exception is thrown for unrecognized values. This prevents other modules from using the render system to add additional properties that bubble up during the rendering process.

https://git.drupalcode.org/project/drupal/-/blob/11.x/core/lib/Drupal/Co...

    // Send a message back if the render array has unsupported #attached types.
    $unsupported_types = array_diff(
      array_keys($attached),
      ['html_head', 'feed', 'html_head_link', 'http_header', 'library', 'html_response_attachment_placeholders', 'placeholders', 'drupalSettings']
    );
    if (!empty($unsupported_types)) {
      throw new \LogicException(sprintf('You are not allowed to use %s in #attached.', implode(', ', $unsupported_types)));
    }
  • Big Pipe makes use of #attached by wrapping the core attachments processor, and removing its own properties from the array before passing the rest on to the decorated method. These properties are only added (and then removed) by Big Pipe during the rendering process.
  • Content Security Policy allows modules to alter a page's policy, which can currently be done by inspecting the libraries attached to a response. Some content, such as media or an iframe may not have an associated library and so site builders may have to allow a certain resource on all pages of a site instead of for only relevant content. If an element contains content which requires a placeholder for a nonce be replaced, there isn't a direct way to pass that information to the response ✨ Provides a filter to add nonce attribute to inline scripts. Needs work
  • Attach Inline allows developers to add inline JS and CSS snippets to a render array element, which are then rendered with the page's libraries in the page header or footer. To allow additional keys for specifying inline JS and CSS, the module replaces the core attachments processor service, which can cause compatibility issues if core's code changes or if another module also needs to replace the service #3096061: [upstream] html_response.attachments_processor service must be replaced instead of decorated → . If the attach inline module is uninstalled while a render array still has the additional key, the exception will be thrown.

Proposed resolution

  • Allow modules to define additional allowed keys. Use an assertion to check for unrecognized keys so that an exception isn't thrown in a production environment for properties that will just be ignored.
  • OR Allow any key
$element['#attached'] = [
  'csp' => [
    'media-src' => ["*"],
    'script-src' => ["'unsafe-inline'"],
  ],
];

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

✨ Feature request
Status

Needs review

Version

11.0 🔥

Component
Render  →

Last updated 2 days ago

Created by

🇨🇦Canada gapple

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @gapple
  • Merge request !8145Configurable supported types in #attached → (Open) created by gapple
  • Status changed to Needs review 25 days ago
  • Comment 25 days ago →
    🇨🇦Canada gapple

    MR moves the supported types to an argument when constructing HtmlResponseAttachmentsProcessor, and checks the keys with an assertion. A module could modify the values with a Service Provider.

    Using a service parameter to set the types would probably be a little easier for modules to modify than altering the service's argument.

  • Pipeline finished with Failed
    25 days ago
    Total: 604s
    #178914
  • Comment 25 days ago →
    🇨🇦Canada gapple

    Doing some code archaeology, the exception for undefined types was added by #2350877: Deprecate/rename drupal_add_feed(), drupal_add_html_head(), drupal_add_html_head_link(), drupal_add_http_header(), and allow to be set declaratively in #attached → , and it looks like part of the original reason for an exception was that drupal_process_attached() changed from using a callback function based on the key name, to a limited set of values mapped to corresponding internal functions. Some discussion of additional keys starts at comment #30, but the only know use case was the contrib libraries API which could transition to extending the core library API.

    It was moved to the attachments processor as part of refactoring in #2350877: Deprecate/rename drupal_add_feed(), drupal_add_html_head(), drupal_add_html_head_link(), drupal_add_http_header(), and allow to be set declaratively in #attached → that removed drupal_process_attached, and the exception was kept because of an existing test for it (comment #39), but not otherwise justified.

  • Pipeline finished with Success
    25 days ago
    Total: 557s
    #179092
  • Comment 25 days ago →
    🇫🇷France andypost

    I'd better keep to throw exception for security reasons, assertions are usually disabled

  • Comment 25 days ago →
    🇨🇦Canada gapple

    I don't think there should be a security concern here. It's already possible to put any extra keys/data in #attached, as long as you decorate or replace so that HtmlResponseAttachmentsProcessor::processAttachments() doesn't see them, like Big Pipe does. HtmlResponseAttachmentsProcessor itself only interacts with the keys it expects and any others would be ignored. Anything put in #attached is also done as part of creating a renderable element - I don't think it should ever see user input, and any module would be responsible for safely handling it's own data anyways after permitting a new key.

    Attach Inline can't safely decorate the service because it still needs to modify a call to AttachedAssets::createFromRenderArray() to use its own extended AttachedAssets class which preserves the additional #attached items to be passed on to the CSS / JSS collection renderers (which it is able to decorate).

    I've implemented an example of CSP using extra data in #attached here: https://git.drupalcode.org/project/csp/-/merge_requests/38/diffs?commit_.... It could do similar to Big Pipe to remove and restore it's additional key to hide it from HtmlResponseAttachmentsProcessor, but disabling the CSP module while a render element still includes the additional key would result in the exception being thrown.

    I think the assertion is even excessive - it would only be a developer hint that they've spelled a key wrong or are missing a module to process the the item, but nothing would break as any unrecognized keys are ignored - I'm inclined to just remove the key check entirely.

  • Comment 15 days ago →
    🇫🇷France andypost

    There's TODO in AttachmentsInterface which could be resolved with the issue

  • Pipeline finished with Success
    14 days ago
    Total: 537s
    #189523
contrib.social Blog FAQ Discussions
Production build 0.69.0 2024