Protobuf: JS: Type not convertible to Uint8Array

[jsievenpiper]

Hi!

I've been trying to get protobuf deserialization working in JavaScript but hitting a wall. I have a small sample message just to test everything is working:

message Sample {
    string text = 1;
    uint32 number = 2;
    bool truthiness = 3;
}

When I create a new Sample message (without deserializing it from anything), the message works fine.
Serializing it to bytes yields me a 24 byte Uint8Array.

However, if I try to create a Sample message from existing bytes, I get an assertion error: Type not convertible to Uint8Array. This is what I've tried:

var Sample = // loaded in from require;
var dataBytes = // some existing uint8array;
var sample = new Sample(dataBytes);
var sample2 = Sample.deserializeBytes(dataBytes);

The first one (sample) will silently fail, but the message is broken from there out (sample.getText() returns 10 instead of a string, as expected.. other fields return garbage as well).

The second one throws the assertion error. I've verified that the Uint8Array I'm receiving and testing with is the same one I get from manually building one via serializeBytes in the console.

Here's my data:

Am I perhaps just using this wrong?
Thanks in advance!

[haberman]

Hi Justin,

new Sample(dataBytes); isn't correct, I would expect this to corrupt your data. But Sample.deserializeBinary(dataBytes); should work. What browser or JS environment are you using?

[jsievenpiper]

@haberman thanks! I did notice there was another deserializeBinary call, so I tried that. Unfortunately I get the same result. However, I was able to get it working by wrapping the Uint8Array binary one more time:

Sample.deserializeBinary(new Uint8Array(dataBytes));

This works and does what's expected. It just felt odd that I couldn't use the Uint8Array I already had directly (this could just be me severely misunderstanding js typed arrays).

I reproduced this in FF26, latest Chrome, and Safari 9.0.2.

[haberman]

@jsievenpiper Hmm, are you sure that dataBytes is a Uint8Array? I'm suspicious because I don't think you can actually wrap Uint8Arrays inside each other -- the constructor will just copy the bytes into a different Uint8Array. The fact that it works when you construct a new Uint8Array makes me wonder whether dataBytes has some other type.

[jsievenpiper]

@haberman Yeah I figured it was suspicious as well, but from what I can tell (original screenshot), every browser is telling me that it is, indeed a Uint8Array. For giggles, I created an Int32Array, and the browser reported it as such (didn't tell me it was a different typed array) -- so I imagine it should work.

RE the constructor pattern I was trying:

new Sample(dataBytes);

FWIW I found this generated documentation that lead me to believe it would work:

/**
 * Generated by JsPbCodeGenerator.
 * @param {Array=} opt_data Optional initial data array, typically from a
 * server response, or constructed directly in Javascript. The array is used
 * in place and becomes part of the constructed object. It is not cloned.
 * If no data is provided, the constructed object will be empty, but still
 * valid.
 * @extends {jspb.Message}
 * @constructor
 */
proto.SampleModel = function(opt_data) { ... snip ... }

[haberman]

Can you post a reproducible test case for this? We should get to the bottom of it.

In the documentation you posted, "data array" isn't the same as "array of serialized bytes." Our implementation somewhat-publicly exposes its internal representation, which is a JavaScript array where data members are indexed by their field number. This is used somewhat frequently inside Google, but we're trying to deemphasize/discourage that externally, at least for now. I can see why this might have been confusing, and it might be worth clarifying in the comments.

[sincerity711]

@jsievenpiper if you are using buffer of node.js, a simple workaround is using buffer.buffer instead of buffer.

[bootstraponline]

I just encountered this issue with electron native image. I'm creating a Uint8Array but protobuf errors because the Uint8Array constructor check fails. I've worked around the bug by redefining the constructor.

\node_modules\google-protobuf\google-protobuf.js:62 Uncaught AssertionError: Failure: Type not convertible to Uint8Array.

$ pngArray.constructor === Buffer
true
$ pngArray instanceof Uint8Array
true

var pngPath = "path/to/some/screenshot.png";
var pngNativeImage = nativeImage.createFromPath(pngPath);
var pngArray = pngNativeImage.toPNG();

pngArray.constructor = Uint8Array;

var isUint8 = pngArray instanceof Uint8Array;
var isUint8Construct = pngArray.constructor === Uint8Array;

console.log("is uint8? " + isUint8);
console.log("is constructor uint8? " + isUint8Construct)

[jfromaniello]

The only way I have found so far to deserialize a buffer in node.js without getting this error is using this:

Sample.deserializeBinary(Array.from(buffer));

I tried with buffer.buffer but it didn't work.

[kbahr]

I'm having this problem in Chrome due to an interaction between a custom Buffer implementation and this function:

jspb.utils.byteSourceToUint8Array = function(a) {
  if (a.constructor === Uint8Array) return a;
  if (
    a.constructor === ArrayBuffer ||
    a.constructor === Buffer ||
    a.constructor === Array
  )
    return new Uint8Array(a);
  if (a.constructor === String)
    return goog.crypt.base64.decodeStringToUint8Array(a);
  goog.asserts.fail("Type not convertible to Uint8Array.");
  return new Uint8Array(0);
};

The custom Buffer implementation is FaceBook's LiteBuffer which attempts to abstract over various types of byte buffers:
https://github.com/rsocket/rsocket-js/blob/master/packages/rsocket-core/src/LiteBuffer.js
I'm getting data off the wire and it runs through this conversion function:
https://github.com/rsocket/rsocket-js/blob/master/packages/rsocket-core/src/LiteBuffer.js#L121
Then I attempt to deserialize using Protobuf which goes through this BinaryReader function:

jspb.BinaryReader.alloc = function(a, b, c) {
  if (jspb.BinaryReader.instanceCache_.length) {
    var d = jspb.BinaryReader.instanceCache_.pop();
    a && d.decoder_.setBlock(a, b, c);
    return d;
  }
  return new jspb.BinaryReader(a, b, c);
};

setBlock ultimately calls the utility function I mentioned above. The buffer I pass in falls through all the .constructor checks and reads as a non-buffer. However, it *is a Uint8Array as is apparent from the LiteBuffer function I linked. FaceBook just swapped the prototype in order to add instance methods. The buffer passes an instanceof Uint8Array check - shouldn't that be the criterion?

Is there a general fix in the works for this? This issue is 2 years old at this point.

[acozzette]

@kbahr Do you want to send us a pull request with your proposed fix?

[kbahr]

Thanks @acozzette , I'll take a stab at it - I didn't see obvious documentation for contributing. Shall I just fork and submit a PR from there? Are there guidelines for style and testing and whatnot that I can read somewhere?

[acozzette]

Thanks @kbahr! You can just create a fork and send us a pull request; the only real process to go through is that there's a CLA agreement to sign here: https://cla.developers.google.com/clas We roughly go by the style guide here but I wouldn't worry about it too much.

[graup]

I also keep running into this issue. @kbahr are you working on a fix?

[kbahr]

The fates have conspired against me, I'm afraid - I haven't had time to
look at it.

On Mon, Nov 19, 2018 at 12:06 AM Paul Grau [e-mail] wrote:

I also keep running into this issue. @kbahr https://github.com/kbahr
are you working on a fix?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/protocolbuffers/protobuf/issues/1319#issuecomment-439803961,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ADtqFmOlct0usQTx4LNVKWLpZlR8XUruks5uwmaDgaJpZM4HvhHB
.

[graup]

Wouldn't it be enough if we change

if (a.constructor === Uint8Array) return a;

to

if (a.constructor === Uint8Array || a instanceof Uint8Array) return a;

?

[kbahr]

I think so and that’s what I planned to do. It’s the forking and testing that I felt like I didn’t have time for :)

On Nov 19, 2018, at 7:04 PM, Paul Grau [e-mail] wrote:

Wouldn't it be enough if we change

if (a.constructor === Uint8Array) return a;
to

if (a.constructor === Uint8Array || a instanceof Uint8Array) return a;
?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

[luckyr13]

@jsievenpiper if you are using buffer of node.js, a simple workaround is using buffer.buffer instead of buffer.

this is the best solution, thank you