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

Correctly handle null prototypes #32

Merged
merged 2 commits into from
May 31, 2023

Conversation

msealand
Copy link
Contributor

@msealand msealand commented Mar 5, 2023

This fixes an error thrown when using is-what on an object with a null prototype (i.e. an object created via Object.create(null)).

Before:

> var iw = require('./dist/index.cjs');
> iw.isObject(Object.create(null));
Uncaught TypeError: Cannot read properties of null (reading 'constructor')
    at isPlainObject (/Users/mspoly/Documents/Development/3rdParty/is-what/dist/index.cjs:40:22)
    at Object.isObject (/Users/mspoly/Documents/Development/3rdParty/is-what/dist/index.cjs:49:12)

After:

> var iw = require('./dist/index.cjs');
> iw.isObject(Object.create(null));
true
> iw.isFullObject(Object.create(null));
false
> iw.isPlainObject(Object.create(null));
true

@mesqueeb
Copy link
Owner

mesqueeb commented Mar 5, 2023

@msealand i'm open to merging but what's the use case of creating objects like this ?

@msealand
Copy link
Contributor Author

msealand commented Mar 5, 2023

@mesqueeb It's just a method of creating a valid javascript object that doesn't have a prototype attached. The created object otherwise works as you would expect:

Welcome to Node.js v19.7.0.
Type ".help" for more information.
> var obj = Object.create(null);
undefined
> obj.a = 1;
1
> obj
[Object: null prototype] { a: 1 }
> Object.getPrototypeOf(obj);
null

I don't actually use it much personally, so I'm hesitant to say what the typical use case is. I suspect it's just to get an even lighter weight object. It is used quite a bit in the javascript ecosystem: https://github.com/search?q=Object.create%28null%29+extension%3Ats+extension%3Ajs&type=Code&ref=advsearch&l=&l=. I found the issue with is-what when trying to use it on objects coming from Koa.

@jcbhmr
Copy link
Collaborator

jcbhmr commented May 26, 2023

what's the use case of creating objects like this ?

@mesqueeb Use case is usually a Record<string, SomeThing> so that setting myMap[userInput] = anything doesn't have the ability to set __proto__ or .constructor or any other "special" keys. For instance in @msealand's case it was a Koa object that probably had a bunch of user-specific query params or something like ?name=Tim&age=34 into [null prototype] { name: "Tim", age: "34" } which is good! That means sending ?__proto__=hi won't cause the object to magically become a String. It's just [null prototype] { __proto__: "hi" } instead of [String(hi)] { } (instanceof String is true).

@mesqueeb
Copy link
Owner

@msealand sorry for taking so long. But i've decided the following:

your proposal

isPlainObject(Object.create(null)) // true
isAnyObject(Object.create(null)) // true

my decision

isPlainObject(Object.create(null)) // false
isAnyObject(Object.create(null)) // true

I made it so because I believe an object without any prototype should not be regarded as a "plain object".

However, I did made sure the isPlainObject / isAnyObject does not crash anymore thanks to this PR!

If you want to keep using the Object.create(null) method of creating objects, then please just use isAnyObject to make your checks : )

If you really want we can also add a new isPrototypelessObject function, which returns true if it's any object without a prototype.

@mesqueeb mesqueeb merged commit d0aa1d9 into mesqueeb:production May 31, 2023
@msealand
Copy link
Contributor Author

Thanks @mesqueeb, that works for me! And no worries about the timing; we're all busy 🙂. Also, thanks @jcbhmr for explaining the use case!

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

Successfully merging this pull request may close these issues.

3 participants