Should there be a usable ModuleBlock constructor?

[littledan]

#24 adds a Module Block constructor such that you can do new on it, with a string, to get a new module block with those contents. This is designed to be analogous to the function constructor. Should this exist? An alternative is that we could make this constructor always throw.

[Jack-Works]

How to handle import.meta.url?

[jridgewell]

What's the utility in it? It should be possible to eval a module block (eval(`module {}`)), right? So this would just be another way to cause an eval, which seems like a negative.

[ljharb]

Why would it be possible to eval a module block, if the block isn't toStringable? eval only accepts strings.

[jridgewell]

@ljharb: My example got garbled by the markdown syntax. I'm passing the string "module {}" to eval, not a module instance. I'd expect that should do the same as if I had written the literal source-text:

module {}

My point being, we can already dynamically create module blocks with this syntax via eval, so adding the ability to eval with new Module isn't strictly necessary.

[ljharb]

true - however, eval in a general sense is dangerous in a way Function (and the other function constructors) is not, and as such, a ModuleBlock constructor would avoid the need to either use eval itself, or to use Function('return ' + moduleString)().

[littledan]

I could go either way on the utility question here (I don't understand @ljharb 's argument--is the danger about direct eval?).

For import.meta.url, that's a good question. I think, in HTML, it should come from the "document"--acting as if the module block were declared at the top level of the application.

[ljharb]

@littledan yes, direct eval.

i don’t think it would be a good thing for it to come from the document - that would mean different modules have the same import.meta.url.

[littledan]

@ljharb If you believe that, please file an issue explaining why so we can have a focused discussion there. As the README explains, the proposed HTML integration is that the import.meta.url of a module block would generally match the surrounding module. This is important for things like asset references through new URL(..., import.meta.url).

[ljharb]

Sure, #26.

[littledan]

I can see @jridgewell 's point in #25 (comment) , and could go either way on this question. I want to suggest that this is a question that we need to resolve by Stage 3: it's not a core semantics question, but it's definitely an important detail to nail down before implementation.

[koto]

I would prefer if there were not yet another eval-like construct without a really strong need for it.

For one - we should have less dynamic evaluation, not more. Additionally, while it'd be hooked to the dynamic compilation in the implementations, as it's not known to the existing linters, scanners, security reviewers etc, it might likely lead to introducing new injection vulnerabilities.

Why is it preferable to new Function(`return ${moduleString}`) ?

[littledan]

In the January 2021 TC39 meeting, we heard a fair bit of skepticism about this constructor. I think we should follow through with removing it (in particular, throwing an exception if it's called).

[ljharb]

I still am not clear on why it was ok to add AsyncFunction, GeneratorFunction, and AsyncGeneratorFunction - with no concern about injection vectors or a new eval source - but ModuleBlock is somehow different.

[surma]

Just because something was done in the past, doesn’t mean it’s still good to do it today. I don’t have a strong opinion on this, I added the constructor for symmetry, but I don’t think it’s essential. I have always found it odd that “eval() is evil” is thrown around, and yet more variants of eval() were added to the language.

[koto]

I still am not clear on why it was ok to add AsyncFunction, GeneratorFunction, and AsyncGeneratorFunction

I don't know, frankly, but I would've had the same position on those constructors as well. To reiterate, I think we should have try to have less evaluation from strings, not more. And it this case it seems quite workable without the constructor.

[surma]

Reopening this! I did just remove the constructor from the spec, but we can still talk about this

[ljharb]

Dropping the constructor would make ModuleBlock the only form of reusable code, aside from class (which produces a function anyways), that doesn't have a code-string-taking constructor. Dropping this constructor would be consistent with not having AsyncFunction, GeneratorFunction, and AsyncGeneratorFunction, but since we have those, I think we must have this too.

CSP is the way to prevent eval on the web, node has policies, and both could ensure that all these constructors throw before any user code runs.

[surma]

I agree that having the symmetry is nicer aesthetically. However, I worry introducing another way to effectively eval() code could introduce a new exploit vector for systems that don’t/can’t rely on CSP but sanitize code manually.

Apart from that, I don’t see a benefit of using new ModuleBlock('...') over eval('module { ... }'), because in contrast to the Function constructor, ModuleBlocks don’t take any parameters. So in the end:

ModuleBlockConstructor = code => eval(`module { ${code} }`);

All in all, breaking symmetry seems worthwhile to me. Anne said in #29 that ObjectURLs are “not to be extended further”. So while they continue to exist for Blobs et al, ModuleBlocks will kinda be breaking symmetry there, too. While I understand that that is HTML and this is TC39, it seems to me that — judging by the feedback I got in TC39 — code-evaluating constructors are heading towards a similar fate.

[surma]

I’ll bring this up at a TC39 meeting and ask for more explicit opinions on this :)

[ljharb]

What is different now wrt community/implementer thinking about eval versus in 2019 that would merit making a different decision than was made for AsyncGeneratorFunction?

[bathos]

I'm confused by the comment regarding "sanitizing" JS manually. Is there an example of this which is meaningfully secure but which also has been broken each time a new function constructor was added in the last few years? (This might be a rhetorical question now that I think about it, but it didn't start that way...)

I don't understand what's gained from the inconsistency - there's no such thing as "more" eval, it's just blocked or it isn't.

[koto]

there's no such thing as "more" eval, it's just blocked or it isn't.

.. and because of that (dynamic code evaluation being either fully enabled or not) many applications need to leave it enabled (https://github.com/w3c/webappsec-trusted-types/wiki/design-history describes the problem a bit, same here: https://swag.cispa.saarland/papers/steffens2021blockparty.pdf).

Eval-enabling applications (most on the web platform) still want to be secure, so they employ manual code reviews and static tooling to see where eval-equivalent constructs are called and what data is passed to them. Each new eval vector adds to that problem, as both the tooling and people need to understand that it also needs to be reviewed.

[ljharb]

@koto that suggests that what the web needs is more granular CSP controls, so they don't have to resort to manual review.

[koto]

I think so too, but even with those controls in the future I don't think we should introduce new evaluation sinks without a strong reason for it, as dynamic evaluation should be discouraged - we should, in my opinion, drive the language away from it and not add to the problem.

[nicolo-ribaudo]

https://github.com/tc39/proposal-compartments is working on introducing an usable Module constructor.

This proposal will continue throwing for new Module, so that the Compartments proposal can explore this space without module blocks limiting their possibilities.

[ljharb]

I don’t think it’s viable to introduce this proposal without a string-taking constructor, per #25 (comment).

[mhofman]

A Module doesn't encapsulate just source code, so taking source in the form of string wouldn't make sense.

The layer 0 of the compartment proposal has just what you're asking for though, as a ModuleSource constructor.

[ljharb]

Neither does a function, and yet Function exists.

[mhofman]

As currently proposed, Module will not take a string as argument. I don't see how asking for it to take a string now is compatible with that.

[ljharb]

It’s not, which is why I’m making it clear that as currently proposed, i don’t think this proposal meets a consistency bar i find important.

[kriskowal]

The language would be eventually consistent, with module blocks and first-class modules. Would you be satisfied if they were conflated?

[mhofman]

I fail to see why a new evaluator should have to directly accept source as a string, and why we can't introduce a level of indirection in the form of ModuleSource.

[kriskowal]

Synchronizing Module Harmony would be a good topic for next week’s call.

[ljharb]

@mhofman i explained it in #25 (comment)

@kriskowal i agree that if both proposals were to eventually land at the same time, then it seems like it would be fine, but i don’t think that’s sufficient reason to combine the proposals, and i don’t think this one should advance (without this concern addressed) with the risk that the other won’t.

[kriskowal]

So you would be satisfied if their advancement were coupled? (I’m not endorsing conflating the layers)

[ljharb]

I would be satisfied if this proposal contains a way to create a module block from a string, just like every other form of reusable code does - if coupling the proposals does that, then great, but i suspect there’s much easier ways to meet the constraint.

[kriskowal]

Would you be satisfied by the current proposal that includes a level of indirection, using ModuleSource to amortize the cost of lifting a string? That is consistent with Function.

[ljharb]

While I think it would be inconsistent to not have new Whatever(string) instanceof Whatever for reusable code, the capability is what i'm insisting on, so new ModuleSource(string) instanceof Module would meet my constraint, even tho it would introduce a new kind of inconsistency.

[mhofman]

To be clear, the proposal is for

const source = new ModuleSource(codeString);
const mod = new Module(source);
source instanceof ModuleSource;
mod instanceof Module;

[ljharb]

@mhofman ah, the current proposal spec github pages doesn't seem to include that. Can that be updated, so i can review it?

[mhofman]

This is in the Compartment layer 0 proposal. That part actually seem to have some preliminary spec text, though likely outdated.

[kriskowal]

Layer 0 spec text is current cc @caridy

…

On Fri, Oct 21, 2022 at 9:28 AM Mathieu Hofman ***@***.***> wrote: This is in the Compartment layer 0 proposal <https://github.com/tc39/proposal-compartments/blob/master/0-module-and-module-source.md>. That part actually seem to have some preliminary spec text <https://github.com/tc39/proposal-compartments/blob/master/0-module-and-module-source.emu>, though likely outdated. — Reply to this email directly, view it on GitHub <#25 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAOXBQL23GBQ6OLE6MCWILWELAEPANCNFSM4U2P7FAA> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[ljharb]

Then yes, per #25 (comment) i would consider that to resolve my intolerable inconsistency, in favor of a merely unpleasant inconsistency.