Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

objectOf should not require hasOwnProperty to be a function #183

Closed
tor-ocean opened this issue May 21, 2018 · 4 comments
Closed

objectOf should not require hasOwnProperty to be a function #183

tor-ocean opened this issue May 21, 2018 · 4 comments

Comments

@tor-ocean
Copy link

When using objects as dictionaries, and to avoid littering code with hasOwnProperty checks, it can be useful to use object = Object.create(null) instead of object = {} when creating the object.

Unfortunately, this causes the PropTypes.objectOf checker to throw:

Warning: Failed prop type: propValue.hasOwnProperty is not a function

as the code assumes that every value will have an hasOwnProperty method in its prototype chain:

if (propValue.hasOwnProperty(key)) {

In my case, I will have to work around it by using a regular object and refactor the rest of my own code to do the same hasOwnProperty check.

I would still argue that this is a bug and needs to be fixed. When checking that a value is an object where every own key maps to a value of a certain type, it is self-contradictory that the code simultaneously requires that one specific key is going to map to a function. What was supposed to check that the value is a dictionary of a specific type now risks crashing the app when that is the case.

Imagine the case where the object is truly being used as a dictionary, even with a regular {} object, what's to stop the key "hasOwnProperty" to map to some other value? Malicious users might even cause a component to crash for other users by exploiting this.

@ljharb
Copy link
Collaborator

ljharb commented May 22, 2018

This is a good fix; not because null objects are reasonable or because almost anyone would use them, but because delete Object.prototype.hasOwnProperty is a thing.

@dferber90
Copy link
Contributor

dferber90 commented May 28, 2018

I am getting the same warning when defining this prop-type validation:

PropTypes.shape({
  pathname: PropTypes.string.isRequired,
  query: PropTypes.objectOf(PropTypes.string),
}),

And when the passed value is:

{
  "pathname": "/foo",
  "query": {
    "perPage": "6",
    "sortBy": "createdAt",
    "sortDirection": "desc"
  }
}

It results in this error:

Warning: Failed prop type: propValue.hasOwnProperty is not a function

However, if I only use

PropTypes.shape({
  pathname: PropTypes.string.isRequired,
  query: PropTypes.object,
})

it works fine as pointed out in the issue description.

Observed with prop-types@15.6.1.

@likerRr
Copy link

likerRr commented Jun 18, 2018

Any updates on it? I also faced the same issue, when validating object, created with Object.create

@dferber90
Copy link
Contributor

#187 is approved and solves this. It's waiting for a merge still.

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

No branches or pull requests

4 participants