The _filter_html_image_secure_process() filter in FilterHtmlImageSecure checks for image tags in content. Is short, it does this:

<?php
$html_dom = Html::load($text);
$images = $html_dom->getElementsByTagName('img');
foreach ($images as $image) {
  // Perform checks and replacements.
}
$text = Html::serialize($html_dom);
return $text;
?>

The Html::serialize is always performed, even if the content does not have images. This leads to double serializing when the 'Correct faulty and chopped off HTML' filter is enabled as well, which currently results in HTML like this: text. Note the duplicate xml:lang attributes. The double attribute tag bug is tackled in another issue (#1333730: [Meta] PHP DOM (libxml2) misinterprets HTML5), but while debugging #2346989: [PP-1] 'Restrict images to this site' leads to incorrect HTML with language attribute, I found this issue.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

BarisW created an issue. See original summary.

BarisW’s picture

And here is the patch that simply returns the source text if it does not contain an image tag.

Wim Leers’s picture

Version: 8.1.x-dev » 8.0.x-dev
Priority: Normal » Minor
Issue tags: +Quickfix, +Performance
+++ b/core/modules/filter/filter.module
@@ -773,6 +773,12 @@ function _filter_html_image_secure_process($text) {
+  // No further processing needed if the content does not contain images.

I think even the comment is unnecessary TBH :)

I also don't see why this could only go into 8.1 since this is an absolutely harmless optimization.

Wim Leers’s picture

Status: Needs review » Reviewed & tested by the community
alexpott’s picture

Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs tests

Seems like a test could be written considering core provides both filters. And I;m not sure what the patch does to fix the double serialising if both filters are enabled and there are images...

In fact if there are images I'm not sure how we can prevent the double serializing.

aerozeppelin’s picture

In this statement ,

  $images = $html_dom->getElementsByTagName('img');

$images would contain a 'DOMNodeList' object, so instead of

if (empty($images)) {
    return $text;
  }

it would be appropriate to check for

if (empty($images->length)) {
    return $text;
  }
heykarthikwithu’s picture

my ide is showing an error (Closing tag name missing) on filter.module file, line number 744. which is from this commit http://cgit.drupalcode.org/drupal/commit/?id=8fe50513608c519f36ee42f658f...

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

dimaro’s picture

Add test against 8.2.x on #5.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.4 was released on January 3, 2018 and is the final full bugfix release for the Drupal 8.4.x series. Drupal 8.4.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.5.0 on March 7, 2018. (Drupal 8.5.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.5.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Ivan Berezhnov’s picture

Issue tags: +CSKyiv18

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.6 was released on August 1, 2018 and is the final bugfix release for the Drupal 8.5.x series. Drupal 8.5.x will not receive any further development aside from security fixes. Sites should prepare to update to 8.6.0 on September 5, 2018. (Drupal 8.6.0-rc1 is available for testing.)

Bug reports should be targeted against the 8.6.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.8.x-dev

Drupal 8.6.x will not receive any further development aside from security fixes. Bug reports should be targeted against the 8.8.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.9.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.7 was released on June 3, 2020 and is the final full bugfix release for the Drupal 8.8.x series. Drupal 8.8.x will not receive any further development aside from security fixes. Sites should prepare to update to Drupal 8.9.0 or Drupal 9.0.0 for ongoing support.

Bug reports should be targeted against the 8.9.x-dev branch from now on, and new development or disruptive changes should be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.2.x-dev

Drupal 8 is end-of-life as of November 17, 2021. There will not be further changes made to Drupal 8. Bugfixes are now made to the 9.3.x and higher branches only. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.15 was released on June 1st, 2022 and is the final full bugfix release for the Drupal 9.3.x series. Drupal 9.3.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.4.x-dev branch from now on, and new development or disruptive changes should be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

larowlan’s picture

Still needs work for tests here

smustgrave’s picture

Status: Needs work » Postponed (maintainer needs more info)

After testing it out I was not seeing the double encoding. Think the issue summary needs to be updated.

Currently I don't think it's an issue. If there are images it loops through them. If not it doesn't.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.9 was released on December 7, 2022 and is the final full bugfix release for the Drupal 9.4.x series. Drupal 9.4.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.5.x-dev branch from now on, and new development or disruptive changes should be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

DanielVeza’s picture

Version: 9.5.x-dev » 10.1.x-dev

I was rerolling this because on the surface it does make sense to skip all the processing if there is no images.

I had one thought while I was doing it that if we aren't calling Html::serialize anymore, things like script won't be stripped anymore since that happens in the serialize function. So if there is a text format with only this filter enabled, scripts will get through.

In which case we could change the early return to be return Html::serialize($text);. But I'm not sure if thats worth doing since the code skipped by the early return would be minimal.

Interested in hearing thoughts.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.