Idenitifier self is not declared, breaks when `self` its leaked.

[anodynos]

The self identifier is NOT declared locally in the module and hence it comes from global - in

rxjs/src/util/root.ts

Line 21 in 3793e47

export let root: any = (objectTypes[typeof self] && self) || (objectTypes[typeof window] && window);

So, depending on what window.self value might be (i.e from leaked variables, a common mistake of devs), RxJS might break.

RxJS version:
5.0.0-beta.11

Code to reproduce:
Add a window.self variable with different values and run tests.

[kwonoj]

As far as I know as same as other global variables like window, it is looking existing global variable to check if global already has defined
(The Window.self read-only property returns the window itself - https://developer.mozilla.org/ko/docs/Web/API/Window/self).

I think it's expected behavior to allow setup your own environment.

/cc @Blesh , @jayphelps since I could be wrong..

[anodynos]

Well, it's not read only in Chrome Version 53.0.2785.92 (64-bit) - I assume in other browsers also...

-> window.self
Window {speechSynthesis: SpeechSynthesis, caches: CacheStorage, localStorage: Storage, sessionStorage: Storage, webkitStorageInfo: DeprecatedStorageInfo…}
->  window.self = {some:'value'};
Object {some: "value"}
->  window.self
Object {some: "value"}

This makes RxJS very prone to errors, especially for libs etc that run in uncontrolled code sites.

[anodynos]

Same for Firefox 48.0

-> window.self = {'some':'value'}; window.self
Object { some="value"}

[jayphelps]

Did this actually happen in real life or just discussing the possibility?

self is important because in WebWorkers window does not exist. Using it is common, even some of the most popular libraries around like underscore.

Any alternatives?

[benlesh]

This seems like trying to defend against developers defining undefined.

[jayphelps]

I see @Blesh's point--but this is indeed a bit more insidious:

function UserClassOfSomething() {
  self = this;
}

user = new UserClassOfSomething();
// self === user
// whoops

where as undefined = 'something' would be much more obviously stupid to do, and also in every compliant ES5 runtime (all browsers I tested several years ago) redefining it in this way is a noop. undefined = 123; alert(undefined); does not alert 123.

It's only a problem in ES5 when shadowing with a new declaration var undefined = 123; and only within a function's scope, not globally.

(function () { var undefined = 132; alert(undefined); })()

..but that's even less likely to affect us cause it would mean someone did this in code that wraps our code. There's little point to worrying about that because there are a billion similar things people could do to fuck up our code if they're wrapping it in another function.

that said I don't think in practice this self business is a major issue. We could certainly make it a safer for the 99% case..something like typeof self === 'object' && self.self === self && self. Then it's only an issue if you reassign it inside a Worker that uses Rx..at which point you're SOL. Reasonable vs. exhaustive efforts I think applies in these cases given the nature of JavaScript.

[anodynos]

@jayphelps yes, it's a real case, we had an incident with a client's code that was overwriting self by mistake, and we found out the hard way that RxJs was the issue... It certainly needs to have a guard like the above and default to window or global if they are available.

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.