isaacs/github

README rendering doesn't handle relative/local SVG images

stuartpb opened this issue ยท 43 comments

See https://github.com/litl-license/litl-license/tree/e50196aa3dba3f667728483eb801022d9c113925 - I had to switch the inline image here to PNG for it to display on GitHub, which is lame and means that it's going to look lousy on Retina displays. (I would later go on to fix this by passing it through a gh-pages URL, which gives the added benefit of the code working when copied to repos that don't have this image.)

SVG does not work from READMEs by design for security concerns: http://stackoverflow.com/a/21521184/895245

Interestingly, however, it seems that is has started to show on blob show: https://github.com/blog/1902-svg-viewing-diffing https://github.com/************/test/blob/master/svg.svg , so maybe this request might be accepted now?

SVG does not work from READMEs by design for security concerns

Putting the security angle aside for a moment, SVGs totally do work in READMEs: it's how shields.io does badges (h5bp/lazyweb-requests#150), along with other services like Gitter.

This is a bug, one that is only present for SVG files relative to the README in the repository. The bug is caused (as far as I can tell, with a tip of the hat to Karsten S.) by the Markdown renderer rewriting remote URLs to point to rehosted images at camo.githubusercontent.com (which handles SVG's content-type correctly), but directing local urls to a route that points to raw.githubusercontent.com (which serves SVG files as text/plain with a header coercing the browser not to interpret it as SVG).

The solution should be to fix raw.githubusercontent.com to handle SVGs the same way as it handles other images (serving them with the appropriate content-type), or (if that's unfeasible) to have the Markdown render path rewrite local SVG references to rehost and point to camo.githubusercontent.com (as it currently does for remote images).

(edited 2017-07-23 to bold the line with the solution for anybody coming in from github/markup#556)


Coming back to the security question, there's no actual security problem to displaying SVGs. As the links on that question mention, SVG images don't run scripts, and even if they did, it would be in a sandboxed document no more dangerous than visiting the image separately/in an iframe.

Regarding fixing raw.githubusercontent.com: X-Content-Type-Options: nosniff is a half-assed measure against hotlinking that breaks hotlinking for files only when the content-type is incorrect. raw.githubusercontent.com doesn't get the content-type wrong for raster images, so they hotlink just fine, even though breaking <img> was half of the rationale given for implementing nosniff. GitHub complained about people using raw.githubusercontent.com as a CDN due to it being implemented as a Rails view with poor performance; the solution should have been to rewrite raw.githubusercontent.com to not go through Rails, not to rely on bugs and some headers that make it coincidentally bad for some use cases.

If GitHub has a problem with raw.githubusercontent.com having bad performance as a CDN, the fix is staring them right in the face: put it on the deployment system gh-pages uses (#212), minus the Jekyll rendering step. This is how people are already working around this problem (I've even renamed my master branch to gh-pages on projects I even remotely might want to include files from).

On 12/19/2014 8:34 AM, GitHub Staff wrote:

Hi Stuart,

Thanks for your feedback! I can understand your frustration. I have added your feedback to our list for our team to consider.

GreLI commented

What security concerns are you talking about? Browsers are already disabling scripting and other security-related SVG features in an <img> tag. See for example SVG as an Image on MDN.

@GreLI That's what I said:

Coming back to the security question, there's no actual security problem to displaying SVGs. As the links on that question mention, SVG images don't run scripts, and even if they did, it would be in a sandboxed document no more dangerous than visiting the image separately/in an iframe.

A workaround for others hitting this issue from a search engine (as I did) is to use the cdn.rawgit.com service.

Before:
https://raw.githubusercontent.com/zeromq/netmq/master/docs/Images/NetMQLogo.svg

After:
https://cdn.rawgit.com/zeromq/netmq/master/img/NetMQLogo.svg

+1

+1

๐Ÿ‘

๐Ÿ‘

The bugs hasn't been fixed yet?

rtens commented

rawgit not a workaround for private repos

๐Ÿ‘

Please forgive me for a flooding above. I used git-rebase few times. And every time GitHub created a record of a link. I'm very sorry!

๐Ÿ‘

It would be great to get some attention on this (especially since it's getting close to a two year old bug report). I'd like to move more of my writing to GitHub but this is actually a bigger sticking point than it looks like.

EDIT: They're doing this now: https://github.com/seagreen/svg-on-github-test ๐Ÿ˜Ž

At minimum the link should still show up as text instead of disappearing entirely. Content appearing in a slightly more inconvenient form is alright, content appearing nonsensical due to entirely missing images is a bigger deal.

Adding ?raw=fixyourshitgithub is a workaround that will still break local editing but may be less gnar than using fully specifying the sin-ugly https://raw.githubusercontent.com url.

Edit: whoops, this ticket is for SvG specifically? I just was annoyed that relateives path never work for anything. We have a lot of examples that work locally that github breaks. Gee, local paths sure would be nice in markdown.

Adding ?raw=fixyourshitgithub is a workaround that will still break local editing but may be less gnar than using fully specifying the sin-ugly https://raw.githubusercontent.com url.

Could you be able to provide an example of that?

+1

๐Ÿ‘

Apparently this is now being worked on!!

@ioquatix Where did you hear that?

Just to put a point on my aforementioned exasperation at this noise that's coming up again in github/markup#556 where people misunderstand the issue being with repository-local paths as being a problem, somehow, with SVG (as if there need to be extra steps to accommodate it), here is an example of an SVG image being displayed in GitHub Markdown right now:

tiger

The problem is not with SVG images being unsupported (they're supported fine), it's with repository-local file paths not being rehosted correctly, an issue that is only visible (due to content-type fumbling) with SVG images (but present for any such repository-local image).

@stuartpb For what it's worth, that SVG is not being displayed right now (I'm assuming from your comment I should actually see the tiger inline). Doesn't necessarily invalidate your point but does add confusion:
image

@hickeng That's strange, because for me, both Safari 11.0 and Chrome 59.0 currently display it perfectly fine inline. What version of Chrome breaks it for you?

@sorenmortensen Version 59.0.3071.115 (Official Build) (64-bit) - opening the image directly in a new tab functioned as expected so it's not directly a firewall or content filter issue. In fact, having just reloaded the page I now see it inline and no variant of opening via different links is causing the failure :( Not a fan of transient failures.

How about base64 in a img tag?

@stuartpb Is it possible to find the "camo.githubusercontent.com path" of an SVG file hosted in a private GitHub repo? How to do so? Thanks in advance for you help. ๐Ÿ˜Š

+1 :-(

I might be mistaken but doesn't this issue resolve the problem?

Fix relative SVG rendering github/markup#556

@Potherca: Yep, it's solved. You can see in my test that both the PNG and the SVG appear now.

In which version of GitHub Enterprise is this fixed?

I was going to close this, but github/markup#556 (comment) suggests it's still an issue.

@stuartpb #556 was closed recently and I believe it is not an issue anymore.

I think this issue can be closed. Embedding SVG in Markdown is officially supported by GitHub. If there is still any issue, it would be considered bug instead of lack of feature, so user should report in https://github.com/github/markup.

TPS commented

Per above & https://stackoverflow.com/a/16462143, this is solved