tc39 / proposal-array-is-template-object

TC39 proposal to identify tagged template string array objects
https://tc39.es/proposal-array-is-template-object/
MIT License
44 stars 7 forks source link

Advance to stage 3 #15

Open koto opened 3 years ago

koto commented 3 years ago

Entry criteria for stage 3:

koto commented 3 years ago

// cc @ljharb @_shu @smooshMap @bakkot as current ES editors.

koto commented 3 years ago

// cc @erights @jridgewell as reviewers. Justin, I think you already reviewed this, but #9 might give the wrong impression that this was for stage 2 advancement. Explicitly asking for LGTM.

Relevant discussion in #9.

jridgewell commented 3 years ago

LGTM.

ljharb commented 3 years ago

Spec text LGTM from an editorial perspective (please also ensure the other editors have deferred or approved)

koto commented 3 years ago

cc @michaelficarra @syg for real (I used Twitter handles before :( )

michaelficarra commented 3 years ago

Spec text LGTM. Splitting out IsTemplateObject is a bit unnecessary, but I'm fine with it.

syg commented 3 years ago

Editorially lgtm with small nits (punctuation and capitalization) that can be addressed later.

I have the same question that @michaelficarra has: why aren't the algorithm steps for Array.isTemplateObject inline?

ljharb commented 3 years ago

I assume to mimic IsArray and other “is” checkers, which have a separate abstract operation. It could always be inline now and factored back out to an op if it’s needed in a second place, but i don’t see the harm in leaving it separate now either.

koto commented 3 years ago

I inlined the algorithm, it's more compact that way. @bakkot, can you TAL? All other ES editors gave LGTM for stage 3. Thanks in advance!

ljharb commented 3 years ago

@koto the name shouldn’t be explicitly defined in a note; the method signature already handles that.

koto commented 3 years ago

Fixed, thanks!

ljharb commented 3 years ago

Updates LGTM. You may also want to list champions/authors in the readme.

bakkot commented 3 years ago

Strictly speaking I'm not sure it's defined to read the value of a slot which might not be present. So I think this should probably be

1. If Type(_value_) is Object and _value_ has a [[TemplateObject]] internal slot and _value_.[[TemplateObject]] is *true*, then

(Also there's a miscapitalization in Array.isTemplateObject 1.a: s/return/Return/.)

Otherwise LGTM.

ljharb commented 3 years ago

Alternatively, it could use IsArray to ensure it’s an array (and also an Object), which also ensures it has the slot - since there shouldn’t ever be a non-array with this slot.

koto commented 3 years ago

@erights any thoughts about the proposal?

erights commented 3 years ago

Is there an answer to my objections? Where?

koto commented 3 years ago

https://github.com/tc39/proposal-array-is-template-object/issues/10#issuecomment-734296514

erights commented 3 years ago

Thanks. That seems dismissive of the concerns rather than addressing them. The general Foo.prototype.method.call(aFooInstance, ...args) is not the normal way one applies that method to that instance. The typical way, aFooInstance.method(...args) works fine across membranes. Your isTemplate (or isTemplateObject) check has no safe form of use.

Also, having it work transparently across realms and compartments negates your own motivating use case. You acknowledge that it only serves a purpose when eval is disabled or restricted. But which eval? No one is in a position to disable or restrict every eval everywhere. Your own motivating use case is only meaningful relative to the evals that have been disabled or restricted. If a template object made by an unrestricted eval passes the isTemplate tests you're trying to use for your stated purposes, it doesn't work. You need to reconcile the stated motivation with the diversity of co-existing evals in the overall environment, both from different realms and different compartments, in order for your motivation to justify your design.

Attn @mikesamuel who understood the motivation to make it eval relative and agreed to do so.

koto commented 3 years ago

Thanks. That seems dismissive of the concerns rather than addressing them. The general Foo.prototype.method.call(aFooInstance, ...args) is not the normal way one applies that method to that instance. The typical way, aFooInstance.method(...args) works fine across membranes. Your isTemplate (or isTemplateObject) check has no safe form of use.

Can you explain what 'safe' means for you in this context? Realms are not security boundaries, so I don't see how interacting with a template object created by a foreign Realm might be any more, or less safe.

Also, having it work transparently across realms and compartments negates your own motivating use case. You acknowledge that it only serves a purpose when eval is disabled or restricted. But which eval? No one is in a position to disable or restrict every eval everywhere.

That's not true. The host environment is in a position to disable eval - be it based on the passed realms, or globally.

Your own motivating use case is only meaningful relative to the evals that have been disabled or restricted. If a template object made by an unrestricted eval passes the isTemplate tests you're trying to use for your stated purposes, it doesn't work.

That is correct and part of the design, it stems from the fact that Realms are not security boundaries. Relying on isTemplateObject check for security ("this value came from syntax") is only meaningful if the untrusted dynamic code evaluation didn't happen yet in any of the Realms that can reach the current one. I'm aware of and OK with that limitation.

erights commented 3 years ago

The terms "security" and "security boundary" are unclear umbrella terms that mean different things to different people. See A Taxonomy of Security Issues for understanding language-based security and modularity and please explain in more precise terms what you mean. I suspect this remains the heart of our disagreement.

I'm aware of and OK with that limitation.

I am not.

koto commented 3 years ago

Thanks, @erights, I continued the discussion in #10, as that ticket has the most context.