search

jiffyclub / snakeviz

An in-browser Python profile viewer
https://jiffyclub.github.io/snakeviz/
Other
2.36k stars 139 forks source link

Remove third party-requests #159

Open phryk opened 3 years ago

phryk commented 3 years ago

I just noticed that snakeviz tries loading data from cdnjs.cloudflare.com, which is behavior that actively makes snakeviz users trackable to cloudflare – amongst other things exposing the full filepath of the currently read .prof file, which will most often also expose things like usernames.

Please stop doing that and remove all third-party requests. JS dependencies should be bundled in order to avoid breaking peoples privacy.

Helveg commented 2 years ago

So the offending lines are

https://github.com/jiffyclub/snakeviz/blob/a236c7bdfbde6163d5c772eeca33b71cd065998e/snakeviz/templates/viz.html#L248-L276

To address privacy concerns this could probably be changed into an opt-in CLI flag:

snakeviz --cdn-fallback
snakeviz --allow-cdn
phryk commented 2 years ago

Oh, didn't even know there already are bundled versions.

With the versions loaded from CDN being hardcoded like that I also fail to see what advantage loading using the CDN instead of the local, bundled files brings…

If there is an advantage, I'm not at all against offering that option and a CLI flag seems like a good way to enable that – personally I'm a fan of privacy by default and would argue that --allow-cdn would be better than --cdn-fallback. :)

jiffyclub commented 2 years ago

There's a reason there's both. SnakeViz mostly operates as a single page app that can be loaded once from the local server and then does not need to communicate with the local server again, so the overall browser can load the local vendor JS then. The exception is that snakeviz uses web workers, which need to load the JS every time one is booted up (which happens when interacting with the graphic). There are contexts in which people use snakeviz without the local server running, especially when using it in a jupyter notebook via the magic command. When that's the case the only option for booting up a web worker is to get the vendor JS from a CDN since the local server is not running. We might be able to rearrange things so we try the local server first, but I don't want to break the usage contexts in which the local server is not running.

Helveg commented 2 years ago

I'd say we turn off the CDN loading behaviour, add the --allow-cdn flag, and only when we detect that we're in a serverless context we automatically toggle it on, leaving perhaps even a little note in the docs that we do that. I can PR this on a rainy day if @jiffyclub could specify what requirements they have for the conditions under which I should detect that the CDN requests should be made!