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

Remove third party-requests #159

Open
phryk opened this issue Feb 28, 2021 · 4 comments
Open

Remove third party-requests #159

phryk opened this issue Feb 28, 2021 · 4 comments

Comments

@phryk
Copy link

phryk commented Feb 28, 2021

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
Copy link

Helveg commented Oct 12, 2022

So the offending lines are

self.onmessage = function (event) {
// Try loading JS from CDN in case snakeviz server is off
try {
importScripts(
"https://cdnjs.cloudflare.com/ajax/libs/immutable/3.8.1/immutable.min.js",
"https://cdnjs.cloudflare.com/ajax/libs/lodash.js/4.17.4/lodash.min.js");
}
// If the user is offline try loading from the SnakeViz server
catch (e) {
try {
importScripts(
event.data['url'] + "/static/vendor/immutable.min.js",
event.data['url'] + "/static/vendor/lodash.min.js");
}
catch (e) {
throw 'Could not load JS libraries in worker.';
}
}
var depth = 0;
var max_depth = event.data['depth'];
var cutoff = event.data['cutoff'];
var node_name = event.data['name'];
var parent_name = event.data['parent_name'];
var node_time = stats[node_name]['stats'][3];
self.postMessage(JSON.stringify(
sv_build_hierarchy(
node_name, depth, max_depth, cutoff, node_time, parent_name
)));
};

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

snakeviz --cdn-fallback
snakeviz --allow-cdn

@phryk
Copy link
Author

phryk commented Oct 13, 2022

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
Copy link
Owner

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
Copy link

Helveg commented Oct 13, 2022

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!

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

No branches or pull requests

3 participants