Repository navigation
Contain CLR type and member resolution - #3052
Conversation
e5685f8 to
7cab1ac
Compare
lahma
left a comment
There was a problem hiding this comment.
Reviewed as the incremental diff against sebros/stack-host-error-redaction. This closes a real escape: the old NamespaceReference.GetPath searched the calling/executing assemblies and then fell back to System.Type.GetType(path) — which accepts assembly-qualified names, i.e. script-triggered assembly loading. Replacing that with a closed allow-list plus an effective-public declaring-chain check, and filtering the static Type.GetType family out of member resolution unconditionally, is exactly right. The per-assembly ConditionalWeakTable type-name index is collectible-assembly-safe and engine-independent, and moving the static-member and constructor caches onto TypeResolver keyed by resolution profile finally fixes the weak-keyed process-wide TypeReference cache that AGENTS.md has been flagging as a hazard. The explicit-capability story is coherent throughout: a host-exported TypeReference stays a capability (including the CWT round-trip through typeToObject/objectToType), while anything discovered through a namespace passes the policy. Benchmark numbers show restrictive construction allocation-neutral and the nested-lookup cache paying for itself. Approving.
Notes:
- Breaking changes to catalog in the release notes (all deliberate):
AllowClr(...)no longer implicitly searches the calling/executing assembly orType.GetType— an embedder who relied on that must now pass their assemblies explicitly (parameterlessAllowClr()keeps core-lib types working);clrHelper.unwrapnow requiresAllowGetType; staticType.GetType(string)is filtered even whenAllowGetTypeis true (a host that wants it can export a delegate). - Subtlety worth a doc line:
ClrTypeResolutionPolicy.AllowschecksAllowedAssemblies.Contains(type.Assembly)on the resolved type, andAssembly.GetTypefollows type forwarders. A host that allow-lists a facade assembly (e.g.System.Runtime) will have its types resolve into core-lib and then be denied because the forwarded type'sAssemblyis not the facade. "Allow-list the assembly that defines the type (typeof(X).Assembly)" is the guidance to write down. GetConstructorscaches perTypewithout the resolution profile in the key — currently fine because constructor filtering only consults resolver-level state (_memberFilter, cleared on change), not the engine profile. IfFilterever grows a profile-dependent branch that applies toConstructorInfo, this key needs the profile too; a one-line comment on the cache would keep that from being missed.
Stack merge still gated on the #3035 fix below.
7cab1ac to
e58ed6e
Compare
e58ed6e to
1a5525b
Compare
…lly has Main has moved 181 commits past v4.16.0 and nothing records what an embedder has to react to. Most of it is invisible to a compiler: every entry below still compiles exactly as it did in 4.16 and behaves differently at run time, and six of them flip a default. `docs/v5-migration.md` is the artefact those entries go in. It is deliberately not a second README: a table per change, before/after code where it helps, and the rationale left in the pull request it cites. Seeded from git history, every claim checked against the tree rather than against the pull request title: * sebastienros#3054 `Interop.AllowWrite` true -> false. A projected CLR write is now silently ignored in sloppy mode and a TypeError in strict mode. * sebastienros#3056 `Interop.ArrayConversion` LiveView -> Copy. `Array.isArray` flips to true, `push`/`length` stop throwing, and CLR-side mutations after the crossing stop being visible. * sebastienros#3057 `Constraints.StackOverflowGuard` false -> true. RangeError instead of a process that ends with no exception in the log. * sebastienros#3058 `AgentCanSuspend` true -> false. `Atomics.wait` is a TypeError before a waiter is registered; `waitAsync` is untouched. * sebastienros#3052 namespace type discovery loses its implicit fallback to `Assembly.GetCallingAssembly()` / `GetExecutingAssembly()` / `Type.GetType(name)` and becomes the `AllowedAssemblies` allow-list. * sebastienros#3051 host exception, module-load and CLR-resolution messages are redacted from script; `ExposeDetailedErrors()` restores all three. * sebastienros#3035 concurrent `Engine` use fails fast, and an engine stays reserved for the lifetime of a returned async Task. * sebastienros#3036 `LimitMemory` charges allocations across async continuations, so a budget can now trip where one synchronous segment never reached it. * sebastienros#3252 every `*Async` entry reports the operation's failures through the task and only a usage error out of the call. * sebastienros#3248 an array-like `length` above 2^32-1 stops answering differently per target framework. * sebastienros#3037, sebastienros#3045, sebastienros#3046 add parser, module-graph and result bounds that all default to unlimited, so they change nothing until configured; sebastienros#3059 and sebastienros#3060 add the diagnostics and the hardened profile on top. Two entries the prompt for this work had slightly differently, both checked and written as the tree has them: the stack-overflow guard raises a catchable `RangeError`, not a `RecursionDepthOverflowException`, and `ArrayOperations` is internal, so sebastienros#3248 removes no public member — its break is what a script sees. Sections 2, 3 and 6 (removed API, renamed API, AOT) are explicit empty placeholders. Nothing public has been removed or renamed since v4.16.0, and a parallel task is measuring the AOT state. The target-framework section is written and marked pending: `Jint.csproj` still lists net462, and the net472 change is not on main yet. `Jint/AGENTS.md` gains the rule that makes the guide stay current — a change to anything in its public-contract table is a row in the guide, in the same pull request, including a change that breaks nothing at compile time. The root `AGENTS.md` is untouched; it is at 23 KB against a 24 KiB budget. README's "Branches and releases" described `main` alone and did not mention the `3.x` branch at all. It now has a row per live branch, in the same wording sebastienros#3291 gives the 4.x branch's own copy, plus a pointer to the guide. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014W5mbjGhyvgAS4pivXoc4S
…lly has (#3294) Main has moved 181 commits past v4.16.0 and nothing records what an embedder has to react to. Most of it is invisible to a compiler: every entry below still compiles exactly as it did in 4.16 and behaves differently at run time, and six of them flip a default. `docs/v5-migration.md` is the artefact those entries go in. It is deliberately not a second README: a table per change, before/after code where it helps, and the rationale left in the pull request it cites. Seeded from git history, every claim checked against the tree rather than against the pull request title: * #3054 `Interop.AllowWrite` true -> false. A projected CLR write is now silently ignored in sloppy mode and a TypeError in strict mode. * #3056 `Interop.ArrayConversion` LiveView -> Copy. `Array.isArray` flips to true, `push`/`length` stop throwing, and CLR-side mutations after the crossing stop being visible. * #3057 `Constraints.StackOverflowGuard` false -> true. RangeError instead of a process that ends with no exception in the log. * #3058 `AgentCanSuspend` true -> false. `Atomics.wait` is a TypeError before a waiter is registered; `waitAsync` is untouched. * #3052 namespace type discovery loses its implicit fallback to `Assembly.GetCallingAssembly()` / `GetExecutingAssembly()` / `Type.GetType(name)` and becomes the `AllowedAssemblies` allow-list. * #3051 host exception, module-load and CLR-resolution messages are redacted from script; `ExposeDetailedErrors()` restores all three. * #3035 concurrent `Engine` use fails fast, and an engine stays reserved for the lifetime of a returned async Task. * #3036 `LimitMemory` charges allocations across async continuations, so a budget can now trip where one synchronous segment never reached it. * #3252 every `*Async` entry reports the operation's failures through the task and only a usage error out of the call. * #3248 an array-like `length` above 2^32-1 stops answering differently per target framework. * #3037, #3045, #3046 add parser, module-graph and result bounds that all default to unlimited, so they change nothing until configured; #3059 and #3060 add the diagnostics and the hardened profile on top. Two entries the prompt for this work had slightly differently, both checked and written as the tree has them: the stack-overflow guard raises a catchable `RangeError`, not a `RecursionDepthOverflowException`, and `ArrayOperations` is internal, so #3248 removes no public member — its break is what a script sees. Sections 2, 3 and 6 (removed API, renamed API, AOT) are explicit empty placeholders. Nothing public has been removed or renamed since v4.16.0, and a parallel task is measuring the AOT state. The target-framework section is written and marked pending: `Jint.csproj` still lists net462, and the net472 change is not on main yet. `Jint/AGENTS.md` gains the rule that makes the guide stay current — a change to anything in its public-contract table is a row in the guide, in the same pull request, including a change that breaks nothing at compile time. The root `AGENTS.md` is untouched; it is at 23 KB against a 24 KiB budget. README's "Branches and releases" described `main` alone and did not mention the `3.x` branch at all. It now has a row per live branch, in the same wording #3291 gives the 4.x branch's own copy, plus a pointer to the guide. Claude-Session: https://claude.ai/code/session_014W5mbjGhyvgAS4pivXoc4S Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…t no longer exists (#3440) `Jint/Runtime/Interpreter/AGENTS.md` told anyone touching a shared interop cache that `TypeReference` keeps a process-wide static member cache "on a *weaker* key -- just `(Type, member name)`, with no requirement or profile -- so it is shared even between engines whose interop configuration differs". That described `TypeReference._memberAccessors`, which #3052 removed. Static-member resolution now goes through `TypeResolver.GetStaticAccessor`, whose key is `StaticAccessorCacheKey(Type, member name, InteropResolutionProfile)` and which runs the same `IsShareable` / `IsConverterNeutral` exclusions as the instance lane. The one real opt-out left is an enum reference, which resolves per instance because `EnumConversion` decides whether a constant is a `JsString` or a `JsNumber` and the key cannot express it -- which the code says in a comment and the instruction file did not. Found while auditing every process-wide cache for #3426: the paragraph as written makes an agent record a defect that is not there, and miss the enum exception that is. Claude-Session: https://claude.ai/code/session_014W5mbjGhyvgAS4pivXoc4S Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Summary
AllowGetTypefor widening helpers while preserving wrapper-specific explicitTypeReferenceprovenanceConditionalWeakTableStack
Base: #3051 (
sebros/stack-host-error-redaction)#3051 -> #3046 -> #3045 -> #3037 -> #3036 -> #3035 -> #3030
Integration and review
The two previously reviewed containment commits were transplanted unchanged onto live base
aa5a91b8ea2c31c8c357df4516534920061ae8c9; they applied without manual conflicts. Integration then sanitized unresolved-member and generic-binding failures, prevented raw generic exceptions under default policy, and preserved fatal resource/control exceptions.Fresh specialist review found two
Assembly.GetTypevisibility bypasses: CLR+names could reach non-public nested types, and internal top-level types remained resolvable. Both were fixed with an effective-public check across the complete declaring-type chain. The final follow-up security review was clean.Validation
Jint/Jint.csprojbuildJint.Tests: 5,774 passed, 4 skipped (default and host-contract verification)Jint.Tests.PublicInterface: 1,707 passed / 9 skipped default; 1,711 passed / 5 skipped with host-contract verificationTypeReference, nested-type, cache, GC, reflection, debugger, module, result, and security coverage included in the full suitesgit diff --checkNuGet validation used only
https://packagefeedproxy.microsoft.io/nuget/v3/index.json; the proxy-compatible analyzer downgrade was local-only and the manifest is restored exactly.Performance
Default BenchmarkDotNet job, fresh-engine probe:
Restrictive construction is allocation-neutral and within noise of compatibility mode; the process cache avoids about 9.76 us and 40.9 KB per fresh-engine nested lookup versus the former scan shape.