Should JSON modules be frozen?

[dandclark]

During the July TC39 meeting, @FUDCo and @erights (and maybe others who I'm missing?) stated that the JSON object exported by a JSON module should be frozen (via Object.freeze) so that each importer has a guarantee that the JSON object they are importing has not been modified from another import site. Meeting notes here. The concern was that not doing so could lead to bugs from cross-module interference, and importers could never be sure that they were getting a JSON object that hadn't been modified.

I'm not fully convinced that this is needed. JS modules are mutable by default, and unless there's a clear history of bugs caused by this I'm not sure that we should change this behavior; it seems like a potential source of confusion if default mutability differs across module type.

If there are cases where it's necessary to freeze an import or guarantee that a given import site gets a fresh copy, that seems like a use case for an evaluator attribute that could be used to achieve this with JSON modules as well as other potential module types. This seems more consistent and flexible than choosing different per-module-type defaults.

I also found this point from @ljharb compelling:

JHD: JSON modules in node for CJS always had this property, bringing in a mutable object, just like the default for every module. I think the vast majority of people do not bother to freeze them, and they haven’t run into these issues. That’s just the way JS works, things are mutable by default...

If this has historically not been an issue with node/CJS then that suggests to me that it would not be a problem for ES JSON modules.

Thoughts?

As a (potentially irrelevant) historical note, when JSON modules were previously added to the HTML spec, they were not frozen.

[ljharb]

This was discussed here: https://github.com/tc39/proposal-import-assertions/issues/54 and the resolution was that they should not be frozen.

If you want a given JSON module to be frozen, and you're first-run code (if you're not, then you have no ability to lock anything down anyways), you can import it first and freeze it for all future consumers that would otherwise share the same unfrozen object.

If you want any arbitrary JSON module to be frozen for consumers you don't control, I'd love to understand why you want that.

[dandclark]

Whoops, I totally missed that issue. Transferred it to this repo: #3.

I'm not sure there was really agreement in that thread though. The last comment is from @littledan:

OK, given this discussion, I'm going to leave the proposal as a Realm-wide mutable object for now. Let's keep this open to discuss alternatives; we can also talk about it at the next TC39 meeting where module attributes are discussed.

I'd like to consider the frozen vs not-frozen question to be something that's an important detail to figure out before Stage 3, but that we can go to Stage 2 just based on the "core" decisions (namely, that JSON modules can be imported by with type: "json" in all environments, and that they consist of a single default export). If you disagree with this categorization, I'd like to hear from you!

@xtuc , the thread was closed with tc39/proposal-import-attributes#74, was there discussion I missed that really resolved this? I'm wondering if #3 should be reopened. Overall I agree with @ljharb that JSON modules should not be frozen and I would like to put this issue to rest, but I want to make sure we really have closure given that this was a sticking point during the July meeting and this was what led to us splitting the import assertions and JSON modules proposals.

[FUDCo]

@dandclark I don't want this to wrap up with the impression that we have consensus on the mutability vs. immutability question. I for one will continue to advocate for immutability where it is feasible. Jordon has a good point when he argues that if you're not first run code you're pretty much hosed, but I think that's a world we'd like to move away from when we can. In that world everybody wants to be first run code, which is just not a stable equilibrium. Guaranteed immutability is one of the most powerful tools in our toolbox for preventing one piece of code from interfering in the operation of another piece of code.

In my opinion, the greatest source of program errors and security problems, broadly speaking, is non-local causality, of which this is an example. For example, if a module imports JSON configuration data, another module might be able to mess with that configuration in a way that the first module would be unable to defend itself against. Essentially this is the inverse of the case Jordan described: why would you want somebody's JSON module to be mutable by anyone else who cares to fiddle with it?

[erights]

I agree with @FUDCo .

To emphasize a point in this thread that I fear may be getting lost: Yes, JS is mutable by default. But a JS module is in a position to decide whether on not to freeze its exports before any other code may possibly access them. A JSON module is not. This asymmetry is why the forced choice must be the safe one.

[xtuc]

@dandclark we apparently didn't document the conclusion, it has been discussed during the July TC39 meeting (notes https://github.com/tc39/notes/blob/master/meetings/2020-07/july-21.md#import-conditions-for-stage-3). However, nothing formal since we couldn't ask for stage 3.

[ljharb]

@erights the author of a JSON module can make that choice by becoming a JS module.

[erights]

@ljharb so why introduce JSON modules at all?

[ljharb]

In that scenario, it's still valuable to have the data source be able to be a JSON module - but also because most use cases and users won't need it to be frozen at all.

[erights]

Most use cases and users don't need it to not be frozen.

[ljharb]

Sure. But if it starts mutable, the use cases that need it to be frozen can freeze it - if it starts frozen, the use cases that need it to be mutable are screwed.

[erights]

if it starts frozen, the use cases that need it to be mutable are screwed.

That's a valid point. What are some of those use cases?

[ljharb]

Wanting to load a deeply nested JSON object, and make a single change (in a leaf) without having to use a "deep copy" algorithm. For example, loading a test fixture and modifying a piece of it (for example, to minimize diffs).

[erights]

Thanks for that. It is a realistic and useful example.

[dandclark]

During the July meeting there was this exchange:

JHD: Your Object.freeze call would happen after all the import calls have happened, but since your Object.freeze call would run first, before any other user code, it would be indistinguishable to later code that it was ever mutable in the first place.

MM: How do you get your Object.freeze call to run first?

JHD: The same way you do any initial lockdown of the environment.

CM: Here it is data that has been interpreted and is given back as an object

JHD: For a known JSON module specifier, it’s trivial, and I’m happy to talk about it offline. But like you said CM, if it’s just a regular object literal, it would just be mutable unless you froze it.

CM: If I did that, then I could freeze it before sharing with someone. Here, I don't get the opportunity to freeze it before it gets shared. Modules can defend themselves, data can’t.

@ljharb can you clarify what method you had in mind for ensuring that a JSON module from a given specifier is frozen before other code importing it can run? On the web I guess you could just put a module script right at the top of the page, and this script imports all the JSON modules you want to be immutable and calls Object.freeze() on them. As long as no module scripts get added above the freezer script, then it will be guaranteed to execute before any other module. Is there any more elegant technique that I'm not thinking of?

[ljharb]

@dandclark yes, that's exactly right - the same way any deniable invariants are preserved, by controlling the order code runs in, so you can run your preservation code first.

[jorendorff]

I'm cursed to see both sides on this one. The following is not meant to be persuasive in either direction.

Part of the problem is that this will be the first and only cross-platform way to load a JSON file.

On the web, freezing the JSON is a fine default, because it's so easy to get the opposite if needed. You just say what you want the code to do:

// fooDataAdjusted.js - module that exposes the foo data
let data = (await fetch("fooDataSource.json")).json();
applyTweaks(data);
export default data;

In fact this module is better than a mutable JSON import:

• The data is only exposed after the mutation is applied. So it doesn't have to "run first" in order to avoid other modules accidentally using un-tweaked data.

• Anyone debugging weirdness in the data will naturally find this code where it's is loaded and mutated.

• Most importantly, you won't have the situation where someone else independently loads the same JSON file and is astonished to find that what they get is not what the file says. A separate fetch("fooDataSource.json") will get a fresh copy, no surprises.

The use cases with mutation are real, but that doesn't mean JSON imports support them well. :-\

[jorendorff]

How will other kinds of imports handle this? As a user I'd like to be able to import a text or binary file, I guess producing a String or ArrayBuffer. Strings can't be mutable. ArrayBuffers can't be immutable. ...Great, I guess this is no help either.

[annevk]

I was wondering if it was reasonable for the importer to decide, but then you have to deal with conflicts somehow.

Note that a JSON module cannot simply become a JS module as all the import assertion would have to change (or allow for multiple somehow, not sure where that discussion ended up).

Aside: note that there are requests for immutable ArrayBuffers at WICG/reducing-memory-copies#1 (and probably elsewhere).

[matthewp]

Is the freeze intended to be a "deep" freeze, meaning are nested objects also frozen or only the outer object? If only the outer object then I don't think there's any added value here. If it is a deep freeze then we really need a Object.deepFreeze first.

[littledan]

The idea of this would be that the structure is deeply, not shallowly, frozen. Adding Object.deepFreeze would be a separate proposal.

[matthewp]

That's what I was thinking. Would Object.deepFreeze be a prerequisite of this, or is it ok the internal implementation to be able to do something that userland cannot?

[ljharb]

Discussion in today's plenary ended up with the temperature of the room implying that JSON modules might change to "immutable". I misunderstood that, and did not speak up during plenary, so I wanted to concretely say that I plan to object to stage 3 unless JSON modules are mutable. Hopefully when this proposal comes back for consensus in January, it'll remain as-is, and this won't come up.

[xtuc]

Platforms seems to prefer different defaults and my feeling is that we won't be able to agree on one.

@dandclark already mentioned it but I wanted to bring it up again. Using evalutator attributes we can let the module consumer decide:

import data from "file.json" with { mutability: true };

I believe this could ensure a consistent behaviour across platforms, as long as they support the attributes.

[ljharb]

I would prefer the reverse - default mutability (like the rest of the language), and pursue a follow-on proposal to allow it to be made immutable (or to retrieve the original object, mutably; or both)

[littledan]

I'd prefer that we have a strong default, one way or the other, which is consistent across platforms. I'd be sad if you have to opt-in to portability.

[dandclark]

I'd prefer that we have a strong default, one way or the other, which is consistent across platforms. I'd be sad if you have to opt-in to portability.

I agree. One of our listed motivations is to guarantee consistency across hosts. I would prefer not to lose that if possible.

I would prefer the reverse - default mutability (like the rest of the language), and pursue a follow-on proposal to allow it to be made immutable (or to retrieve the original object, mutably; or both)

Some rough brainstorming on evaluator attributes for this:
If JSON module are default mutable:

// Default behavior: 
import notFrozenJson from "./foo.json" assert {type: "json"};

// Gets a deeply frozen JSON object.  Evaluator attributes *are* part of the cache key,
// so this can be done alongside the prior import, and is evaluated as a different module.
import deeplyFrozenJson from "./foo.json" assert {type: "json"} with { frozen: "true" };

// Or alternatively, we could have an attribute that always re-evaluates the module,
// roughly equivalent to appending a randomly generated URL #fragment-identifier to the module specifier.
// This provides a mutable JSON object that is guaranteed to be unmodified, since the importer always
// gets a fresh copy.  This is also potentially interesting for other module types, even JS.
import freshMutableCopy from "./foo.json" assert {type: "json"} with { alwaysEvaluate: "true" };

Or, if JSON modules were default immutable:

// Default behavior: 
import deeplyFrozenJson from "./foo.json" assert {type: "json"};

// Gets a mutable JSON object.  Evaluator attributes *are* part of the cache key,
// so this can be done alongside the prior import, and is evaluated as a different module.
import notFrozenJson from "./foo.json" assert {type: "json"} with { frozen: "false" };

[xtuc]

To be clear, I'm also in favour of a good default. If platforms can't agree on a default, using evaluator attribute to maintain consistency from a developer point of view would be an acceptable solution in my opinion.

[littledan]

I haven't heard strong opinions from any environment maintainer that one or the other option would be unacceptable, so I would prefer to continue with the assumption that we are deciding here on the cross-environment semantics, unless we hear otherwise.

[acutmore]

Outcome: Consensus for stage 3, with mutable semantics

https://github.com/tc39/notes/blob/master/meetings/2021-01/jan-25.md#conclusionresolution-3