Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

angular copy should throw exception when source and destination are not the same type. #15444

Closed
m-amr opened this issue Nov 27, 2016 · 2 comments

Comments

@m-amr
Copy link
Contributor

m-amr commented Nov 27, 2016

Note: for support questions, please use one of these channels: https://github.com/angular/angular.js/blob/master/CONTRIBUTING.md#question. This repository's issues are reserved for feature requests and bug reports.

Do you want to request a feature or report a bug?
bug

What is the current behavior?

In the following cases

1 - angular copy a source (Array Type) and destination (Object Type), then the app crashes
TypeError: destination.push is not a function
at copyRecurse (angular.js:911).......

2 - angular copy a source (Object Type) and destination ( Array Type) then source will equal empty array []

3 - angular copy a source (String Type) and destination ( Object Type) then

var result = angular.copy('abc', {}); // output {"0":"a","1":"b","2":"c"}

If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem via https://plnkr.co or similar (template: http://plnkr.co/edit/tpl:yBpEi4).

Plunker for the case no 1
https://plnkr.co/edit/BwmL7DHUF9xxo4G58orM?p=preview

Plunker for case no 2,3
https://plnkr.co/edit/vmn7wv8sI33PvSxVtM1q?p=preview

What is the expected behavior?

I suggest if the source and the destination are not the same type it should throw an exception

What is the motivation / use case for changing the behavior?
Prevent the app for crashing for the first case, and make sure that copy method is working as expected for all the cases.
since that Javascript is not static typed language we can't make sure that the developer will always enter a source and destination of the same type

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.
1.5.8

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

In case of destination in not undefined check if source and destination are not the same type then throw exception

 function copy(source, destination) {

  var stackSource = [];
  var stackDest = [];

  if (destination) {
    // Add this case
    if(typeof source !== typeof destination){
       throw ngMinErr('cpi', 'Can\'t copy! Source({0} type) and destination({1} type) are not the same type', typeof source, typeof destination);
  }

if (isTypedArray(destination) || isArrayBuffer(destination)) {
  throw ngMinErr('cpta', 'Can\'t copy! TypedArray destination cannot be mutated.');
}
if (source === destination) {
  throw ngMinErr('cpi', 'Can\'t copy! Source and destination are identical.');
}
@m-amr m-amr changed the title angular copy should throw exception when source and destenation are not the same type. angular copy should throw exception when source and destination are not the same type. Nov 27, 2016
@gkalpak
Copy link
Member

gkalpak commented Nov 28, 2016

IMO, it works as expected. There might be legitimate usecases for copying data to a slightly different object type. In any case, if the user explicitly passed a destination object, we should not prevent them from using that - it's their responsibility to provide something that works. also, throwing a more descriptive error won't prevent the app from crashing.

@Narretz
Copy link
Contributor

Narretz commented Dec 2, 2016

since Javascript is not static typed language we can't make sure that the developer will always enter a source and destination of the same type

That's right, but doesn't the developer need to make sure that his objects are of the same kind? This is the kind of application logic that needs to be sound in the first place. This is also why we have Typescript, Flow etc - to make Javascript type-safer, and to prevent developer errors.
Can you describe a scenario where you would reasonanly expect angular.copy to receive two different typed, incompatible objects?

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

Successfully merging a pull request may close this issue.

3 participants