Downloading remote images doesn't work with protocol-less srcs

pr-welcome

(Leo McArdle) #1

The image above, embedded with ![Long](//discourse-meta.s3-us-west-1.amazonaws.com/original/3X/9/a/9a3bc7ce270b38e31e0c1425043b3e51539e3b3a.jpg) will be downloaded locally, and the image src will be replaced with the new url.

The image below, embedded with ![cat](//discourse-meta.s3-us-west-1.amazonaws.com/original/3X/7/6/7685d7c6a3ecc9df487f7e4c32d3cb4ba571b032.jpg) won’t, because of the bug.

What happens is the image is correctly downloaded locally, but then the url replacement won’t work, because the code is looking for a src with http: prepended to it.

(I’m going to look like a right numpty if this demonstration doesn’t work, but anyway, there’s a fix here: FIX: Handle img src starting with “//” in pull_hotlinked_images job by LeoMcA · Pull Request #4643 · discourse/discourse · GitHub)


(Jeff Atwood) #2

Seems to work fine?

![cat](//i.imgur.com/7Z8brhn.jpg)


(Rafael dos Santos Silva) #3

@LeoMcA imgur won’t work to prove that here on meta, try with another hosting service.


(Leo McArdle) #4

Aaaah, cheers! The first post now shows the bug, after moving to embedding from the Mozilla Wiki (it was the last place I thought you were going to have blacklisted… and I was right!)

Just to clarify, this is about the behaviour triggered by enabling the download remote images to local site setting.

The issue isn’t whether the image is displayed or not, but where it’s hosted. Both images were embedded from the Mozilla Wiki, but only the first has been downloaded locally to your cdn. (The second has been downloaded, but the regexp used to edit the post failed because it was looking for a url with http: at the start)

This has also exposed another bug, the url in the code block also got replaced. It initially read:

Do we also want to fix this?


System user replaces wrong URL when it downloads local copy of image
(Régis Hanol) #5

Sure if you’re up to it :wink: Beware, there lies :dragon:


(Jeff Atwood) #6

Is this still an issue after the massive commonmark refactor @LeoMcA?


(Leo McArdle) #7

Let’s see:

Nightly

![Nightly|50x50](//discourse-meta.s3-us-west-1.amazonaws.com/original/3X/4/f/4f2567048904a08ba6390cd498a77ec6b9753d0b.png)

Nightly

![Nightly|50x50](//discourse-meta.s3-us-west-1.amazonaws.com/original/3X/4/f/4f2567048904a08ba6390cd498a77ec6b9753d0b.png)

<img src='//discourse-meta.s3-us-west-1.amazonaws.com/original/3X/4/f/4f2567048904a08ba6390cd498a77ec6b9753d0b.png' width="50" height="50">

<img src='//discourse-meta.s3-us-west-1.amazonaws.com/original/3X/4/f/4f2567048904a08ba6390cd498a77ec6b9753d0b.png' width="50" height="50">


(Régis Hanol) #8

I don’t think it’s fixed since it’s not a markdown issue but a PostCookedProcessor issue.


(Leo McArdle) #9

I can’t even get it to trigger remote image downloading - let alone replacing the url in the code block - so I’m not sure what I’m doing wrong.