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

fix(parseKeyValue): ignore properties in prototype chain. #8070

Closed
wants to merge 1 commit into from

Conversation

caitp
Copy link
Contributor

@caitp caitp commented Jul 3, 2014

Previously, properties (typically functions) in the prototype chain
(Object.prototype) would shadow query parameters, and cause them to be
serialized incorrectly.

This CL guards against this by using hasOwnProperty() to ensure that only own
properties are a concern.

Fixes #8068

@caitp
Copy link
Contributor Author

caitp commented Jul 3, 2014

@matsko can you give this a quick review? I'd like to check this in shortly as this is a kind of embarassing bug

var oldQ = Object.prototype.q;
Object.prototype.q = function(a,b,c,d,e,f,g) {};

expect(parseKeyValue('q=123')).toEqual({
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

all browsers will fail this without this fix.

Currently, any query parameter name which shadows a property of Object.prototype will cause issues, notably watch in Firefox for which this issue was reported.



it('should ignore properties higher in the prototype chain', function() {
expect(parseKeyValue('toString=123')).toEqual({
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Refactored the test to not modify Object.prototype --- still fails without the change in Chrome

Previously, properties (typically functions) in the prototype chain (Object.prototype) would shadow
query parameters, and cause them to be serialized incorrectly.

This CL guards against this by using hasOwnProperty() to ensure that only own properties are a concern.

Closes angular#8070
Fixes angular#8068
@caitp caitp closed this in cb42766 Jul 4, 2014
caitp added a commit to caitp/angular.js that referenced this pull request Jul 4, 2014
Previously, properties (typically functions) in the prototype chain (Object.prototype) would shadow
query parameters, and cause them to be serialized incorrectly.

This CL guards against this by using hasOwnProperty() to ensure that only own properties are a concern.

Closes angular#8070
Fixes angular#8068
caitp added a commit that referenced this pull request Jul 4, 2014
Previously, properties (typically functions) in the prototype chain (Object.prototype) would shadow
query parameters, and cause them to be serialized incorrectly.

This CL guards against this by using hasOwnProperty() to ensure that only own properties are a concern.

Closes #8070
Fixes #8068
ckknight pushed a commit to ckknight/angular.js that referenced this pull request Jul 16, 2014
Previously, properties (typically functions) in the prototype chain (Object.prototype) would shadow
query parameters, and cause them to be serialized incorrectly.

This CL guards against this by using hasOwnProperty() to ensure that only own properties are a concern.

Closes angular#8070
Fixes angular#8068
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Firefox-only - Angular runs into problem with a query parameter named 'watch'
2 participants