Problem/Motivation

Discovered in #3270734: Update Editor + CKEditor 5 module to not use CKEditor 4 in tests there is an edge case that causes HTMLRestrictions::mergeAllowedElementsLevel() to fail when presented with a minimal test case of <ol type="1"> being merged into itself.

This was previously discovered and considered fixed in #3274648: HTMLRestrictions::merge() and ::toGeneralHtmlSupportConfig() fail on allowed attribute values that can be interpreted as integers but that only covers the case of <ol type="1"> being merged into something different; when both sides of the merge contain the "1" then the bug occurs.

This method is recursive. Eventually this gets recursed down into:

$array1['1'] = TRUE;
$array2['1'] = TRUE;

When called with these arguments, the method incorrectly returns [1, TRUE]. This is because array_keys($array1) casts the string key 1 to int!

php > var_dump(array_keys(['1' => TRUE]));
array(1) {
  [0]=>
  int(1)
}

And when we have a numeric key, array_merge() appends instead of overwriting keys, and so a corrupted array is produced.

Steps to reproduce

See test case.

Proposed resolution

Manually merge the arrays instead of using array_merge(), as it cannot be convinced to overwrite when numeric keys are present (even if they were once strings).

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#2 3305621-2.patch1.62 KBlongwave
#2 3305621-2-test-only.patch897 byteslongwave

Comments

longwave created an issue. See original summary.

longwave’s picture

Status: Active » Needs review
StatusFileSize
new897 bytes
new1.62 KB
longwave’s picture

Issue summary: View changes

The last submitted patch, 2: 3305621-2-test-only.patch, failed testing. View results

wim leers’s picture

Status: Needs review » Reviewed & tested by the community

👏

  • lauriii committed 1921b1b on 10.1.x
    Issue #3305621 by longwave, Wim Leers: HTMLRestrictions::...

  • lauriii committed 13884e1 on 10.0.x
    Issue #3305621 by longwave, Wim Leers: HTMLRestrictions::...

  • lauriii committed 5368b3d on 9.5.x
    Issue #3305621 by longwave, Wim Leers: HTMLRestrictions::...
lauriii’s picture

Status: Reviewed & tested by the community » Fixed

Committed 1921b1b and pushed to 10.1.x. Also cherry-picked to 10.0.x, 9.5.x, and 9.4.x. Thanks!

  • lauriii committed 06a12b8 on 9.4.x
    Issue #3305621 by longwave, Wim Leers: HTMLRestrictions::...

Status: Fixed » Closed (fixed)

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