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

Safari: ArrayBuffers are sometimes not instanceof ArrayBuffer #166

Closed
jvilk opened this issue Jul 23, 2017 · 6 comments
Closed

Safari: ArrayBuffers are sometimes not instanceof ArrayBuffer #166

jvilk opened this issue Jul 23, 2017 · 6 comments

Comments

@jvilk
Copy link

jvilk commented Jul 23, 2017

Summary

I'm storing and retrieving ArrayBuffer objects from IndexedDB, and I noticed that in my test environment, the ArrayBuffer objects are not always instanceof ArrayBuffer in Safari. As a result, the Buffer package throws the following exception:

TypeError: First argument must be a string, Buffer, ArrayBuffer, Array,or array-like object.

This behavior does not occur in any other web browser.

Detailed Explanation

The test environment is that of the Karma test runner, which uses an iframe to isolate test code from the runner's code. As a result, the ArrayBuffer constructor object in the code's context is not referentially equivalent to the ArrayBuffer constructor object in the main window context. This is similar to the age-old Array problem, where arrays from other contexts are not necessarily instanceof Array, necessitating the use of Array.isArray.

Safari chooses the main window's constructor for the ArrayBuffer object retrieved from IndexedDB, so it is not an instanceof ArrayBuffer in the test code's context.

Here's a screenshot, just so you know I'm not totally mad here. This is evaluated at a breakpoint in the stack frame that throws the TypeError:
screen shot 2017-07-22 at 11 06 09 pm

I do not have a good solution to the problem without resorting to some hack (e.g., is the byteLength property present?).

feross added a commit that referenced this issue Aug 4, 2017
Fixes: #166

ArrayBuffers from another context (i.e. an iframe) do not pass the
`instanceof` check but they should be treated as valid.
@feross
Copy link
Owner

feross commented Aug 4, 2017

Interesting... instanceof is really quite terrible, isn't it?

Please see this PR with a potential solution: #170 Let me know if that works for you. If so, I'll merge it and do a new release.

@jvilk
Copy link
Author

jvilk commented Aug 4, 2017

Interesting... instanceof is really quite terrible, isn't it?

I want to agree with you, but I can't really blame JavaScript on this one! instanceof is only intended to check if the object was constructed by a particular function, which is totally reasonable. I'd blame the unnecessarily complex native interface between JavaScript code and the web browser, which makes it possible to have multiple constructors for the same type of object! Really, browsers should have a special type of handoff between contexts that appropriately adjusts native objects like this one to use the appropriate constructor object. (Sharing constructors is a no-go for security reasons -- e.g., monkey-patching and such.)

With that said, it seems like the other browsers do not have this issue; perhaps they already use the appropriate constructor object from the context in which programs invoke native APIs that create ArrayBuffer objects. (That sentence is a mouthful...)

Please see this PR with a potential solution: #170 Let me know if that works for you.

I just ran my test suite against it, and it appears to work wonderfully!

Safari 10.1.2 (Mac OS X 10.12.6): Executed 667 of 667 SUCCESS (24.15 secs / 20.861 secs)

Thanks for fixing this!

@feross
Copy link
Owner

feross commented Aug 5, 2017

Released as 5.0.7

@maximerety
Copy link

Hi!

I found this bug report while investigating a similar issue (not using the buffer library) and wanted to add two things:

Best regards,

@feross
Copy link
Owner

feross commented May 23, 2018

@maximerety Thanks for sharing. Hope it gets fixed one day so we can remove this hack!

@jvilk
Copy link
Author

jvilk commented May 23, 2018

@maximerety Thanks for reminding me of this issue! I just added a minimal complete test case to that bug report.

jasongrout added a commit to blink1073/jupyterlab that referenced this issue Aug 28, 2020
…views and array buffers.

Using instanceof to determine ArrayBuffers is fraught with problems in Javascript. See, for example, lodash’s implementation of isArrayBuffer, or the many isArrayBuffer libraries out there. Apparently, ArrayBuffers from a different context (like an iframe, or perhaps something in a test setup) so instanceof won’t work always. See feross/buffer#166 for some more examples.
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