Make WordPress Core

Opened 6 weeks ago

Closed 5 weeks ago

#62083 closed defect (bug) (fixed)

Text_Diff::_check() throws a fatal error when $prevtype is null.

Reported by: hellofromtonya's profile hellofromTonya Owned by: hellofromtonya's profile hellofromTonya
Milestone: 6.7 Priority: normal
Severity: normal Version: 5.6
Component: External Libraries Keywords: has-patch has-unit-tests
Focuses: Cc:

Description

Text_Diff::_check() will throw a fatal error when $prevtype is null.

Fatal error: Uncaught Error: Class name must be a valid object or a string

instanceof requires the class name's term to be an object or string.

See it in action here https://3v4l.org/rQ0s4#veol.

Looking at the existing code (using the 6.6.2 tagged version as its the latest release as of this ticket),

$prevtype = null;
foreach ($this->_edits as $edit) {
    if ($edit instanceof $prevtype) {
        trigger_error("Edit sequence is non-optimal", E_USER_ERROR);
    }
    $prevtype = get_class($edit);
}

this means $prevtype must be an object or string. On the first loop, $prevtype is null.

For contextual history, [49194] changed from get_class() to instanceof.

Reference:

Change History (5)

#1 @hellofromTonya
6 weeks ago

  • Keywords has-patch has-unit-tests commit added
  • Owner set to hellofromTonya
  • Status changed from assigned to reviewing

Patch: https://github.com/WordPress/wordpress-develop/pull/7375/commits/5b0db944a16fdcc0db1c14a445715e1029d2c672

This specific commit in PR 7375 adds a test and fixes the bug. Props to @jrf for its discovery and the patch.

#2 @hellofromTonya
6 weeks ago

In 59070:

External Libraries: Skip instanceof check when null in Text_Diff::_check().

On the first foreach loop in Text_Diff::_check(), $prevtype is null. As instanceof` requires the class name term to be an object or string, a fatal error is thrown:

Fatal error: Uncaught Error: Class name must be a valid object or a string on line 279

This change:

  • Adds a simple test for the Text_Diff::_check() method, which is how the bug was discovered as the test could never pass with the code as-is.
  • Adds a defensive guard to protect against the fatal. It checks if $prevtype is not null as a pre-condition to for checking the instance. This bugfix also resolves the failing test.

Follow-up to [49194], [7747].

Props jrf, hellofromTonya.
See #62083.

#3 follow-up: @peterwilsoncc
5 weeks ago

  • Keywords commit removed

Ticket/report maintenance following the earlier commit. I've left it on the milestone, @hellofromTonya are you able to bump it if you don't think the rest of the patch will make it?

#4 in reply to: ↑ 3 ; follow-up: @jrf
5 weeks ago

Replying to peterwilsoncc:

Ticket/report maintenance following the earlier commit. I've left it on the milestone, @hellofromTonya are you able to bump it if you don't think the rest of the patch will make it?

The rest of the patch belong with another ticket (#62061), so I think this ticket can be closed (as solved).

#5 in reply to: ↑ 4 @peterwilsoncc
5 weeks ago

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

Replying to jrf:

The rest of the patch belong with another ticket (#62061), so I think this ticket can be closed (as solved).

Thanks Juliette, I appreciate the follow up.

Note: See TracTickets for help on using tickets.