Skip to content
This repository has been archived by the owner on Nov 18, 2021. It is now read-only.

README rendering doesn't handle relative/local SVG images #316

Closed
stuartpb opened this issue Dec 18, 2014 · 43 comments
Closed

README rendering doesn't handle relative/local SVG images #316

stuartpb opened this issue Dec 18, 2014 · 43 comments

Comments

@stuartpb
Copy link

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.)

@cirosantilli
Copy link
Collaborator

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/cirosantilli/test/blob/master/svg.svg , so maybe this request might be accepted now?

@stuartpb
Copy link
Author

stuartpb commented Dec 20, 2014

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.

@stuartpb
Copy link
Author

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).

@stuartpb
Copy link
Author

stuartpb commented Jan 8, 2015

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.

tfausak referenced this issue in bolshakov/stoplight Feb 4, 2015
tfausak referenced this issue in AaronLasseigne/active_interaction Feb 4, 2015
bkimminich added a commit to juice-shop/juice-shop that referenced this issue Feb 18, 2015
SJLC added a commit to VCTLabs/elc2015-presentations that referenced this issue Mar 14, 2015
take the "raw=true" back off, thanks to
isaacs/github#316
UltCombo pushed a commit to JSRocksHQ/harmonic that referenced this issue Mar 24, 2015
@GreLI
Copy link

GreLI commented Mar 31, 2015

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.

@stuartpb
Copy link
Author

@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.

kiliankoe added a commit to kiliankoe/ParkenDD that referenced this issue Apr 14, 2015
Somewhat documented as an issue in this repo:
isaacs/github#316
kini added a commit to kini/spacemacs that referenced this issue Jun 2, 2015
Attribution required by CC-BY 2.5 license.

<img/> hack to workaround isaacs/github#316 .
@drewnoakes
Copy link

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

gagath pushed a commit to haum/PCBlastifieuse that referenced this issue Jul 3, 2015
scottgonzalez added a commit to scottgonzalez/j5-shift-seven that referenced this issue Jul 4, 2015
GitHub doesn't handle relative paths to SVG images properly.
isaacs/github#316
yoshuawuyts added a commit to man-n/logo that referenced this issue Jul 5, 2015
apparently relative svg images bug out locally. See:
isaacs/github#316
tuxlife added a commit to Motorbootslalom/Parcours that referenced this issue Jul 16, 2015
igrr pushed a commit to esp8266/Arduino that referenced this issue Jul 22, 2015
@ioquatix
Copy link

+1

@stuartpb
Copy link
Author

stuartpb commented Aug 3, 2018

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

@gucu112
Copy link

gucu112 commented Aug 11, 2018

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

@FranklinYu
Copy link

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.

fhemberger pushed a commit to nodejs/nodejs.org that referenced this issue Feb 9, 2019
We cannot see the 'schedule.png', so according to this submit
(nodejs/Release@2bf2ea3), it seems the img was removed and we've used
'schedule.svg' instead.

The reason why I use the parameter `?sanitize=true` is that we MUST make
sure that the response type should be of img (Content-Type should be
`img/svg+xml` instead of `text/plain`).

For more about svg rendering on GitHub's server, please see these
related posts:

1. isaacs/github#316 (comment)
2.
https://stackoverflow.com/questions/13808020/include-an-svg-hosted-on-github-in-markdown
(See 'Linking to RAW files (Does not work)').

Fix for: #2055
@TPS TPS closed this as completed Feb 25, 2019
@TPS
Copy link
Collaborator

TPS commented Feb 25, 2019

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

dlipovetsky pushed a commit to dlipovetsky/etcdadm that referenced this issue Mar 7, 2019
The upstream issue that blocked this was resolved (isaacs/github#316)
dlipovetsky pushed a commit to dlipovetsky/etcdadm that referenced this issue Mar 7, 2019
The upstream issue that blocked this was resolved (isaacs/github#316)
dlipovetsky pushed a commit to dlipovetsky/etcdadm that referenced this issue Mar 8, 2019
The upstream issue that blocked this was resolved (isaacs/github#316)
gichan-jang added a commit to gichan-jang/nnstreamer that referenced this issue Apr 17, 2020
Since badge is cached in github, badge is not updated.
For example, current coverage is 83.7%, however, badge on NNStreamer mainpage indicated old value.
Reference : https://stackoverflow.com/questions/13808020/include-an-svg-hosted-on-github-in-markdown/16462143#16462143
Related issure : isaacs/github#316

Signed-off-by: gichan-jang <gichan2.jang@samsung.com>
filipemarruda pushed a commit to filipemarruda/grandma that referenced this issue Oct 22, 2021
filipemarruda pushed a commit to filipemarruda/grandma that referenced this issue Oct 22, 2021
filipemarruda pushed a commit to filipemarruda/grandma that referenced this issue Oct 22, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests