Make WordPress Core

Opened 4 years ago

Closed 3 years ago

#51839 closed defect (bug) (fixed)

When get_feed_permastruct() return false, get_feed_link() return incorrect non feed url

Reported by: hauvong's profile hauvong Owned by: johnbillion's profile johnbillion
Milestone: 5.7 Priority: normal
Severity: minor Version: 5.5
Component: Feeds Keywords: has-patch has-unit-tests commit
Focuses: Cc:


When $this->permalink_structure is empty & $this->feed_structure is not set, the function get_feed_permastruct() would return boolean false causing get_feed_link() return incorrect non-feed url.


Patch file to fix is attached.

Attachments (3)

class-wp-rewrite.php.patch (556 bytes) - added by hauvong 4 years ago.
51839.diff (534 bytes) - added by peterwilsoncc 4 years ago.
51839.2.diff (2.6 KB) - added by peterwilsoncc 3 years ago.

Download all attachments as: .zip

Change History (11)

#1 @peterwilsoncc
4 years ago

  • Keywords needs-unit-tests added
  • Milestone changed from Awaiting Review to 5.7

Thanks for the report, I've verified the issue. It was introduced in [47808] during the 5.5 release.

I'll move this to the 5.7 milestone. It would be good to include some tests for get_feed_link() when the permalink structure is undefined so I've added the needs-unit-tests keyword too.

4 years ago

#2 @peterwilsoncc
4 years ago

In 51839.diff I've moved the change to get_feed_link() rather than changing the return value and type of $wp_rewrite->get_feed_permastruct().

This is to avoid backward compatibility breaking for theme or plugin developers using the condition if ( $wp_rewrite->get_feed_permastruct() === false ) {}.

This ticket was mentioned in Slack in #core by hellofromtonya. View the logs.

3 years ago

This ticket was mentioned in Slack in #core by peterwilsoncc. View the logs.

3 years ago

#5 @peterwilsoncc
3 years ago

  • Keywords has-unit-tests added; needs-unit-tests removed

In 51839.2.diff :

  • Same change to get_feed_link() as earlier, using a truthy check rather than strict check
  • Unit tests x3 sets
    • plain permalinks
    • pretty links without a plain text prefix
    • pretty links with a plain text prefix

All looks to be passing but another set of eyes would be ideal.

#6 @SergeyBiryukov
3 years ago

  • Keywords commit added

Thanks @hauvong for catching the issue and @peterwilsoncc for the tests!

51839.2.diff looks good to me.

#7 @johnbillion
3 years ago

  • Owner set to johnbillion
  • Status changed from new to reviewing

Agreed and the tests fail as expected if the strict check is reintroduced.

#8 @johnbillion
3 years ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

In 50354:

Feeds: Fix the URL returned by get_feed_link() when pretty permalinks are not in use.

Props hauvong, peterwilsoncc, SergeyBiryukov

Fixes #51839

Note: See TracTickets for help on using tickets.