Skip to content

Limits: the bounds are named properties, and a preset is something you adjust - #3459

Merged
lahma merged 4 commits into
sebastienros:mainfrom
lahma:v5-c6-limits-are-properties
Aug 27, 2026
Merged

lahma merged 4 commits into
sebastienros:mainfrom
lahma:v5-c6-limits-are-properties

Conversation

@lahma

@lahma lahma commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator

Item C6 of the v5 API campaign: optional configuration is init properties, never a positional constructor — 3.3's rule applied to the three places that still ignored it.

Nothing became looser

Stated first because a wrong default here has no compile-time hint. No dimension a host used to state acquired a default, and no default moved.

  • The eight dimensions that were required positional parameters are still required — as C# required members. Omitting one is CS9035 at the call site, not a weaker limit. That is the strongest available protection during the rewrite this breaking change forces: a host cannot drop TimeSpan.FromSeconds(5) and silently inherit a 10.
  • The seven that already had defaults keep byte-identical defaults, so a host that never named them is bounded exactly as before. Jint.Tests.PublicInterface now pins those seven numbers, because nothing else would notice a weaker one: the value is in no signature a compiler checks, and a looser limit fails no test by failing to fire.
  • UntrustedCodeLimits.Default is new API, so nothing can regress through it.

UntrustedCodeLimits

Fifteen constructor parameters, eight of them required and positional, four of those adjacent TimeSpans. Every call site was a column of unlabelled values in an order nobody remembers, and regexTimeout and promiseTimeout could be swapped without a diagnostic. This repository's own integrator suite had written a wrapper with eight nullable parameters and eight ?? fallbacks to avoid it.

// before — position is the only thing that says which TimeSpan is which
var limits = new UntrustedCodeLimits(
    TimeSpan.FromSeconds(1), 100_000, 16_000_000, 64, 10_000,
    TimeSpan.FromMilliseconds(250), TimeSpan.FromSeconds(1), TimeSpan.FromSeconds(2));

// after
var limits = new UntrustedCodeLimits
{
    TimeoutInterval = TimeSpan.FromSeconds(1),
    MaxStatements = 100_000,
    MemoryLimit = 16_000_000,
    MaxRecursionDepth = 64,
    MaxArraySize = 10_000,
    RegexTimeout = TimeSpan.FromMilliseconds(250),
    PromiseTimeout = TimeSpan.FromSeconds(1),
    MaxOperationDuration = TimeSpan.FromSeconds(2),
};

// ...or start from Jint's own conservative profile
var tuned = UntrustedCodeLimits.Default with { MaxStatements = 5_000 };

Which are required, and why

required should mean a value the host genuinely must decide. The call sites decide the answer, and they are unanimous: across the README example, the threat model, the benchmark and four test suites, every one of the eight varies — timeoutInterval is 5 s / 1 s / 2 s / 750 ms, maxStatements is 100,000 / 500 / 50,000 / 12,345, maxOperationDuration is 10 s / 2 s / 3 s / 5 s. The seven with defaults are overridden only where a test is deliberately proving that they can be. So the existing split is where the evidence puts it, and it stays.

property default why
TimeoutInterval required one entry's wall clock; a property of the host's request, not of Jint
MaxStatements required same
MemoryLimit required same
MaxRecursionDepth required unchanged from 4.16; a ceiling a workload decides
MaxArraySize required unchanged from 4.16
RegexTimeout required unchanged from 4.16; must also match the value untrusted code was prepared with
PromiseTimeout required unchanged from 4.16
MaxOperationDuration required the host's request budget; only the host knows it
MaxSourceLength 1_000_000 exactly the old parameter default
MaxNodeCount 250_000 exactly the old parameter default
MaxModuleCount 100 exactly the old parameter default
MaxTotalModuleSourceBytes 10_000_000 exactly the old parameter default
MaxModuleGraphDepth 32 exactly the old parameter default
MaxModuleResolutionHops 1_000 exactly the old parameter default
ResultLimits ResultLimits.Conservative exactly the old parameter default (null already meant this)

UntrustedCodeLimits.Default is the one place Jint picks the eight itself, so it takes the stricter value of the two deployment examples Jint documents, dimension by dimension: TimeoutInterval 1 s (README 1 s, threat model 2 s), MaxStatements 50,000 (100,000 / 50,000), MemoryLimit 16,000,000 (both), MaxRecursionDepth 64 (both), MaxArraySize 10,000 (10,000 / 100,000), RegexTimeout 250 ms (both), PromiseTimeout 500 ms (1 s / 500 ms), MaxOperationDuration 2 s (2 s / 3 s). The other seven are the property defaults, so Default is exactly "name only the eight, with Jint's own budget filled in". Its documentation says what it is: Jint guessing on the host's behalf, which is what the required members exist to prevent.

ResultLimits

Same treatment; five optional parameters become five init properties with the same unlimited defaults, Unlimited and Conservative stay, and Conservative with { MaxStringLength = 4_096 } is now the natural way to adjust one dimension.

JsonSerializer takes its limits on the constructor

The way JsonParser already takes its depth. The real overload count, since the item's estimate was close but not exact: there were seven methods named Serialize plus SerializeWithLimits — eight serialization entry points. Four used Options.ResultLimits; three took a trailing ResultLimits that had to be repeated at every call site, where forgetting it silently fell back to the engine's limits, which default to unlimited; SerializeWithLimits was the three-argument overload with its replacer and space omitted. Eight → four.

// before
var serializer = new JsonSerializer(engine);
serializer.SerializeWithLimits(value, limits);
serializer.Serialize(value, replacer, space, limits);
serializer.Serialize(value, writer, limits);
serializer.Serialize(value, replacer, space, writer, limits);

// after
var serializer = new JsonSerializer(engine, limits);
serializer.Serialize(value);
serializer.Serialize(value, replacer, space);
serializer.Serialize(value, writer);
serializer.Serialize(value, replacer, space, writer);

new JsonSerializer(engine) is unchanged and still takes Options.ResultLimits — now read once at construction rather than once per call, which is not observable because an engine's Options are frozen before anything can serialize through it.

Presets compose on every target framework

Default with { … } needs the record copy constructor to carry [SetsRequiredMembers], and required / init need RequiredMemberAttribute / IsExternalInit, which PolySharp already generates (Jint.csproj configures it). Verified two ways: the net472 and netstandard2.0 legs of Jint/Jint.csproj build the presets, and a separate cross-assembly net472 probe confirmed a consumer can also write Default with { … } against Jint's metadata rather than only Jint itself. Jint.Tests.PublicInterface — the only project without InternalsVisibleTo — runs the composition tests on net472 as well as net8.0 and net10.0.

Tests

In Jint.Tests.PublicInterface:

  • TheDefaultPresetIsAdjustedWithoutRestatingTheRest — with satisfies the required members, keeps all fourteen dimensions it does not name, and leaves the preset alone.
  • TheDimensionsThatCarryDefaultsCarryTheOnesTheyAlwaysCarried — the seven defaults, by value.
  • ThePresetProfileStillBoundsWhatTheProfileBounds — behaviour, not values: an engine built from Default with { MaxStatements = 500 } still overrides AllowClr, still refuses eval, still trips the statement budget, and still reports a clean security configuration.
  • ASerializerConstructedWithLimitsAppliesThemToEveryOverload — all four overloads, plus proof that the engine's own unlimited ResultLimits is not merged in.
  • ASerializerWithoutExplicitLimitsTakesTheEnginesOwn, AResultLimitsPresetSurvivesBeingAdjusted, AnAdjustedPresetIsStillValidated.

The two positional-avoidance wrappers are deleted: UntrustedCodeProfileTests.CreateLimits (eight nullable parameters, eight ??) and HostResultLimitsTests.Limits (five). Both become a preset plus with, which is the readable proof — Fixture with { MaxArraySize = 2 } at the call site, and the sixteen validation rows now read Fixture with { TimeoutInterval = TimeSpan.Zero } against nameof(UntrustedCodeLimits.TimeoutInterval).

Also in this PR

  • Five regenerated TFM baselines, and UndocumentedPublicApi.txt shrinks by two (both new constructors ship documented).
  • docs/v5-migration.md §2.6 (the serializer's limits are its own) and §3.16 (the two limit bags are named properties), plus four rows in the §2 removal table.
  • README's "Running untrusted code" and .github/THREAT_MODEL.md's hardened baseline rewritten to the new shape.

Verification

dotnet build -c Release (solution) and dotnet test -c Release green, including the net472 legs of Jint.Tests (7,414), Jint.Tests.PublicInterface (2,522) and Jint.Tests.CommonScripts. Jint.Tests.PublicInterface also run once with JINT_HOST_CONTRACT_VERIFICATION=1, all three frameworks green. test262: 102,495 passed / 0 failed / 189 skipped.

🤖 Generated with Claude Code

https://claude.ai/code/session_014W5mbjGhyvgAS4pivXoc4S

@lahma
lahma force-pushed the v5-c6-limits-are-properties branch 2 times, most recently from fefd211 to e6897d7 Compare August 27, 2026 03:23
@lahma
lahma force-pushed the v5-c6-limits-are-properties branch 3 times, most recently from 88b8fe4 to cf49b71 Compare August 27, 2026 04:13
lahma and others added 4 commits August 27, 2026 07:32
…u adjust

`UntrustedCodeLimits` took fifteen constructor parameters, eight of them
required and positional, four of those adjacent `TimeSpan`s. Every call site
was a column of unlabelled values in an order nobody remembers, and
`regexTimeout` and `promiseTimeout` could be swapped without a diagnostic. The
repository's own integrator suite had written a wrapper with eight nullable
parameters to avoid it. `ResultLimits` had the same shape with five.

Both are now records with `init` properties, which is the rule sebastienros#3310
established for `Options` applied to the two bags that still ignored it.

The eight that were required are still required, as C# `required` members:
omitting one is CS9035 rather than a weaker limit nobody notices. The seven
that had defaults keep byte-identical defaults. `UntrustedCodeLimits.Default`
is new API, and a preset composes: `Default with { MaxStatements = 5_000 }`
satisfies the required members and keeps every dimension it does not name.
Validation moved from the constructor to the property, so it also runs for a
dimension `with` changes, and names the property rather than the parameter.

`JsonSerializer` takes its limits on the constructor, the way `JsonParser`
already takes its depth. That deletes `SerializeWithLimits` and the three
`Serialize` overloads whose trailing argument had to be repeated at every call
site, collapsing eight serialization entry points to four.

Two wrappers that existed only to avoid a positional constructor are deleted
and replaced by a preset plus `with`.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014W5mbjGhyvgAS4pivXoc4S
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014W5mbjGhyvgAS4pivXoc4S
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014W5mbjGhyvgAS4pivXoc4S
sebastienros#3453 landed OptionsCloneReadOnlyTests while this branch was open, and it
constructs UntrustedCodeLimits through the positional constructor this
change replaces:

    error CS1739: The best overload for 'UntrustedCodeLimits' does not
    have a parameter named 'timeoutInterval'

Converted to the object initializer, with the same eight values. This is
the migration the guide's 3.16 row describes, applied to the one call
site that arrived after the rewrite.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014W5mbjGhyvgAS4pivXoc4S
@lahma
lahma force-pushed the v5-c6-limits-are-properties branch from cd6c179 to 392008e Compare August 27, 2026 04:32
@lahma
lahma merged commit 8bf7e5e into sebastienros:main Aug 27, 2026
7 checks passed
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.

1 participant