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

Content Security Policy blocking images #166

Closed
silverwind opened this issue Sep 1, 2014 · 34 comments
Closed

Content Security Policy blocking images #166

silverwind opened this issue Sep 1, 2014 · 34 comments

Comments

@silverwind
Copy link
Member

Looks like Chrome enforces CSP for userstyles, white Firefox doesn't. Github only allows these domains to server images:

assets-cdn.github.com identicons.github.com www.google-analytics.com collector.githubapp.com *.githubusercontent.com *.gravatar.com *.wp.com

I see a few ways to solve this:

  1. Inline all our images. This would bump our size above the limit for userstyles.org
  2. Open an issue at Stylish for Chrome
  3. Serve images from raw.githubusercontent.com
  4. Ask GitHub to include github.io in above list
@Mottie
Copy link
Member

Mottie commented Sep 1, 2014

I do notice the issue when I try to use the default background image in the style, but I usually install my style from userstyles, and that same default image is saved as a uri so no policy issue shows up.

I find it really odd that the main background image I usually use is hosted at githubusercontent.com domain and I haven't had any issues. Basically I copied the url from an attachment from an issue, like this one:

bg-fabric1

Click on that image, then copy the url and paste it into the stylish editor... it always seems to work, and I don't see an error in Chrome.

As a last resort, we could just create an account at photobucket or just use tinypic.com to save the images.

@ddavison
Copy link
Member

ddavison commented Sep 1, 2014

i wonder what the possibility is for # 2... i'm sure that we aren't the only ones experiencing this.. I say it couldn't hurt to ask stylish to do something about this :)

@silverwind
Copy link
Member Author

Alright, opened an issue. As CSP is a pretty new thing, we might be the first to run into this issue, given that the host domain has to specifiy it.

@Mottie
Copy link
Member

Mottie commented Sep 19, 2014

I just added a wiki page on how to add a background image to the style.

@silverwind
Copy link
Member Author

Thanks for contacting GitHub, it something I should have done earlier.

@Mottie
Copy link
Member

Mottie commented Sep 24, 2014

From GitHub:

Hi Rob,

is there something we can do to work around this limitation

Thanks for the question. We don't have plans to change the current CSP, so you're limited to referencing images hosted on hostnames we have listed in the image-src section of our CSP. Currently, those are:

assets-cdn.github.com identicons.github.com www.google-analytics.com collector.githubapp.com *.githubusercontent.com *.gravatar.com *.wp.com

You could either use data URIs to embed base64-encoded binary data, or you could host the images by uploading them in an issue/comment. Those are our recommended approaches to solving this.

Hope that helps.

Thanks,
James

I don't remember if I asked if they would include github.io...

@silverwind
Copy link
Member Author

Hmm, my stance was always to inline everything for performance, so I'm fine with that. With minfied code, we have a bit leeway regarding to style size.

A issue I see is with custom backgrounds. Maybe we should point users to a Data-URI generator instead of asking for an URL.

@silverwind
Copy link
Member Author

By the way: What happens when a user uploads an image on us.org? Does it get embedded as Data-URI?

@silverwind
Copy link
Member Author

@Mottie maybe ask them concisely to include github.io in their CSP. It's their own domain after all and should be on a same trust level as githubusercontent.com in my opinion.

@Mottie
Copy link
Member

Mottie commented Sep 24, 2014

Yeah, userstyles.org does encode the image into a data-URI.

I did send a followup email asking that specifically. I'm just waiting for a response.

@Mottie Mottie added the github :octocat: label Sep 24, 2014
@Mottie
Copy link
Member

Mottie commented Sep 24, 2014

would it be possible to include *.github.io in that list?

We don't have plans to add this to the list currently.

Thanks,
James

@silverwind
Copy link
Member Author

Eh, they should at least explain on why they won't include it 😢.

Anyways, I think we may have to remove that URL background option for Chrome users, unless Jason comes up with a way to subvert CSP in Chrome, if that is even possible.

@Mottie
Copy link
Member

Mottie commented Sep 24, 2014

Maybe we can use grunt-data-uri to automatically encode images.

@silverwind
Copy link
Member Author

If I inline our default tile background, there wouldn't be a single github.io URL left in the style. Should I go for it? We might go above 100kB unminified.

@Mottie
Copy link
Member

Mottie commented Sep 24, 2014

Sure go for it.

@silverwind
Copy link
Member Author

Maybe we can use grunt-data-uri to automatically encode images.

That could work actually, so we don't include double url() when someone builds a custom theme.

silverwind added a commit that referenced this issue Sep 24, 2014
@silverwind
Copy link
Member Author

97599 bytes unminfied 😌.

Did you resolve the issue with pasting the minifed style into Chrome by the way?

@silverwind
Copy link
Member Author

Once we remove the URL option on userstyles, we'll be free (and this issue is solved)!

@Mottie
Copy link
Member

Mottie commented Sep 24, 2014

Nope, I can copy and paste the full uncompressed style without any problems. So, I'd have to guess that it is a limitation of a single line of code. Maybe if we inject some carriage returns.

@silverwind
Copy link
Member Author

I'd rather suggest opening an issue on Stylish for Chrome.

@Mottie
Copy link
Member

Mottie commented Sep 24, 2014

It might be a limitation of the textarea, and not Stylish

@silverwind
Copy link
Member Author

Well, a few newlines won't hurt I suppose, if there's a way to add them.

@silverwind
Copy link
Member Author

https://github.com/JasonBarnabe/stylish-chrome/blob/master/edit.html
https://github.com/JasonBarnabe/stylish-chrome/blob/master/edit.js

Nothing obvious that would limit the textarea. But that CodeMirror in there is pretty ancient, which might be the issue.

@silverwind
Copy link
Member Author

Reported stylish-userstyles/stylish-chrome#21 for the cut-off issue.

@silverwind
Copy link
Member Author

I'd say this issue is basically resolved with us inlining everything.

@Mottie
Copy link
Member

Mottie commented Oct 3, 2014

Oh, thanks for reporting that issue!

@Mottie
Copy link
Member

Mottie commented Oct 11, 2014

Well it finally happened. I guess it was the addition of the mobile styles.

2014-10-11 14_27_50-editing github dark - userstyles org

Unfortunately, I can't seem to upload an svg type image (octocat-spinner) to any of the aforementioned locations. So, I'm kind of stuck with updating the userstyles site.


Notes:

@Mottie
Copy link
Member

Mottie commented Oct 11, 2014

Update

I used the grunt usermin compressed css version on userstyles.org and it worked! 😹

@silverwind
Copy link
Member Author

Ah, I thought you were minifying for release already.

By the way, Jason merged my PR to fix pasting code in Stylish for Chrome a few days ago, so that should work too next release.

@Mottie
Copy link
Member

Mottie commented Oct 12, 2014

Hmm, ok the compressed version on userstyles does have some issues.

So far I've noticed:

  • text selection coloring is broken
  • message notification color (the dot) is missing styling - it's transparent, or something.

@Mottie
Copy link
Member

Mottie commented Oct 12, 2014

Ok, I got the non-minified version working.

I took the octocat-spinner-smil.svg, optimized it, re-encoded it and it ended up being slightly smaller. Small enough to work.

I looked at the minified version and noticed that it still removes some of the comments:

::selection{background:0 0!important;color:#fff!important}::-moz-selection{background:0 0!important;color:#fff!important}

I'll see if I can fix that.

@silverwind
Copy link
Member Author

Careful with these SVG optimizers! I've tried running it through svgo, only to get the smil data destroyed.

@Mottie
Copy link
Member

Mottie commented Oct 12, 2014

It looks okay... it says it basically removed the extra decimal places in the path

<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 512 512" width="512" height="512">
    <path fill="#333" d="m332.289429 87.087219c60.033295 -20.4 114.4 -21.8 172.9 1.047241l-11.403595 32.850952c-43.333344 -15.5 -104.1 -16 -148.5 -0.7" transform="rotate(45 418.757 96.1885)">
        <animate attributeName="fill" values="#333;#eee;#333;#333" begin="0s" dur="1s" repeatCount="indefinite"/>
    </path>
    <path fill="#333" d="m398.527466 246.388596c60.033325 -20.4 114.4 -21.8 172.9 1.047241l-11.403564 32.850967c-43.333374 -15.5 -104.1 -16 -148.5 -0.7" transform="rotate(90 484.995 255.49)">
        <animate attributeName="fill" values="#333;#eee;#333;#333" begin="0.125s" dur="1s" repeatCount="indefinite"/>
    </path>
    <path fill="#333" d="m331.332214 404.823669c60.033325 -20.4 114.4 -21.8 172.9 1.047241l-11.403534 32.850952c-43.333405 -15.5 -104.1 -16 -148.5 -0.7" transform="rotate(135 417.8 413.925)">
        <animate attributeName="fill" values="#333;#eee;#333;#333" begin="0.250s" dur="1s" repeatCount="indefinite"/>
    </path>
    <path fill="#333" d="m171.58223 470.323669c60.03331 -20.4 114.4 -21.8 172.9 1.047241l-11.403534 32.850952c-43.333405 -15.5 -104.1 -16 -148.5 -0.7" transform="rotate(180 258.05 479.425)">
        <animate attributeName="fill" values="#333;#eee;#333;#333" begin="0.375s" dur="1s" repeatCount="indefinite"/>
    </path>
    <path fill="#333" d="m13.0822 406.074005c60.033301 -20.4 114.4 -21.8 172.9 1.046997l-11.404007 32.850983c-43.332993 -15.5 -104.1 -16 -148.5 -0.7" transform="rotate(-135 99.55 415.175)">
        <animate attributeName="fill" values="#333;#eee;#333;#333" begin="0.500s" dur="1s" repeatCount="indefinite"/>
    </path>
    <path fill="#333" d="m-53.713486 247.091553c60.03331 -20.4 114.4 -21.8 172.9 1.046997l-11.404022 32.850983c-43.332977 -15.5 -104.1 -16 -148.5 -0.7" transform="rotate(-90 32.754 256.193)">
        <animate attributeName="fill" values="#333;#eee;#333;#333" begin="0.625s" dur="1s" repeatCount="indefinite"/>
    </path>
    <path fill="#333" d="m12.883621 87.061485c60.033295 -20.4 114.4 -21.8 172.9 1.047249l-11.403595 32.850952c-43.333344 -15.5 -104.1 -16 -148.5 -0.7" transform="rotate(-45 99.3514 96.1628)">
        <animate attributeName="fill" values="#333;#eee;#333;#333" begin="0.750s" dur="1s" repeatCount="indefinite"/>
    </path>
    <path fill="#333" d="m172.906631 20.929741c60.03331 -20.4 114.4 -21.8 172.9 1.04723l-11.403595 32.85096c-43.333344 -15.5 -104.1 -16 -148.5 -0.7">
        <animate attributeName="fill" values="#333;#eee;#333;#333" begin="0.875s" dur="1s" repeatCount="indefinite"/>
    </path>
   <path fill="#eee" d="m197.75 459.850006c0 0 -0.1 -48.8 -0.1 -48.850006c0 -0.1 -4.8 -4.4 -33 -1c-28.25 3.4 -62.5 -66.6 -65.2 -69.75c-2.75 -3.1 14 -5.6 26.5 5c12.5 10.6 38.2 33.4 38.5 33.25c0.25 -0.1 31.8 0.6 31.8 0.5c0 -0.1 3 -44.1 20.8 -37.5c17.75 6.6 -72.5 1.4 -97.8 -61.25c-25.25 -62.6 19.2 -124.9 20.2 -115.399994c1 9.5 -17 -35 -10.2 -46.850006c6.75 -11.8 19.2 -9.4 30.2 -5.5c11 3.9 34.2 20.4 39.2 21.25c5 0.9 18.2 -8.2 59.2 -7.5c41 0.7 52.8 6.1 56.8 6.5c3.980804 0.4 30.5 -17.4 38.3 -22.557701c7.730896 -5.2 18.7 -9.1 28.7 0.78849c10.000092 9.9 4.9 67 3.1 55.519211c-1.826904 -11.5 36.2 70.6 6 122.100006c-30.25 51.5 -99.2 52.8 -87.8 55.899994c11.5 3.1 15 33.4 14.8 49.850006l-0.25 65.899994l-22.875 3.875l-0.5 -74.75c0.125 -9.2 -9.1 -10.6 -9.6 -9.75l-0.25 85.875l-21.25 1.25l-0.25 -86.75l-10.5 0l0 86.5l-20.75 -1l-0.25 -84.25c0 -0.1 -8.8 -1.1 -8.2 10.75l-0.5 72.2"/>
</svg>

@silverwind
Copy link
Member Author

Just decimals and whitespace should be fine, yeah. Reminds me I have to fix a few points in the octocat, it's not perfectly symettrical :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants