Skip to content

Widgets: Prevent an infinite loop in wp_widget_rss_process(). - #15892

Open
rejaulalomkhan wants to merge 1 commit into
WordPress:trunkfrom
rejaulalomkhan:fix/62341-rss-process-link
Open

rejaulalomkhan wants to merge 1 commit into
WordPress:trunkfrom
rejaulalomkhan:fix/62341-rss-process-link

Conversation

@rejaulalomkhan

Copy link
Copy Markdown
Member

Trac ticket: Core-62341

Description

wp_widget_rss_process() removes characters from the start of a feed's channel link until the link starts with http. If the link has no http in it, the string becomes empty and the while loop never ends, so the request runs until PHP's execution time limit.

On the ticket, abcd95 couldn't reproduce this because esc_url() adds http:// to a link with no scheme. It doesn't do that for other cases:

  • A channel link with another allowed scheme, such as <link>ftp://example.com/</link>, stays as it is and reaches the loop.
  • A channel with no <link> element makes get_permalink() return null, which becomes an empty string. On PHP 8.1 and later, strip_tags() also raises a deprecation notice for the null.

This adds the ! empty( $link ) guard that the matching loops in wp_widget_rss_output() and WP_Widget_RSS::widget() already have, and casts the permalink to a string. The function now returns an empty link for these feeds.

Not changed here: WP_Widget_RSS::widget() also passes get_permalink() to strip_tags() without a cast, so it can raise the same deprecation notice.

Testing Instructions

  1. Run the new test:
    vendor/bin/phpunit --filter test_wp_widget_rss_process_channel_link
    On trunk, the FTP URL data set never finishes, so use a timeout there, for example timeout 30 vendor/bin/phpunit --filter 'test_wp_widget_rss_process_channel_link@FTP URL'. The no link element data set errors on the strip_tags() deprecation.
  2. Run the related groups:
    vendor/bin/phpunit --group widgets
    vendor/bin/phpunit --group feed
    vendor/bin/phpunit -c tests/phpunit/multisite.xml --group widgets

Results on PHP 8.3: widgets 508 tests and 2019 assertions, feed 134 and 510, multisite widgets 508 and 2019, all passing. widgets also passes with --order-by=reverse. PHPCS is clean on both files, and PHPStan reports no errors on widgets.php.

Trac ticket: https://core.trac.wordpress.org/ticket/62341


This Pull Request is for code review only. Please keep all other discussion to the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

Use of AI Tools

AI assistance: Yes
Tool(s): Claude Code
Model(s): Claude Opus 5.5
Used for: reproducing the bug, drafting the fix and test, running the checks listed above, and drafting this description. I reviewed every line and take responsibility for the change.

The loop that trims a feed's channel link never ends when the link does not contain `http`, such as an FTP link or a channel without a link. Add the guard the other RSS widget loops already use, and cast the permalink to a string since it can be null.

Fixes #62341.
@github-actions

Copy link
Copy Markdown

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs.

Core Committers: Use this line as a base for the props when committing in SVN:

Props rejaulalomkhan.

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.


Props Bot commands

You can trigger Props Bot actions by commenting on this PR:

  • @propsbot add <username> to give props to a GitHub user. Requires write access.
  • @propsbot remove <username> to leave a user out of the props. Requires write access.
  • @propsbot refresh to refresh this list. Adding the props-bot label works too.

@github-actions

Copy link
Copy Markdown

Test using WordPress Playground

The changes in this pull request can previewed and tested using a WordPress Playground instance.

WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser.

Some things to be aware of

  • All changes will be lost when closing a tab with a Playground instance.
  • All changes will be lost when refreshing the page.
  • A fresh instance is created each time the link below is clicked.
  • Every time this pull request is updated, a new ZIP file containing all changes is created. If changes are not reflected in the Playground instance,
    it's possible that the most recent build failed, or has not completed. Check the list of workflow runs to be sure.

For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation.

Test this pull request with WordPress Playground.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant