fix: escape XML special characters in sitemap generation

[NathanDrake2406]

Summary

• sitemapToXml() interpolated user-supplied values directly into XML without escaping
• URLs with & (extremely common with query params like ?q=a&b=2) produced invalid XML
• Video titles/descriptions with <, >, ", or & also broke the XML structure
• Search engines silently reject malformed sitemaps, causing invisible SEO loss

Fix

Add escapeXml() helper that handles all five XML special characters (& < > " ') and apply it to every user-supplied string value in the sitemap serializer: URLs, alternate hrefs, hreflang values, image locs, video titles/descriptions/tags/URLs, uploader names/info, restriction/platform attributes.

Numeric fields (duration, view_count, rating) and controlled enums (family_friendly, live) are left unescaped since they can't contain XML-special characters.

This intentionally diverges from Next.js's resolveSitemap() which also raw-interpolates. The existing parity test "matches Next's raw interpolation for XML-sensitive values" has been updated to assert correct XML escaping instead.

Test plan

• New test: URLs with & produce & in <loc>, <image:loc>, and href attributes
• New test: video fields with &, <, >, " are properly escaped
• Updated existing XML-sensitive values test to expect correct escaping
• All 54 metadata-routes tests pass
• Sitemap-related shim tests pass
• CI: full Vitest + Playwright suite

[pkg-pr-new]

Open in StackBlitz

npm i https://pkg.pr.new/vinext@505

commit: d0873b7

[james-elicx]

/bigbonk review

[ask-bonk]

Solid fix. Unescaped & in sitemap URLs is a real-world problem that silently breaks XML parsing by search engines. The escapeXml() helper is correct (importantly, & is replaced first to avoid double-encoding), the coverage is thorough, and the intentional divergence from Next.js is well-documented in both code comments and the test name.

All 54 metadata-routes tests pass.

Two minor observations below — neither blocks merge.

[ask-bonk]

Nit: expiration_date and publication_date are typed string | Date. When they're strings, they're interpolated raw here. In practice these will always be ISO date strings (no XML-special chars), but for consistency with the rest of the function, you could escapeXml(String(...)) them too. Not a real risk — just a completeness note.

Same applies to changeFrequency (line 270) and lastModified string values (line 267), though those are even less likely to contain special characters since changeFrequency is a union type and lastModified goes through serializeDate.

[ask-bonk]

Nice that the test uses 'fr"CA' as a hreflang key to cover quote escaping in attributes. Consider adding an assertion that the hreflang attribute value itself is escaped:

Suggested change

	expect(xml).toContain('href="https://example.com/fr?a=1&b=2"');
	expect(xml).toContain('hreflang="fr"CA"');
	expect(xml).toContain('href="https://example.com/fr?a=1&b=2"');

Currently only the href is asserted, so the quote-in-hreflang escaping is tested implicitly (by the well-formedness check at line 416) but not explicitly.

[ask-bonk]

Review posted. Here's a summary:

Approved the PR with two minor non-blocking observations:

1. Date fields not escaped — expiration_date and publication_date are interpolated raw when they're strings. In practice these are always ISO date strings (no XML-special chars), but for consistency with the rest of the function they could be wrapped in escapeXml() too.

2. Missing hreflang assertion — The test cleverly uses 'fr"CA' as a hreflang key to exercise quote escaping in attributes, but only asserts the href value is escaped. Adding an explicit assertion on the hreflang attribute value would make the coverage more visible.

The core fix is correct: escapeXml() replaces & first (avoiding double-encoding), covers all five XML special characters, and is applied to every user-supplied string. The intentional divergence from Next.js's raw interpolation is well-justified and documented. All 54 tests pass.

github run