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

Broken in Chrome 49.0.2620.0 canary (64-bit) #94

Closed
LinusU opened this issue Jan 13, 2016 · 8 comments
Closed

Broken in Chrome 49.0.2620.0 canary (64-bit) #94

LinusU opened this issue Jan 13, 2016 · 8 comments
Labels

Comments

@LinusU
Copy link
Contributor

LinusU commented Jan 13, 2016

My chrome updated during the night and now my stuff doesn't work anymore, fun times, I get to debug!

The first thing was actually already fixed by 5f5478a, I just had to update to the latest version. @feross Was that commit intentionally fixing a Chrome bug?

The next problem is that .subarray(start, end) seems broken when the __proto__ is tampered with.

screen shot 2016-01-13 at 16 54 53

The following code fixes the problem but I'm not sure that it's the right approach:

diff --git a/index.js b/index.js
index 15d7c50..cbb1516 100644
--- a/index.js
+++ b/index.js
@@ -782,7 +782,9 @@ Buffer.prototype.slice = function slice (start, end) {

   var newBuf
   if (Buffer.TYPED_ARRAY_SUPPORT) {
+    this.__proto__ = Uint8Array.prototype
     newBuf = this.subarray(start, end)
+    this.__proto__ = Buffer.prototype
     newBuf.__proto__ = Buffer.prototype
   } else {
     var sliceLen = end - start
@LinusU LinusU mentioned this issue Jan 13, 2016
@feross
Copy link
Owner

feross commented Jan 13, 2016

No, 5f5478a was just changing Safari 5-7 to use the typed array implementation.

It sounds like Chrome broke something in Canary. I opened an issue on the V8 tracker so we can fix this before this makes it farther in the release cycle. https://bugs.chromium.org/p/v8/issues/detail?id=4665

@LinusU
Copy link
Contributor Author

LinusU commented Jan 13, 2016

Hehe, that is some timing though 😆

The output of this script in the above stated version of Chrome alerts false and true. In Chrome Stable, Version 47.0.2526.106 (64-bit), it alerts true and true.

function typedArraySupportBroken () {
  function Bar () {}
  try {
    var arr = new Uint8Array(1)
    arr.foo = function () { return 42 }
    arr.constructor = Bar
    return arr.foo() === 42 && // typed array instances can be augmented
        arr.constructor === Bar && // constructor can be set
        typeof arr.subarray === 'function' && // chrome 9-10 lack `subarray`
        arr.subarray(1, 1).byteLength === 0 // ie10 has broken `subarray`
  } catch (e) {
    return false
  }
}

function typedArraySupportWorks () {
  try {
    var arr = new Uint8Array(1)
    arr.foo = function () { return 42 }
    return arr.foo() === 42 && // typed array instances can be augmented
        typeof arr.subarray === 'function' && // chrome 9-10 lack `subarray`
        arr.subarray(1, 1).byteLength === 0 // ie10 has broken `subarray`
  } catch (e) {
    return false
  }
}

alert(typedArraySupportBroken())
alert(typedArraySupportWorks())

Conclusion: The constructor cannot be set in Chrome 49.

@feross
Copy link
Owner

feross commented Jan 13, 2016

Conclusion: The constructor cannot be set in Chrome 49.

It's actually throwing the exception on arr.subarray(1, 1). But it only does that because the constructor property was set.

Crazy timing.

@feross
Copy link
Owner

feross commented Jan 13, 2016

Actually, the tests aren't failing because arr.subarray is throwing an exception. They're failing because it's returning the full array!

Here's a simple test case to repro:

function Buffer (arg) {
  var arr = new Uint8Array(arg)
  arr.__proto__ = Buffer.prototype
  return arr
}
Buffer.prototype.__proto__ = Uint8Array.prototype
Buffer.__proto__ = Uint8Array

var buf = new Buffer(10)

var buf2 = buf.subarray(2)

console.log(buf2.length) // returns 10 in Canary, instead of 8 as expected

@LinusU
Copy link
Contributor Author

LinusU commented Jan 13, 2016

That's true, I must have missed typing out the end of that sentence :)

Hopefully this will get fixed sooner rather than later in Chrome and than we won't need to do anything at all here...

@feross
Copy link
Owner

feross commented Jan 13, 2016

Yeah, I think let's wait a couple days. If there's no indication they plan to fix it in Chrome in a timely manner, we can do a new release with your PR and add a big TODO to the code to remove that as soon as possible.

@dcousens dcousens added the bug label Jan 13, 2016
@littledan
Copy link
Contributor

This has already been fixed in Chrome Canary. Thanks for the bug report.

@nolanlawson
Copy link
Collaborator

Closing based on #95 (comment)

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

No branches or pull requests

5 participants