Paranoia with null object

[shadowspawn]

I'm on a roll with a paranoia series... Another interesting question that I don't have relevant experience to base a strong opinion on.

@ljharb asked an interesting question in #26:

I hope result.flags is a null object?

Short version: no, should we do that?

---

Long version

The current answer is no, initialised with

const result = {
    flags: {},
    values: {},
    positionals: []
  };

I like the purity of a null object, but it has a slightly alarming behaviour in logging for users unaware of the behaviour:

 % node -e 'a=Object.create(null); a.foo="bar"; console.log(a)'
[Object: null prototype] { foo: 'bar' }

The documentation covers at some lengths the complications:

• https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/create

which can lead to some support questions:

• https://stackoverflow.com/questions/56298481/how-to-fix-object-null-prototype-title-product/56298574

I like the idea of using a null object in theory. With zero-config parsing in parseArg there is arguably more reason to consider null prototype. However, I don't have any practical experience with working with null objects. I am interested in comments from people with experience, and of how new users cope.

[ljharb]

I don’t think the logging thing is an issue; CVEs because someone names an option toString i think is an issue.

[bcoe]

The trick we do in yargs-parser is to perform all the parsing with Object.create(null) but then the object we return has the full prototype:

https://github.com/yargs/yargs-parser/blob/main/lib/yargs-parser.ts#L1045

We had a couple CVEs opened related to prototype polution in yargs-parser, we defintiely want to avoid a similar situtation in Node core, as getting a patch out is much more of a pain...

---

Perhaps a question to ask, do we need to return objects with a prototype? My concern is people will expect this right?

[ljharb]

Yes, they will; I think we should.

[shadowspawn]

Nice simple approach to keep the more subtle usage internal. That addresses my concern.

[bcoe]

@shadowspawn @ljharb feels like this conversation has a lot in common with #32, perhaps we can just state one of the goals in the approach we land on to prvent prototype pollution should be that the public interface still feels natural.

[shadowspawn]

There is definitely overlap in conversation, due in part to my issue creation with fuzzy understanding.

My current understanding is there are three things we are taking into account:

1. immune to existing prototype pollution (Paranoia with Primordials #31 refactor: use primordials #37)
2. correct vanilla-like function in the face of end-user or author using interesting keys (like toString)
3. prevent prototype pollution in the face of end-user or author using problematic keys (like __proto__)

The solutions being discussed which cover (2) and help with (3) are internally using a null object, or more explicitly, a map. Most of that conversations has been in #32.

Happy closing this issue as asking an implementation decision and the wider goal is being discussed deeper in #32.

[shadowspawn]

Quoted goal in #32, closing here.