Skip to content

Merge: Fix support for feeds with XML preample + DTD, entities#37

Merged
Alkarex merged 3 commits intofreshrssfrom
merge-915
Jun 24, 2025
Merged

Merge: Fix support for feeds with XML preample + DTD, entities#37
Alkarex merged 3 commits intofreshrssfrom
merge-915

Conversation

@Alkarex
Copy link
Member

@Alkarex Alkarex commented Jun 23, 2025

Upstream: simplepie#915
Alternative of #35

Alternative of simplepie#914 with support for XML feeds using HTML entities

Simple feeds with XML preample + DTD are crashing (see example in test) due to regression from simplepie@162a3d3
Its implementation is buggy, as it creates a second DOCTYPE declaration even when there is an existing one, which is completely invalid.

Example of feed: https://blog.plover.com/index.rss
Downstream Issue: FreshRSS/FreshRSS#7514
Downstream PR: #35

Note that the culprit feature added support for html entities in xml seems to come from a misunderstanding of
https://www.rssboard.org/rss-encoding-examples
HTML entities are only allowed in XML when there is a DTD declaring them. SimplePie is even allowing Atom documents with undeclared HTML entities - which I am not sure is on purpose.

Downstream issue: FreshRSS/FreshRSS#7687

Alkarex added 3 commits June 24, 2025 01:00
Alternative of simplepie#914 with support for XML feeds using HTML entities

Simple feeds with XML preample + DTD are crashing (see example in test) due to regression from simplepie@162a3d3
Its implementation is buggy, as it creates a second DOCTYPE declaration even when there is an existing one, which is completely invalid.

Example of feed: https://blog.plover.com/index.rss
Downstream Issue: FreshRSS/FreshRSS#7514
Downstream PR: #35

Note that the culprit feature *added support for html entities in xml* seems to come from a misunderstanding of
https://www.rssboard.org/rss-encoding-examples
HTML entities are only allowed in XML when there is a DTD declaring them. SimplePie is even allowing Atom documents with undeclared HTML entities - which I am not sure is on purpose.

Downstream issue: FreshRSS/FreshRSS#7687
Copy link
Member

@Frenzie Frenzie left a comment

Choose a reason for hiding this comment

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

I'm not quite sure what to think but it shouldn't be any worse. :-)

@Alkarex Alkarex merged commit 4aed604 into freshrss Jun 24, 2025
20 checks passed
@Alkarex Alkarex deleted the merge-915 branch June 24, 2025 06:32
Alkarex added a commit to Alkarex/FreshRSS that referenced this pull request Jun 24, 2025
fix FreshRSS#7687

FreshRSS/simplepie#37
Upstream: simplepie/simplepie#915

Partial revert of FreshRSS#7515

HTML entities are normally only allowed in XML when there is a DTD declaring them. SimplePie is even allowing Atom documents with undeclared HTML entities - which I am not sure is on purpose.
Alkarex added a commit to FreshRSS/FreshRSS that referenced this pull request Jun 24, 2025
fix #7687

FreshRSS/simplepie#37
Upstream: simplepie/simplepie#915

Partial revert of #7515

HTML entities are normally only allowed in XML when there is a DTD declaring them. SimplePie is even allowing Atom documents with undeclared HTML entities - which I am not sure is on purpose.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants