query-string: result from `parse` no longer has `hasOwnProperty` method

in 3.0.1, parse had the following behavior:

> qs.parse('my-param=my-val')
{ 'my-param': 'my-val' }
> qs.parse('my-param=my-val').hasOwnProperty
[Function: hasOwnProperty]
> qs.parse('h=j').hasOwnProperty
[Function: hasOwnProperty]
> qs.parse('').hasOwnProperty
[Function: hasOwnProperty]

in 3.0.2, the object returned no longer has the hasOwnProperty method:

> qs.parse('hbud-fixture=not-logged-in')
{ 'hbud-fixture': 'not-logged-in' }
> qs.parse('hbud-fixture=not-logged-in').hasOwnProperty
undefined
> qs.parse('hbudfixture=not-logged-in').hasOwnProperty
undefined
> qs.parse('hbudfixture=notloggedin').hasOwnProperty
undefined
> qs.parse('').hasOwnProperty
[Function: hasOwnProperty]
> qs.parse('h=j').hasOwnProperty
undefined

I am unsure whether this was considered part of the API, but some consumers rely on the behavior - https://github.com/mjackson/history/blob/master/modules/useQueries.js#L13

About this issue

Original URL
State: closed
Created 8 years ago
Comments: 18 (4 by maintainers)

Commits related to this issue

Close #53 PR: support object properties - fixes #47. Fixes #47, Fixes #50 — committed to sindresorhus/query-string by SamVerschueren 8 years ago

Most upvoted comments

I reverted that change and did a patch release. Sorry for the trouble. I didn’t realize users were dependent on the behavior…

sindresorhus on Mar 19, 2016

You’re right, but a lot of people write it as obj.hasOwnProperty instead of going up to Object.prototype.hasOwnProperty. It’s a pretty big usability caveat.

Either way, though, this change in practice triggers enough bugs in consumers of this code that it should be released as breaking.

taion on Mar 18, 2016

No, that wasn’t what I was recommending. What if someone relied on the toString method of an object? You’d have to define that as well. What if someone tried to parse "hasOwnProperty=hello+from+a+cheeky+user"? To even allow overwriting hasOwnProperty, you’d have to specify writable: true. Even when you did, the 'hasOwnProperty' property would be non-enumerable so Object.keys(parsedParams) would return an empty array.

I am recommending something like this:

/**
 * Safely set a value on a dictionary.  Do *not* trigger special behavior from any setters.  An example of a setter is `Object.prototype.__proto__`
 */
function safeSet(obj, key, value) {
    if(key in obj) {
        Object.defineProperty(obj, key, {
            value: value,
            enumerable: true,
            configurable: true,
            writable: true
        });
    } else {
        obj[key] = value;
    }
}

All that work just to keep Object.prototype in the prototype chain, even though any properties might be overwritten by parsed query params. With Object.create(null) or {__proto__: null}, you avoid ever giving anyone a false sense of security about calling methods on the returned object.

cspotcode on Mar 18, 2016