Problem/Motivation

Warnings are logged when an RSS feed includes markup with an empty srcset attribute.

Steps to reproduce

  1. Create a node:
    • body: <source srcset=""/>
    • format: Full HTML
    • promote to front page
  2. View /rss.xml
  3. The following error message is logged:
    Warning: Uninitialized string offset 0 in Drupal\Component\Utility\Html::transformRootRelativeUrlsToAbsolute() (line 486 of /var/www/html/web/core/lib/Drupal/Component/Utility/Html.php)

Proposed resolution

Don't assume that the srcset attribute has a non-empty string — instead, check and skip the absolute-URL transformation if there is no URL.

Remaining tasks

Review.
Update title to state what is being fixed

User interface changes

n/a

API changes

n/a

Data model changes

n/a

Release notes snippet

n/a

Original report by mxmilkiib

I'm not sure if this is the right component to log this under. It's from https://libreav.org, the site is mostly about Feeds. I keep getting runs of this errors in the logs, but it's apparently not directly related to feed import times. It does say Rss though, but IDK!

Notice: Uninitialized string offset: 0 in Drupal\Component\Utility\Html::transformRootRelativeUrlsToAbsolute() (line 474 of /var/www/html/core/lib/Drupal/Component/Utility/Html.php)

#0 /var/www/html/core/includes/bootstrap.inc(305): _drupal_error_handler_real()
#1 /var/www/html/core/lib/Drupal/Component/Utility/Html.php(474): _drupal_error_handler()
#2 /var/www/html/core/lib/Drupal/Core/EventSubscriber/RssResponseRelativeUrlFilter.php(63): Drupal\Component\Utility\Html::transformRootRelativeUrlsToAbsolute()
#3 /var/www/html/core/lib/Drupal/Core/EventSubscriber/RssResponseRelativeUrlFilter.php(29): Drupal\Core\EventSubscriber\RssResponseRelativeUrlFilter->transformRootRelativeUrlsToAbsolute()
#4 [internal function]: Drupal\Core\EventSubscriber\RssResponseRelativeUrlFilter->onResponse()
#5 /var/www/html/core/lib/Drupal/Component/EventDispatcher/ContainerAwareEventDispatcher.php(142): call_user_func()
#6 /var/www/html/vendor/symfony/http-kernel/HttpKernel.php(191): Drupal\Component\EventDispatcher\ContainerAwareEventDispatcher->dispatch()
#7 /var/www/html/vendor/symfony/http-kernel/HttpKernel.php(179): Symfony\Component\HttpKernel\HttpKernel->filterResponse()
#8 /var/www/html/vendor/symfony/http-kernel/HttpKernel.php(80): Symfony\Component\HttpKernel\HttpKernel->handleRaw()
#9 /var/www/html/core/lib/Drupal/Core/StackMiddleware/Session.php(57): Symfony\Component\HttpKernel\HttpKernel->handle()
#10 /var/www/html/core/lib/Drupal/Core/StackMiddleware/KernelPreHandle.php(47): Drupal\Core\StackMiddleware\Session->handle()
#11 /var/www/html/core/modules/page_cache/src/StackMiddleware/PageCache.php(191): Drupal\Core\StackMiddleware\KernelPreHandle->handle()
#12 /var/www/html/core/modules/page_cache/src/StackMiddleware/PageCache.php(128): Drupal\page_cache\StackMiddleware\PageCache->fetch()
#13 /var/www/html/core/modules/page_cache/src/StackMiddleware/PageCache.php(82): Drupal\page_cache\StackMiddleware\PageCache->lookup()
#14 /var/www/html/modules/contrib/advban/src/AdvbanMiddleware.php(57): Drupal\page_cache\StackMiddleware\PageCache->handle()
#15 /var/www/html/core/modules/ban/src/BanMiddleware.php(50): Drupal\advban\AdvbanMiddleware->handle()
#16 /var/www/html/core/lib/Drupal/Core/StackMiddleware/ReverseProxyMiddleware.php(47): Drupal\ban\BanMiddleware->handle()
#17 /var/www/html/core/lib/Drupal/Core/StackMiddleware/NegotiationMiddleware.php(52): Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle()
#18 /var/www/html/modules/remove_http_headers/src/StackMiddleware/RemoveHttpHeadersMiddleware.php(49): Drupal\Core\StackMiddleware\NegotiationMiddleware->handle()
#19 /var/www/html/vendor/stack/builder/src/Stack/StackedHttpKernel.php(23): Drupal\remove_http_headers\StackMiddleware\RemoveHttpHeadersMiddleware->handle()
#20 /var/www/html/core/lib/Drupal/Core/DrupalKernel.php(706): Stack\StackedHttpKernel->handle()
#21 /var/www/html/index.php(19): Drupal\Core\DrupalKernel->handle()
#22 {main}

Issue fork drupal-3195583

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

mxmilkiib created an issue. See original summary.

mohit.bansal623’s picture

Status: Active » Needs review
StatusFileSize
new1.1 KB

Added a check for the same.

xamount’s picture

StatusFileSize
new35.76 KB

If you go to the culprit view and change the Show Content field to say use Title, you will not get the error. (This doesn't mean the problem is solved by doing this).

See screenshot: screenshot

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

Drupal 9.1.10 (June 4, 2021) and Drupal 9.2.10 (November 24, 2021) were the last bugfix releases of those minor version series. Drupal 9 bug reports should be targeted for the 9.3.x-dev branch from now on, and new development or disruptive changes should be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

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.

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.

smustgrave’s picture

Status: Needs review » Needs work
Issue tags: +Needs Review Queue Initiative, +Needs tests, +Needs issue summary update

This issue is being reviewed by the kind folks in Slack, #needs-review-queue-initiative. We are working to keep the size of Needs Review queue [2700+ issues] to around 400 (1 month or less), following Review a patch or merge request as a guide.

As this is a bug it will need a test case to show the issue.

Also an issue summary update for steps to reproduce, proposed solution, remaining tasks, etc. Should follow the default template.

ericdsd’s picture

Hi i experience the same issue on core 9.5.9 and it also generates large error messages that get stored in session table (when trying to debug) which can generate exceding max_allowed_packet error when querying session table on common mysql config.

smokris’s picture

Issue summary: View changes

Updated issue summary.

smokris’s picture

StatusFileSize
new1.29 KB

Test-only patch; should fail.

smokris’s picture

Version: 9.5.x-dev » 10.1.x-dev
Status: Needs work » Needs review
StatusFileSize
new2.29 KB

Fix + test; should pass.

smokris’s picture

Issue summary: View changes

The last submitted patch, 10: 3195583-test-only.patch, failed testing. View results

smustgrave’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs tests, -Needs issue summary update
StatusFileSize
new340.88 KB

test seems to show the issue pretty clearly.

Followed the steps from the issue summary and definitely see the issue

issue

Patch #11 does solve the issue.

quietone’s picture

Version: 10.1.x-dev » 11.x-dev
Status: Reviewed & tested by the community » Needs work

I am doing triage on the core RTBC queue.

The issue summary and proposed resolution are clear and complete. Thank you, that really helps the reviewers and committers. I read the comments this still needs a code review. Setting to Needs review.

I tested this on Drupal 11.x, using the steps in the issue summary and reproduced that problem.
I then took a look at the patch.

+++ b/core/lib/Drupal/Component/Utility/Html.php
@@ -471,8 +471,11 @@ public static function transformRootRelativeUrlsToAbsolute($html, $scheme_and_ho
+          if (isset($image_candidate_string[0]) && isset($image_candidate_string[1])) {

Since this is really just checking for an empty string this could be much simpler.

        $image_candidate_strings = array_filter(array_map('trim', $image_candidate_strings));
        foreach ($image_candidate_strings as $key => $image_candidate_string) {
          if ($image_candidate_string[0] === '/' && $image_candidate_string[1] !== '/') {
            $image_candidate_strings[$key] = $scheme_and_host . $image_candidate_string;
          }

Now, setting to Needs work.

quietone’s picture

Issue summary: View changes

This also needs a title to say what is being fixed here.

sourabhjain’s picture

Status: Needs work » Needs review
StatusFileSize
new2.79 KB

Rerolled the patch against 11.x and updated it as suggested in #15.

smustgrave’s picture

Status: Needs review » Needs work
Issue tags: +Needs title update

Per #16

smokris’s picture

Title: Uninitialized string offset: 0, Html::transformRootRelativeUrlsToAbsolute(), Html.php:474 » Fix warning when RSS feed includes markup with an empty srcset attribute
Status: Needs work » Needs review
Issue tags: -Needs title update

Updated issue title, as requested.

smustgrave’s picture

Status: Needs review » Reviewed & tested by the community

Thanks!

xjm’s picture

Status: Reviewed & tested by the community » Needs work

Patch does not apply. Thanks!

smokris’s picture

Status: Needs work » Needs review
StatusFileSize
new2.63 KB

Re-rolled for current 11.x-dev.

smustgrave’s picture

Status: Needs review » Needs work

Patch failed to apply.

sourabhjain’s picture

Assigned: Unassigned » sourabhjain

Let me work on it.

sourabhjain’s picture

Assigned: sourabhjain » Unassigned
Status: Needs work » Needs review
StatusFileSize
new2.77 KB

Updated the patch as per latest 11.x version.
Please review.

smustgrave’s picture

Status: Needs review » Reviewed & tested by the community
StatusFileSize
new2.83 KB

#25 didn't apply either.

I just went ahead and rerolled it

smokris’s picture

Status: Reviewed & tested by the community » Needs review
StatusFileSize
new2.76 KB
new722 bytes

Thanks. #26 has an unnecessary variable assignment which wasn't present in #17. I've attached a modified patch that removes that line (and an interdiff).

smokris’s picture

Status: Needs review » Reviewed & tested by the community

(Unintentionally changed issue status. Sorry for the noise.)

smustgrave’s picture

lets see what the tests say but that line I believe was new to the existing code. But if not needed then good cleanup.

smokris’s picture

Assigned: Unassigned » smokris
Status: Reviewed & tested by the community » Needs work

Both patches failed. Investigating…

smokris’s picture

Assigned: smokris » Unassigned
Status: Needs work » Reviewed & tested by the community
StatusFileSize
new1.4 KB
new2.64 KB
new1.45 KB

Revised test data since the transformation now removes empty attribute values.

needs-review-queue-bot’s picture

Status: Reviewed & tested by the community » Needs work

The Needs Review Queue Bot tested this issue.

While you are making the above changes, we recommend that you convert this patch to a merge request. Merge requests are preferred over patches. Be sure to hide the old patch files as well. (Converting an issue to a merge request without other contributions to the issue will not receive credit.)

smustgrave’s picture

Status: Needs work » Reviewed & tested by the community

Think this was a false positive by the bot.

smustgrave’s picture

Status: Reviewed & tested by the community » Needs work

Nope needs an MR now and patches hidden.

smokris’s picture

Status: Needs work » Reviewed & tested by the community
xjm’s picture

I triggered the test-only job for this on GitLab CI. Results:

There was 1 error:
1) Drupal\Tests\Component\Utility\HtmlTest::testTransformRootRelativeUrlsToAbsolute with data set "weepwas0, empty srcset" ('<weepwas0 srcset>empty test</weepwas0>', 'http://example.com', false)
Uninitialized string offset 0
/builds/issue/drupal-3195583/core/lib/Drupal/Component/Utility/Html.php:490
/builds/issue/drupal-3195583/core/tests/Drupal/Tests/Component/Utility/HtmlTest.php:345
/builds/issue/drupal-3195583/vendor/phpunit/phpunit/src/Framework/TestResult.php:728
/builds/issue/drupal-3195583/vendor/phpunit/phpunit/src/Framework/TestSuite.php:684
/builds/issue/drupal-3195583/vendor/phpunit/phpunit/src/Framework/TestSuite.php:684
/builds/issue/drupal-3195583/vendor/phpunit/phpunit/src/TextUI/TestRunner.php:651
/builds/issue/drupal-3195583/vendor/phpunit/phpunit/src/TextUI/Command.php:144
/builds/issue/drupal-3195583/vendor/phpunit/phpunit/src/TextUI/Command.php:97
xjm’s picture

Saving issue credits.

  • xjm committed 217797fb on 11.x
    Issue #3195583 by smokris, smustgrave, mohit.bansal623, xjm, quietone:...

  • xjm committed 9383c598 on 10.2.x
    Issue #3195583 by smokris, smustgrave, mohit.bansal623, xjm, quietone:...
xjm’s picture

Version: 11.x-dev » 10.1.x-dev
Status: Reviewed & tested by the community » Needs work

Committed to 11.x and 10.2.x, thanks!

This is also eligible for backport to 10.1.x as a non-disruptive bugfix; however, it does not cherry-pick cleanly (nor does #31 apply to 10.1.x). Marking "Needs work" for a 10.1.x backport of the final version of the patch to be created. Thanks!

xjm’s picture

Status: Needs work » Patch (to be ported)

Better status.

smokris’s picture

Status: Patch (to be ported) » Reviewed & tested by the community

  • xjm committed 6f456ae2 on 10.1.x
    Issue #3195583 by smokris, smustgrave, mohit.bansal623, xjm, quietone:...
xjm’s picture

Status: Reviewed & tested by the community » Fixed

Committed to 10.1.x. Thanks @smokris!

Status: Fixed » Closed (fixed)

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