WordPress.org
Get WordPress

Make WordPress Core

Opened 21 months ago

Closed 17 months ago

Last modified 17 months ago

#62426 closed defect (bug) (fixed)

Interactivity API directives support invalid data attribute characters on the server

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

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
21 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.
21 months ago

#3 @desrosj
21 months ago

  • Keywords needs-patch added
  • Milestone Awaiting Review → 6.8

This ticket was mentioned in ​PR #8048 on ​WordPress/wordpress-develop by ​@jonsurrell.
20 months ago #4

  • Keywords has-patch added; needs-patch removed

See ​https://github.com/WordPress/gutenberg/blob/ca616014255efbb61f34c10917d52a2d86c1c660/packages/interactivity/src/vdom.ts#L20-L32

Trac ticket: https://core.trac.wordpress.org/ticket/62426

## Todo:

  • [ ] Add tests
  • [ ] Add note to source in client to keep regexes in sync.

This ticket was mentioned in ​Slack in #core by audrasjb. ​View the logs.
18 months ago

#6 @audrasjb
18 months ago

  • Keywords needs-testing needs-testing-info added

#7 @SirLouen
18 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.
18 months ago

#9 follow-up: @jonsurrell
18 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
18 months ago

  • Keywords has-unit-tests dev-feedback added
  • Severity normal → 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 18 months ago by SirLouen (previous) (diff)

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

This ticket was mentioned in ​Slack in #core by audrasjb. ​View the logs.
17 months ago

#13 @joemcgill
17 months ago

  • Owner set to joemcgill
  • Status new → reviewing

This ticket was mentioned in ​Slack in #core by audrasjb. ​View the logs.
17 months ago

#15 @joemcgill
17 months ago

  • Resolution → fixed
  • Status reviewing → 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:
17 months ago #16

Thanks for reviewing and landing this!

Note: See TracTickets for help on using tickets.