Rss and atom feeds are dropping some characters

[pm24601]

Blog post with title:

& > test <

has the less than ( < ) is stripped from the atom and rss feed.

[redsweater]

Confirmed in WordPress trunk. The test case example loses the final "<" character, specifically the final output in the feed looks like:

<![CDATA[&amp; &gt; test ]]>

[Denis-de-Bernardy]

See also #9992

[peaceablewhale]

The PHP's built-in "strip_tags" function has caused the error. It also seems not necessary to have it applied to the_title_rss(). I have uploaded a patch to remove it.

[Denis-de-Bernardy]

trouble with the patch is the strip_tags() call might be there for a reason. isn't it possible to have html in a post title?

[Denis-de-Bernardy]

punting per IRC discussion

[peaceablewhale]

Yes. However, WordPress will escape the HTML (via the "ent2ncr" function) and therefore they are no longer HTML.

I think calling strip_tags is problmatic because an author can have a post title like "Use <h1> to <h6> for headings, <p> for paragraphy, but not formatting". strip_tags will remove the HTML unexpectedly.

[Denis-de-Bernardy]

some core dev feedback would be useful here

[peaceablewhale]

The priorities of ent2ncr are no longer modified by the patch. The new patch only removes the unnecessary 'strip_tags' filter.

[peaceablewhale]

Marking this report "needs-testing" in order to gain attention in the upcoming bug hunt.

[ryan]

There are other areas where we need a smarter strip. Let's figure it out in 3.0.

[dd32]

Punting due to impending beta

[peaceablewhale]

Patch updated for 3.1.

"& > test <" now works even without the patch, but "<test>This is a test</test>" not.

I still think there is no reason for having 'strip_tags' in feed title as they are escaped already.

[nacin]

Bumping once more.

[MikeHansenMe]

I was able to reproduce the problem in trunk (3.5-beta2-22252). Confirmed the patch fixes the problem.

[stevenkword]

Unit tests for the_title_rss

[stevenkword]

Errors still exists. Patch 9993-tests.diff adds test cases for both lone XML entities and XML tags.

[mdgl]

Replying to peaceablewhale:

I still think there is no reason for having 'strip_tags' in feed title as they are escaped already.

According to the RSS Best Practices Profile (​http://www.rssboard.org/rss-profile), the <title> element for both channels and items is plain text character data and so should not contain embedded HTML tags.

I also don't see how post titles are already "escaped".

[stevenkword]

This directly relates to #29621 in the Export component.

[stevenkword]

Replying to mdgl:

Replying to peaceablewhale:

I still think there is no reason for having 'strip_tags' in feed title as they are escaped already.

According to the RSS Best Practices Profile (​http://www.rssboard.org/rss-profile), the <title> element for both channels and items is plain text character data and so should not contain embedded HTML tags.

I also don't see how post titles are already "escaped".

I believe what @peaceablewhale is saying is that the filters applied to the the_title_rss include methods for escaping HTML. If an additional filter was added to the_title_rss that also escaped the disallowed XML characters, there would be no reason to strip the tags.

The string, <h1>, would be disallowed inside an XML element, but <h1> is valid even thought <h1> would be best.

The point is, properly escaped HTML is technically plain text. The only necessary addition would be filter to encode the disallowed characters.

[stevenkword]

Refresh @ 4.5.1

[stevenkword]

I'm standing behind (attachment:9993.2.patch​) as a reasonable approach for the time being. Since the output is also passed through esc_html(), it is not necessary to strip these tags. It also opens up the benefit of having parity between the web view and the feed content.

Unit tests, (attachment:9993-tests.diff​), are present and show failure before the patch is applied and success after.

[netweb]

@stevenkword Maybe add tests for Atom also?

[stevenkword]

Replying to netweb:

@stevenkword Maybe add tests for Atom also?

This might be a case where the naming conventions don't quite make sense. Both Atom and RSS feeds use the same the_title_rss() method in their respective templates. The only difference being that the Atom template wraps the output in CDATA ,which would be unnecessary, but not problematic after this change.

Ideally, since both templates use XML (the export component too), naming the method the_title_xml() would have been more appropriate. While we do have the ability to add unit tests for Atom as of 4.5, I think the tests proposed in this ticket cover those test cases. As a future part of adding test coverage for feeds, I would like to add a new common.php to /tests/feed/ to help make things more obvious.

[netweb]

Replying to stevenkword:

This might be a case where the naming conventions don't quite make sense. Both Atom and RSS feeds use the same the_title_rss() method in their respective templates. The only difference being that the Atom template wraps the output in CDATA ,which would be unnecessary, but not problematic after this change.

Ideally, since both templates use XML (the export component too), naming the method the_title_xml() would have been more appropriate. While we do have the ability to add unit tests for Atom as of 4.5, I think the tests proposed in this ticket cover those test cases. As a future part of adding test coverage for feeds, I would like to add a new common.php to /tests/feed/ to help make things more obvious.

Because the functionality is different isn't this even more reason to add unit tests for Atom?

Also looking and the_title_rss() is also used in src/wp-admin/includes/export.php#L426, sadly there are zero export unit tests at the moment, there is a patch adding export unit tests in #22435 but they are dependant on the new Export API enhancements in that ticket.

[ocean90]

The strip_tags() call exists since b2, see ​https://github.com/ericandrewlewis/b2/blame/e0de2a065d3e1d139fbb635e505e7fed5271954e/b2-include/b2template.functions.php#L269.
I think one of the reasons might be that titles can include HTML like <br> or <strong> for visual purposes and not as a part of the title. These shouldn't be displayed in a feed reader.

[stevenkword]

Replying to ocean90:

The strip_tags() call exists since b2, see ​https://github.com/ericandrewlewis/b2/blame/e0de2a065d3e1d139fbb635e505e7fed5271954e/b2-include/b2template.functions.php#L269.
I think one of the reasons might be that titles can include HTML like <br> or <strong> for visual purposes and not as a part of the title. These shouldn't be displayed in a feed reader.

How would you feel about str_replace() those explicity rather than running them through strip_tags()?

[ocean90]

Replying to stevenkword:

How would you feel about str_replace() those explicity rather than running them through strip_tags()?

Those were just two examples, it can be any HTML.

Maybe we can add another filter for the_title_rss which runs before strip_tags() and encodes < which are not part of a HTML tag as <.

[desrosj]

Found this going through a list of tickets missing a milestone.

Since it has been a good amount of time since this was last investigated, looking at the current state of this report in the context of today's code base would be a good first step to reviving.

I'm going to remove the ticket owner to make it more clear that no one is actively working on this one.

[redsweater]

I just tested against a WordPress.com blog (most convenient) and the bug is still there. It seems minor, but it would still be nice to fix. Probably still an issue in .org blogs as well.

[sukhendu2002]

Reproduction Report

Description

This report validates whether the issue can be reproduced.

Environment

• WordPress: 6.9-alpha-60093-src
• PHP: 8.2.29
• Server: nginx/1.29.0
• Database: mysqli (Server: 8.4.5 / Client: mysqlnd 8.2.29)
• Browser: Chrome 138.0.0.0
• OS: macOS
• Theme: Twenty Fifteen 4.0
• MU Plugins: None activated
• Plugins:
  • Hello Dolly 1.7.2
  • Test Reports 1.2.0

Actual Results

✅ Error condition does not occur .

• When creating a post with the title & > test <, the generated RSS/Atom feed encodes the < character as < (numeric entity), not as a missing or stripped character.
• The feed output is: `xml <title> Comments on: &#038; &gt; test &#060; </title> `
• The < character is present and correctly encoded, not stripped.

Additional Notes

• The original concern was that < was missing from the feed output. However, with the current filter configuration, < is encoded as <.

[redsweater]

For what it's worth, I just reproduced this with the latest trunk sources. I think that perhaps the test @sukhendu2002 performed made it appear that the bug is fixed because an entity encoding was created by the web admin interface when creating the post. When using the XMLRPC API, for example, to send a post to the blog, the "<" character is still allowed to reach the database, whereupon the bug can be observed.

I don't know that it's necessarily worth fixing after all these years, but the bug still exists.

[SirLouen]

Given that @redsweater has reproduced it, but has not provided exact instructions, I think it would be nice to have them

[redsweater]

As I mentioned, I'm not sure it's worth fixing this after all these years. Anybody who would work on a fix would probably be able to create arbitrary XML-RPC requests for testing at will, so the steps to reproduce are:

1. Establish a test blog e.g. at ​http://localhost:8889/
2. POST to the ​http://localhost:8889/xmlrpc.php endpoint with XML representing a new post request, where the title is "& > test <"
3. Using a newsreader or curl, access the feed for the site:

curl ​http://localhost:8889/feed/

Expected: the text within <title></title> tag on the post in the feed should include a "<" character at the end.
Actual: It's stripped.