angular.copy doesn't copy typed subarrays

[zhukov]

Regression bug:

What is the current behavior?

It copies the whole buffer, not a slice.
Here is the demo: http://plnkr.co/edit/nU76OITTchFc48jFab2a?p=preview

What is the expected behavior?

When copying typed subarray expected behavior is to return the value of the same length.

What is the motivation / use case for changing the behavior?

I use a big byte buffer for the whole server API response and I make slices (via subarrays) in it for response object fields when deserializing. Then I need to copy object with fields to use at another place. With current behavior new copy is invalid: all byte fields have wrong contents.

Which versions of Angular, and which browser / OS are affected by this issue? Did this work in previous versions of Angular? Please also test with the latest stable and snapshot (https://code.angularjs.org/snapshot/) versions.

I can reproduce the bug in v1.5.7. It worked fine in previous angular version I used: v1.4.12

I think the bug was introduced in this commit:
7b51243
#13641

Other information (e.g. stacktraces, related issues, suggestions how to fix)

I think it doesn't work as expected because of this code:

  function copyType(source) {
    switch (toString.call(source)) {
      case '[object Int8Array]':
      case '[object Int16Array]':
      case '[object Int32Array]':
      case '[object Float32Array]':
      case '[object Float64Array]':
      case '[object Uint8Array]':
      case '[object Uint8ClampedArray]':
      case '[object Uint16Array]':
      case '[object Uint32Array]':
        return new source.constructor(copyElement(source.buffer));

it always copies the whole buffer, even if the view has shorter byte length. We can check that source is subarray by checking e.g. byteOffset value e.g.

In v1.4.12 the code was like this:

    if (isArray(source)) {
      destination = [];
      needsRecurse = true;
    } else if (isTypedArray(source)) {
      destination = new source.constructor(source);

[gkalpak]

The commit you mention is not the culprit (it still uses new source.constructor(source)).
It's 986647a that changed that.

/cc @jbedard

[jbedard]

I think when copying the *Array we we just need to also copy the byteLength and byteOffset?

[gkalpak]

Something like that. From a quick look, it seems that we could fix this by changing:

new source.constructor(copyElement(source.buffer))

to:

new source.constructor(copyElement(source.buffer), source.byteOffset, source.byteLength)

@zhukov , would you like to have the honor of submitting a PR with the fix? 😛

[zhukov]

@gkalpak sure! :) On it.

[zhukov]

@gkalpak Done. Took me a while.
Had to replace source.byteLength with source.length, because the constructor docs look like this: new Uint8Array(buffer [, byteOffset [, length]]); and tests would fail.

Btw, small remark about contributing. I had to install process package via npm manually, because otherwise grunt test would fail.