Generate MatrixOf<T> properties for OneOrMoreDimension types (#3625) - #3868
Merged
Conversation
Closes the original PR #3619 TODO and extends the MatrixOf<T> opt-in to four sites in the source generator: 1. DataType structure fields: typed properties + Variant.From / WriteEncodeableMatrix encode/decode. 2. VariableType State classes: typed template parameter (e.g. XYArrayItemState<MatrixOf<XVType>>). 3. PropertyState / BaseDataVariableState instances: typed (e.g. EnumDictionaryEntries: PropertyState<MatrixOf<NodeId>>, FailureSystemIdentifier: BaseDataVariableState<MatrixOf<byte>>). 4. Client/Server API method parameters: typed MatrixOf<T> instead of Variant. DiagnosticInfo + abstract numeric bases (BaseDataType/Number/Integer/UInteger) keep the Variant fallback. Also adds the parameterless IDecoder.ReadEncodeableMatrix<T>(string) overload across BinaryDecoder/JsonDecoder/XmlDecoder/XmlParser and the PubSub wrapper. Fixes #3625
marcschier
commented
Jun 9, 2026
… strings, RS TestData - MigrationGuide.md: use 1.5.378 (master378) 'new Variant(new Matrix(...))' patterns in before-samples (#3868 review). - MigrationGuide.md: drop the IsTemplateParameterRequired generator-internal paragraph. - IDecoder.cs: cite OPC UA Part 6 v1.05 section 5.2.5 (Matrix encoding) in the new ReadEncodeableMatrix overload remarks. - NodeStateGeneratorTests.cs: drop 'Site' and PR-number references in the snippet test comments. - Quickstarts.Servers/TestData/Generated/TestDataDesign.xml: mirror the new MatrixValueDataType in the reference server design too. - GenerateCodeTests.cs: convert escaped-quote strings to C# 11 raw-string literals to drop the backslash noise.
marcschier
marked this pull request as ready for review
June 9, 2026 15:09
…Matrix<T>(string) Codecov flagged the parameterless ReadEncodeableMatrix<T>(string?) overloads in Stack/Opc.Ua.Types/Encoders/JsonDecoder.cs and Stack/Opc.Ua.Types/Encoders/XmlDecoder.cs as uncovered. Add three round-trip tests: - XmlDecoderReadEncodeableMatrixNoEncodingIdRoundTrips (streaming XML) - JsonDecoderReadEncodeableMatrixNoEncodingIdRoundTrips (positive) - JsonDecoderReadEncodeableMatrixNoEncodingIdMissingFieldReturnsDefault (negative) All 6 EncodeableMatrixRoundTripTests pass locally on net10.0.
koepalex
reviewed
Jun 10, 2026
koepalex
reviewed
Jun 10, 2026
…on overflow
The MatrixOf<T>(ReadOnlyMemory<T>, int[]) constructor used unchecked Int32
multiplication when validating the dimensions array against the element
count: dimensions.Aggregate((a, b) => a * b). With this PR the constructor
is reachable through new attacker-controlled wire paths via the new
IDecoder.ReadEncodeableMatrix<T>(string?) overloads added in this branch,
and was already reachable via the pre-existing encoding-id matrix overload
and every BinaryDecoder.ReadVariantValue matrix branch.
Concrete reachable case (verified by the security-review sub-agent):
dimensions=[65536, 65536] + empty element array -> product wraps to 0 ==
Memory.Length, so the length-mismatch guard accepts the message and exposes
a matrix whose declared shape claims 4_294_967_296 elements. Downstream
Array.CreateInstance / Span<T> indexing converts this into OOM or
OverflowException, i.e. exception-driven decoder DoS / data-integrity
bypass.
Hardening:
1. Wrap the dimension product in checked { } so any Int32 overflow throws
OverflowException -> ArgumentException at the constructor boundary.
2. Reject negative individual dimensions explicitly: [-1, -1] multiplies to
the positive value 1 and would otherwise pass the length-match guard
for a single-element payload, only to crash downstream consumers.
Tests: 3 new MatrixOfTests cases (2-dim overflow, 3-dim overflow, negative
dimension); 36/36 MatrixOfTests pass, full Opc.Ua.Types.Tests 7559 pass.
Verified by the security-review sub-agent independently; cross-checked
against Security Agent MCP scans of service tree
59eec07a-6f20-42b9-b41b-d20e0a6322da (no findings attributable to this
branch in CodeVulnerabilities, SecretsUsage, ThreatModels, PolicyCoverage,
CodeIntegrity, or AttackPaths).
…vice param contract test Three follow-up items from plans/security-assessment.md addressed in this commit: 1. PubSubJsonDecoder.ReadEncodeableMatrix<T> (both overloads) now throws NotSupportedException with a clear 'PubSub JSON matrix encoding is not yet implemented' message, replacing the prior silent default-return behavior. Surfaces the gap explicitly instead of dropping attacker- controlled payload content. (Resolves S2 / T3.) 2. Added a HeaterStatus enum + HeaterStatusMatrix field to TestDataDesign.xml MatrixValueDataType, and extended GenerateMatrixValueDataType_EmitsMatrix OfPropertiesAndCalls in GenerateCodeTests.cs to assert the property type MatrixOf<HeaterStatus>, encode call Variant.From(HeaterStatusMatrix), and decode call GetEnumerationMatrix<HeaterStatus>(). (Resolves T1.) 3. Added MatrixServiceParameterContract_GetDotNetTypeNameMatchesGeneratorIntegration in ModelDesignExtensionsTests.cs - 7 parameterised cases asserting the exact integration composition both ClientApiGenerator.CollectParameters and ServerApiGenerator.CollectParameters apply, including the DiagnosticInfo->Variant fallback. (Resolves T2.) Local verification: 8/8 source-gen tests pass (1 snippet + 7 contract); PubSub 9315/9412 pass with the only failure being an unrelated [LongRunning] MQTT MetaData timing test that does not touch ReadEncodeableMatrix code.
koepalex
approved these changes
Jun 11, 2026
The HeaterStatus enum added to TestDataDesign.xml in commit 4ec55f429 (to exercise the typed-enum matrix codegen path through HeaterStatusMatrix in MatrixValueDataType) causes the source generator to emit one additional output file, bringing the total per-model file count for TestDataDesign in line with the 9 files DemoModel produces. Updated ModelGeneratorTests.GenerateAndCompileTestDataDesignTest assertion and added an explanatory inline comment.
marcschier
pushed a commit
that referenced
this pull request
Jun 12, 2026
…anager.DoSimulation DoSimulation can race with Dispose() on shutdown: the simulation timer fires and gets past the m_simulationEnabled guard, then Dispose() runs (which intentionally does NOT wait for in-flight timer callbacks - see the comment on Dispose, lines 79-87) and disposes m_semaphore while DoSimulation is about to call m_semaphore.Wait(). The resulting ObjectDisposedException is caught by the outer try/catch and logged at LogLevel.Error as "Unexpected error doing simulation", producing misleading noise in every CI test log that disposes a ReferenceNodeManager. The race itself is acknowledged and accepted in the Dispose() comment as a better trade-off than blocking Dispose on a Timer.Dispose(WaitHandle). The log entry is the only problem: it labels an expected, documented teardown race as an unexpected error. Filter ObjectDisposedException specifically, gated on `m_simulationTimer is null` so the suppression only kicks in once Dispose has actually run. A genuine premature semaphore-disposal bug elsewhere would still surface loudly via the generic catch. Observed in build 14598 (mac PR build for #3868), History.Tests fixture teardown for AlarmClientIntegrationTests: SERVER 04:54:05.298 [Quickstarts.ReferenceServer.ReferenceNodeManager] Unexpected error doing simulation #1. [ObjectDisposedException] Cannot access a disposed object. Object name: 'System.Threading.SemaphoreSlim'.
marcschier
added a commit
to marcschier/UA-.NETStandard
that referenced
this pull request
Jun 14, 2026
Brings in 7 upstream commits including OPCFoundation#3866 (Managed DevOps Pool CI migration), OPCFoundation#3865 (dotnet format sweep), OPCFoundation#3854 (V2 SetTriggering), OPCFoundation#3868 (MatrixOf<T> properties for OneOrMoreDimension), OPCFoundation#3864 (ManagedSession revalidation tests), OPCFoundation#3863 (Part 13 Aggregates compliance), and OPCFoundation#3828 (stop emitting unimplemented Optional children on singleton instances). One conflict in `UA.slnx`: upstream added two new project entries between `ConsoleReferenceSubscriber` and `Quickstarts.Servers` — * `Applications/McpServer/Opc.Ua.Mcp.csproj` — upstream's renamed path. Our branch never adopted the rename and still ships the project at `Applications/Opc.Ua.Mcp/Opc.Ua.Mcp.csproj`, which is already listed two lines below. Dropped: the McpServer path is stale build artifacts only (bin/obj), no .csproj. * `Applications/PumpDeviceIntegrationServer/PumpDeviceIntegrationServer.csproj` — upstream's intentional new project. Project file exists in our branch; kept. UaLens build clean (0 warnings / 0 errors). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Closes the original PR #3619 TODO and extends the
MatrixOf<T>opt-in to every place the source generator previously fell back toVariantforValueRank="OneOrMoreDimensions".Generator changes (
Tools/Opc.Ua.SourceGeneration.Core)Site 1 — DataType structure fields (
DataTypeGenerator.cs)Fields declared with matrix rank now render as
global::Opc.Ua.MatrixOf<T>instead ofVariant, mirroring the existingArrayOf<T>treatment forArrayrank. Encode/decode is routed per the field'sDataType:DataTypeBoolean,Int32,String, …)MatrixOf<bool>etc.WriteVariant(name, Variant.From(field));ReadVariant(name).GetBooleanMatrix();(etc.)Structure/ abstract structureMatrixOf<ExtensionObject>WriteVariant(name, Variant.From(field));ReadVariant(name).GetExtensionObjectMatrix();IEncodeable(e.g.Vector)MatrixOf<Vector>WriteEncodeableMatrix(name, field);ReadEncodeableMatrix<Vector>(name);MatrixOf<MyEnum>WriteVariant(name, Variant.From(field));ReadVariant(name).GetEnumerationMatrix<MyEnum>();BaseDataType/Number/Integer/UIntegerMatrixOf<Variant>WriteVariant(name, Variant.From(field));ReadVariant(name).GetVariantMatrix();Site 2 — VariableType State classes (
NodeStateGenerator.cs)Replaces the original
// TODO: matrixOf;from #3619. Concrete matrix variable types now inherit the typed<MatrixOf<T>>chain. Standard model:XYArrayItemState : ArrayItemState<MatrixOf<XVType>>.Implementation<StructureBuilder<XVType>>Abstract bases (
ArrayItemType,CubeItemType,ImageItemType,NDimensionArrayItemType— allDataType="BaseDataType") keep the existing<T>early-return.Site 3 — Property / Variable instances (
ModelDesignExtensions.GetNodeStateClassName)Adds a
OneOrMoreDimensionsbranch after the existingArraybranch and refinesIsTemplateParameterRequired/IsIndeterminateTypeso matrix-rank concrete types are no longer treated as indeterminate. Standard model:EnumDictionaryEntries: PropertyState<MatrixOf<NodeId>>FailureSystemIdentifier: BaseDataVariableState<MatrixOf<byte>>Site 4 — Method parameters (
ClientApiGenerator/ServerApiGenerator)CollectParametersopts in touseMatrixTypeInsteadOfVariantviaSupportsMatrixOf(). Zero standard-service impact (no OPC UA service declares matrix args).Encoder / decoder API
Adds the parameterless overload
MatrixOf<T> ReadEncodeableMatrix<T>(string? fieldName) where T : IEncodeable, new()toIDecoderand all implementations (BinaryDecoder,JsonDecoder,XmlDecoder,XmlParser,PubSubJsonDecoder), mirroring the existingReadEncodeableArray<T>(string?)shape so the generator does not need a runtime encoding-id lookup.Consumer migration
Applications/Quickstarts.Servers/ReferenceServer/ReferenceNodeManager.cs—CreateXYArrayItemVariableupdated fromVariant.FromStructure(ArrayOf.Wrapped(...))tonew XVType[] { … }.ToMatrixOf(5)to match the new typedXYArrayItemState.Valuesurface.Tests added
GenerateMatrixValueDataType_EmitsMatrixOfPropertiesAndCalls(custom data-type matrix fields) andNodeStateGeneratorEmitsMatrixOfTemplateParameterForStandardModel(standard-model VariableType + instance types).EncodeableMatrixRoundTripTestsexercisesWriteEncodeableMatrix↔ReadEncodeableMatrix<T>(string?)through Binary, XML, and parameterless overloads with null-matrix and populated payloads.GetDotNetTypeName_OneOrMoreDimensionsValueRankWithBasicDataTypes_ReturnsCorrectMatrixOfTypeandIsTemplateParameterRequired_OneOrMoreDimensionsWith…tables;SupportsMatrixOfcoverage includingDiagnosticInfo → falseandArgumentNullExceptionfor null receiver.TestDataDesign.xml,DemoModel.xml,TestModel.xml): newMatrixValueDataType/SampleStructureWithMatrixFieldsexercise every branch end-to-end through the existingGenerateAndCompileDesignFileAsyncintegration test.Documentation
Docs/MigrationGuide.md— new sub-section "Generated data type fields with ValueRank=OneOrMoreDimensions" with a per-BasicDataTypemapping table, theIDecoder.ReadEncodeableMatrix<T>(string?)overload note for custom decoder implementations, and a sibling block covering VariableType State classes, property-state instances, and service parameters with before/after code samples.Verification
Opc.Ua.Types.TestsOpc.Ua.SourceGeneration.Core.TestsOpc.Ua.SourceGeneration.Stack.TestsOpc.Ua.SourceGeneration.TestsOpc.Ua.Server.Tests(focused onReferenceServer/ArrayItem)UA.slnxbuildThe
// TODO: matrixOf;comment is gone fromNodeStateGenerator.cs.Related Issues
Checklist