Change extend util to check hasOwnProperty

[stephen]

The lack of a hasOwnProperty check can accidentally cause functions on the prototype to bubble up to being a parameter in an API request.

For example... defining a polyfill:

Object.prototype.entries = function* entries(obj) {
  for (let key of Object.keys(obj)) {
    yield [key, obj[key]];
  }
};

will cause an extra entries param to exist in a request in the query string, e.g. https://www.googleapis.com:443'/calendar/v3/calendars/stephen%40stephenwan.net/events?entries=.

[ryanseys]

Please add a test.

[stephen]

@ryanseys - Added a test that fails before the changes to extend, but passes currently. Satisfactory? Just checks that a prototyped function doesn't get copied over.

[ryanseys]

Ahaha, jslint won't even let the build pass with it because extending Object is so bad. I guess we'll have to the scrap the test. I would highly suggest you don't extend native objects like that and would go as far to say that it's really not our fault if you do that and our code breaks because of it, but because this change is relatively straightforward and using hasOwnProperty is sort of "standard practice" in JavaScript due to limitations of for .. in .. I'll let it slide. God's speed for .. of ..

[stephen]

@ryanseys - I added a jshint ignore for the one line, so we can keep the test around and not have travis complain.

Generally agree with not extending native objects, but I believe Object.entries is (was?) being considered at some point for ES7 support, and is fairly useful - https://esdiscuss.org/topic/object-entries-object-values.

[ryanseys]

Cool :) Thanks! Any way you could finally squash into 1 commit? Sorry for the troubles and thanks for your PR.

[stephen]

No worries - was just in the middle of rebasing actually. 😸

[ryanseys]

Thanks! Released in v1.1.5!

[robertrossmann]

I shall only add here as a tip that extending native objects this way is indeed considered a terrible practice exactly because of the side-effects you observed.

If you really need to do so, you must do it "properly", making sure your extensions are not in an enumerable property.

Object.defineProperty(Array.prototype, 'entries',
  { enumerable: false // default if omitted
  , value: function entries () { /* logic here */ }
  }
)

[stephen]

@Alaneor - thanks for the tip. Realized that this was also causing a problem in request as well, and ended up switching to defineProperty.