Make WordPress Core

Opened 12 months ago

Closed 8 months ago

Last modified 8 months ago

#62426 closed defect (bug) (fixed)

Interactivity API directives support invalid data attribute characters on the server

Reported by: jonsurrell's profile jonsurrell Owned by: joemcgill's profile joemcgill
Milestone: 6.8 Priority: normal
Severity: major Version: 6.5
Component: Interactivity API Keywords: has-patch has-unit-tests dev-feedback
Focuses: Cc:

Description

The Interactivity API client filters directive data attributes to ignore invalid data attributes. The server does not.

This leads to a situation where the server processes directives that the client does not, when the two implementation should agree.

For example:

<style>
.be\[red\] {background-color: red;}
.b { border: 3px dotted black;}
</style>
<div data-wp-interactive="example" data-wp-context='{"a":true}'>
  <div data-wp-class--be[red]="context.a">Demo Div</div>
</div>
<button data-wp-on--click="toggleCtxA">toggle ctx a</button>

This produces a red div on the server. The client does not recognize the data-wp-class--be[red] directive. Clicking the button toggles the context (adding and removing a border) but the class triggering the red background is ignored by the client and cannot be toggled.

Change History (16)

#1 @jonsurrell
12 months ago

There was some discussion in this Gutenberg PR. The conclusion was the the server should be more restrictive to match the client.

CC @luisherranz.

This is related to #62131

This ticket was mentioned in Slack in #core by desrosj. View the logs.


12 months ago

#3 @desrosj
12 months ago

  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to 6.8

This ticket was mentioned in PR #8048 on WordPress/wordpress-develop by @jonsurrell.


11 months ago
#4

  • Keywords has-patch added; needs-patch removed

This ticket was mentioned in Slack in #core by audrasjb. View the logs.


8 months ago

#6 @audrasjb
8 months ago

  • Keywords needs-testing needs-testing-info added

#7 @SirLouen
8 months ago

  • Keywords needs-testing removed

@jonsurrell can you add some testing instructions?

This ticket was mentioned in Slack in #core by sirlouen. View the logs.


8 months ago

#9 follow-up: @jonsurrell
8 months ago

  • Keywords needs-testing-info removed

Sure. Process interactivity directives that include the characters that are expected to be invalid in the attribute name of the directive:

<?php
$html = <<<HTML
<div data-wp-interactive="example" data-wp-context='{"x":1}'><div data-wp-class--bottom-[-24rem]="context.x">Should not have any class attribute.</div></div>
HTML;
echo wp_interactivity_process_directives( $html );

Here, data-wp-class--bottom-[-24rem] is an invalid directive name because it includes [ and ].

On trunk, the server process the directive and adds a class attribute. This is unexpected, the client will never process those directives and therefore they can never be interactive (see #62131). Trunk output looks like this after server processing:

<div data-wp-interactive="example" data-wp-context="{&quot;x&quot;:1}"><div class="bottom-[-24rem]" data-wp-class--bottom-[-24rem]="context.x">Should not have any class attribute.</div></div>

In PR 8048, the server does not process the directives. This is expected and aligns with the client behavior. Here's the processed HTML from the server on this branch:

<div data-wp-interactive="example" data-wp-context="{&quot;x&quot;:1}"><div data-wp-class--bottom-[-24rem]="context.x">Should not have any class attribute.</div></div>

Note the difference, trunk includes class="bottom-[-24rem]" (evidence of the undesirable processing) while PR 8048 does not.

It's also good to confirm that allowed directives continue to work as expected. For example, the following directive is the same except for the removal of [] from the directive name:

<?php
$html = <<<HTML
<div data-wp-interactive="example" data-wp-context='{"x":1}'><div data-wp-class--bottom--24rem="context.x"><em>Must</em> have class attribute.</div></div>
HTML;
echo wp_interactivity_process_directives( $html );

In this case, both trunk and the PR produce the same expected result with the class="bottom--24rem" class applied:

<div data-wp-interactive="example" data-wp-context="{&quot;x&quot;:1}"><div class="bottom--24rem" data-wp-class--bottom--24rem="context.x"><em>Must</em> have class attribute.</div></div>

#10 in reply to: ↑ 9 @SirLouen
8 months ago

  • Keywords has-unit-tests dev-feedback added
  • Severity changed from normal to major

Test Report

Description

This report validates that the indicated patch addresses the issue.

Patch tested: PR 8048

Environment

  • WordPress: 6.8-beta1-59933-src
  • PHP: 8.2.27
  • Server: nginx/1.27.4
  • Database: mysqli (Server: 8.4.4 / Client: mysqlnd 8.2.27)
  • Browser: Chrome 133.0.0.0
  • OS: Windows 10/11
  • Theme: Twenty Twenty-Five 1.1
  • MU Plugins: None activated
  • Plugins:
    • Test Reports 1.2.0

Reproduction steps

  1. Using interactivity-testing.php in a working WP environment results expected.
  • For the first test case without the patch, the class is present and this is wrong (class="bottom-[-24rem]")
  • For the first test case with the patch, class should not be present
  1. For the second test case with the patch, class should be present in both scenarios.

Actual Results with Patch

  • ✅ Invalid directive with square brackets is not processed (no class attribute added)
  • ✅ Valid directive without square brackets is processed correctly (class attribute added)
  • ✅ Unit Tests asserts passing.

Additional Notes

Supplemental Artifacts

File: interactivity-testing.php

<?php
require_once __DIR__ . '/wp-load.php';

// Test Case 1: Invalid directive with square brackets
$html = <<<HTML
<div data-wp-interactive="example" data-wp-context='{"x":1}'><div data-wp-class--bottom-[-24rem]="context.x">Should not have any class attribute.</div></div>
HTML;
echo "Test Case 1 (Invalid directive):\n";
echo wp_interactivity_process_directives( $html );
echo "\n\n";

// Test Case 2: Valid directive without square brackets
$html = <<<HTML
<div data-wp-interactive="example" data-wp-context='{"x":1}'><div data-wp-class--bottom--24rem="context.x">Must have class attribute.</div></div>
HTML;
echo "Test Case 2 (Valid directive):\n";
echo wp_interactivity_process_directives( $html );
echo "\n\n";
Last edited 8 months ago by SirLouen (previous) (diff)

This ticket was mentioned in Slack in #core-test by sirlouen. View the logs.


8 months ago

This ticket was mentioned in Slack in #core by audrasjb. View the logs.


8 months ago

#13 @joemcgill
8 months ago

  • Owner set to joemcgill
  • Status changed from new to reviewing

This ticket was mentioned in Slack in #core by audrasjb. View the logs.


8 months ago

#15 @joemcgill
8 months ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

In 60070:

Interactivity API: Apply the same directive name restrictions as the client.

This adds the same logic to filter directive data attributes to ignore invalid data attributes that is applied in the client to avoid processing directives on the server that will not be processed in the client.

Props jonsurrell, SirLouen.
Fixes #62426.

@jonsurrell commented on PR #8048:


8 months ago
#16

Thanks for reviewing and landing this!

Note: See TracTickets for help on using tickets.