Skip to content

SimplePie use single constant for default HTTP Accept header#5083

Merged
Alkarex merged 4 commits intoFreshRSS:edgefrom
Alkarex:fix-simplepie-http-accept
Mar 4, 2023
Merged

SimplePie use single constant for default HTTP Accept header#5083
Alkarex merged 4 commits intoFreshRSS:edgefrom
Alkarex:fix-simplepie-http-accept

Conversation

@Alkarex
Copy link
Member

@Alkarex Alkarex commented Feb 8, 2023

Follow-up of simplepie/simplepie@5d966b9

  • Use single constant for default SimplePie HTTP Accept
  • Missing headers in SimplePie_Locator::body()

Fix #5079 (comment)
The */* breaks Mastodon content negotiation

And add missing headers in `SimplePie_Locator::body()`
Follow-up of simplepie/simplepie@5d966b9
Fix FreshRSS#5079 (comment)
The `*/*` breaks Mastodon content negotiation
@Alkarex Alkarex added SimplePie 🍰 Feed problem 🗞️ Feeds that have issues while loading/reading in FreshRSS labels Feb 8, 2023
@Alkarex Alkarex added this to the 1.21.0 milestone Feb 8, 2023
class SimplePie_File
{

const DEFAULT_HTTP_ACCEPT = 'application/atom+xml, application/rss+xml, application/rdf+xml;q=0.9, application/xml;q=0.8, text/xml;q=0.7, text/html;q=0.6, text/plain;q=0.5, application/octet-stream;q=0.1';
Copy link
Member

@Frenzie Frenzie Feb 8, 2023

Choose a reason for hiding this comment

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

Hold on, are you absolutely sure you want to remove */*? If Mastodon doesn't work that's Mastodon's problem. That doesn't mean we should break everything.

Copy link
Member

Choose a reason for hiding this comment

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

Also I'd like to emphasize there's nothing "fix" about "work around Mastodon's problem".

Copy link
Member Author

@Alkarex Alkarex Feb 8, 2023

Choose a reason for hiding this comment

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

The thing is that we are not able to read other things, so we are actually lying when saying */*.
But I do believe there is a bug in Mastodon, and I might try to have a look (but not in the coming days).
We can postpone to 1.22 to give it a longer try in edge.

Copy link
Member

@Frenzie Frenzie Feb 8, 2023

Choose a reason for hiding this comment

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

How I've interpreted this header for the past decade is that the idea is to tell the server to send over its stuff and we'll give it a go, because it might be XML we can handle with some random incorrect mimetype.

Imho it's far from obvious that we should give broken Mastodon preference, because broken server configurations are usually just a minor user mistake of some sort in e.g. Apache or Nginx which are otherwise not broken.

Copy link
Member Author

Choose a reason for hiding this comment

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

I have reverted the change of HTTP Accept for now, especially since there seems to be an easy fix for Mastodon (only some versions seem broken), by being explicit about the RSS feed address. Example: https://framapiaf.org/@davidrevoy.rss

@Alkarex
Copy link
Member Author

Alkarex commented Feb 8, 2023

Partial upstream PR: simplepie/simplepie#784

@Alkarex Alkarex modified the milestones: 1.22.0, 1.21.0 Feb 8, 2023
@Alkarex Alkarex modified the milestones: 1.21.0, 1.22.0 Feb 9, 2023
@Alkarex Alkarex modified the milestones: 1.22.0, 1.21.0 Mar 4, 2023
@Alkarex Alkarex changed the title Fix SimplePie HTTP Accept SimplePie use single constant for default HTTP Accept header Mar 4, 2023
@Alkarex Alkarex merged commit 32acd6c into FreshRSS:edge Mar 4, 2023
@Alkarex Alkarex deleted the fix-simplepie-http-accept branch March 4, 2023 10:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Feed problem 🗞️ Feeds that have issues while loading/reading in FreshRSS SimplePie 🍰

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants