Context Navigation

← Previous Ticket
Next Ticket →

#62083 closed defect (bug) (fixed)

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

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

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:

https://www.php.net/manual/en/language.operators.type.php

Change History (5)

#1 @hellofromTonya
22 months ago

Keywords has-patch has-unit-tests commit added
Owner set to hellofromTonya
Status assigned → 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
22 months 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: ↓ 4 @peterwilsoncc
22 months 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: ↓ 5 @jrf
22 months 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
22 months ago

Resolution → fixed
Status reviewing → 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.

Download in other formats:

Make WordPress Core

Context Navigation

#62083 closed defect (bug) (fixed)

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

Description

Change History (5)

#1 @hellofromTonya 22 months ago

#2 @hellofromTonya 22 months ago

#3 follow-up: ↓ 4 @peterwilsoncc 22 months ago

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

#5 in reply to: ↑ 4 @peterwilsoncc 22 months ago