Skip to content

Commit 054b2c3

Browse files
spbsolublespbsoluble
committed
fix: type field as object + drop dead nested helper (issue #7 follow-up)
The PR-#2 first pass on issue #7 fixed constructor-signature reflection (the DI-container surface) so the plugin loaded on gateway 25.x. The reporter then ran the live lab against the post-fix DLL and showed a SECOND failure path: gateway 25.5.0 ships IAnyCAPlugin v3.2.0.0, the plugin loads cleanly through Initialize at CA-registration time, but the very first Enroll trips TypeLoadException — the JIT eagerly resolves the declared types of instance fields when it compiles ANY method on the class, and `_domainValidatorFactory` was still declared as `IDomainValidatorFactory` (a type the v3.2 assembly doesn't ship). Changes: 1. CERTInextCAPlugin.cs:48 — `_domainValidatorFactory` field re-typed from `volatile IDomainValidatorFactory` to `volatile object`. CLR class-load no longer eagerly tries to resolve the v3.3-only IDomainValidatorFactory type. 2. CERTInextCAPlugin.cs:63 — new private property `DomainValidatorFactory` returns `_domainValidatorFactory as IDomainValidatorFactory`. The cast is inside the property body and therefore JIT-lazy per-method, so the type is only resolved when the property actually runs (on hosts where the type exists). All read sites that need typed access go through the property. 3. CERTInextCAPlugin.cs:1387 — the nested helper class `DomainValidatorConfigProvider : IDomainValidatorConfigProvider` was dead code (declared but never instantiated). Deleted — a nested type declaring a v3.3-only base interface is itself a class-load hazard, and the cost of removing it is zero. 4. Null-check guards (`_domainValidatorFactory != null`, `_domainValidatorFactory == null`) remain as field reads — comparing `object` to null doesn't touch the v3.3 type at all. Tests: - CERTInextCAPluginPublicSurfaceTests gained three new regression guards: * NoInstanceField_DeclaredTypeReferencesV3Point3OnlyTypes — walks every instance field via reflection and asserts none are declared as a v3.3-only type. This is the exact regression that slipped through the first issue-#7 pass. * NoNestedType_ImplementsV3Point3OnlyInterface — walks nested type interface lists. Catches the dead-code nested helper pattern. * NoPublicMethod_SignatureReferencesV3Point3OnlyTypes — walks public methods' return and parameter types so reflection-driven hosts don't trip on signature metadata. - DcvEnroll_CompletesWithoutThrowing still passes against the live sandbox — the DomainValidatorFactory property cast hot path on v3.3 works correctly. 156/156 unit tests pass. Release build 0 warnings, 0 errors.
1 parent d4467e9 commit 054b2c3

2 files changed

Lines changed: 118 additions & 21 deletions

File tree

‎CERTInext.Tests/CERTInextCAPluginPublicSurfaceTests.cs‎

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,91 @@ public void NoPublicConstructor_ReferencesV3Point3OnlyTypes()
5858
}
5959
}
6060

61+
[Fact]
62+
public void NoInstanceField_DeclaredTypeReferencesV3Point3OnlyTypes()
63+
{
64+
// The .NET JIT eagerly resolves the declared types of all instance fields
65+
// when it first compiles ANY method on a class. If an instance field is
66+
// declared with a missing-type-on-this-host type, TypeLoadException fires
67+
// the very first time Initialize / Enroll / Synchronize / anything is
68+
// invoked — independent of whether the field is read on that code path.
69+
//
70+
// Issue #7's original fix patched constructor-signature reflection (the
71+
// DI-container surface). The follow-up comment showed a separate failure
72+
// path where Enroll trips on field-type loading. This test guards against
73+
// a regression of either: field types must use only types the v3.2 host
74+
// ships, with `object` as the typical neutral-typed storage and an `as`
75+
// cast inside method bodies (JIT-lazy) for actual use.
76+
var fields = typeof(CERTInextCAPlugin)
77+
.GetFields(BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance);
78+
79+
foreach (var field in fields)
80+
{
81+
string fieldTypeName = field.FieldType.FullName ?? field.FieldType.Name;
82+
V3Point3OnlyTypeNames.Should().NotContain(fieldTypeName,
83+
$"instance field '{field.Name}' (declared type {fieldTypeName}) on " +
84+
$"{field.DeclaringType?.FullName} would trigger TypeLoadException when the JIT " +
85+
$"first compiles any method on the class on a v3.2 gateway host. " +
86+
$"Re-type the field as `object` and cast to the v3.3 type inside method " +
87+
$"bodies — see issue #7 follow-up.");
88+
}
89+
}
90+
91+
[Fact]
92+
public void NoNestedType_ImplementsV3Point3OnlyInterface()
93+
{
94+
// Nested types declared with a base/interface reference to a v3.3-only
95+
// interface put that interface in the containing class's nested-type
96+
// metadata. CLR class-load behaviour around nested-type interface
97+
// resolution is fragile across .NET versions, so we forbid it outright
98+
// as a belt-and-braces measure.
99+
var nestedTypes = typeof(CERTInextCAPlugin)
100+
.GetNestedTypes(BindingFlags.Public | BindingFlags.NonPublic);
101+
102+
foreach (var nested in nestedTypes)
103+
{
104+
foreach (var iface in nested.GetInterfaces())
105+
{
106+
string ifaceName = iface.FullName ?? iface.Name;
107+
V3Point3OnlyTypeNames.Should().NotContain(ifaceName,
108+
$"nested type '{nested.FullName}' implements v3.3-only interface " +
109+
$"'{ifaceName}', which would leak into the containing class's " +
110+
$"reflection surface on a v3.2 host. Delete the nested type or " +
111+
$"refactor it to not declare the v3.3 interface in its base list.");
112+
}
113+
}
114+
}
115+
116+
[Fact]
117+
public void NoPublicMethod_SignatureReferencesV3Point3OnlyTypes()
118+
{
119+
// Reflection-driven hosts (anything calling Type.GetMethods()) eagerly
120+
// resolve return-type and parameter-type metadata on each method. Public
121+
// method signatures must therefore avoid v3.3-only types the same way
122+
// public constructors do. SetDomainValidatorFactory's `object` parameter
123+
// is the safe pattern.
124+
var publicInstanceMethods = typeof(CERTInextCAPlugin)
125+
.GetMethods(BindingFlags.Public | BindingFlags.Instance | BindingFlags.DeclaredOnly);
126+
127+
foreach (var method in publicInstanceMethods)
128+
{
129+
// Property accessors get caught here too — that's intentional.
130+
string returnTypeName = method.ReturnType.FullName ?? method.ReturnType.Name;
131+
V3Point3OnlyTypeNames.Should().NotContain(returnTypeName,
132+
$"public method '{method.Name}' returns v3.3-only type '{returnTypeName}'. " +
133+
$"Change the return type to `object` and have callers cast at the use site.");
134+
135+
foreach (var param in method.GetParameters())
136+
{
137+
string paramTypeName = param.ParameterType.FullName ?? param.ParameterType.Name;
138+
V3Point3OnlyTypeNames.Should().NotContain(paramTypeName,
139+
$"public method '{method.Name}' parameter '{param.Name}' is " +
140+
$"v3.3-only type '{paramTypeName}'. Change the parameter to `object` " +
141+
$"and cast inside the method body — see SetDomainValidatorFactory.");
142+
}
143+
}
144+
}
145+
61146
[Fact]
62147
public void ParameterlessConstructor_IsPublic()
63148
{

‎CERTInext/CERTInextCAPlugin.cs‎

Lines changed: 33 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -35,17 +35,33 @@ public class CERTInextCAPlugin : IAnyCAPlugin, IDisposable
3535
private CERTInextConfig _config;
3636
private ICERTInextClient _client;
3737
private ICertificateDataReader _certificateDataReader;
38-
// Not readonly: SetDomainValidatorFactory mutates this post-construction.
39-
// A v3.3 gateway host can call SetDomainValidatorFactory between `new` and
40-
// Initialize() to wire up DCV; on a v3.2 host where the factory type doesn't
41-
// exist, the field remains null and DCV gracefully no-ops.
38+
// Typed as `object` — NOT `IDomainValidatorFactory` — so the .NET JIT does not
39+
// eagerly resolve the v3.3-only IDomainValidatorFactory type when it compiles
40+
// any method on this class. Resolving an instance field's declared type is
41+
// part of the JIT's per-class metadata load, distinct from constructor-signature
42+
// reflection (which we already protected in the issue #7 first pass). On a
43+
// gateway host whose IAnyCAPlugin assembly is v3.2.0.0 (no IDomainValidatorFactory),
44+
// declaring the field with the missing type causes TypeLoadException the first
45+
// time ANY instance method on the class is compiled — typically Initialize.
4246
//
43-
// `volatile` because the field is written by SetDomainValidatorFactory and
44-
// read by EnrollNewAsync/TryRunDcvDuringSyncAsync, which can run on different
45-
// threads. The standard gateway lifecycle wires the factory before the first
46-
// operation, but no contract forbids a later re-wire and the cost of `volatile`
47-
// is negligible compared to the time spent inside Enroll/Synchronize.
48-
private volatile IDomainValidatorFactory _domainValidatorFactory;
47+
// Reads of this field perform an `as IDomainValidatorFactory` cast inside method
48+
// bodies (see DomainValidatorFactory below). Casts in method bodies are JIT-lazy
49+
// per-method, so the type is only resolved on hosts that actually have it.
50+
//
51+
// `volatile` because the field is written by SetDomainValidatorFactory and read
52+
// by EnrollNewAsync / TryRunDcvDuringSyncAsync, which can run on different threads.
53+
// See GitHub issue #7 for the full reasoning.
54+
private volatile object _domainValidatorFactory;
55+
56+
/// <summary>
57+
/// Returns the injected <see cref="IDomainValidatorFactory"/> when one is
58+
/// available, or <c>null</c> when DCV is not wired up. The cast is inside this
59+
/// property body (and therefore JIT-lazy) so the missing-type case on a v3.2
60+
/// gateway host stays compileable and never triggers <c>TypeLoadException</c>
61+
/// at runtime. All read sites in this class go through this property.
62+
/// </summary>
63+
private IDomainValidatorFactory DomainValidatorFactory =>
64+
_domainValidatorFactory as IDomainValidatorFactory;
4965

5066
// True when the client was passed in via a test-injection constructor and therefore
5167
// should not be disposed by this class (the test owns the mock's lifetime).
@@ -1125,16 +1141,12 @@ private static bool IsDcvNotYetReady(Exception ex)
11251141
return hasPhrase && !hasOtherEmsCode;
11261142
}
11271143

1128-
/// <summary>
1129-
/// Passes connector configuration to DNS provider plugins for per-domain configuration lookup.
1130-
/// </summary>
1131-
private sealed class DomainValidatorConfigProvider : Keyfactor.AnyGateway.Extensions.IDomainValidatorConfigProvider
1132-
{
1133-
public Dictionary<string, object> DomainValidationConfiguration { get; }
1134-
1135-
public DomainValidatorConfigProvider(Dictionary<string, object> config)
1136-
=> DomainValidationConfiguration = config ?? new Dictionary<string, object>();
1137-
}
1144+
// (`DomainValidatorConfigProvider` nested helper removed — it declared an
1145+
// implementation of `Keyfactor.AnyGateway.Extensions.IDomainValidatorConfigProvider`,
1146+
// a v3.3-only interface, but the type was never instantiated anywhere in the
1147+
// plugin. Keeping a nested type whose base list references a missing assembly
1148+
// type is a hazard for CLR class-load on v3.2 hosts (see issue #7). Dead code
1149+
// that costs nothing to remove.)
11381150

11391151
/// <summary>
11401152
/// Best-effort DCV retry for an order that may still be pending validation.
@@ -1414,7 +1426,7 @@ private async Task<bool> PerformDcvIfNeededAsync(
14141426
: _config.DcvTxtRecordTemplate;
14151427
string hostname = string.Format(template, domain);
14161428

1417-
var validator = _domainValidatorFactory.ResolveDomainValidator(domain, "dns-01");
1429+
var validator = DomainValidatorFactory.ResolveDomainValidator(domain, "dns-01");
14181430
if (validator == null)
14191431
throw new InvalidOperationException(
14201432
$"No DNS provider plugin is configured for domain '{domain}'. " +

0 commit comments

Comments
 (0)