Should ScriptableObject.sealObject also prevent modifications of parent and prototype

[rPraml]

Please consider this PR first as question, if I (mis)understand documentation or if it is really a bug

My use case is, that I have a shared scope and I want to prevent that scope from any modifications.

So I thougt, it would be a good idea to call scope.sealObject()

I noticed, that no normal property is writeable - but I can change the __parent__ and __proto__ of such sealed objects.
(This is also what I try to fix in this PR)

I planned also to implement sealObject in NativeJavaObject, so that you can seal them too, to prevent from further modification. For example block put operations and especially __parent__ and __proto__ property (I'm aware that I can use UnmodifiableMap and so on, but this does not prevent modification of the special props)

Then I found in the documentation of ScriptableObject:

It is possible to change the value of an existing property

This seems not to be true, at least what I have found in my tests. I dig further into documentation and found #676 and https://262.ecma-international.org/11.0/#sec-object.seal which confirms that existing properties should be modifiable.

So there IS a difference between java ScriptableObject.sealObject and js Object.seal (the later one does not call the first one and objects sealed with Object.seal throw only an EcmaError in strict mode, while objects sealed with ScriptableObject.sealObject throw an EvaluatorException)

Maybe someone can explain, what's the intention of sealObject

[rPraml]

When these two were swapped, the setPrototype some lines below is not needed

[rPraml]

This would prevent modification of SpecialRefs (TODO: Should I make 2 constants SpecialRef.PARENT / SpecialRef.PROTO)?

[p-bakker]

I think the JavaScript Object.seal and ScriptableObject.sealObject aren't necessarily to do the same thing.

JavaScript's Object.seal probably got added long after ScriptableObject.sealObject. Also, besides Object.seal there's also the more restrictive Object.freeze

To my knowledge, the purpose of ScriptableObject.sealObject is to prevent modifications on shared scopes, in which case I would say that the parent and proto properties should also not be writable.

Then again: no experience here with shared scopes 🙂

Changing the behavior would be a potentially breaking change imho, so something to postpone for v2.0.0?

[rPraml]

I've updated this PR.
My use case is still to protect a shared scope from any modification withScriptableObject.sealObject (also __parent__ and __proto__) This is what this PR would fix.

Nevertheless, it would be great if someone could explain the difference and intention of ScriptableObject.sealObject and Object.seal

[p-bakker]

I think there's no one around anymore to detail the difference between ScriptableObject.sealObject and Object.seal, but I did find this in old release notes: https://p-bakker.github.io/rhino/releases/new_in_rhino_1.5r5.html#seal-and-changes-in-semantic-of-sealed-objects and https://p-bakker.github.io/rhino/releases/new_in_rhino_1.5r5.html#api-for-context-sealing

Just guessing here, but it looks like ScriptableObject.sealObject was added before Object.seal existed and that they ought to behave similarly?

Object.seal(...) however allows changing the values of existing properties, which is not desirable when 'sealing' scopes using ScriptableObject.sealObject(...) to make the scope sharable. so in this respect, ScriptableObject.sealObject(...) is more similar in behavior to Object.freeze(...) (which is like Object.freeze, but also prevents adding new properties).

Note that in browsers, Object.seal(...) makes the proto property readonly, as modifying it could introduce new properties, which Object.seal(...) is to prevent.

Also note that __parent__ is deprecated and non-existing in most JS engines, but as long at its still there in Rhino, I think when sealing (or freezing) an object it should be treated similarly as proto.

So, in summary:

• I think I agree that ScriptableObject.sealObject(...) should also prevent modification of the proto/parent property
• ScriptableObject.sealObject(...) behavior is more like Object.freeze than Object.seal
• Then I found in the documentation of [ScriptableObject](https://github.com/mozilla/rhino/blob/master/src/org/mozilla/javascript/ScriptableObject.java#L1942): It is possible to change the value of an existing property I think this is an anomaly in the JavaDocs, since that behavior seems to have been changes in 1.5r5

So maybe the entire ScriptableObject.sealObject(...) implementation should be rewired to use the Object.freeze implementation, as it seems that are both trying to achieve the same thing

[rPraml]

So maybe the entire ScriptableObject.sealObject(...) implementation should be rewired to use the Object.freeze implementation, as it seems that are both trying to achieve the same thing

Full ack!

But one must note that Object.seal currently only works in strict mode. (AFAIR! I didn't check, if it prevents write access in non strict mode).

[p-bakker]

Full ack!

Do you mean 'full acknowledgement', as in agreed?

But one must note that Object.seal currently only works in strict mode. (AFAIR! I didn't check, if it prevents write access in non strict mode).

I'm not aware of that. If that is indeed the case, a case should be created about that I think, cause I dont think there is one