Fix the Opc.Ua.Types defects found by the read-only audit by romanett · Pull Request #4474 · OPCFoundation/UA-.NETStandard

romanett · 2026-09-11T14:47:20Z

Description

Fixes the defects a read-only audit reported against Opc.Ua.Types at a05a360: 98 of its 99 findings for Variant/TypeInfo, the identifier types, the four codecs, NodeState/Nodes, Schema/Utils/Polyfills and the WoT binding, plus 21 further defects found while reviewing those fixes — several of which were regressions the first pass introduced. Every fix carries a regression test that fails without it.

Opc.Ua.Types is the lowest-level assembly in the stack, so most of these are silent wrong answers in code every other project sits on rather than crashes.

The higher-impact ones

Variant equality, hashing and ordering. ValueEquals and CompareTo — the ContentFilter comparison surface, whose only consumer is FilterEvaluator — now convert between numeric built-in types as Part 4 §7.7.3's implicit-conversion and data-precedence rules require. Variant.Equals deliberately does not: it stays strict on BuiltInType, so it remains consistent with GetHashCode and safe as a Dictionary/HashSet key. GetHashCode read Float/Double/Enumeration/DateTime through the wrong union field and hashed byte arrays by reference, so equal values hashed apart. Comparing a null variant against a typed one gave a different answer depending on which side it was written on; that is now decided once and symmetrically, and a numeric or boolean zero stays unequal to a null variant — letting it compare equal collapsed both into one bucket of every Dictionary/HashSet keyed on Variant, made Equals intransitive, and made a HashSet's count depend on insertion order. CompareTo reports an unrelated pair as not comparable instead of ordering it by raw union bits, and the comparison operators now reject that sentinel — reading int.MinValue as an ordinary negative result made both a < b and b < a true, which OrderBy detects and throws on.

NodeId.CompareTo was not transitive. The null case reported "less than" in both directions for every identifier shape except a namespace-zero numeric id, so a SortedSet<NodeId> could not find a NodeId.Null it had just inserted.

Encoding limits. MaxStringLength counts the bytes of the encoded string (Part 3 §5.6.4, Part 5 §6.3.2). The XML codecs counted UTF-16 code units, so one message context gave two answers and a string a binary peer rejects survived an XML round trip. The measurement now lives in a single EncodingLimits helper, replacing three separate copies across the codecs. The JSON decoder additionally enforces the array, byte-string and string limits it never had — including on the base64 and shorthand spellings that bypassed the checks entirely, so a peer could opt out of a limit just by choosing the other legal encoding. An encoding-limit breach is also no longer downgraded into a successful decode that keeps the raw over-limit body.

Node.Write threw out of a status-code API. The attribute type gate compared built-in types only, which does not separate a scalar from an array, so an array value passed and the unconditional cast behind it threw InvalidCastException out of a method contracted to return a ServiceResult. It now compares the value rank too, and the structured permission attributes became writable at all (their data type has no built-in type, so the gate had rejected every value).

CoreUtils.Match. Rejects an unmatched trailing character without breaking the single-character wildcard — an earlier version of this fix made every pattern ending in ? return false, which no existing test covered.

Spec conformance was checked against Part 3, 4, 5 and 6 for each behavioural change: implicit conversion and data precedence (4 §7.7.3), NumericRange BNF (4 §7.27/A.3), returnDiagnostics bits (4 §7.32), MinimumSamplingInterval as a Duration (3 §5.6.2), ServerIndex as UInt32 (6 §5.2.2.10), DateTime UTC (6 §5.3.1.6), Union SwitchField (6 §5.2.8, §5.3.7, H.10), and the verbose JSON Enumeration form (6 §5.4.4.2).

Reviewer notes

One deliberate behaviour tightening. XML with more than MaxStringLength non-ASCII bytes is now rejected where it previously passed on character count. Nothing in the repo's fixtures trips it, but it is a real narrowing for a deployment with a low MaxStringLength and non-ASCII display names.
One finding deliberately left open. NodeState.Initialize still does not carry over the optional storage that CopyTo copies. It instantiates a node from a prototype rather than cloning it, and propagating the prototype's RolePermissions makes the GDS certificate-group nodes unreadable for users that could read them before (CustomCertificateGroupNodeExistsInAddressSpaceAsync fails with BadUserAccessDenied). Whether an instance inherits its prototype's permissions is a security decision for the address space, so the omission is now documented in place rather than quietly closed. Happy to split it into its own issue.
Nine existing tests were updated because they pinned the buggy behaviour. Each is worth a look: EqualsWithDifferentNumericTypesReturnsFalse asserted the opposite of Part 4 §7.7.3 and was renamed with a replacement negative case; XmlElementGetHashCodeShouldReturnCorrectHashCode pinned the textual hash that broke the Equals contract; the CompareTo-versus-null cases in NodeIdTests/ExpandedNodeIdTests asserted the non-antisymmetric answers; three DateTimeUtcTests parse cases depended on the local time zone.
21 of the fixes (5,558 insertions across 74 files) came out of an adversarial review of the first pass, with each candidate independently verified against the built assembly rather than by inspection — the Variant null-equality, NodeId.CompareTo, Node.Write, CoreUtils.Match and JSON-limit items above are all in that group.

Related Issues

No tracking issue exists — this came from an internal read-only audit rather than a reported defect. Happy to open one, or to split the WoT, encoder and Variant groups into separate issues if the maintainers prefer that shape for a change this size.

Fixes #

Checklist

Put an x in the boxes that apply. You can complete these step by step after opening the PR.

I have signed the CLA and read the CONTRIBUTING doc.
I have added tests that prove my fix is effective or that my feature works and increased code coverage.
I have added all necessary documentation.
I have verified that my changes do not introduce (new) build or analyzer warnings.
I ran all tests locally using the UA.slnx solution against at least .net framework and .net 10, and all passed.
I fixed all failing and flaky tests in the CI pipelines and all CodeQL warnings.
I have addressed all PR feedback received.

Notes on the checklist

Tests: every fix has a regression test; 9 new test files, ~130 new test cases. Several were verified discriminating by temporarily reverting the production line and confirming the test fails.
Warnings: full UA.slnx build is clean, and dotnet format whitespace --verify-no-changes reports the same 101 pre-existing findings in the same file set as an unmodified worktree at the same commit — none in changed files.
Test runs: all suites were run on net10.0 only, so that box is left unchecked pending a .NET Framework run: Types 10374, Core 4490, Core.Encoders 2929, Core.Schema 157, Configuration 227, Server 5540, Client 2217, Gds 1117, PubSub 1369, WotCon 1496, ComplexTypes 2773, InformationModel 1080, Di 425 — all passing, 0 failures. Two failures in Core.Diagnostics.Tests and OpenUsd.Tests are pre-existing on master and caused by a German locale decimal separator, confirmed against a clean baseline worktree at the same commit.
Documentation: no separate docs; the new and changed members carry XML doc comments explaining the spec clause behind each behavioural decision.

🤖 Generated with Claude Code

Addresses 98 of the 99 findings the read-only audit of the Opc.Ua.Types assembly reported for Variant/TypeInfo, the identifier types, the four codecs, NodeState/Nodes, Schema/Utils/Polyfills and the WoT binding, plus 21 further defects found while reviewing those fixes. Every fix carries a regression test that fails without it. Variant and TypeInfo - TryGetDecimal read the narrow signed scalars through m_union.Int32, so a negative SByte/Int16 lost its sign. - Equality and comparison now convert between numeric built-in types, as Part 4 7.7.3's implicit conversion and data precedence rules require, and CompareTo reports an unrelated pair as not comparable instead of ordering it by raw union bits. The comparison operators reject that sentinel rather than reading int.MinValue as an ordinary negative result, which otherwise made both a < b and b < a true. - Comparing a null variant against a typed one gave a different answer depending on which side it was written on. It is now decided once and symmetrically, and a numeric or boolean zero stays unequal to a null variant - letting it compare equal collapsed both into one bucket of every Dictionary and HashSet keyed on Variant. - GetHashCode read Float/Double/Enumeration/DateTime through the wrong union field and hashed byte arrays by reference, breaking the Equals contract. - The bitwise operators, Expand, Collapse and ConvertTo handle Enumeration and the narrow integer widths instead of silently producing a null. Identifiers - NodeId.CompareTo and ExpandedNodeId.CompareTo are antisymmetric against a null and against a foreign type; the null case used to report "less than" in both directions, which is not even transitive, so a SortedSet could not find a NodeId.Null it had just inserted. - ExpandedNodeId parses svr= as a UInt32 (Part 6 5.2.2.10) through the server mapping table, and reports a malformed index instead of silently turning a remote node id into a local one. - NumericRange rejects an empty dimension, renders multi-dimensional ranges, and treats a null range as a no-op in every overload. - DateTimeUtc forces UTC on every parse path (Part 6 5.3.1.6) and saturates its arithmetic in both directions on every target framework. - DiagnosticInfo keeps the UserPermission bit when shifting the returnDiagnostics mask down to the operation level (Part 4 7.32). - QualifiedName.Format emits a namespace prefix when the name itself contains a colon, so the text parses back into the namespace it came from. - XmlElement hashes structurally, matching the structural Equals. Encoders - MaxStringLength counts the bytes of the encoded string (Part 3 5.6.4, Part 5 6.3.2) in all four codecs. The XML codecs counted UTF-16 code units, so one message context gave two answers and a string a binary peer rejects survived an XML round trip. The measurement now lives in one place, replacing three separate copies. - The JSON decoder enforces the array, byte string and string limits it never had, including on the base64 and shorthand spellings that bypassed the checks the audit asked for. - A Union selector of 0 means no field and an out of range one is an error (Part 6 5.2.8, 5.3.7, H.10); the absent SwitchField no longer indexes -1. - An encoding limit breach is no longer downgraded into a successful decode that keeps the raw over-limit body. - Nested codecs inherit the namespace and server mappings of the message they are part of. - BinaryEncoder.Close no longer leaves a disposed writer behind for Dispose to flush, and EncodeableFactory normalizes a namespace zero URI on lookup as well as on registration, without collapsing a server scoped id onto the local entry. NodeState and Nodes - Node.Write compares the attribute's built-in type and value rank, so an array no longer reaches a cast that throws InvalidCastException out of a method that reports failure as a status code, and the structured permission attributes became writable at all. - CopyTo carries the optional storage and reports the source's change mask rather than the bits its own setters raise. - MinimumSamplingInterval is written as the Duration it is (Part 3 5.6.2). Utils, Schema and WoT - CoreUtils.Match rejects an unmatched trailing character without breaking the single character wildcard, and no longer reads past an unterminated character set. - The culture aware string.Replace polyfill advances by the length that actually matched; under a culture sensitive comparison a match can span a different number of characters than the value searched for. - The virtual file system keeps the bytes a stream never wrote. - The WoT resolvers unescape the portable nsu= namespace form the way every other reader of the same member does, and the NodeSet converter matches a reference type by alias or numeric id consistently. Notably NOT changed: NodeState.Initialize still does not carry over the optional storage that CopyTo copies. It instantiates a node from a prototype rather than cloning it, and propagating the prototype's RolePermissions makes the GDS certificate group nodes unreadable for users that could read them before. Whether an instance inherits its prototype's permissions is a security decision for the address space, so the omission is now documented instead of quietly closed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Copilot

🟡 Changes recommended

Several fixes remain incomplete, including telemetry attribution, inferred WoT DataTypes, XML serialization, and polyfill compatibility.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Fixes broad correctness defects in the foundational Opc.Ua.Types assembly and adds regression coverage.

Changes:

Corrects identifier, Variant, node-state, schema, filesystem, and telemetry behavior.
Strengthens binary, XML, JSON, and WoT processing.
Adds extensive regression tests and updates obsolete expectations.

File summaries

File	Description
`tests/Opc.Ua.WotCon.Tests/WotConnectivityNodeManagerTests.cs`	Clarifies typed-null assertion
`tests/Opc.Ua.Types.Tests/Wot/WotAuditRegressionTests.cs`	Adds WoT regression coverage
`tests/Opc.Ua.Types.Tests/Utils/UtilsAuditRegressionTests.cs`	Adds utility regression coverage
`tests/Opc.Ua.Types.Tests/State/NodeStateOptionalStorageTests.cs`	Updates clone and notification tests
`tests/Opc.Ua.Types.Tests/Schema/SchemaAuditRegressionTests.cs`	Adds schema regression coverage
`tests/Opc.Ua.Types.Tests/Encoders/JsonAuditRegressionTests.cs`	Adds JSON codec coverage
`tests/Opc.Ua.Types.Tests/Encoders/EncodeableFactoryTests.cs`	Updates namespace normalization expectations
`tests/Opc.Ua.Types.Tests/Encoders/CodecAuditRegressionTests.cs`	Adds binary/XML codec coverage
`tests/Opc.Ua.Types.Tests/Encoders/BinaryDecoderTests.cs`	Tests diagnostic depth boundaries
`tests/Opc.Ua.Types.Tests/BuiltIn/XmlElementTests.cs`	Tests structural hash consistency
`tests/Opc.Ua.Types.Tests/BuiltIn/VariantStorageTests.cs`	Updates typed-union expectations
`tests/Opc.Ua.Types.Tests/BuiltIn/VariantCoverageTests.cs`	Verifies bitwise result types
`tests/Opc.Ua.Types.Tests/BuiltIn/VariantAuditRegressionTests.cs`	Adds Variant regression coverage
`tests/Opc.Ua.Types.Tests/BuiltIn/NodeIdTests.cs`	Updates null ordering tests
`tests/Opc.Ua.Types.Tests/BuiltIn/IdentifierAuditRegressionTests.cs`	Adds identifier regression coverage
`tests/Opc.Ua.Types.Tests/BuiltIn/ExpandedNodeIdTests.cs`	Updates null comparison expectation
`tests/Opc.Ua.Types.Tests/BuiltIn/DateTimeUtcTests.cs`	Makes UTC parsing tests deterministic
`tests/Opc.Ua.Core.Tests/Types/ContentFilter/FilterEvaluatorTests.cs`	Tests numeric comparison conversion
`tests/Opc.Ua.Core.Encoders.Tests/EncodeableFactoryTests.cs`	Mirrors factory normalization tests
`src/Opc.Ua.Types/Wot/WotProjectionResolver.cs`	Preserves base URI queries
`src/Opc.Ua.Types/Wot/WotNodeSetConverter.WoT.cs`	Corrects WoT node synthesis
`src/Opc.Ua.Types/Wot/WotNodeSetConverter.DataTypes.cs`	Corrects DataType synthesis
`src/Opc.Ua.Types/Wot/WotNodeSetConverter.Analog.cs`	Rejects non-finite ranges
`src/Opc.Ua.Types/Wot/WotNativeProjection.cs`	Avoids duplicate derived members
`src/Opc.Ua.Types/Wot/WotJsonResidue.cs`	Hardens residue processing
`src/Opc.Ua.Types/Wot/WotEventSelectionResolver.Definitions.cs`	Tracks definition ownership
`src/Opc.Ua.Types/Wot/WotEventSelectionResolver.cs`	Uses owning document context
`src/Opc.Ua.Types/Wot/WotDocumentNodeResolver.cs`	Corrects portable-name parsing
`src/Opc.Ua.Types/Utils/ServiceResultException.cs`	Handles null results
`src/Opc.Ua.Types/Utils/FileSystem/VirtualFileSystem.cs`	Tracks write high-water marks
`src/Opc.Ua.Types/Utils/FileSystem/LocalFileSystem.cs`	Allows concurrent readers
`src/Opc.Ua.Types/Utils/CoreUtils.cs`	Corrects wildcard matching
`src/Opc.Ua.Types/State/NodeStateCollection.cs`	Fixes server URI mapping
`src/Opc.Ua.Types/State/NodeState.cs`	Corrects cloning and writes
`src/Opc.Ua.Types/State/MethodState.cs`	Clones arguments and saves flags
`src/Opc.Ua.Types/State/DataTypeState.cs`	Raises definition change masks
`src/Opc.Ua.Types/State/BaseVariableTypeState.cs`	Copies source values correctly
`src/Opc.Ua.Types/State/BaseVariableState.cs`	Copies status with values
`src/Opc.Ua.Types/State/BaseDataVariableState.cs`	Clones enumeration strings
`src/Opc.Ua.Types/Schema/XmlSchemaValidator2.cs`	Safely filters schema imports
`src/Opc.Ua.Types/Schema/UANodeSetHelpers.cs`	Preserves unresolved identifiers
`src/Opc.Ua.Types/Schema/TypeDictionaryValidator.cs`	Handles fieldless base types
`src/Opc.Ua.Types/Schema/SchemaValidator.cs`	Corrects side-by-side lookup
`src/Opc.Ua.Types/Schema/NodeSetComparer.cs`	Expands alias-aware attributes
`src/Opc.Ua.Types/Schema/BinarySchemaValidator.cs`	Fixes imports and diagnostics
`src/Opc.Ua.Types/Polyfills/System.cs`	Implements comparison-aware operations
`src/Opc.Ua.Types/Nodes/VariableNode.cs`	Writes Duration as Double
`src/Opc.Ua.Types/Nodes/ReferenceNode.cs`	Aligns hashing with equality
`src/Opc.Ua.Types/Nodes/NodeTable.cs`	Corrects reference indexing/removal
`src/Opc.Ua.Types/Nodes/NodeSet.cs`	Corrects namespace translation
`src/Opc.Ua.Types/Nodes/Node.cs`	Validates attribute rank and type
`src/Opc.Ua.Types/Encoders/XmlParser.cs`	Hardens XML decoding
`src/Opc.Ua.Types/Encoders/XmlEncoder.cs`	Corrects XML serialization
`src/Opc.Ua.Types/Encoders/XmlDecoder.cs`	Hardens streaming XML decoding
`src/Opc.Ua.Types/Encoders/Union.cs`	Recovers verbose union selectors
`src/Opc.Ua.Types/Encoders/JsonEncoder.cs`	Enforces JSON limits consistently
`src/Opc.Ua.Types/Encoders/EncodingLimits.cs`	Centralizes string limit measurement
`src/Opc.Ua.Types/Encoders/EncodeableFactory.cs`	Normalizes namespace-zero identifiers
`src/Opc.Ua.Types/Encoders/BinaryEncoder.cs`	Corrects limits and disposal
`src/Opc.Ua.Types/Encoders/BinaryDecoder.cs`	Preserves nested decoding state
`src/Opc.Ua.Types/Diagnostics/TelemetryContextBase.cs`	Derives telemetry source identity
`src/Opc.Ua.Types/BuiltIn/XmlElement.cs`	Aligns structural equality and hashing
`src/Opc.Ua.Types/BuiltIn/TypeInfo.cs`	Accepts enumeration variants
`src/Opc.Ua.Types/BuiltIn/QualifiedName.cs`	Preserves namespace-zero names
`src/Opc.Ua.Types/BuiltIn/NumericRange.cs`	Corrects parsing and null ranges
`src/Opc.Ua.Types/BuiltIn/NodeId.cs`	Corrects parsing, hashing, ordering
`src/Opc.Ua.Types/BuiltIn/MatrixOf.cs`	Allows null reference elements
`src/Opc.Ua.Types/BuiltIn/ExpandedNodeId.cs`	Corrects parsing and equality
`src/Opc.Ua.Types/BuiltIn/DiagnosticInfo.cs`	Preserves diagnostic mask flags
`src/Opc.Ua.Types/BuiltIn/DateTimeUtc.cs`	Adds UTC parsing and saturation
`src/Opc.Ua.Types/BuiltIn/ArrayOf.cs`	Corrects replacement bounds

Review details

Files reviewed: 74/74 changed files
Comments generated: 8
Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

github-actions · 2026-09-11T15:21:53Z

Code coverage

✅ Coverage gate passed.

Check	Result	Threshold
✅ Project line rate	86.95% (262050/301376 lines)	>= 70.00%
✅ Project branch rate	77.06%	>= 60.00%
✅ Patch coverage	88.07% (812/922 changed lines)	>= 75.00% (> 100 changed lines)
ℹ️ Baseline delta (advisory)	+13.35 pp	73.60% recorded

Uncovered changed lines

src/Opc.Ua.Types/Encoders/XmlDecoder.cs: 374, 375, 376, 377, 378, 1075, 1076, 1077, 1078, 1079, 1993, 2054, 2056, 2057, 2058, 2059, 2060, 2061, 2200, 2201, 2202
src/Opc.Ua.Types/Encoders/XmlParser.cs: 372, 373, 374, 375, 376, 1071, 1072, 1073, 1074, 1075
src/Opc.Ua.Types/Wot/WotProjectionResolver.cs: 1564, 1608, 1613, 1614
src/Opc.Ua.Types/Encoders/EncodeableFactory.cs: 218, 222, 226
src/Opc.Ua.Types/State/DataTypeState.cs: 303
src/Opc.Ua.Types/Utils/FileSystem/VirtualFileSystem.cs: 705
src/Opc.Ua.Types/Utils/CoreUtils.cs: 812, 827, 829, 835, 875
src/Opc.Ua.Types/Nodes/Node.cs: 636
src/Opc.Ua.Types/Schema/UANodeSetHelpers.cs: 2754
src/Opc.Ua.Types/BuiltIn/Variant.cs: 1090, 2808, 2809, 7424, 8299
src/Opc.Ua.Types/Wot/WotNodeSetConverter.DataTypes.cs: 1708, 1950
src/Opc.Ua.Types/Schema/TypeDictionaryValidator.cs: 421
src/Opc.Ua.Types/Wot/WotNodeSetConverter.NodeSet.cs: 1718
src/Opc.Ua.Types/Encoders/BinaryDecoder.cs: 834, 835, 836
src/Opc.Ua.Types/Encoders/JsonDecoder.cs: 1958, 2019, 2020, 2021, 2852, 2853, 2854, 2961, 2962, 2963, 2964, 2965
src/Opc.Ua.Types/Wot/WotDocumentNodeResolver.cs: 228
src/Opc.Ua.Types/Wot/WotNodeSetConverter.WoT.cs: 2032, 2033, 2034, 2035, 2036, 2037, 2038, 2039
src/Opc.Ua.Types/State/NodeState.cs: 6187
src/Opc.Ua.Types/Wot/WotJsonResidue.cs: 1463, 1550, 1567, 1568, 1569, 1570, 1571, 1572, 1573, 1574, 1575, 1577, 1578, 1580, 1581, 1582, 1583, 1584, 1585
src/Opc.Ua.Types/Schema/SchemaValidator.cs: 307, 308, 310, 311, 313
src/Opc.Ua.Types/Diagnostics/TelemetryContextBase.cs: 91, 93, 94
src/Opc.Ua.Types/BuiltIn/DateTimeUtc.cs: 577
src/Opc.Ua.Types/BuiltIn/XmlElement.cs: 272

Coverage is above the recorded baseline - consider ratcheting coverage-thresholds.json.

_{Thresholds live in coverage-thresholds.json. Whole report before exclusions: line 86.09%, branch 76.24%.}

codecov · 2026-09-11T15:22:50Z

Codecov Report

❌ Patch coverage is 80.80260% with 177 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.00%. Comparing base (2d90881) to head (6d82e99).

Files with missing lines	Patch %	Lines
src/Opc.Ua.Types/Encoders/XmlDecoder.cs	38.46%	21 Missing and 3 partials ⚠️
src/Opc.Ua.Types/Wot/WotJsonResidue.cs	65.67%	19 Missing and 4 partials ⚠️
src/Opc.Ua.Types/Encoders/JsonDecoder.cs	77.52%	12 Missing and 8 partials ⚠️
src/Opc.Ua.Types/Encoders/XmlParser.cs	65.78%	10 Missing and 3 partials ⚠️
src/Opc.Ua.Types/Wot/WotNodeSetConverter.WoT.cs	82.53%	8 Missing and 3 partials ⚠️
src/Opc.Ua.Types/BuiltIn/Variant.cs	94.70%	5 Missing and 4 partials ⚠️
src/Opc.Ua.Types/Utils/CoreUtils.cs	65.21%	5 Missing and 3 partials ⚠️
src/Opc.Ua.Types/Wot/WotProjectionResolver.cs	50.00%	4 Missing and 4 partials ⚠️
src/Opc.Ua.Types/Schema/SchemaValidator.cs	0.00%	3 Missing and 2 partials ⚠️
src/Opc.Ua.Types/Encoders/EncodeableFactory.cs	73.33%	3 Missing and 1 partial ⚠️
... and 28 more

Additional details and impacted files

@@           Coverage Diff            @@
##           master    #4474    +/-   ##
========================================
  Coverage   80.99%   81.00%            
========================================
  Files        2050     2051     +1     
  Lines      300779   301376   +597     
  Branches    52053    52177   +124     
========================================
+ Hits       243628   244138   +510     
+ Misses      39185    39164    -21     
- Partials    17966    18074   +108

Flag	Coverage Δ
actions	`81.00% <80.80%> (+<0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
src/Opc.Ua.Types/BuiltIn/DiagnosticInfo.cs	`94.52% <100.00%> (ø)`
src/Opc.Ua.Types/BuiltIn/MatrixOf.cs	`79.67% <100.00%> (+0.87%)`	⬆️
src/Opc.Ua.Types/BuiltIn/QualifiedName.cs	`99.27% <100.00%> (+0.01%)`	⬆️
src/Opc.Ua.Types/Encoders/EncodingLimits.cs	`100.00% <100.00%> (ø)`
src/Opc.Ua.Types/Encoders/Union.cs	`72.32% <100.00%> (+1.52%)`	⬆️
src/Opc.Ua.Types/Nodes/NodeSet.cs	`74.17% <100.00%> (ø)`
src/Opc.Ua.Types/Nodes/NodeTable.cs	`90.56% <100.00%> (+2.83%)`	⬆️
src/Opc.Ua.Types/Nodes/ReferenceNode.cs	`89.88% <ø> (-0.12%)`	⬇️
src/Opc.Ua.Types/Nodes/VariableNode.cs	`75.17% <100.00%> (+3.54%)`	⬆️
src/Opc.Ua.Types/Schema/NodeSetComparer.cs	`92.85% <100.00%> (+0.42%)`	⬆️
... and 44 more

... and 38 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@context

CI build failures, in this PR's own new test files and only on the .NET Framework targets, which the net10.0 builds never compiled: - A range indexer over an array needs RuntimeHelpers.GetSubArray, which .NET Framework does not have. - SHA256.HashData is .NET 5 and later only. Two AOT test failures, where the TUnit suite carries a second copy of an assertion that pins behaviour this PR deliberately changed: - Clone is a full copy and now carries AccessRestrictions; handing out a copy less restricted than the node it was made from was the defect. - A bitwise operator whose operand does not resolve to an integer now yields null rather than a zero that cannot be told from a real result (OPC 10000-4 7.7.3). Neither spelling ever exposed the packed payload's raw bits, which is what the test is named for. Review findings: - XmlEncoder.WriteXmlElement parses the body once and writes the parsed element. An XML declaration parses, so the validation accepted it, and WriteRaw then put it in the middle of the document where it is illegal. - VirtualFileSystem.SetLength raises the high water mark before the equal length early return, so asking for the length the file already has no longer truncates it to nothing on dispose. - MapJsonSchemaToDataType only returns an inferred DataType identity when the caller asks for it. Inference walks the properties and runs after all mapping, so an action argument or event field naming a custom uav:dataTypeName was typed against a Node that is never written. - The string.Replace polyfill throws for a null or empty oldValue, as the framework overload and its own ordinal branch already do, instead of silently returning the target for a culture sensitive comparison. - The EventType reference chain's cycle detector keys on the document a reference is read in as well as its text. The same text resolves to different definitions under different @context mappings, so a chain through two documents was rejected as cyclic. - Component reconciliation compares the normalized reference type as well as direction and target, so a forward HasComponent no longer satisfies an inverse HasProperty and leaves that relation one directional. - Removed a spliced doc comment that left AddOwnedComponent undocumented. Not addressed: Assembly.GetCallingAssembly in TelemetryContextBase still resolves to Opc.Ua.Types for the call sites that reach it through the TelemetryExtensions helpers, which live in the same assembly, and the answer depends on whether those helpers are inlined. Fixing it means passing the assembly in from the call site, which changes the public ITelemetryContext surface, so it is left for a separate decision. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

.NET Framework's Double.GetHashCode folds the two zeroes together but hashes the raw bits of a NaN, while Equals is true for any two NaNs. So a variant holding a NaN broke the Equals/GetHashCode contract on the .NET Framework targets - a Dictionary keyed on one could fail to find a key equal to the one being looked up - even though .NET Core 3.0 and later normalize the hash themselves and the net10.0 tests therefore passed. The regression test only covered Double, so Float is now covered too; the same defect applied there and nothing would have caught it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Copilot

🟡 Changes recommended

Four confirmed defects remain in foundational comparison, hashing, attribute-write, and pattern-matching paths.

Get a fresh assessment by requesting another Copilot review.

Review details

Files reviewed: 77/77 changed files
Comments generated: 4
Review effort level: Balanced

Three review findings, all confirmed by a test that fails without the fix: - Variant.CompareTo still equated the null variant with every zero valued scalar. It substituted the typed side's type info and then read the null variant's zeroed union storage, so Null.CompareTo(Variant(0)) answered 0 in both directions while Equals answered false, and a SortedSet or SortedDictionary collapsed the two into one entry. CompareTo now decides the null case the way Equals does, including reporting equality for a typed variant whose reference payload is absent, so the two agree. - Node.Write rejected every DataTypeDefinition write with BadTypeMismatch. The gate resolved the value's built-in type by way of its data type id, and for a structured value that id is the concrete structure rather than Structure itself, which is not a built-in type at all. The value's own built-in type is now read directly, which also removes the round trip that needed a message context it never had. - CoreUtils.Match accepted an unterminated character set, and a set whose closing bracket ended the pattern matched every character rather than only its members: the scan loop ended on its bounds check instead of through the "end of set, no match" path, and the switch then carried on as though the set had matched. A set now has to be closed, and a match has to be recorded for the set to count. Not changed, having been measured rather than assumed: Variant.Equals does not convert between numeric built-in types - Int32/Float, Int32/Double, Int32/Int64, SByte/Int32, Int16/Int32 and UInt32/Int32 all compare unequal - so the differing hashes break no contract. IsConvertible pairs only Int32 with Enumeration and ByteString with byte[], and both hash alike. Cross numeric equality belongs to ValueEquals, which serves the ContentFilter operators of OPC 10000-4 7.7.3 and is deliberately not the hash contract. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

romanett

LGTM

# Description **Server/core audit fixes with regression tests. Latest review-fix validation, merge validation and earlier acceptance results are distinguished below.** This addresses a 95-finding server/core report. Every finding was reviewed with the OPC UA specification knowledge base or, where the standard does not prescribe a C# implementation, the corresponding ownership/configuration contract. Incorrect or withdrawn subclaims remain qualified rather than being presented as additional fixes. The original checkpoint covers session identity/certificate validation and revocation; certificate/key ownership and transactional updates; TCP reconnect, receive/admission and buffer lifetime; HTTP quotas; subscription admission, durability and pooled payloads; file-operation lifecycle; historian aggregates; fluent/state-machine behavior; and related utilities. It includes targeted regressions and directly related documentation. The approved acceptance cleanup is separate in the commit history: | Cleanup | Verified result | | --- | --- | | Invalid identity-policy and KeyCredential fixture inputs | Fixtures explicitly advertise Username and use its generated policy ID. Credential persistence uses valid clear mode inside the existing encrypted admin context. Original permission, lockout and negative encryption assertions remain. | | Native GDS lifetime and concurrent connection failures | Retain ApplicationInstance until client teardown. A deterministic native reproducer proved the test-only channel cap of 10 was exhausted; removing that override restores the existing bounded default of 1000. No global serialization or production quota change. | | Stress certificate ownership and close accounting | Balance two helper references and the fake transport's server-certificate ownership. Replace the invalid event-count bound with exactly-once physical disposal, opens = closes, zero active entries and one logical entry lifetime. | | Participant deadlines | Use the injected TimeProvider with the existing cross-framework WaitAsync overload. Tests pin one tick before, exact expiry, terminal notification, recovery and timely completion. | | Reproducibility and platform assertions | Preserve canonical WoT bytes/hashes, explicit display-culture contracts and exact numeric-vector assertions. Accept Apple's CryptographicException subtype without weakening disposal balance. | | Missing user-signature proof | Reject null/empty proofs before OS crypto. Return BadUserSignatureInvalid per Part 4, 5.7.3.3. None-policy and valid-signature controls remain. A CI-only test nullable-context error was corrected without changing production behavior. | | Durable sample restore and shared test storage | Dequeue waits for actual batch residency instead of treating failed restore completion as success, and restored files close before deletion. Five deterministic regressions failed before the fix. The two fixtures that clear the shared batch directory are isolated; all original 3,000-item assertions remain unchanged. | ## Latest review follow-up Commit `828856850219518970e8e2f580b8f94b3c1fc748` fixes the deferred robotics disposal reported in the latest review. `RobotIntentNodeManager.DisposeAsync` now joins a shared controller-host drain and then explicitly awaits base node-manager disposal. A controller's bounded shutdown wait may defer its cleanup, but no longer lets the node-manager override finish early. Concurrent callers, including an asynchronous call after synchronous `Dispose`, cannot bypass the drain after the host list has been detached. Synchronous disposal remains nonblocking for deferred work. Deferred initialization failures are logged and retained, and base cleanup failures propagate to asynchronous callers. The existing controller-level bounded disposal contract is unchanged; the node-manager contract is documented in `docs/Robotics.md`. | Regression requirement | Evidence | | --- | --- | | Wait for deferred hosts; preserve synchronous compatibility and concurrent joins | `NodeManagerAsyncDisposalWaitsForDeferredHostsAsync`: four cases covering both entry-point orders and zero/positive host shutdown waits. All four failed before the fix. | | Wait through base cleanup and observe failures | `NodeManagerDisposalWaitsForBaseCleanupAndReportsItsFailureAsync`: four cases hold the actual base-resource semaphore, verify pending callers after the host finishes, then check success or the same propagated exception and exactly-once cleanup. | The focused `IntentHostingCoverageTests` and `IntentControllerHostTests.Dispose` selection passed **30/30 on net10.0 and 30/30 on net48**, with no failures or skips. The final net10 solution build passed with zero errors and only the four previously documented CA2249 warnings in unchanged fuzz-test code. No new warnings, analyzer suppressions, dependencies, or weakened assertions were added. The review thread received a commit-specific reply and was explicitly resolved. This verification is scoped to the robotics feedback; the earlier full Subscriptions-run limitation remains recorded in the merge section below. ## Documentation review follow-up (2026-09-14) Commit `f9a9686fcc5d3cfd19cbbc8c2d826e20f5c7e72f` addresses the six follow-up threads on documentation and implementation rationale. Each has an in-thread reply citing the commit and an explicit resolution. Multiline XML documentation now covers the declarations introduced or modified throughout the entire PR, including internal contracts, samples, tests, and public members of related private helpers. A syntax-aware inventory checked **2,031 declarations across all 256 PR C# paths with zero documentation gaps**. A supplementary check also covers **75 previously unchanged public members** of private helpers involved in the changed code. Token comparison across the conditional-compilation branches proves this follow-up changes **no executable C# or test assertions**. Project-file changes are rationale comments only. The implementation questions are documented at their source: | Question | Resolution | | --- | --- | | Certificate assembly friendship | Server Push configuration reuses the internal explicit-subject CSR helper with the existing key; no public crypto API was added. | | Core.Types assembly friendship | Server builders use internal transition callbacks/revision guards; Server.Tests checks the revision deterministically without creating a public compatibility contract. | | Always-single-reader notifications | One dequeuer preserves event order; optional concurrency fans out within one event and joins before the next event. | | Asynchronous disposal completion | `DisposeAsync` already joins the completion published after admitted operations and all cleanup attempts. The compatibility `Dispose` remains nonblocking to avoid deadlocking an admitted callback against its own drain. | The requested detailed review disposition follows. It distinguishes actual fixes from confirmed policies and incorrect subclaims rather than counting every comment as a new runtime change. ### Detailed disposition of review 5191795284 The [SDK review](#4477 (review)) was submitted against `1c4a9df6c`, before the subsequent merge and feedback fixes. Its **15 inline threads** have each received an action-specific reply and an explicit resolution. A fresh GraphQL query verified those replies and resolution states; the fixes in `b8a59990f` remain ancestors of the current branch. | Review concern | Action and disposition | Concrete regression evidence | | --- | --- | --- | | Injected certificate-store providers bypassed | **Fixed.** Registration, configuration snapshots, and validation use the injected providers. Explicit custom types remain authoritative; the built-in Directory catch-all is deferred so native X509 paths and later custom providers are not captured. Failed store opens dispose the created store. | `InjectedProviderResolvesRegisteredAndConfiguredTrustStores` covers default-provider ordering and both registration paths; provider validation/open-failure cases and the unchanged `TrustedCertificateInjectedViaIndependentX509StoreIsObservedAsync` cover real use and independent-store visibility. | | Unguarded timer-thread leadership callbacks | **Fixed.** Each subscriber's exception is logged independently and cannot suppress subsequent subscribers or fail lease operations. `IsLeader` reads the deadline without invoking application callbacks. | `ThrowingLeadershipSubscriberDoesNotFailAcquireOrDisposeAsync`, `ExpiryNotifiesOtherSubscribersWhenOneThrowsAsync`, and `IsLeaderDoesNotDispatchExpiryNotificationsAsync` were all demonstrated failing before the fix. | | Legacy pending-key provider accepts an unfinishable regenerated CSR | **Fixed by explicit capability validation.** A provider implementing only `IPendingCertificateKeyStore` receives `BadNotSupported` before key generation or persistence when regeneration is requested. Existing-key CSR/update remains supported. Regeneration requires matching-only consumption; no unsafe destructive probing was introduced. | `LegacyPendingProviderRejectsRegenerationBeforeCreatingOrSavingAKeyAsync` checks no key or store mutation; `LegacyPendingProviderCompletesExistingKeySigningAndUpdateAsync` proves the supported flow completes. | | Shutdown waits behind user deactivation | **Fixed.** Accepted deactivations drain before the master's lifecycle gate is taken. Configuration retains its binding until the shared drain completes, including concurrent shutdown callers and direct master disposal. | Four cases of `ShutdownDrainsDeactivationOutsideTheSessionClosingGateAsync` exercise real configuration/binding/ServerInternalData/master reentry, retained address space, exact session/subscription closure, and no production cancellation shortcut. | | Subclass cleanup runs under the operation-lifetime lock | **Fixed.** Cleanup starts outside that lock. A pre-published completion joins all disposers, and only the active asynchronous teardown context may use guarded helpers. Captured contexts lose that privilege when the callback exits. | `SynchronousOwnedCleanupLetsSamplingWorkerReachClosedAdmissionAsync`, `DisposeCoreCanUseGuardedMembersWithoutReopeningCallerAdmissionAsync`, captured-context rejection, and exact cleanup-failure/ownership assertions cover both synchronous and suspended cleanup. | | Discovery requestId-zero chunk read after pool return | **Fixed by the earlier merge.** Discovery validation precedes `SaveIntermediateChunk`; no read occurs after that path can return the buffer. The ordering is retained. | `DiscoveryChannelReadsTheFirstChunkBeforeItIsReturnedAsync` uses a poisoning pool and the requestId-zero path. | | T1.8 OPN cleanup misses post-decryption throws | **Fixed by the earlier merge and pinned with new regressions.** Both sequence rejection and certificate mismatch release the decrypted body and input ownership. | `OpenSecureChannelRejectionAfterDecryptionReturnsEveryRentalAsync` checks the exact audit status and pool conservation. Omitting either corresponding release in an isolated snapshot made its case fail; no mutation remains. | | T1.2 mount-root protection depends on the provider | **Fixed at the host boundary.** Separator-only aliases resolve to the protected root. Traversal, empty/whitespace segments, backslashes, and drive/stream qualifiers are rejected before arbitrary providers are invoked. Valid nested paths and root destinations remain supported. | A deliberately permissive provider is used by `NonCanonicalObjectPathsCannotDeleteMoveOrCopyAsync` and invalid-target cases; exact zero-call checks are paired with legitimate nested-path/root-destination controls. | | Inline trusted/issuer entries now face CRL checks | **Intentional strict behavior, documented and tested.** A configured backing store with strict unknown-status rejection requires current revocation information. Explicit-only lists without a store retain their no-store behavior. Actual revocation cannot be suppressed; a matching store entry's policy takes precedence. | `InlineIssuerWithStoreHonorsStrictUnknownRevocationPolicyAsync`, `InlineIssuerRevocationIsNeverSuppressedAsync`, store-precedence cases, and explicit-only controls preserve the missing-information versus actual-revocation distinction in Part 4 6.1.3. | | RSA minimum applied to unspecified ECC identifiers | **Fixed at the application-certificate check.** The RSA minimum is compared only with an actual RSA public key. An unspecified ECC identity is not replaced simply because its key size is below an RSA threshold. | `SilentCertificateCheckPreservesUnspecifiedEccIdentityAsync` checks DER, thumbprint, store contents, and private-key retention; explicit RSA and HTTPS minimum controls remain. | | Failed directory refresh wedges corrective mutations | **Fixed.** Recoverable automatic pre-refresh failures are logged and leave reconciliation pending, but do not block corrective provider mutations. Explicit refresh errors, capacity admission, caller cancellation, and actual provider denials remain enforced. | `CorrectiveDeleteRecoversAfterPersistentOverLimitRefreshFailureAsync` converges an externally over-limit directory back to its exact allowed contents. Capacity, cancellation/token, and six refresh-error controls check negative paths. | | Timed-transition rejection logs at 10 Hz | **Fixed.** Repeated `(state, revision, status)` warnings are deduplicated without changing the retry cadence. A different status or state entry permits another warning. | `RepeatedTimedRejectionLogsOnceAndRecoversAsync` checks one warning over eleven rejections and success on the next attempt. Status-change and same/different-state reentry controls pin reset behavior. | | Bad unsubscribe strands retired-generation bookkeeping | **Fixed.** Retired all-event bookkeeping completes even when unsubscribe reports Bad; the first Bad is still returned to the caller. | Good and BadCommunicationError cases of `RetiredAllEventsDeletionCompletesBookkeepingAndReleasesGenerationAsync` check event/snapshot removal, generation release, and exactly one unsubscribe without finalization replay. | | CSR subject parsing and fallback | **Fixed for existing and regenerated keys.** Omitted subjects fall back to the actual stored certificate. Regeneration resolves the public certificate's subject before creating a key. Malformed or unresolvable subjects return `BadInvalidArgument` before pending-key mutation. | Store-only fallback, real-generator null/empty cases, and missing-subject/malformed-DN controls verify the signed CSR, matching fresh pending key, unchanged active identity, and no destructive side effects. | | Committed RemoveUser change reported as failed | **Fixed for RemoveUser and ModifyUser.** Accepted cleanup is still awaited and close failures remain logged, but cannot replace the committed mutation result. Precommit failures retain their original result and side effects. | `CommittedUserChangeKeepsResultAndPublishedUsersWhenCloseFailsAsync` checks published Users, Good result, logging, and continued closes. `FailedUserChangeKeepsPublishedUsersAndDoesNotCloseSessionsAsync` checks BadNotFound with no publication or close. | | Alleged pooled decoded-request aliasing | **No runtime redesign needed; reviewer rebuttal confirmed.** The decoder copies the relevant input data. Misleading ownership comments were corrected rather than introducing a new dispatch model. | `DecodedRequestRetainsValuesAfterInputBuffersAreReturnedAsync` checks the exact decoded NodeId and ByteString after returned arrays are poisoned. | The current documentation-only follow-up rechecked **104 Core, 473 Server, and 4 Configuration cases** covering these concerns: **581 passed, zero failed or skipped**. The final net10 solution build also passed with **zero warnings and errors**. The complete earlier Core/Server/Configuration and net48 results remain listed separately rather than being presented as rerun current-head acceptance. ## Earlier review follow-up Commit `b8a59990fe2e8e967a12ffd5d9158beed7831143` addresses all 19 review threads. Each thread has an in-thread reply naming this commit and was explicitly resolved after verification; the final paginated query found 19 resolved and zero unresolved threads. The follow-up fixes injected certificate-store selection (including default provider ordering and native X509 paths), actual-RSA minimum enforcement, and CSR capability/subject validation before key mutation. Node-manager teardown uses simpler scoped ownership and drains accepted user revocation outside the master gate; committed user results and retired-event cleanup remain correct when cleanup reports failure. File mutations can recover after failed refresh without bypassing capacity or cancellation, provider paths protect mount roots, and repeated timed rejections no longer flood logs. Leadership subscriber failures are isolated without callbacks from `IsLeader`. Regressions also pin the already-merged OPN/discovery buffer fixes and prove decoded request values do not borrow returned input buffers. The strict inline issuer revocation policy is documented and tested, not weakened. Existing tests were changed only for the reviewed behavior: the incorrect committed-user result was corrected, and inline-issuer/root-alias controls were expanded. ### Review-fix validation | Suite | Scope | Framework | Passed | Existing TRX NotExecuted | | --- | --- | --- | ---: | ---: | | Core | Complete project | net10.0 | 4,970 | 88 | | Server | Complete project | net10.0 | 6,414 | 69 | | Configuration | Complete project | net10.0 | 231 | 0 | | Core | Certificate, lease and buffer compatibility slice | net48 | 153 | 0 | | Server | Reviewed lifecycle, user, file, timer and CSR compatibility slice | net48 | 692 | 0 | | Configuration | Complete project | net48 | 231 | 0 | | CSR provider regressions | Final portable-mock follow-up | net10.0 | 11 | 0 | All runs exited successfully with normal concurrency and leak detection. The Server console reports 5 skips; its TRX additionally records existing assumption/explicit cases, hence the 69-record count above. The Framework build caught two modern-only `ValueTask.CompletedTask` usages in new mock setup; only those expressions changed to the equivalent portable `default(ValueTask)`. The complete modern suites precede that test-only correction; all production files remained byte-identical, and the corrected fixture passed on both targets. The final net10 solution build succeeded with 0 errors and the same 4 CA2249 warnings in unchanged fuzz-test code. Diff checks and changed-line whitespace checks are clean; whole-file formatting still identifies 10 unchanged baseline lines, which were not reformatted outside the feedback scope. The committed 35-file tree was hash-verified against the validation inputs. This is the review-feedback verification scope, not a rerun or sign-off of the historical complete platform/native/soak acceptance campaign below. ## Latest upstream merge Merge `9d9bf679f317164c5b2b90924cd3cdc27a86d451` integrates `master` through `162fc5ebb` (#4486), including the AddNodes child-name index, shared certificate-error reporting, CTT sample-data corrections, and the accompanying regressions and documentation. The CreateSession conflict preserves this branch's certificate ownership, required certificate, application-URI, and channel-certificate checks. The validation result now retains nested errors through `ThrowIfInvalid`, allowing the upstream shared reporting helper to mask trust, policy, and revocation failures consistently without losing audit detail. The new indexed AddNodes regression initially attempted to reuse a deterministic NodeId after renaming its existing node. Renaming frees the BrowseName, not that NodeId. `IndexedBrowseNameReusePreservesExistingNodeIdentityAsync` now proves both contracts: the derived-ID collision leaves the original node intact, then a fresh explicitly requested ID can reuse the old name while the new name remains protected against duplicates. No production identity rule was relaxed. The incoming seeded random-index test uses the existing `UnsecureRandom` wrapper with the same seed and sequence; no new analyzer suppression was added. | Local merge validation | Framework | Passed | Existing exclusions | | --- | --- | ---: | ---: | | Core, complete project | net10.0 | 5,081 | 88 | | NodeState / BaseInstanceState | net10.0 | 579 | 0 | | Server node management, certificate and session-security selection | net10.0 | 112 | 0 | | InformationModel node-management services | net10.0 | 29 | 1 | | NodeState / BaseInstanceState | net48 | 578 | 0 | | Core certificate validation, error reporting and TCP buffers | net48 | 111 | 0 | | Server node management, certificate and session-security selection | net48 | 112 | 0 | All selected runs exited successfully at normal concurrency: **6,602 passing test executions**. Modified test assemblies were explicitly rebuilt, and the combined name/identity regression was verified by name in both net10 and net48 results. No skips or weakened assertions were introduced. The final `dotnet build UA.slnx -c Release --no-restore -m:1` with `CustomTestTarget=net10.0` succeeded with **zero warnings and zero errors**. The resolved source and adjusted tests pass whitespace verification, and their source hashes match the validation inputs. This is merge-specific verification. Earlier review results and the previously reported incomplete full Subscriptions runs below remain scoped to their own commits; they were not reopened or relabeled as passing by this operation. ## Previous upstream merge (2026-09-15 morning) Merge `c4694e20ab089c555af233642042133ff3a03ac8` integrates `master` through `484d6c887`, retaining #4481's shared Like parser/GDS query fixes, #4488's RequestHandle recovery in ServiceFaults, and #4487's expired-session and durable-transfer corrections. Three conflict resolutions preserve both sides: - Alias searches now use the shared `LikePattern` implementation, including upstream malformed-list validation. A timeout overload retains this branch's 100 ms per-alias deadline and `BadTimeout` reporting without keeping a second wildcard translator. The shared matcher's original unlimited overload remains unchanged for existing GDS/ContentFilter callers. New regressions cover bounded prefix/list scans, reuse after timeout, and the alias service result. - Expired activation closes outside the session-manager gate using upstream's once-only timeout claim, audit, and diagnostic accounting. This branch's original-channel and security-policy checks remain in place. - TCP buffer tests retain this branch's ownership, poisoning-pool and post-decryption checks alongside the upstream RequestHandle/fault overload tests; neither set was discarded. The upstream HTTPS binary behavior is intentionally retained: a decoder `ServiceResultException` produces HTTP 200 with an OPC UA ServiceFault carrying the decoder status, response timestamp, and recoverable RequestHandle, rather than the previous HTTP 500 text response. Other exceptions retain their existing HTTP error handling. | Local validation | Framework | Passed | TRX NotExecuted | | --- | --- | ---: | ---: | | Core, complete project | net10.0 | 5,054 | 88 | | Sessions, complete project | net10.0 | 780 | 610 | | Server aliases, expiry/security and subscriptions | net10.0 | 288 | 0 | | GDS database, paging and live directory calls | net10.0 | 263 | 0 | | Shared Like patterns and bounded evaluation | net10.0 | 109 | 0 | | Subscriptions transfer/durability/service selection | net10.0 | 23 | 1 | | HTTPS WebApi, complete project | net10.0 | 346 | 0 | | InformationModel aliases | net10.0 | 54 | 5 | | Shared Like patterns and bounded evaluation | net48 | 109 | 0 | | Core fault handles, TCP/HTTPS and filters | net48 | 268 | 0 | | Server aliases, expiry/security and subscriptions | net48 | 288 | 0 | These selected runs exited successfully with normal test concurrency: **7,582 passing test executions**. Sessions' console reports 379 skips; the TRX also contains conditional/assumption exclusions, so the table uses the actual 610 NotExecuted records. No skips or weakened assertions were added. The new `EvaluationTimeoutBoundsWorkAndPreservesPatternReuse` and `SharedPatternTimeoutRemainsAnAliasServiceError` tests passed on both targets. **Validation limitation:** two unfiltered Subscriptions runs did not produce a final result and were stopped, including a retry with a hang guard. They are not counted as passing. The isolated failover fixture passed 2/2, and the transfer/durability/service selection above passed. The stall's cause was not established; no unrelated code or test was changed to conceal it. The final `dotnet build UA.slnx -c Release --no-restore -m:1` with `CustomTestTarget=net10.0` succeeded with **0 warnings and 0 errors**. Whitespace verification passed for all hand-reconciled files, and staged source hashes match the validation inputs. These are merge-specific checks, not a new full platform/native/soak acceptance sign-off. Earlier review and acceptance evidence below remains dated to its own commit. ## Previous upstream merge (2026-09-14) Merge `c3e09553e4d1509502d4ba751960d58491bea2cf` integrates `master` through `6313e38a0` (#4478), including the CTT-driven Part 13 aggregate corrections and FindAlias argument validation. The upstream tests and CTT documentation are retained. The single conflict in `AliasNameWildcardMatcher` is reconciled through the branch's shared regex compiler: upstream malformed-pattern rejection and `IsValidPattern` coexist with OPC UA character escaping, whole-string/newline matching, the 100 ms timeout, and service-level error reporting. The upstream ReferenceTypeFilter checks and chronological aggregate behavior are preserved. Local merge verification: | Suite / selection | Framework | Passed | Existing skips | | --- | --- | ---: | ---: | | Server aliases, aggregates and historian | net10.0 | 1,011 | 0 | | Server aliases, aggregates and historian | net48 | 1,011 | 0 | | InformationModel aliases | net10.0 | 54 | 5 | | Features aliases | net10.0 | 11 | 0 | | Client aliases | net10.0 | 28 | 0 | | History aggregate/historical-access consumers | net10.0 | 265 | 0 | All selected tests passed at normal concurrency. The final net10 solution build passed with 0 errors and 0 warnings. The resolved file's whitespace check and the staged diff check passed. These are merge-specific checks, not a new full platform/native/soak acceptance claim. ## Previous upstream merges Integrates `master` through `8e874d073`, including the Client, Types, Core and source-generator audit fixes in #4471, #4473, #4474 and #4475, plus the event queue-size revision from #4480. The merge commits are `150ea0439` (26 conflict resolutions) and `a0e17c07c` (the base update that arrived during validation). The overlapping fixes are reconciled rather than replacing either branch: - **Rejected-certificate retention follows this branch, as explicitly requested:** zero means unlimited history; negative values disable new storage while preserving existing backend trimming behavior (Directory clears prior history; shared key/value leaves it intact). The processor and manager policy are not redefined, and documentation/tests describe the distinction. - Preserve owned certificate snapshots and borrowed validation-core lifetimes alongside upstream cache invalidation, store-first CRL checks and `BadCertificateRevocationUnknown` for unsupported revocation lookup. - Combine exactly-once chunk ownership, receive-loop cancellation, stale-epoch rejection and detach-before-adoption with upstream signature-before-padding verification and secret cleanup. - Preserve pooled `HttpClient` per-request configuration together with bounded response reads and deadlines that remain active through body consumption. Regression cases exercise already-used clients and both timeout precedence cases. - Retain the binary-compatible channel-key constructor and normalize omitted profiles; preserve exact profile filtering and upstream binary-first HTTPS/WSS endpoint ordering. - Preserve monotonic lease expiry and stale-confirmation fencing while adding ordered, non-reentrant leadership notifications outside the state lock. ## Validation Local validation of the conflict-resolution merge: | Suite | Framework | Passed | Existing skips | | --- | --- | ---: | ---: | | Core | net10.0 | 4,940 | 88 | | Client | net10.0 | 2,278 | 0 | | Server | net10.0 | 6,088 | 5 | | Types | net10.0 | 10,394 | 1 | | SourceGeneration.Core | net10.0 | 3,940 | 8 | | SourceGeneration | net10.0 | 185 | 0 | | Core | net48 | 4,855 | 104 | After the subsequent four-file #4480 update, the related Server slice passed 811 tests with 5 existing skips, the real durable queue-size cases passed 6/6, and EventManager's 21 cases passed again after analyzer-only expected-array cleanup. The final `dotnet build UA.slnx -c Release` with `CustomTestTarget=net10.0` succeeded: 0 errors and 4 pre-existing CA2249 warnings in unchanged fuzz-test code. Whitespace verification passed for the resolved files and the follow-up. Merge-specific proof includes `RequestTimeoutRemainsActiveWhileReadingTheOpenApiBodyAsync` on already-used shared clients with either the request or HttpClient timeout shorter, the existing ownership/expiry regressions from both branches, and both branches' retention tests adjusted only to the explicitly chosen zero/negative contract. Earlier acceptance at `1c4a9df6c` passed hosted Linux/macOS CI, CodeQL, image builds and the canonical coverage gate (87.15% line, 77.27% branch, 92.14% patch). Both Framework project matrices, all three Windows native suites (183 cases), and the unfiltered net10 stress project (230 cases, including full 60-minute chaos and 30-minute memory soaks) also passed. Those are **pre-merge results**, not a claim that the full framework/platform/native/soak matrix has been rerun on this merge. The previous acceptance workflow remains separate from this review-feedback operation. Security checks, meaningful assertions, leak detection, default native concurrency, full soak durations and coverage thresholds remain intact. The user-set ready-for-review state is unchanged. No architectural or complete merged-head acceptance sign-off is claimed. ## Related Issues - Fixes #4476 ## Checklist - [ ] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. - [ ] I fixed **all** failing and flaky tests in the CI pipelines and **all** CodeQL warnings. - [x] I have addressed **all** PR feedback received.

romanett requested a balanced review from Copilot September 11, 2026 14:48

Copilot started reviewing on behalf of romanett September 11, 2026 14:48

View session

Copilot AI reviewed Sep 11, 2026

View reviewed changes

romanett and others added 2 commits September 11, 2026 18:30

marcschier approved these changes Sep 12, 2026

View reviewed changes

marcschier marked this pull request as ready for review September 12, 2026 09:15

Merge branch 'master' into romanett/opc-ua-types-bugs-6c0393

ed2e0e2

marcschier requested a balanced review from Copilot September 12, 2026 09:15

Copilot started reviewing on behalf of marcschier September 12, 2026 09:16

View session

Copilot AI reviewed Sep 12, 2026

View reviewed changes

Comment thread src/Opc.Ua.Types/BuiltIn/Variant.cs

Comment thread src/Opc.Ua.Types/BuiltIn/Variant.cs Outdated

Comment thread src/Opc.Ua.Types/Nodes/Node.cs

Comment thread src/Opc.Ua.Types/Utils/CoreUtils.cs

marcschier added the ready Ready to merge once CI Passes label Sep 12, 2026

romanett commented Sep 13, 2026

View reviewed changes

romanett mentioned this pull request Sep 13, 2026

[CTT] Reference server: close sample address-space gaps and document project settings for tests the CTT skips #4479

Closed

12 tasks

marcschier merged commit 5362ec5 into master Sep 14, 2026
278 of 279 checks passed

marcschier mentioned this pull request Sep 14, 2026

Fix server/core validation, lifecycle and protocol regressions #4477

Merged

4 of 7 tasks

Copilot AI mentioned this pull request Sep 15, 2026

Fix telemetry component assembly identification #4490

Merged

4 of 7 tasks

marcschier deleted the romanett/opc-ua-types-bugs-6c0393 branch September 29, 2026 09:32

Navigation Menu

Repository navigation

Fix the Opc.Ua.Types defects found by the read-only audit - #4474

Fix the Opc.Ua.Types defects found by the read-only audit#4474
marcschier merged 5 commits into
masterfrom
romanett/opc-ua-types-bugs-6c0393

romanett commented Sep 11, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Sep 11, 2026 •

edited

Loading

Uh oh!

codecov Bot commented Sep 11, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

romanett left a comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

romanett commented Sep 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

The higher-impact ones

Reviewer notes

Related Issues

Checklist

Notes on the checklist

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Sep 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code coverage

Uh oh!

codecov Bot commented Sep 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

romanett left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

romanett commented Sep 11, 2026 •

edited

Loading

github-actions Bot commented Sep 11, 2026 •

edited

Loading

codecov Bot commented Sep 11, 2026 •

edited

Loading