search

chocolatey / choco-wiki

The content from this repository has been moved to https://github.com/chocolatey/docs. If you have found an issue, or want to submit a fix, then please open an issue, or a PR, on that repository.
Other
81 stars 70 forks source link

Policy on CDNs for github IconUrl should be changed #183

Open riedel opened 4 years ago

riedel commented 4 years ago

https://github.com/chocolatey/choco-wiki/blob/c8f6f919fb475a55df4871a35dbb3983cadc486b/CreatePackages.md#L294

Microsoft recommends using raw git links in nuspec's iconURL (apart from it being deprecated): https://docs.microsoft.com/en-us/nuget/reference/nuspec#iconurl

Rawgit made sense before, it has become obsolete and now directs to:

HTTP/1.1 301 Moved Permanently
...
Content-Type: text/html; charset=utf-8                                                                              
Link: <https://rawgit.com/>; rel="sunset"; title="RawGit will soon shut down. Please stop using it."
Location: https://raw.githubusercontent.com/StreamWhatYouHear/SWYH/1.5.0/SWYH/Resources/Icons/swyh128.png

I recently submitted a package without even noticing. I guess many people don't and this is OK (until they remove the redirect server)!

The reason for using it was mostly fixing githubs content type and increasing loading speeds. However it turns out https://raw.githubusercontent.com/ has become a valid CDN for most usecases (no sign of ratelimiting I am aware of). They are using https://varnish-cache.org/ served from different locations.

HTTP/1.1 200 OK
...
Content-Type: image/svg+xml
Via: 1.1 varnish
X-Served-By: cache-hhn4053-HHN
X-Cache: MISS, MISS
X-Cache-Hits: 0, 0
X-Timer: S1590576479.474429,VS0,VE200

Even if not. The chocolatey website has its own image cache. Either this is the only use, there is no need for a CDN (ratelimiting by github would have become an issue much earlier since rawgit started only redirecting). If there are client applications showing images they should probably better use https://chocolatey.org/content/packageimages/ instead of loading iconUrl.

pauby commented 4 years ago

Just to be clear:

Loading those random URLs could probably even be in violation of the GDPR if the author did not reference the CDN in his Terms of Use

This is not in violation of GDPR.

riedel commented 4 years ago

This is not in violation of GDPR.

I changed the comment because I do not want to evoke discussion on this here (and it is actually not CDN specific): IMHO serving external tracking pixels (as an extreme example to the same effect as using random external images) without mentioning it would be a problem. So I think for an GUI application it would be still safer to load from a source it explicitly mentions to the user like chocolatey.org. I think this one of the reasons Microsoft deprecates iconUrls

tylerszabo commented 4 years ago

Rather than speculate on the reasons for the deprecation of <iconUrl> we can read the discussion in NuGet/Home#352 where the change was made and NuGet/NuGetGallery#2613 which discusses some of the issues encountered when using URLs rather than embedded resources.

I think the best fix here would be to also adopt the embedded <icon> and <license>. I'm content to drop the <iconUrl> and <licenseUrl> from packages I maintain as soon as I know it's supported by the package repository UI.

riedel commented 4 years ago

I am fine to close this issue in favour of rather adopting <icon>and <licence> . This really solves the problem, rather than having to add more and more policy for reviewer.

Thanks for linking the issues. At least one commenter there shares my general concern:

This also prevents leaking search data to other domains, which is currently the case for any https:// icons.

But sure other issues probably were the more direct cause. I am happy if I don't have to chose a CDN.