#65785 closed enhancement (fixed)
No safe way to add XML namespaces to feeds
| Reported by: | pfefferle | Owned by: | westonruter |
|---|---|---|---|
| Priority: | normal | Milestone: | 7.2 |
| Component: | Feeds | Version: | 2.8 |
| Severity: | normal | Keywords: | has-patch has-unit-tests commit |
| Cc: | Focuses: |
Description (last modified by )
rss2_ns and atom_ns are echo-only actions. Callbacks print their attributes directly into the <rss> / <feed> element, so a callback can not know what was already printed. When two callbacks add the same namespace, the element ends up with a duplicate attribute:
<rss version="2.0" xmlns:source="http://source.scripting.com/" xmlns:source="http://source.scripting.com/" >
A duplicate attribute is a well-formedness error in XML, and conforming parsers treat it as fatal. Some feed readers recover from it, but every strict consumer refuses the whole feed: browsers that render the feed as XML, and everything based on XSLT, which needs a proper XML parse before it can transform.
XML Parsing Error: duplicate attribute
This is easy to hit: two plugins (or a plugin and a theme) that support the same standard will add the same namespace, and both do nothing wrong. There is no API to check for it, so the only defensive option is to buffer the complete action output. This works, but it is more a hack than a solution, and it only helps if everyone does it.
Maybe we could add a filter with the prefix as key, next to the existing actions:
<?php $namespaces = apply_filters( "{$type}_ns_list", array() ); foreach ( $namespaces as $prefix => $uri ) { printf( ' xmlns:%s="%s"', sanitize_key( $prefix ), esc_url( $uri ) ); } do_action( "{$type}_ns" );
With the prefix as key, duplicates would not be possible anymore, and the actions could stay for backwards compatibility. The same would work for rdf_ns, rss2_comments_ns and atom_comments_ns.
Change History (13)
This ticket was mentioned in โPR #12812 on โWordPress/wordpress-develop by โ@pfefferle.
6 weeks ago
#2
- Keywords has-patch has-unit-tests added
The rss2_ns and atom_ns actions are echo-only, so two plugins that add the same namespace produce a duplicate attribute, and the whole feed becomes invalid XML.
This adds get_feed_namespaces() and feed_namespaces() (named after feed_content_type()) and a wp_feed_namespaces filter. Namespaces are keyed by their prefix, so the same namespace can not be printed twice. The default namespaces of the templates moved into the filtered list, otherwise a plugin could still duplicate them. The *_ns actions stay untouched for backwards compatibility.
What it does not solve: a plugin that still echoes a namespace via the old actions can produce a duplicate, like before. That would need output buffering around the actions, and I did not want to go there.
#3
@
6 weeks ago
- Keywords has-patch has-unit-tests removed
Hey @abcd95 I also have prepared a PR. Will also push it right now.
#5
@
5 weeks ago
- Component General โ Feeds
- Description modified (diff)
- Milestone Awaiting Review โ Future Release
โ@westonruter commented on โPR #12812:
5 weeks ago
#6
This is looking good to me. My only question is whether we should prefix the functions with wp_. There is wp_title_rss() already. The new wp_feed_namespaces filter has it too. So why not wp_feed_namespaces() and wp_get_feed_namespaces() too?
โ@pfefferle commented on โPR #12812:
3 weeks ago
#7
@westonruter I think both are fine. I was using the pattern of feed_content_type and get_feed_build_date. But I also was not consistent and might have to rename the filter if we keep it without wp_.
โ@pfefferle commented on โPR #12812:
3 weeks ago
#8
@westonruter do you think we can ignore URNs for now? โhttps://github.com/WordPress/wordpress-develop/pull/12812#discussion_r3731205335
โ@westonruter commented on โPR #12812:
2 weeks ago
#9
@westonruter do you think we can ignore URNs for now? โ#12812 (comment)
@pfefferle Sorry for the delay. โYes!
โ@pfefferle commented on โPR #12812:
2 weeks ago
#10
Sorry for the delay.
No problem :)
![(please configure the [header_logo] section in trac.ini)](/chrome/site/your_project_logo.png)
Hey there! ๐
Introducing a new associative array-based filter (e.g.,
{$type}_ns_list) is the best path forward:[ 'prefix' => 'uri' ]inherently deduplicates prefixes.sanitize_key()andesc_url()) before printing.do_action( "{$type}_ns" ).This aligns perfectly with how we already securely construct dynamic attributes in other areas (like filtering
body_classbefore output). I'll attach a patch for this.