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

uBlock removes WebSocket methods (addEventListener) #1604

Closed
sateffen opened this issue May 2, 2016 · 33 comments
Closed

uBlock removes WebSocket methods (addEventListener) #1604

sateffen opened this issue May 2, 2016 · 33 comments

Comments

@sateffen
Copy link

sateffen commented May 2, 2016

Describe the issue

The method addEventListener gets removed by this plugin from window.WebSocket. This breaks a lot of applications and websites. Deactivating this plugin fixes the problems.

One or more specific URLs where the issue occurs

Every website that uses this code:

var w = new window.WebSocket('wss://echo.websocket.org');
w.addEventListener('open', function () { console.log('YEAH'); });

Throws addEventListener is not a function.

Example: https://jsfiddle.net/84do11v6/

Steps for anyone to reproduce the issue

  1. Install chrome
  2. Install uBlock Origin version 1.7.0
  3. Open any website with the above code (like this: https://jsfiddle.net/84do11v6/)
  4. Open console (F12) and see the error.

Your settings

  • Browser/version: Chrome 50.0.2661.94 m (64-bit)
  • uBlock Origin version: 1.7.0

If any questions are left, text me.

This is pretty critical to me... Hopefully this will be fixed soon.

@sateffen
Copy link
Author

sateffen commented May 2, 2016

Looks like this file might be the problem: https://github.com/gorhill/uBlock/blob/ffc313136b94cb90b827e7507295ee471e489a3b/platform/chromium/websocket.js

Here you create a copy of the websocket API, but don't copy the prototypes properly. The WebSocket prototype inherits from the EventTarget prototype, which adds the methods addEventListener, removeEventListener and dispatchEvent. This functions are missing, as far as I see.

@brianj64
Copy link

brianj64 commented May 2, 2016

http://twitch.tv/twitch (example) is also affected. The IRC takes ages to load and disabling this extension completely(disabling it for a specfiic website does not) fixes it. I'm not sure if it's the same problem but i'm assuming it is(Any of the WebSocket issues).

@ghost
Copy link

ghost commented May 2, 2016

+1, this issue breaks our site

@gorhill
Copy link
Owner

gorhill commented May 2, 2016

I uploaded a new version 1.7.2 which remove WebSocket support for the time being.

@octatone
Copy link

octatone commented May 2, 2016

👍

Breaking a lot of the internet for me :(

@gorhill
Copy link
Owner

gorhill commented May 2, 2016

@octatone You have specific URLs which I can use as test cases?

@octatone
Copy link

octatone commented May 2, 2016

https://www.wunderlist.com/webapp for example ... even with the site whitelisted messing with the WebSocket class is breaking binding to websockets

@gorhill
Copy link
Owner

gorhill commented May 2, 2016

I uploaded 1.7.2 to the Chrome store, but it now all depends on how fast they allow it to go through.

@ghost
Copy link

ghost commented May 2, 2016

http://www.pwnwin.com/dashboard - the header and sidebar of this site does not load

@RobbieClarken
Copy link

Thanks for the emergency update @gorhill. The 1.7.0 release broke Jupyter Notebook for me: notebooks would fail to communicate with the IPython kernel.

@gorhill
Copy link
Owner

gorhill commented May 2, 2016

@sowbug is there a way to make the emergency fix I submitted to be approved as fast as possible? (I am melting here..)

@xfalcox
Copy link

xfalcox commented May 2, 2016

Just adding that this is breaking websites even on intranets with ublock disabled on the page 😢

@gorhill
Copy link
Owner

gorhill commented May 2, 2016

That was a bad release. Now just waiting for the store to enable 1.7.2.

@wereHamster
Copy link

Also, WebSocket.{CONNECTING,OPEN,CLOSING,CLOSED} is missing. Those constants must be present on the WebSocket object, not its prototype.

@gorhill
Copy link
Owner

gorhill commented May 2, 2016

@wereHamster Yes, I saw this with Twitch, fixed locally. Turns out these must be present in both WebSocket and WebSocket.prototype.

@tonyp
Copy link

tonyp commented May 2, 2016

Thanks @gorhill for the quick fix, I had the same problems with Jupyter notebooks as @RobbieClarken, 1.7.2 fixed it.

@gorhill
Copy link
Owner

gorhill commented May 2, 2016

Thinking more about the issue, I think the real fix to bring back WebSocket awareness to uBO will work this way:

  • An optional companion extension to uBO: installable by users -- say "uBlock Origin WebSocket".
    • If installed, uBO will become aware of WebSocket connection attempts.
    • If not installed, uBO will work just as it did before.

This will remove worries that uBO could cause unexpected breakage due to unforeseen WebSocket usage, and if it does cause unforeseen breakage, a user can simply uninstall the companion extension.

Wish I had tought of this before releasing 1.7.0.

@gorhill
Copy link
Owner

gorhill commented May 2, 2016

1.7.2 is now available in the Chrome store. Will work on the companion extension to make it available ASAP -- I do feel strongly about WebSocket connections being visible to uBO.

@ChaosCom
Copy link

ChaosCom commented May 2, 2016

Hello Raymond, in light of what happened, can I propose that updates to uBlock Origin don't happen silently anymore and we get a visual (popup?) confirmation that the extension has been updated ?

@ghost
Copy link

ghost commented May 2, 2016

Those are usually (at least in my opinion) somewhat invasive (think about 10+ extensions all creating notifications) which is why @gorhill hasn't added any kind of notification dialogue AFAIK. Maybe this could be optional.

I'm surprised so few tech-savvy people were using the dev version btw.

@gorhill
Copy link
Owner

gorhill commented May 2, 2016

which is why @gorhill hasn't added any kind of notification dialogue AFAIK

Yes, that is the reason, it's also why uBO does not have a first-install page, and of course won't ever have an uninstall page.

This was a bad release, sorry about this, all my fault. If leaving this release aside, there was never really a rationale to notify users. What needs to change is not me to add some sort of notification that uBO has been updated, but rather to be much more paranoid about a release breaking stuff. I thought I had become quite fearful about this (I do get anxious when submitting a new release, this will probably get worst now..), obviously I need to be more fearful.

The lesson in the current case is that for such feature, it needs to be opt-in, at least until enough time has passed that I can confidently conclude it's mature. I had thought about putting the feature behind a setting, and opt-in, but given the nature of the WebSocket solution, this was not feasible. The companion extension though work perfectly, I already have it working fine on my side. (Other blockers could even make use of it.)

@lnostdal
Copy link

lnostdal commented May 2, 2016

Had to force an extension upgrade to get things rolling again: http://www.howtogeek.com/64525/how-to-manually-force-google-chrome-to-update-extensions/

@ChaosCom
Copy link

ChaosCom commented May 2, 2016

Oh, I'm not blaming, I merely wanted to make a point that had there been an update notification, then the instant breaking of all the websockets in this particular case would've been immediately relatable to the extension update. Control over websockets (filtering / blocking) is quintessential imo to close the loophole certain ad-networks use to bypass filtering, so i regard this as a required feature (though i figure there might be easier solutions like filtering the domain in question)

@connor4312
Copy link

The fact that code can get in which is so broken (no support for addEventListener? Mildly rigorous testing should have turned that one up) and update silently to all its users is unfortunate. I'm not sold on the "opt-in until mature", particularly since you immediately said it would not have been possible to do with the websocket feature and that you 'had it working fine.' It seems like you're implying that the websocket feature would have been released anyway, even in a feature-flag-enabled situation.

Is there something that can positively be done to prevent this sort of thing in the future? It seems like having a system to notify users of updates and issues would be a good first step.

@jjohns71
Copy link

jjohns71 commented May 2, 2016

@connor4312,

You're at the right place to help prevent it from happening in the future. Follow the updates of the software you use and help bug test the applications while they are in beta before they get released to a wider audience.

@tonyp
Copy link

tonyp commented May 2, 2016

@gorhill: Please do not beat yourself up about it, mistakes happen to the best and you've resolved it as quickly as possible.
I'm against the notifications in general, unless there is a major change which really requires users to be notified (the WebSockets might fall into that category).

@ghost
Copy link

ghost commented May 2, 2016

I can confirm 1.7.2 fixed my problem on tradingview.com)!
Thanks for the quick solution.
I'll donate, if bitcoin is accepted.

edit: it does (bitcoin acceptance)

Protip: add a QR code to the website, it makes life much more pleasant.

gorhill added a commit that referenced this issue May 2, 2016
@gorhill
Copy link
Owner

gorhill commented May 2, 2016

Since the fix (a revert really) is in the Chrome store now, I will close this issue.

A new companion extension (with issues reported here fixed) is now available in the Chrome store: https://chrome.google.com/webstore/detail/ublock-origin-websocket/pgdnlhfefecpicbbihgmbmffkjpaplco.

@gorhill gorhill closed this as completed May 2, 2016
@ghost
Copy link

ghost commented May 2, 2016

@ghost
Copy link

ghost commented May 2, 2016

Whoops, lol.
Fuckers managed to scam me from 0.01BTC then! :-)

@Couchy
Copy link

Couchy commented May 3, 2016

Will this feature be merged back into the main extension when it's stable?

@gorhill
Copy link
Owner

gorhill commented May 3, 2016

@Couchy Unlikely, because ultimately although the WebSocket wrapper seems to work fine at this point (after the fixes), it's just impossible to make the wrapper look exactly like the native WebSocket. So the final solution is to install the companion extension. Eventually I expect the Chromium issue will be solved, which will render the companion pointless.

@steelbrain
Copy link

@gorhill I would like to suggest you have a look at reconnecting-websocket because it provides an API exactly like WebSocket, similar to this extension

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