Skip to content

fix: sitemap content-type check breaks on content-type parameters#2442

Merged
barjin merged 3 commits intomasterfrom
fix/sitemap-content-type
May 6, 2024
Merged

fix: sitemap content-type check breaks on content-type parameters#2442
barjin merged 3 commits intomasterfrom
fix/sitemap-content-type

Conversation

@barjin
Copy link
Member

@barjin barjin commented May 3, 2024

According to the RFC1341, the Content-type header can contain additional string parameters.

This is sometimes used to communicate the encoding of the content (or other stuff(?)), like in text/plain; charset=utf-8.

The current sitemap implementation didn't expect this and skipped perfectly good sitemaps.

@barjin barjin added bug Something isn't working. adhoc Ad-hoc unplanned task added during the sprint. labels May 3, 2024
@barjin barjin requested a review from janbuchar May 3, 2024 13:24
@barjin barjin self-assigned this May 3, 2024
@github-actions github-actions bot added this to the 88th sprint - Tooling team milestone May 3, 2024
@github-actions github-actions bot added the t-tooling Issues with this label are in the ownership of the tooling team. label May 3, 2024
@janbuchar
Copy link
Contributor

Can we adopt https://www.npmjs.com/package/whatwg-mimetype and be done with this? It gives you a isXml method, which is all we ever wanted.

Copy link
Contributor

@janbuchar janbuchar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@barjin barjin merged commit db7d372 into master May 6, 2024
@barjin barjin deleted the fix/sitemap-content-type branch May 6, 2024 13:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

adhoc Ad-hoc unplanned task added during the sprint. bug Something isn't working. t-tooling Issues with this label are in the ownership of the tooling team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants