Interactivity API: Bind directives may access uninitialized string offset

[jonsurrell]

When using bind directives with short attribute names (less than 5 characters), the following warning is produced:

Warning: Uninitialized string offset 4 in /var/www/html/wp-includes/interactivity-api/class-wp-interactivity-api.php on line 579

This should be fixed to prevent the warning.

Fix the following warning when using an interactivity bind directive with a short attribute name, e.g. wp-data-bind--id.

Warning: Uninitialized string offset 4 in /var/www/html/wp-includes/interactivity-api/class-wp-interactivity-api.php on line 579

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

👋 @c4rl0sbr4v0 @DAreRodz for review.

Possible to add tests for this?

I tried adding tests and would like to, but I couldn't get them to fail on the warning, that may be a configuration option or my lack of knowledge, but I couldn't find any way of saying "warnings on _this test_ should cause the test to fail."

There is actually a test that has a short attribute and should hit this code path, but it doesn't fail the test suite or seem to generate any warning:

​https://github.com/WordPress/wordpress-develop/blob/a6783cfbee4f72b38d67744c2a456b89dce626ab/tests/phpunit/tests/interactivity-api/wpInteractivityAPI-wp-bind.php#L368-L381

I can see the error when running tests. I might be valuable to add another test that handles the boolean value:

The same check needs to be added in another line which only will trigger when passing true:

if ( is_bool( $result ) && strlen( $bound_attribute ) > 5 && '-' === $bound_attribute[4] ) {

Good catch! I pushed a change to apply the same fix to that warning.

You also need a test like the one I shared to trigger the warning in the second updated line. The value needs to be true.

You also need a test like the one I shared to trigger the warning in the second updated line. The value needs to be true.

I've pushed a test for this, but I don't see the warnings when running the tests 😕

You also need a test like the one I shared to trigger the warning in the second updated line. The value needs to be true.

I've pushed a test for this, but I don't see the warnings when running the tests 😕

I pushed a test that does error if I revert the changes.

I'll address the test failures.

I fixed the tests and updated them to use assertSame. We're comparing values of types we expect to match, mostly strings, so assertSame is the appropriate assertion.

[swissspidy]

In 57835:

Interactivity API: Prevent warning when using a bind directive with a short attribute name.

Adds new tests and improves existing ones by using assertSame to do type comparison as well.

Props jonsurrell, cbravobernal, swissspidy, gziolo.
Fixes #60758.

Committed in https://core.trac.wordpress.org/changeset/57835

[swissspidy]

In 57837:

Interactivity API: Prevent warning when using a bind directive with a short attribute name.

Adds new tests and improves existing ones by using assertSame to do type comparison as well.

Reviewed by gziolo.
Merges [57835] to the to the 6.5 branch.

Props jonsurrell, cbravobernal, swissspidy, gziolo.
Fixes #60758.