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

Layering: Distinguish template literals to embedders #1350

Closed
wants to merge 1 commit into from

Conversation

littledan
Copy link
Member

This patch gives the frozen Arrays passed to tagged templates
a [[TemplateLiteral]] internal slot. Such a slot can be used
as a brand check to prove that the object came from a template
literal in the program. Combined with CSP [1] or similar
restrictions, this can prove that the Array came from a tagged
template literal in the provided original application code.
Such proof may be useful in JavaScript embeddings like the Web
in the Trusted Types proposal [2].

This patch does not introduce a JavaScript API to perform the check.
It has been suggested that this check should have certain integrity
properties. There are various ideas about introducing standard
library integrity properties in the get-originals [3] and
JS-Standard-Library [4] proposals. When these proposals are more
developed, they will show the way for how such a check API should
be introduced into JavaScript.

[1] https://w3c.github.io/webappsec-csp/
[2] https://github.com/WICG/trusted-types
[3] https://github.com/domenic/get-originals
[4] https://github.com/tc39/proposal-javascript-standard-library

spec.html Show resolved Hide resolved
spec.html Show resolved Hide resolved
spec.html Show resolved Hide resolved
@ljharb
Copy link
Member

ljharb commented Nov 15, 2018

Wouldn’t this mean i could save an array created from a template literal, and pass it directly into another tag function that wasn’t used on a literal?

@devsnek
Copy link
Member

devsnek commented Nov 15, 2018

hosts can add their own slots right? if the spec itself isn't using the check this shouldn't be needed

@ljharb
Copy link
Member

ljharb commented Nov 15, 2018

That came up in #1306; it seems like currently hosts can’t exactly add their own slots - but since they could indistinguishably store the object in a weakmap to simulate a slot, im also not sure why it’s preferred to have an otherwise useless slot in this spec.

@domenic
Copy link
Member

domenic commented Nov 15, 2018

I'll try to repeat myself from #1306. Hosts can do whatever they want---slots, weak maps, intentional violations of 262, whatever. What is at stake is always the issue of clarity and good layering (including "good citizenship" in the spec ecosystem).

The host could indeed monkey-patch the spec, and say "whenever this syntactic production is encountered in ECMAScript, instead of running the algorithm specified in the ECMAScript spec spec, run this algorithm, which is the same except that after step 8 you also need to store the result and put it into a weak map". This is terrible practice, and doesn't match how implementations work (there's no "patching" applied from Blink into V8's template literal parsing code). We've increasingly moved away from these kind of willful violations, e.g. the upstreaming of document.all semantics instead of monkey-patching the definitions of ===, typeof, etc.

An editor of ECMA-262 calling things "useless" because they're used by the actual environments which run the JS spec, and reflect how you would actually implement the specification, is frustrating. Encouraging willful violations of ECMA-262 by hosts is not what I would like to see.

@ljharb
Copy link
Member

ljharb commented Nov 15, 2018

I’m sorry that came across in a way i didn’t intend; i meant, it has no usage sites in this spec. Improving layering is an important goal, and I’m not attempting to obstruct it, nor am i recommending a willful violation of the spec. I’m merely raising a question about this approach being the best, or clearest, one to take to solve the problem.

@littledan littledan force-pushed the template-literal-layering branch 2 times, most recently from 8567b70 to a7e27d6 Compare November 15, 2018 17:54
spec.html Show resolved Hide resolved
@littledan
Copy link
Member Author

Given the lack of a note, I can understand the confusion that led to these questions, but hosts aren't expected to actually fill in [[TemplateLiteral]], just check for its presence. I share @domenic 's concern with spec monkey-patching. In this case, the logic would be particularly non-linear. @ljharb @devsnek Given this explanation, do you have concerns with this change?

This patch gives the frozen Arrays passed to tagged templates
a [[TemplateLiteral]] internal slot. Such a slot can be used
as a brand check to prove that the object came from a template
literal in the program. Combined with CSP [1] or similar
restrictions, this can prove that the Array came from a tagged
template literal in the provided original application code.
Such proof may be useful in JavaScript embeddings like the Web
in the Trusted Types proposal [2]

This patch does not introduce a JavaScript API to perform the check.
It has been suggested that this check should have certain integrity
properties. There are various ideas about introducing standard
library integrity properties in the get-originals [3] and
JS-Standard-Library [4] proposals. When these proposals are more
developed, they will show the way for how such a check API should
be introduced into JavaScript.

[1] https://w3c.github.io/webappsec-csp/
[2] https://github.com/WICG/trusted-types
[3] https://github.com/domenic/get-originals
[4] https://github.com/tc39/proposal-javascript-standard-library
@devsnek
Copy link
Member

devsnek commented Nov 15, 2018

@littledan my only thought on this is that if this pattern is going to exist we should generalize it more. adding some hook for hosts to add whatever slots they need or just putting slots on everything. just a suggestion though.

my personal approach would be something like "Any object, at the time of creation, may be the target of host-specific internal property creation" or something similar, which would already wrap at least HTMLDDA and this.

spec.html Show resolved Hide resolved
@littledan
Copy link
Member Author

littledan commented Nov 15, 2018

@devsnek The thing is, most objects that ECMAScript creates have some kind of way that embedders can check already. Template literals are one of the few that don't. It turns out that it would be useful for a particular application, so this PR adds it. I think we could go about this on a case-by-case basis, rather than pre-emptively adding brand slots to everything.

@ljharb
Copy link
Member

ljharb commented Nov 15, 2018

I’d like to see #1350 (comment) addressed, so i can understand what guarantees this brand check offers. Knowing that a tag is called from syntax seems valuable, but it’s not clear to me that there’s value in knowing if the array was initially created from syntax or not.

spec.html Show resolved Hide resolved
@littledan
Copy link
Member Author

@ljharb I don't understand your comment in #1350 (comment) .

I’d like to see #1350 (comment) addressed, so i can understand what guarantees this brand check offers.

The value of the slot is always undefined. How would you like this to be addressed?

Knowing that a tag is called from syntax seems valuable, but it’s not clear to me that there’s value in knowing if the array was initially created from syntax or not.

I can't think of a good way to tell the tag that it was called from syntax, without changing the calling conventions of templates, so this patch settles for the next best thing, which is marking the array. You don't get any proof that the tag was called directly with this template object, but you can prove that the source code contained that template object literally. For the trusted types proposal, this could be valuable, see w3c/trusted-types#96 .

@ljharb
Copy link
Member

ljharb commented Nov 15, 2018

That was my question, thank you - specifically, confirming that the guarantee you want is not actually provided by this PR, and that this PR is instead an approximation of what you want.

@littledan
Copy link
Member Author

@ljharb What do you think of the acceptability of the PR, in this case?

@ljharb
Copy link
Member

ljharb commented Nov 15, 2018

While I think we should re-examine the approach of adding things to 262 that a reader of only 262 would think are extraneous, that shouldn't block any specific PR until an acceptable alternative gets consensus, including this one and #1306.

I think this change itself is fine, but I'm still a bit curious how an "almost" guarantee of "coming from syntax" is helpful - the linked resources are lengthy and don't make that clear to me. I'm sure there's something simple I'm missing :-)

@littledan
Copy link
Member Author

If eval is disabled (which ECMA-262 has hooks for), this provides an absolute guarantee of coming from syntax (or at least that's the goal).

To follow up on #1350 (comment) , yes, you can pass forward a template object to a different tag, and that other tag can't tell, but as @mikesamuel says in w3c/trusted-types#96 (comment) , there are circumstances where just literal-ness is useful, even if we don't have proof that it hasn't been passed from another tag.

@littledan
Copy link
Member Author

In the November 2018 TC39 meeting, I intend to ask for "consensus, awaiting embedder feedback". That is, I would like to say that this patch is ready to merge, if it is indeed useful for embedders. Note that this is only "phase 1" and the intended "phase 2" would be to expose a JS API to check for the brand as well. But, I would like to not block Phase 1 on Phase 2 consensus.

@gibson042
Copy link
Contributor

I can't think of a good way to tell the tag that it was called from syntax, without changing the calling conventions of templates, so this patch settles for the next best thing, which is marking the array. You don't get any proof that the tag was called directly with this template object, but you can prove that the source code contained that template object literally.

Why not provide that proof, though? It looks to me like you can do so by updating ArgumentListEvaluation rather than GetTemplateObject such that the returned List uses an internal slot to differentiate parenthesized argument lists from templates, and keeping the new text explaining why ECMA-262 itself does not use that slot.

@allenwb
Copy link
Member

allenwb commented Nov 16, 2018

@gibson042

Lists are not objects. They don't have internal slots.

It's important that ArgumentListEvaluation does not reify the arguments as an object.

@gibson042
Copy link
Contributor

Feedback acknowledged, but I think that sidesteps the point. To rephrase in a way that avoids reification, the goal seems like it would be satisfied by updating ArgumentListEvaluation to return a { [[Elements]], [[FromTemplateLiteral]] } Record rather than a naked List.

@littledan
Copy link
Member Author

Here's another possibility: what if we store a reference to the original tag that was used in a property of the template object? Would this cause any problems?

@erights
Copy link

erights commented Nov 17, 2018

Yes, problems. The tag happens per local evaluation. It can differ from one evaluation to another. The template object happens per evaluation of the enclosing script. Within that script, the same template object is used for all local evaluations of the same template literal expression.

@littledan
Copy link
Member Author

littledan commented Nov 17, 2018

Right, of course, how could I forget. OK, it seems like we would need to change the calling convention somehow (as @gibson042 suggests). Given the implementation complexity of new.target, I am not sure we want to go that way again.

In particular, a tail-called tagged template wouldn't be able to traverse up the stack to find out what's going on; the information would need to be passed some other way.

@littledan
Copy link
Member Author

Subsumed by the Array.isTemplateObject proposal.

@littledan littledan closed this Sep 2, 2019
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.

8 participants