Skip to content

Revisions: Avoid comparison timeouts for very long lines (66095) #14540

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ArkaPrabhaChowdhury wants to merge 1 commit into
base: trunk
Choose a base branch
from
Open

Revisions: Avoid comparison timeouts for very long lines (66095) #14540

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions src/wp-includes/class-wp-text-diff-renderer-table.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -305,6 +305,11 @@ public function _changed( $orig, $final ) { // phpcs:ignore Universal.NamingConv
// Compute word diffs for each matched pair using the inline diff.
foreach ( $orig_matches as $o => $f ) {
if ( is_numeric( $o ) && is_numeric( $f ) ) {
// Avoid expensive word-level diffs for long lines; show the complete lines instead.
if ( strlen( $orig[ $o ] ) > 10000 || strlen( $final[ $f ] ) > 10000 ) {
continue;
}

$text_diff = new Text_Diff( 'auto', array( array( $orig[ $o ] ), array( $final[ $f ] ) ) );
$renderer = new $this->inline_diff_renderer();
$diff = $renderer->render( $text_diff );
Expand Down
102 changes: 101 additions & 1 deletion tests/phpunit/tests/diff/wpTextDiffRendererTable.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,115 @@ class Tests_Diff_WpTextDiffRendererTable extends WP_UnitTestCase {

public static function set_up_before_class() {
parent::set_up_before_class();
require_once ABSPATH . 'wp-includes/Text/Diff.php';
require_once ABSPATH . 'wp-includes/Text/Diff/Renderer.php';
require_once ABSPATH . 'wp-includes/class-wp-text-diff-renderer-table.php';
require_once ABSPATH . 'wp-includes/Text/Diff/Renderer/inline.php';
require_once ABSPATH . 'wp-includes/Text/Exception.php';
require_once ABSPATH . 'wp-includes/wp-diff.php';
}

public function set_up() {
parent::set_up();
$this->diff_renderer_table = new WP_Text_Diff_Renderer_Table();
}

/**
* Long lines should remain complete without expensive word-level highlighting.
*
* @ticket 66095
* @dataProvider data_long_changed_lines
* @covers WP_Text_Diff_Renderer_Table::_changed
*
* @param string $orig Original line.
* @param string $revised Revised line.
*/
public function test_long_changed_lines_use_line_level_diff( $orig, $revised ) {
foreach ( array( true, false ) as $show_split_view ) {
$renderer = new WP_Text_Diff_Renderer_Table( array( 'show_split_view' => $show_split_view ) );
$actual = $renderer->_changed( array( $orig ), array( $revised ) );

$this->assertStringContainsString( $renderer->deletedLine( htmlspecialchars( $orig ) ), $actual );
$this->assertStringContainsString( $renderer->addedLine( htmlspecialchars( $revised ) ), $actual );
$this->assertStringNotContainsString( '<ins>', $actual );
$this->assertStringNotContainsString( '<del>', $actual );
}
}

/**
* @return array[]
*/
public function data_long_changed_lines() {
return array(
'long original line' => array( str_repeat( 'a', 9995 ) . ' old!!', str_repeat( 'a', 9995 ) . ' new!' ),
'long revised line' => array( str_repeat( 'a', 9995 ) . ' old!', str_repeat( 'a', 9995 ) . ' new!!' ),
'long HTML lines' => array( str_repeat( '<p>a & b</p>', 22000 ), str_repeat( '<p>b & a</p>', 21000 ) ),
'multibyte lines' => array( str_repeat( '日本語', 1200 ) . ' old', str_repeat( '日本語', 1200 ) . ' new' ),
);
}

/**
* @ticket 66095
* @covers WP_Text_Diff_Renderer_Table::_changed
*/
public function test_changed_lines_at_length_limit_keep_word_highlighting() {
$prefix = str_repeat( 'a', 9996 ) . ' ';
$actual = $this->diff_renderer_table->_changed( array( $prefix . 'old' ), array( $prefix . 'new' ) );

$this->assertStringContainsString( '<del>old</del>', $actual );
$this->assertStringContainsString( '<ins>new</ins>', $actual );
}

/**
* @ticket 66095
* @covers WP_Text_Diff_Renderer_Table::_changed
*/
public function test_long_lines_do_not_disable_highlighting_for_other_lines() {
$long_line = str_repeat( 'a', 10001 );
$actual = wp_text_diff(
$long_line . " old\nUnchanged context\nA short old line",
$long_line . " new\nUnchanged context\nA short new line"
);

$this->assertStringContainsString( $long_line . ' old', $actual );
$this->assertStringContainsString( $long_line . ' new', $actual );
$this->assertStringContainsString( 'Unchanged context', $actual );
$this->assertStringContainsString( '<del>old</del>', $actual );
$this->assertStringContainsString( '<ins>new</ins>', $actual );
}

/**
* @ticket 66095
* @covers WP_Text_Diff_Renderer_Table::_changed
*/
public function test_revisions_with_long_lines_can_be_compared_and_restored() {
require_once ABSPATH . 'wp-admin/includes/revision.php';

$original = '<div>' . str_repeat( 'a b ', 60000 ) . '</div>';
$revised = '<div>' . str_repeat( 'b a ', 57000 ) . '</div>';
$post_id = self::factory()->post->create(
array(
'post_type' => 'page',
'post_content' => $original,
)
);
$revision_id = _wp_put_post_revision( $post_id );
wp_update_post(
array(
'ID' => $post_id,
'post_content' => $revised,
)
);

$diffs = wp_get_revision_ui_diff( $post_id, $revision_id, $post_id );
$content_diff = wp_list_filter( $diffs, array( 'id' => 'post_content' ) );
$content_diff = reset( $content_diff );

$this->assertStringContainsString( htmlspecialchars( $original ), $content_diff['diff'] );
$this->assertStringContainsString( htmlspecialchars( $revised ), $content_diff['diff'] );
$this->assertSame( $post_id, wp_restore_post_revision( $revision_id ) );
$this->assertSame( $original, get_post_field( 'post_content', $post_id ) );
}

/**
* @dataProvider data_compat_fields
* @ticket 58898
Expand Down
Loading