subarray returns Uint8Array which can't handle toString('uff-8')

[anion155]

import { Buffer } from 'buffer';

declare const messageRaw: ArrayBuffer;
const message = Buffer.from(messageRaw);
const headCorrect = message.slice(0, size).toString('utf-8'); // 57,57,97,102,101,102,56,57,97....
const headIncorrect = message.subarray(0, size).toString('utf-8'); // 99af...

[personalizedrefrigerator]

I'm also experiencing this issue with version 6.0.3 (running in React Native/Hermes).

Related:

• .subarray method should return Buffer instance #260
• fix: subarray returns Buffer instance, fixes #260 #261

[chjj]

Any runtime which displays this behavior doesn't implement the es2016 "subarray calls child class" behavior and is returning a plain Uint8Array.

Node v4.0.0 to v8.0.0 displayed this behavior, even with the native node.js Buffer object. So if you are running with an older version of v8, it could be argued that feross/buffer is accurately emulating the native node.js Buffer.

This is probably only fixable with an ugly hack. Not sure if people are open to that. Personally, I would just avoid using subarray and stick to slice when using Buffers.

[chjj]

Note: if you are experiencing this in a runtime which does support es2016 (i.e. a modern runtime), please post the runtime/browser/platform here.

If there is actually a popular runtime out there that doesn't implement es2016 properly, we may have to support it.

[hedi-edelbloute]

Hi @chjj 👋

We faced this issue twice, it broke only on this app (React native environment, es2017) :
https://github.com/LedgerHQ/ledger-live/blob/develop/apps/ledger-live-mobile/package.json
https://github.com/LedgerHQ/ledger-live/blob/develop/apps/ledger-live-mobile/tsconfig.json

We polyfill Buffer like this : https://github.com/LedgerHQ/ledger-live/blob/develop/apps/ledger-live-mobile/src/polyfill.ts#L33

And it works perfectly fine usually

Some infos about our issue... it first occured like this :
cardano-foundation/ledgerjs-hw-app-cardano#37

Our partner Strica found that the issue was related to the subarray method, we fixed the issue by gently asking them to use slice instead of subarray (but slice is deprecated on node)

We stumbled on the issue a second time with this PR (that we reverted) : LedgerHQ/ledger-live#6189

Thanks to the analysis of overcat we found some differences between our environments :

The second environment is this one (Electron, React, es2020) :
https://github.com/LedgerHQ/ledger-live/blob/develop/apps/ledger-live-desktop/package.json
https://github.com/LedgerHQ/ledger-live/blob/develop/apps/ledger-live-desktop/tsconfig.json

Issue occurs here with a subarray method call : https://github.com/stellar/js-xdr/blob/master/src/serialization/xdr-writer.js#L79

According to overcat, returned buffer is a Uint8Array 🤔

We are considering doing the ugly hack you are talking about just below our global.Buffer polyfill

Hopefully I gave enough infos to see if there is something to be done on your side or not, and thanks !

[chjj]

@hedi-edelbloute is this affecting iOS, Android, or both? Which versions?

I can't easily check to see if a Safari web view would cause this right now, but I would be shocked if this is occurring inside an Android web view.

edit: The electron one is very strange. Not sure what to make of that.

[chjj]

In the meantime you could try this monkey patch:

if (!(Buffer.alloc(1).subarray(0, 1) instanceof Buffer)) {
  Buffer.prototype.subarray = function subarray() {
    const result = Uint8Array.prototype.subarray.apply(this, arguments);
    Object.setPrototypeOf(result, Buffer.prototype);
    return result;
  };
}

[hedi-edelbloute]

@chjj My phone is a Redmi note 10 Pro on Android 11 and I reproduced the issue, I'm trying to gather more informations. I think it also occured on iOS but i'm going to confirm this.

Thank you for the patch proposal and your help, it's greatly appreciated.

EDIT : We have no issue on Electron environment, sorry if it's confusing, it's only on React native environment

[chjj]

Is your Android web view somehow using an old version of chrome? Is there any way to check? I'm not an expert on mobile web views, but I'm certain chrome version 60.x.x included v8 version 6.0.286 which should have the correct es2016 subarray behavior. (Chrome 60 was released 2017-07-25)

[hedi-edelbloute]

Confirmed that issue also happens on iOS, version 16.6.1 on iPhone 12

Same here, not an expert about this subject 😅 I think I found a solution to get the webview version using chrome://inspect/#devices, let me check !

[vweevers]

stick to slice when using Buffers.

Note that slice() is deprecated in Node.js because the Node.js implementation (which does not copy) is not compatible with TypedArray.prototype.slice() which does copy. Unlikely to ever be removed from the Node.js API though, so you're fine.

[chjj]

@vweevers, agreed.

I think ledger might be in an unfortunate situation though. Their ecosystem is comprised of a number submitted apps from many different developers who are probably expecting subarray to behave the way it does in modern node.js.

I'm still not exactly convinced feross/buffer should account for this, but I am really confused as to why these mobile web views are not exhibiting modern javascript behavior with regards to subarray. It'd be nice to get to the bottom of this.

[chjj]

Here's a smaller test case you can use for testing the subarray behavior of the web view:

'use strict';

function TestBuffer(...args) {
  const buf = new Uint8Array(...args);
  Object.setPrototypeOf(buf, TestBuffer.prototype);
  return buf;
}

Object.setPrototypeOf(TestBuffer.prototype, Uint8Array.prototype);
Object.setPrototypeOf(TestBuffer, Uint8Array);

console.log(TestBuffer(1).subarray(0, 1) instanceof TestBuffer);

Prints true for the proper es2016 behavior, false otherwise.

[overcat]

Here's a smaller test case you can use for testing the subarray behavior of the web view:

'use strict';

function TestBuffer(...args) {
  const buf = new Uint8Array(...args);
  Object.setPrototypeOf(buf, TestBuffer.prototype);
  return buf;
}

Object.setPrototypeOf(TestBuffer.prototype, Uint8Array.prototype);
Object.setPrototypeOf(TestBuffer, Uint8Array);

console.log(TestBuffer(1).subarray(0, 1) instanceof TestBuffer);

Prints true for the proper es2016 behavior, false otherwise.

I am not familiar with this area, but my device (Android 13, Chrome 122.0.6261.90 and Android System WebView 122.0.6261.64) can reproduce the issues encountered by the Ledger team. I executed this code on my device through this way, and it output true on my device.

[hedi-edelbloute]

It's Android System Webview 121.0.6167.178 for me

I think ledger might be in an unfortunate situation though. Their ecosystem is comprised of a number submitted apps from many different developers who are probably expecting subarray to behave the way it does in modern node.js.

Exactly

[chjj]

@overcat, okay, I just want to make sure I have this right:

1. The device prints true when running the above test code.
2. Buffer#subarray does not return a Buffer on that same device.

Is that correct?

Can you verify for sure that number 2 is also happening -- that is, can you isolate it down to a simple test case rather than the code @hedi-edelbloute posted to be absolutely sure Buffer#subarray is being called and you're not accidentally calling subarray on a Uint8Array?

If both of these things are happening at once, there is something very strange going on and I will have to investigate further.

[overcat]

The following is my test.

// main.js
const buffer = require('buffer');
const Buffer = buffer.Buffer
const data = Buffer.alloc(10);
const subarray = data.subarray(0, 5)
const subarray_is_buffer = Buffer.isBuffer(subarray)
document.getElementById("p1").innerHTML = subarray_is_buffer;

browserify main.js -o bundle.js

<!DOCTYPE html>
<html lang="en">
<head>
    <meta charset="UTF-8">
    <meta name="viewport" content="width=device-width, initial-scale=1.0">
    <title>Document</title>
</head>
<body>
    <p id="p1">Loading...</p>
</body>
<script src="./bundle.js"></script>
</html>

The output is true.

Update: I just noticed that browserify is not using the latest version of buffer.
Update: I upgraded the above example to 6.0.3, and the result is still the same.

[chjj]

@overcat, if that's the case then I see only two possibilities for the errors produced by the code mentioned above:

1. subarray was actually being called on a plain Uint8Array.
2. polyfill.ts happens to be pulling in a much older version of feross/buffer. Older versions of buffer/ did not account for the 3-argument subarray reinstantiation.

[chjj]

Update: I just noticed that browserify is not using the latest version of buffer.

Ahh, case solved. That was definitely a rollercoaster. Started questioning reality for a second there.

Yeah, make sure you're using the latest buffer module.

edit: Wait, do you mean to say your example above is not using the latest buffer or the polyfill.ts is not using the latest buffer?

[overcat]

edit: Wait, do you mean to say your example above is not using the latest buffer or the polyfill.ts is not using the latest buffer?

@chjj I'm referring to my example, I generated the code by running browserify main.js -o bundle.js, and then I checked that browserify is still using version 5.x.

[hedi-edelbloute]

@overcat Maybe we should try doing instanceof checks on both the output of subarray, and the original Buffer to see if we are dealing with a different class

instanceof Buffer and instanceof Uint8Array

Would it help @chjj ?

[hedi-edelbloute]

We are using version 6.0.3
https://github.com/LedgerHQ/ledger-live/blob/develop/apps/ledger-live-mobile/package.json#L123

[chjj]

@overcat, hmmm, okay, I guess that makes sense since buffer v5.7 does handle subarray properly, but still doesn't explain our issue then.

@hedi-edelbloute, yeah, it would be useful if you would modify the affected code to print something like:

function functionThatCallsSubarray(buf) {
  console.log(buf instanceof Buffer);
  const res = buf.subarray(x, y);
  console.log(res instanceof Buffer);
  return res;
}

If that prints true false, I'm out of ideas.

[chjj]

I thought of one more idea, but then I need to call it quits for a bit.

If the above test fails, please show me the output of:

'use strict';

function TestBuffer(...args) {
  console.log(args);
  const buf = new Uint8Array(...args);
  Object.setPrototypeOf(buf, TestBuffer.prototype);
  return buf;
}

Object.setPrototypeOf(TestBuffer.prototype, Uint8Array.prototype);
Object.setPrototypeOf(TestBuffer, Uint8Array);

console.log(TestBuffer(1).subarray(0, 1) instanceof TestBuffer);

It should be something like:

[1]
[arrayBuffer, 0, 1]
true

[chjj]

Calling it quits now, but some final thoughts on this issue:

1. This clearly isn't an issue with the mobile web view. It's behaving exactly as it should as evidenced by the TestBuffer code.
2. This isn't an issue with v5.x of feross/buffer as evidenced by @overcat's browserify test (this is almost certain to be true of v6.x as well).

I suspect this is some kind of dependency issue. There may be multiple versions of feross/buffer in play. Perhaps one of them is an older one which doesn't handle subarray properly. I recommend you somehow audit the generated code to really see what's going on: to see if there are multiple buffer modules and what versions they are. It's important that this line is present in all of them if there are multiple.

[hedi-edelbloute]

Hey @chjj don't worry, it's not an urgent matter, I will post results here 😃

Thank you so much already

You could be onto something with the dependency issue as we could have multiple versions at play

[overcat]

I modified the buffer@6.0.3 package in node_modules, added Buffer.version = "6.0.3" in index.js, and I think this will help confirm if we are using this version.

Here is the output in LLM:

console.log(Buffer.version);
> 6.0.3

const data = Buffer.alloc(10);
function functionThatCallsSubarray(buf: any) {
  console.log(buf instanceof Buffer);
  const res = buf.subarray(0, 5);
  console.log(res instanceof Buffer);
  return res;
}
functionThatCallsSubarray(data);

> true
> false

function TestBuffer(...args) {
  console.log(args);
  const buf = new Uint8Array(...args);
  Object.setPrototypeOf(buf, TestBuffer.prototype);
  return buf;
}

Object.setPrototypeOf(TestBuffer.prototype, Uint8Array.prototype);
Object.setPrototypeOf(TestBuffer, Uint8Array);
console.log(TestBuffer(1).subarray(0, 1) instanceof TestBuffer);

[1]
false

[chjj]

[1]
false

Why does it now print false when it printed true in your original test? Did you run this on a different device?

The above output is consistent with pre-es2016 behavior: the TestBuffer constructor is not being invoked by subarray.

[overcat]

@chjj I am using the same device. this test is running on Google Chrome on my phone, and this one is in LLM.

LLM is built on React Native, and then I realized that this is because React Native does not use Android System WebView as its default runtime environment? Then I found this link, it uses Hermes. Then I created a brand new project with React Native, which only contains the above code snippet, and it still outputs [1] and false.

After I turned off the Hermes engine, it output the following results:

[1]
[[], 0, 1]
true

This is a feature supported by Hermes: https://github.com/facebook/hermes/blob/main/doc/Features.md

Update: I have created a test project here, you can clone it if you need.

https://github.com/overcat/LedgerBufferDebugApp/blob/408f57917a2ca1b01c8c297aae7718083f668404/App.tsx#L67-L95
For this piece of code, the output is:

 LOG  Test 1
 LOG  [1]
 LOG  false
 LOG  Test 2
 LOG  false
 LOG  false
 LOG  0,0,0,0,0
 LOG  Test 3
 LOG  true
 LOG  false

cc @hedi-edelbloute

[overcat]

Note: if you are experiencing this in a runtime which does support es2016 (i.e. a modern runtime), please post the runtime/browser/platform here.

If there is actually a popular runtime out there that doesn't implement es2016 properly, we may have to support it.

Hi @chjj, based on the above discussion, I would like to ask if you are interested in adding support for the Hermes engine? It is the default engine for React Native projects.