Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Photon: URLs with & in RSS feeds are not being encoded correctly #8321

Closed
chaselivingston opened this issue Dec 6, 2017 · 17 comments
Closed
Labels
Customer Report Issues or PRs that were reported via Happiness. aka "Happiness Request", or "User Report" [Feature] Photon aka "Image CDN". Feature developed in the Image CDN package and shipped in multiple plugins [Plugin] Boost A feature to speed up the site and improve performance. [Plugin] Jetpack Issues about the Jetpack plugin. https://wordpress.org/plugins/jetpack/ [Pri] Low Triaged [Type] Bug When a feature is broken and / or not performing as intended

Comments

@chaselivingston
Copy link
Contributor

Steps to reproduce the issue

  1. Try adding an extra parameter to a Photon URL via a function.
  2. Notice that the parameter is added in a query string with an & character.
  3. This is not encoded correctly and thus causes RSS feed validation errors.

Example feed with this issue: http://offbeatbride.com/feed/

@chaselivingston chaselivingston added [Feature] Photon aka "Image CDN". Feature developed in the Image CDN package and shipped in multiple plugins [Pri] Normal [Type] Bug When a feature is broken and / or not performing as intended labels Dec 6, 2017
@kellbot
Copy link

kellbot commented Dec 7, 2017

Just a heads up - this is no longer visible at http://offbeatbride.com/feed/. We had to disable the filter as a broken RSS feed is a showstopper for us. Thanks!

@stale
Copy link

stale bot commented Jun 29, 2018

This issue has been marked as stale. This happened because:

  • It has been inactive in the past 6 months.
  • It hasn’t been labeled `[Pri] Blocker`, `[Pri] High`.

No further action is needed. But it's worth checking if this ticket has clear reproduction steps and it is still reproducible. Feel free to close this issue if you think it's not valid anymore — if you do, please add a brief explanation.

@stale stale bot added the [Status] Stale label Jun 29, 2018
@janboddez
Copy link

janboddez commented Sep 10, 2019

My feed reader won't refresh feeds with unescaped ampersands in URLs. In one such, recent example, the invalid URL was the one inside the icon tags, ending in &ssl rather than &ssl. Can't image this is hard to fix.

@jeherve
Copy link
Member

jeherve commented Oct 11, 2019

@tw2113 reported a similar issue in #13722.

Annoyingly, I can't seem to reproduce the issue on my own site right now:
https://www.feedvalidator.org/check.cgi?url=https%3A%2F%2Fjeremy.hu%2Ffeed%2Fatom%2F

As you can see, the icon URL is encoded there:
<icon>https://i0.wp.com/jeremy.hu/wp-content/uploads/big-big-favicon-site_icon.png?fit=32%2C32&#038;quality=80&#038;ssl=1</icon>

@tw2113 Do you happen to use another plugin that may output things in the RSS feed on that site? I wonder if this may be a conflict with another plugin at play.

@tw2113
Copy link

tw2113 commented Oct 11, 2019

I can't say on other plugins part, as this wasn't found/demo'd using my own websites. I had noticed it as i tried to subscribe to others' websites and their atom rss feed. I could try to reach out to the site owner and see if they'd share their current plugin list.

The encoding part looks legit though, as my example one does NOT encode that last & like your personal one does. That'd be an interesting one to stack trace through.

@jeherve
Copy link
Member

jeherve commented Feb 26, 2020

Also reported in 2726497-zen with the feeds created by the PodLove plugin.

@matticbot matticbot added the Customer Report Issues or PRs that were reported via Happiness. aka "Happiness Request", or "User Report" label Feb 26, 2020
@jeherve
Copy link
Member

jeherve commented Apr 30, 2020

@jeherve
Copy link
Member

jeherve commented Sep 6, 2024

Noting that this also happens with spaces in image URLs.

Internal reference: p1725633694454789/1725633597.982689-slack-CDLH4C1UZ

Copy link
Contributor

github-actions bot commented Sep 6, 2024

Support References

This comment is automatically generated. Please do not edit it.

  • 2726497-zen

@oskosk
Copy link
Contributor

oskosk commented Sep 6, 2024

@adnan can the team handle this one please?

@jeherve jeherve added the Triaged label Sep 9, 2024
@jeherve jeherve added [Plugin] Jetpack Issues about the Jetpack plugin. https://wordpress.org/plugins/jetpack/ [Plugin] Boost A feature to speed up the site and improve performance. labels Sep 9, 2024
@oskosk
Copy link
Contributor

oskosk commented Sep 12, 2024

@haqadn re-ping

@haqadn
Copy link
Contributor

haqadn commented Sep 17, 2024

@jeherve, have you had any luck reproducing it? I tried to reproduce it but any URL in the feeds seem to be already encoding the URLs, and WordPress renames the files to replace the space when you upload them.

@jeherve
Copy link
Member

jeherve commented Sep 18, 2024

WordPress renames the files to replace the space when you upload them.

That's been my experience as well. I assumed that it didn't happen in some specific site configurations. That is the root of the issue on those sites, but I think ideally we'd account for that problem on our end, and support sites that have wrong filenames when possible.

@dilirity
Copy link
Member

dilirity commented Sep 27, 2024

Can I get an example of code that adds an & to the URL? I keep trying various filters, but I can't find the right one that changes the photon URL. Maybe the filter is not applied in the image cdn package?

I'm also only running Boost, not Jetpack.

EDIT:

I just noticed, that jetpack_photon_url is only applied present in Jetpack, so Boost can't use it if Jetpack isn't running.

@haqadn
Copy link
Contributor

haqadn commented Oct 1, 2024

@jeherve Can I get your review on Image CDN: URL encode path parts #39560?

It solves spaces and special characters in the file name. E.g. I found that the default mac screenshot contains a "Narrow non-breaking space" character which WP doesn't replace by default and causes an issue with RSS.

I couldn't reproduce the ampersand issue. It seems like it is already encoded. May be it was fixed during all this time?

@haqadn
Copy link
Contributor

haqadn commented Oct 9, 2024

So, we had two issues.

  1. Adding extra URL parameters to the URL broke RSS feeds
  2. Spaces in the URL would break RSS feeds.

We have covered # 2. # 1 is no longer reproducible, and I think it's probably fixed without noticing in the past. Shall we close this issue @jeherve ?

@jeherve
Copy link
Member

jeherve commented Oct 9, 2024

Yup, let's close it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Customer Report Issues or PRs that were reported via Happiness. aka "Happiness Request", or "User Report" [Feature] Photon aka "Image CDN". Feature developed in the Image CDN package and shipped in multiple plugins [Plugin] Boost A feature to speed up the site and improve performance. [Plugin] Jetpack Issues about the Jetpack plugin. https://wordpress.org/plugins/jetpack/ [Pri] Low Triaged [Type] Bug When a feature is broken and / or not performing as intended
Development

No branches or pull requests

9 participants