Simple object check

[Gongreg]

Hello.
First of all thank you for the library, it works really great!

I've just bumped into one issue. In our setup we have iframe & parent window running on same domain and interacting with one another.

One of those interactions is:

const intialState = iframe.someFunction();

const state = create(initialState, ...);

Due to this, even though the returned object is "simple", the Object.getPrototypeOf(value) === Object.prototype check fails.

I can overcome this by using mark functionality:

const state = create(initialState, ..., {  mark: () => "immutable" });

This brings a couple of questions:

1. Is it okay to simply use mark: () => "immutable",? Will this lead to some side effects? I haven't digged too deep into the source code yet, but couldn't find info in the docs.
2. Next to mark docs, it says that (AutoFreeze and Patches should both be disabled). Is that really the case? I've tried to use them with mark and it looked like it works fine.
3. Maybe there is a different kind of "simple" object check that would not break in this crazy scenario?

[unadlib]

hi @Gongreg ,

Is it okay to simply use mark: () => "immutable",? Will this lead to some side effects? I haven't digged too deep into the source code yet, but couldn't find info in the docs.

If you're sure the object structure is all immutable, then you're good to go like this. But be careful, if the object structure has some non-plain objects in it, they might get unexpectedly treated as immutable. Like,

class Foo {
  bar = 1;
}
const initState = { foo: new Foo(), a: { b: 1 } };
const state = create(
  initState,
  (draft) => {
    draft.foo.bar = 2;
  },
  {
    mark: () => 'immutable',
  }
);
expect(state.foo).toBeInstanceOf(Foo);
expect(state.foo).not.toBe(initState.foo);

Any objects in initState that get tweaked will be shallow copied, they've been marked as immutable.

---

Next to mark docs, it says that (AutoFreeze and Patches should both be disabled). Is that really the case? I've tried to use them with mark and it looked like it works fine.

Typically, when using the mark function, you shouldn't enable AutoFreeze and Patches. It could cause some unnecessary mix-ups, and It's not an equivalent operation patch.

For example,

class Foo {
  bar = 1;
}
const initState = { foo: new Foo(), a: { b: 1 }, time: new Date() };
const [state, patches] = create(
  initState,
  (draft) => {
    draft.foo.bar = 2;
    draft.time.setDate(2);
  },
  {
    enableAutoFreeze: true,
    enablePatches: true,
    mark: (value) => {
      if (value instanceof Foo) return 'immutable';
    },
  }
);
expect(state.foo).toBeInstanceOf(Foo);
expect(state.foo).not.toBe(initState.foo);
expect(state.time).toBe(initState.time);

initState.time is mutable, but the patches don't include its update. So, we can't get an equivalent object structure by applying those patches. But if you use mark: () => "immutable", in that specific case, you can totally enable AutoFreeze and Patches.

I've already updated it in the docs.

---

Maybe there is a different kind of "simple" object check that would not break in this crazy scenario?

You can use isSimpleObject as your custom mark.

const isSimpleObject = (value) => {
  const prototype = Object.getPrototypeOf(value);
  if (prototype === null) {
    return true;
  }
  const constructor =
    Object.hasOwnProperty.call(prototype, 'constructor') &&
    prototype.constructor;
  return (
    typeof constructor === 'function' &&
    Function.toString.call(constructor) ===
      Object.prototype.constructor.toString()
  );
};

const initialState = iframe.someFunction();

const state = create(initialState, callback, {
  mark: (value) => {
    if (isSimpleObject(value)) {
      return 'immutable';
    }
  },
});

isSimpleObject function can check for simple objects from across iframe or those without inheritance, like Object.create(null).

---

I'm thinking about adding some potentially useful mark functions to Mutative.

[unadlib]

I've released Mutative v0.7.3, which supports marking simple objects.

For example,

import { create, markSimpleObject } from 'mutative';

const baseState = {
  foo: {
    bar: 'str',
  },
  simpleObject: Object.create(null),
};

const state = create(
  baseState,
  (draft) => {
    draft.foo.bar = 'new str';
    draft.simpleObject.a = 'a';
  },
  {
    mark: markSimpleObject,
  }
);

expect(state.simpleObject).not.toBe(baseState.simpleObject);

[waynebloss]

In this conversation I found that NodeJS code creates simple objects everywhere (in the form of { __proto__: null } see example) so that they don't inherit a polluted Object prototype (for security). They even have an eslint rule to enforce it.

With that in mind, and not knowing what type of data a developer might stick into one of my drafted states, wouldn't it be safer to always check if (prototype === null) by default and mark it as immutable? (Or to just make the whole markSimpleObject mark the default behavior?)

Also, am I protected against this if strict: true during development, even without mark: markSimpleObject?

[unadlib]

hi @waynebloss , mark: markSimpleObject or enabling strict mode does not protect the Object prototype. The mark function only defines the nature of a shallow copy or marks data.

The main issue is that protecting the Object prototype is very costly, and it may vary depending on the browsers, requiring unnecessary overhead. Moreover, prototype chain pollution is not common.

Specifically, there are multiple ways to access the prototype chain, such as: Object.getPrototypeOf(objectOrDraft), objectOrDraft.__proto__, objectOrDraft.constructor.prototype, objectOrDraft.isPrototypeOf.__proto__.__proto__, and so on.

Therefore, I think it is not very necessary for us to do this, don't you think so?

[waynebloss]

@unadlib I was not trying to protect against object prototype pollution - I was surprised when I observed that objects created with Object.create(null) or { __proto__: null }, which live inside in the base state are modified during create.

[waynebloss]

I tried it out and I see that strict mode does catch the issue. Thanks!

e.g.

> var baseState = { a: {}, b: Object.create(null) };
undefined
> var [draft, finalize] = create(baseState, { enablePatches: true, strict: true });
undefined
> draft.a.name = "a";
'a'
> draft.b.name = "b";
Uncaught:
Error: Strict mode: Mutable data cannot be accessed directly, please use 'unsafe(callback)' wrap.
    at checkReadable
    at Object.get