search

Automattic / jetpack

Security, performance, marketing, and design tools — Jetpack is made by WordPress experts to make WP sites safer and faster, and help you grow your traffic.
https://jetpack.com/
Other
1.59k stars 798 forks source link

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

Closed chaselivingston closed 3 weeks ago

chaselivingston commented 6 years ago

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/

kellbot commented 6 years ago

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[bot] commented 6 years ago

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

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.

janboddez commented 5 years ago

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 commented 5 years ago

@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 commented 5 years ago

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 commented 4 years ago

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

jeherve commented 4 years ago

This was also reported here: https://wordpress.org/support/topic/can-i-bypass-jetpack-photon-for-a-specific-field/

jeherve commented 2 months ago

Noting that this also happens with spaces in image URLs.

Internal reference: p1725633694454789/1725633597.982689-slack-CDLH4C1UZ

github-actions[bot] commented 2 months ago

Support References

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

oskosk commented 2 months ago

@adnan can the team handle this one please?

oskosk commented 1 month ago

@haqadn re-ping

haqadn commented 1 month ago

@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 commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

@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 commented 3 weeks ago

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 commented 3 weeks ago

Yup, let's close it!