Commit Graph
8 Commits
Author SHA1 Message Date
romanettandClaude Opus 5.5 9b034f8291 Enable nullable reference types centrally; migrate all unset and annotations-only projects (#4700)
# Description

Nullable reference types are now enabled once, in `common.props`, and
every project in the repository builds warning-clean with them.

**Configuration**
- `<Nullable>enable</Nullable>` lives in `common.props`. All per-project
`<Nullable>` settings are removed: 178 project files (enable,
annotations, and the `disable` in the three Aot test projects).
- 419 per-file `#nullable enable`/`restore` directives and the 4
`#nullable disable` directives are removed.
- Kept on purpose:
- The 5 `<auto-generated>` schema files (`UANodeSet.cs`, `UA Type
Dictionary.cs`, `OPCBinarySchema.cs`, `SecuredApplication.cs`,
`PInvokeHelper.cs`) keep `#nullable enable`. The compiler ignores the
project setting in generated code, so the directive is required there.
- `tests/Opc.Ua.Interop.LegacyPeer` keeps `disable`. It builds against
the 1.5.378 packages and does not import the repository props.
- `docs/DeveloperGuide.md` and `.github/copilot-instructions.md` now
describe the central setting.

**Migration** (the 43 unset and 13 annotations-only projects, roughly
10k diagnostics)
- **Production code: annotated at the declarations.** Covers
`tools/Opc.Ua.SourceGeneration.Core`, `src/Opc.Ua.Lds.Server` and the
LDS sample.
- Members, parameters and returns are nullable where null actually
flows.
- Null-check helpers carry `[NotNullWhen]` (`IsNull`, `IsValidXmlName`,
`IsMethodTypeDesign`).
- `CoreUtils.Format`, `Utils.Format`, `ITemplateWriter.WriteLine` take
`params object?[]`. `Template.AddReplacement` takes `string?` (rendering
already skips null replacements).
- `IRegisteredServerStore.RegisterAsync` returns
`Task<RegistrationEntry?>`: it returns null when a registration is
removed.
- Dereferences backed by an invariant (for example, the validated model)
use `!`, so runtime behaviour is unchanged.
- **Behavioural changes in production code** (small, deliberate):
- `NodeSetToModelDesign` throws `InvalidOperationException` when the
file is not a NodeSet. Before, it threw a NullReferenceException on the
next line.
- `ModelDesignValidator` starts its lookup tables empty instead of null.
A redundant `if (m_nodes == null) return;` guard is gone.
- **Test code: minimal annotations.**
  - `?` on values that can be null, `!` where the test asserts non-null.
  - `= null!` for fixture members assigned in SetUp.
- `TryGetStructure<T>(out T? x)` where the inferred `T?` broke the
`IEncodeable` constraint.
- A few redundant `== null` checks on structs (`NodeId`, `ArrayOf<T>`)
folded into the adjacent `IsNull`/`Count` checks.

**Notes for reviewers**
- The diff is large (~1,500 files) but almost entirely mechanical: `?`,
`!` and directive removal. The non-annotation changes are listed above.
- Known follow-up, not done here:
`IModelDesign.Namespaces/TargetNamespace/NamespaceUris` stay
non-nullable even though the validator returns null before it loads a
dictionary. Making them nullable would ripple through every generator
call site.

## Related Issues

- Fixes #4439

## 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.
- [ ] 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.
- [ ] I have addressed **all** PR feedback received.

The "no new warnings" box covers net10.0 and net48: full non-incremental
builds of `UA.slnx`, `tools/SourceGeneration.slnx`,
`tools/MigrationAnalyzer.slnx` and the Aot test projects finished with 0
warnings and 0 errors under the repository's warnings-as-errors
settings. net9.0 and net8.0 also had no nullable diagnostics, but they
were not rebuilt after the final fixes. Tests have not been run locally
yet.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-08 08:48:13 +02:00
romanettandClaude Opus 5.5 61edaa1ed0 Remove netstandard2.1 and net472 target frameworks (#4624)
# Description

Drops support for **netstandard2.1** and **.NET Framework 4.7.2**.
Libraries now target `net48`, `net8.0`, `net9.0` and `net10.0`.
`Opc.Ua.Types` and `Opc.Ua.SourceGeneration.Core` still also target
`netstandard2.0`, but only because the Roslyn source generators load
them. There is no general netstandard2.0 support.

**Preprocessor directives** (112 files)
- Every `#if`/`#elif` mentioning `NETSTANDARD2_1*`, `NET472*`,
`NET_STANDARD_TESTS`, `SKIP_ECC_CERTIFICATE_REQUEST_SIGNING` or
`NETSTANDARD*` was evaluated against the remaining TFM set (net48,
net8/9/10, plus netstandard2.0 inside `Opc.Ua.Types`/`tools`) and
simplified. Dead branches were removed.
- Example: `NETSTANDARD2_1 || NET8_0_OR_GREATER` → `NET8_0_OR_GREATER`.
- Example: `NET8_0_OR_GREATER || NET472_OR_GREATER` → removed (always
true).
- Netstandard-only code was removed: `UnixFileModeShim`, the CRDT
reflection path in `ReplicatedGossipOptions.CreateReaderOptions`, and
the netstandard2.1-only test variants.

**Custom test logic**
- `targets.props` drops the `net472`, `netstandard2.0` and
`netstandard2.1` `CustomTestTarget` profiles, `NetStandardTests` and
`SKIP_ECC_CERTIFICATE_REQUEST_SIGNING`. Supported values are now
`net48`, `net8.0`, `net9.0`, `net10.0`.
- The `NET_STANDARD_TESTS` define is removed from 29 test projects.
- The "build netstandard shells as net10.0" overrides are removed from
18 projects.
- `RestrictForLegacyTfm` now only applies to `net48`.
- `tests/customtest.bat` is updated to the new values.

**Build / packaging / CI**
- MSBuild TFM conditions are simplified: Kestrel package choice, MQTTnet
v4 (net48 only), BouncyCastle, Di/OpenUsd/Vision/Stress test excludes,
and `System.IO.FileSystem.AccessControl` (dropped from Core).
- The MigrationAnalyzer nuspec drops its net472/netstandard2.1 lib
groups. The Azure signing lists are updated.
- `get-ci-matrix.ps1` removes the net472/netstandard test profiles
(including the PR-gating `linux-netstandard2.1`) and the matching build
legs.

**Breaking API change (2.0 preview):** removed
`HttpsTransportListener.IsWssTransportSupported` and
`ReplicatedGossipOptions.IsGossipStateDecodingSupported`. They only
returned `false` on netstandard2.1 and are now always `true`. The test
guards that used them were removed as well.

**Docs**
- NuGet READMEs, `README.md`, `DeveloperGuide.md`, `PubSub.md`, the
certificate/ECC/identity docs and the 2.0 migration guide (plus its
`.agents` skill mirror) are updated.
- The migration guide notes that 4.7.2 consumers must move to 4.8 and
netstandard2.1 consumers to net8.0+.

### Reviewer notes
- The `System.IO.FileSystem.AccessControl` `PackageVersion` pin is left
in `Directory.Packages.props`. It has no direct reference any more, but
transitive pinning is on, so it was kept to avoid shifting resolved
versions.
- The `#if` rewrite was done mechanically by a truth-table evaluator.
The diff in the `.cs` files is limited to directive lines, removed dead
branches, and stale-comment updates.

### Verified locally
- `dotnet build UA.slnx -p:CustomTestTarget=net48`: 0 errors.
- `dotnet build UA.slnx -p:CustomTestTarget=net10.0`: 0 errors.
- Default multi-target builds (no `CustomTestTarget`) of Opc.Ua.Types,
Bindings.Https, PubSub.Mqtt and Redundancy.Server succeeded.
- net10.0 tests:
  - Opc.Ua.Redundancy.Server.Tests: 840/840 passed.
  - Opc.Ua.Types.Tests: 11363 passed, 1 skipped.
  - ModelDependencyScanner tests: 20/20 passed.
- The full test suite was **not** run locally.

## Related Issues

- None

## 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.
- [ ] 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.
- [ ] 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.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-02 17:09:49 +02:00
20c2d77d57 Fix Core/Types audit findings and conform them to the OPC UA spec (stacked on #4569) (#4573)
> **Stacked on #4569.** The base branch is
`romanett/practical-murdock-fd9c56`, so this diff shows only the commits
of this PR. Retarget to `master` after #4569 merges.

## Summary

Bug audit of `Opc.Ua.Core`, `Opc.Ua.Core.Types`, `Opc.Ua.Types` and
`Opc.Ua.Core.Schema`, with the findings fixed and the fixes grounded
against the OPC UA specification (reference.opcfoundation.org). 133
commits in four groups:

1. **Audit fixes** (`Fix <ID>: ...`): 116 findings from a read-only
audit, each with a regression test. IDs follow the audit scopes: T1-T6
`Opc.Ua.Types`, C1-C4 `Opc.Ua.Core`, S1 `Opc.Ua.Core.Types` +
`Opc.Ua.Core.Schema`.
2. **Review fixes** (`Fix review <ID>: ...`): issues a code review of
group 1 found, plus lower-severity leftovers.
3. **Spec fixes** (`Spec: #<n> ...`): the 18 places where an earlier fix
only partly matched the spec text.
4. Test and analyzer follow-ups.

Where `master` (#4516, #4522, #4540, #4564) or #4569 already fixed the
same issue, their version was kept and this PR only adds what they
didn't cover.

## Behaviour and user-facing changes

### Wire format and interoperability (breaking)
- **PubSub AES-CTR (Aes128-CTR / Aes256-CTR)** now follows Part 14
Tables 156/157:
- The counter block is `KeyNonce[4] | MessageNonce[8] |
BlockCounter[4]`, and the block counter starts at 1.
- The SecurityHeader nonce is 8 bytes: `Random[4] | UInt32
SequenceNumber`, with the sequence number starting at 1.
  - SKS key data is split with a 4-byte KeyNonce, giving 52 / 68 bytes.
- **Not interoperable with earlier releases of this stack**, but matches
the spec. There is a test against the independent RFC 3686 vector.
- **User token signature** (Part 4 6.1.8):
- The signed data is now built from the user token policy's
SecurityPolicy on both client and server.
- It also supports the SecurityMode None layout `ServerNonce |
HASH(ServerCertificate) | ClientNonce`.
- New overload `SecurityPolicyInfo.GetUserTokenSignatureData(...,
MessageSecurityMode)`.
- **Secure channel:**
- The ECC AEAD `LastSequenceNumber` is correct across the 32-bit
sequence wrap (Part 6 6.8.1).
  - The incoming chunk counts against MaxChunkCount/MaxMessageSize.
- The sender no longer lets a message through that is 24 bytes over
MaxMessageSize.
- The OPN response signature is verified without overrunning the buffer
(RSA_DH with differing key sizes).

### Sessions (server)
- **CreateSession nonce:**
- The client nonce is kept on None channels, because the None-mode user
token signature covers it.
- A nonce over 128 bytes returns `Bad_NonceInvalid` on every channel
(Part 4 5.7.2.3).
  - The duplicate-nonce check runs only on secured channels.
- A non-empty nonce under 32 bytes is still accepted on None, for
compatibility.
- **ActivateSession:** an embedded `EccEncryptedSecret` signing
certificate is validated with the server's peer certificate validator
(Part 6 6.8.3). Byte equality with the client certificate is only the
fallback when no validator is available. New opt-in interface
`ICertificateValidatorProvider`.
- **FindServers / GetEndpoints:** `GatewayServerUri` and
`DiscoveryProfileUri` are no longer swapped.

### Write, Read and Call (NodeState / node managers)
- **UserWriteMask:**
- Enforced for non-value attributes and for VariableType Value writes
(`ValueForVariableType`), returning `Bad_UserAccessDenied`.
- An unconfigured UserWriteMask (0, no handler) is not enforced, and
reads back as the WriteMask so Read and Write agree (Part 3 8.60).
- A throwing or Bad `OnReadUserWriteMask` handler fails only that item.
- **Index-range writes:**
- Slices are merged into the cached value under the attribute lock, also
for `OnWriteValueAsync` handlers.
- A data-change notification is still produced when the cache cannot
absorb the slice.
- The slice must match the unclipped range, otherwise
`Bad_IndexRangeDataMismatch` (new status code in `Opc.Ua.Types`).
  - A range past the target returns `Bad_IndexRangeNoData`.
- A handler without index-range support returns `Bad_WriteNotSupported`
(Part 4 7.27, 5.11.4.4).
- **Index-range reads on matrices** return partial results clipped to
the source, instead of allocating the requested size (a Read with
`"0:40000,0:40000"` no longer allocates about 1.6e9 elements).
- **DataTypeDefinition writes:**
- A value that isn't a decoded DataTypeDefinition, or whose kind doesn't
match the DataType category, returns `Bad_TypeMismatch`.
  - Null is rejected for concrete Structure/Enumeration/OptionSet types.
  - WriteMask is checked first (Part 3 5.8.3).
- **AnalogItem:** a null write skips the InstrumentRange check again.
- **Method call:** the async fallback passes the ObjectId to the method
handlers.
- **Redundancy:** `DistributedValueParticipation` merges index-range
writes and applies the index range on reads.
- **Change masks:** `ClearChangeMasks` takes the bits atomically, so
concurrent changes are no longer lost.
- **`AccessLevelEx`:** preserved by Clone/Export.

### Alarms & Conditions (Part 9)
- **Acknowledge / Confirm / AddComment:**
- Accept any EventId the condition or its branches reported while that
state is still pending (5.7.3).
  - An unknown id returns `Bad_EventIdUnknown`.
- An already handled state returns `Bad_ConditionBranchAlreadyAcked` /
`...AlreadyConfirmed`.
- AddComment on a branch no longer changes the trunk, and the branch
table is re-keyed when a branch's EventId changes.
- **Shelving:**
  - OneShotShelve without MaxTimeShelved no longer overflows the timer.
- A timed shelve is measured on the monotonic clock, so wall-clock steps
don't delay the unshelve.
  - The state is named `OneShotShelved`.
- **Audit events:**
  - They report failure for rejected method calls.
  - `SelectedResponse` is a UInt32.
  - A rejected negative Respond leaves SelectedResponse unset.
- **`ContentFilterElement.Validate`:** an unknown FilterOperator is
reported per element as `Bad_FilterOperatorInvalid`, instead of failing
the whole service call.

### ContentFilter evaluation (Part 4 7.7.3, on top of #4564)
- **Bitwise operators:**
  - The result takes the size of the larger operand.
- Boolean, numeric String and one-element array operands are converted
to integers implicitly.
  - Types without an implicit integer conversion give NULL.
- **Strings:** ordering is ordinal, no longer culture dependent.
- **Cast:** restricted to the Table 121 conversions, and QualifiedName
to LocalizedText is added. The general `Variant.ConvertTo` keeps its
wider set.
- **Null and empty operands:**
  - `Like` with a NULL operand is NULL.
  - Empty arrays of the same DataType compare equal.
  - Null-string operands are NULL everywhere.

### Encoders and built-in types
- **XML:**
- Strings keep leading, trailing and whitespace-only content (Part 6
5.3.1.5). NodeSet import opts back into treating whitespace-only strings
as empty with the new `XmlDecoder.TreatWhitespaceOnlyStringsAsEmpty`.
- Layout whitespace around `<Identifier>` values is ignored, and
trailing control characters are stripped from `s=` ids.
  - `XmlElement` ignores DTDs without expanding entities.
- **JSON:**
  - A DateTime without an offset is read as UTC.
- An unmappable `nsu=` NodeId and an unknown `svu=` ExpandedNodeId
decode to a namespace-0 String NodeId (5.4.2.10 / 5.4.2.11).
  - `svr=` is rejected in a NodeId field.
  - Bad matrix dimensions return `BadDecodingError`.
  - The non-strict body stack stays balanced after an error.
  - The nesting limits match between encoder and decoder.
- **Binary:**
  - `MaxByteStringLength` no longer applies to strings.
- ExtensionObject bodies count toward the nesting limit on seekable
streams (and in the XML encoder).
- **Parsing:**
  - The NodeId parser requires `i=`/`s=`/`g=`/`b=`.
  - `ExpandedNodeId.Parse` rejects an empty `nsu=`/`svu=`.
  - NumericRange rejects a trailing `,` with `Bad_IndexRangeInvalid`.
  - RelativePath escapes `#`.
- **Status codes:** `StatusCode.WithSubCode`/`WithCodeBits` keep the
severity bits and look up the symbolic id for the new code.
- **Variant / VariantHelper:**
- `TryCastTo` returns false on a type mismatch. So
`DataValue.GetValue<T>(default)` returns the default, and
`GetValueOrDefault` / `CastTo` throw `BadTypeMismatch` as documented.
  - `System.DateTime` casts are supported.
  - Concrete encodeable arrays (e.g. `Argument[]`) no longer throw.
- `ConvertTo` keeps one-element and empty arrays as arrays and checks
the conversion is legal for empty arrays.
- `CastFrom(object[])`, `CastFrom(Enum[])` and the obsolete
`Variant(object)` constructors keep the array shape (previously a
one-element array collapsed to a scalar and an empty one to Null).
- `TypeInfo.Unknown` / `Variant.Null.TypeInfo` report `ValueRanks.Any`.
- **Equality and hashing contracts fixed:**
- Types: Matrix/MatrixOf, typed-null Variants, NodeId (the namespace
index is now hashed), ExpandedNodeId, LocalizedText, DiagnosticInfo,
StructureDefinition/EnumDefinition, CertificateIdentifier, UserIdentity
and DateTimeUtc.
  - `MatrixOf.Dimensions` returns a copy.
  - Decimal equality and hashing now run in bounded time.
- **Localized text:** a LocalizedText with locale `"mul"` and no usable
translations, or null client locale ids, no longer throw.

### NodeSet, address space and utilities
- **NodeSet export/import:**
- Server index 0 is the local server on import and export, so local ids
are no longer written as `svr=65535`.
- `StructureDefinition.BaseDataType` is filled from the supertype on
import.
- **`NodeTable.Import`:** replaces duplicates, keeping the references
neighbouring nodes hold. `TypeTable.Add` detaches a re-parented type
from its old supertype, and `Node.Clone` gets its own ReferenceTable.
- **`StringTable` / `NamespaceTable`:** capped at UInt16 indexes
(`BadEncodingLimitsExceeded`) for Append and Update, instead of silently
wrapping.
- **Small fixes:**
  - `UnsecureRandom.Shared` is seeded randomly.
  - The peer's RetryAfterMs is clamped to one day.
- The `CancelAsync` and `string.GetHashCode(StringComparison)` polyfills
match BCL behaviour.
  - `CoreUtils.IsEqual` compares strings ordinally.

### Schema generation and ComplexTypeSystem
- **JSON schema:** describes the 1.05 layout with the compact/verbose
null rules, and now validates real `JsonEncoder` output.
- **XSD:**
- `EncodingMask` (`xs:unsignedLong`) only for
StructureWithOptionalFields.
  - Guid and StatusCode are complex types.
  - Standard types are in the `Types.xsd` namespace.
- **Structure-backed OptionSets** are described as `{Value, ValidBits}`.
New `UaTypeDescription.IsStructureOptionSet` property and constructor
overload.
- **ComplexTypeSystem:**
- A malformed type in a server dictionary is skipped per type instead of
discarding the whole dictionary.
  - Cyclic supertype walks are bounded.
  - Declared definitions are no longer changed.
  - A dictionary validator is kept only if the dictionary parsed.

### Certificates and trust lists (Part 4 6.1.3, Part 12)
- **Revocation:**
- Checked for issuers supplied in the peer's chain and for
inline-trusted issuers, against the CRLs in the trusted and issuer
stores.
  - A store without CRL support no longer hides "revocation unknown".
- `RejectUnknownRevocationStatus` is honoured for inline-trusted issuers
unless the entry sets `SuppressRevocationStatusUnknown`. This changes
the strict case of `ExplicitIssuerChainIsHonoredWithoutStorePathAsync`.
- **Basic constraints / path length** violations fail with
`Bad_CertificateInvalid`.
- **`TreatAsInvalid`:**
- Now yields `Bad_CertificateUntrusted`, which can't be suppressed or
auto-accepted.
- It is honoured on individual trust-list entries even when the
certificate is also in a store.
- **`EccEncryptedSecret`:** enforces the result of validating the sender
certificate.
- **`TrustListTransaction`:**
- Routes each CRL to the store holding its issuer
(TrustedCrls/IssuerCrls).
- Validates staged CRLs, including the store's CRL support, before
changing anything.
  - Applies certificates before CRLs.
- **Other:**
- One failing `CertificateChanges` observer no longer stops the others.
- `InMemory:` stores share their contents per path, and
`InMemoryStoreProvider` is `IDisposable`.

### WoT (`Opc.Ua.Types/Wot`)
- **Projection security:** projection security schemes whose qualified
names collide are refused. New diagnostic
`WotDiagnosticCode.ProjectionSecurityConflict = 6063`.
- **Input handling:**
- Invalid Unicode (lone surrogates, bad UTF-8) is rejected as
`JsonException`.
  - External-schema results are keyed per affordance.
  - A duplicate security-floor member is rejected.
  - `nsu=` ids are percent-encoded (Part 6 5.1.12).
  - The configured JSON depth is honoured on re-parse.
  - An oversized document is reported before it is parsed back.
- **Performance and limits:**
- Subtype-graph validation, `componentOf` linking and the namespace
table are now linear.
  - Namespaces are capped at UInt16.

### New or changed public API
- `ICertificateValidatorProvider` (server)
- `SecurityPolicyInfo.GetUserTokenSignatureData(...,
MessageSecurityMode)` overload
- `XmlDecoder.TreatWhitespaceOnlyStringsAsEmpty`
- `UaTypeDescription.IsStructureOptionSet` plus a constructor overload
- `WotDiagnosticCode.ProjectionSecurityConflict`
- `StatusCodes.BadIndexRangeDataMismatch` in the `Opc.Ua.Types` list
- `InMemoryStoreProvider : IDisposable`

## Not changed / open
- **Deferred:**
- S1-14 (BSD OptionSet size needs the base type in `UaTypeDescription`,
and BSD is deprecated).
- C2-7 (restoring the own-certificate trust exemption would accept
certificates the spec says to reject; removing the dead code is a
separate decision).
- A1-4 (config values keep surrounding whitespace, as the pre-April-2026
loader did).
  - H-2 (JsonDecoder has no real nesting counter).
- A2-3 (the StringTable cap also applies to diagnostics tables; default
`MaxArrayLength` already bounds them).
- **Needs a product decision:**
- The general `ConvertToString` writes a QualifiedName as `ns:Name`,
while Table 121 drops the namespace.
  - Short (< 32 byte) client nonces are accepted on None channels.

## Verification
- Every commit added a regression test that fails without its fix, where
practical.
- Before the rebases, with all audit and review fixes (before the `Spec:
#…` commits): 0 build warnings and 16 test suites green on net10.0. The
suites: Types, Core, Core.Encoders, Core.Security, Core.Schema, Client,
Client.ComplexTypes, Server, Sessions, Gds, History, Features,
Redundancy.Server, PubSub, WotCon and WotCon.Bindings. Core hit its
known intermittent end-of-run hang once, and reruns were clean.
- The `Spec: #…` fixes were verified per group on their own suites.
Those include PubSub.Udp, Redundancy.PubSub, PubSub.Server,
Configuration and SourceGeneration.Core.
- **Not yet built or tested on this base.** It was rebased onto master
(resolving overlaps with #4522, #4540 and #4564) and then onto #4569.
This PR stays a draft until the full run on this base is green.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: marcschier <11168470+marcschier@users.noreply.github.com>
Co-authored-by: Marc Schier <marcschier@users.noreply.github.com>
2026-09-29 11:14:15 +00:00
romanettandClaude Opus 5.5 5d26f6f439 Fix JSON encoding fidelity issues found by fuzz crash replay and align with Part 6 (#4540)
# Description

The external crash corpus replayed by `Opc.Ua.Encoders.Fuzz.Tests` still
produced several hundred tolerated *encoding fidelity findings* per run.
They came from the 10 BinaryJson conversion targets (binary decode →
JSON encode → JSON decode). None of them was a crash or an unexpected
exception. Triage reduced them to three JSON encoder defects plus one
oracle rule that was stricter than the specification. While checking the
fixes against OPC 10000-6, two adjacent deviations turned up and are
fixed as well.

### Encoder fixes (`Opc.Ua.Types`)

| Defect | Before | After | Spec |
|---|---|---|---|
| LocalizedText with no Text and an empty/null Locale | `{"Text":null}`
(Verbose) / `{}` (Compact). Reads back as null and re-encodes
differently | JSON `null` (Verbose) / omitted (Compact) | 5.4.2.15 Table
39, 5.1.2 Table 1, 5.4.6 Table 45 |
| LocalizedText with empty Text | `"Text":""` written | Text not
encoded; `LocalizedText("")` is the null LocalizedText | 5.4.2.15 Table
39 ("not encoded if it is null or empty") |
| Variant/DataValue holding a **null array**, Compact | `Value` omitted
→ decoded as a *scalar* default (array rank lost) | `"Value":[]` |
5.4.2.17 Table 41, 5.1.11 |
| Variant/DataValue `Value` omission rule | Omitted for any default
value (including null *arrays*) when `IgnoreDefaultValues`;
`IgnoreNullValues` ignored | Same rules as every other field: a NULL of
a nullable built-in type is omitted when `IgnoreNullValues` or
`IgnoreDefaultValues` is set (like `WriteNull`); a default of a
non-nullable type only when `IgnoreDefaultValues` is set (like
`WriteValueProperty`). An array `Value` is never omitted | 5.4.2.17
Table 41, 5.1.2 Table 1, 5.4.6 Table 45 |
| `ExpandedNodeId.Format(context)` with a ServerIndex/NamespaceUri and a
null identifier | Returned `null`, so JSON wrote `null` and the server
index was lost | `svr=N;i=0` / `nsu=...;i=0`, consistent with the
`IFormatProvider` overload | 5.1.12 Table 6, 5.4.2.11, 5.1.2 Table 1
(NULL is exactly `i=0`) |

### Fuzz oracle and harness (`fuzzing/`)
- The round-trip oracle now treats null and empty arrays, and null/empty
LocalizedText Text/Locale, as equivalent in every mode (5.1.11:
"semantically the same for all DataEncodings… applications shall treat
null and empty arrays as equal"). Previously this only held when
`IgnoreNullValues` was set.
- `FuzzCrashAssets` no longer prints a stack trace or an inline base64
reproducer for *tolerated* fidelity findings. Only the one-line summary
entry is reported, and it now names the actual cause instead of
`TargetInvocationException`. Before, tolerated findings used up the
60-per-target reproducer budget, so a genuine failure later in the
enumeration printed no reproducer. The externally supplied corpus inputs
were also written into the pipeline log on passing runs.
- Three hand-crafted curated reproducers (9–23 bytes, one per encoder
defect) were added under `Assets/Repo/`, which is always enforced
strictly. Each one fails on the previous master and passes with this
change.

### Reviewer notes: observable JSON output changes
- `LocalizedText("")` / `("en","")` now round-trip with `Text == null`
instead of `""` (equivalent per 5.1.11).
- Verbose and Compact scalar Variant output is unchanged.
`IgnoreNullValues` alone now also omits a NULL `Value`, as it does for
every other null field.
- A null *array* in a Variant is written as `"Value":[]` in every mode
instead of being dropped in Compact, and a null *matrix* is written as
`"Value":[]` instead of failing with `BadEncodingError`.
- Variants holding a Variant or DiagnosticInfo still reach the writer
and are rejected as before (`WriteBadVariantThrows`).

### Testing (net10.0, locally)
- Opc.Ua.Types.Tests 10,542 passed, including new JsonEncoder tests for
each rule above
- Opc.Ua.Core.Encoders.Tests 2,929 passed (one expectation updated from
`"Text":""`)
- Opc.Ua.PubSub.Tests 1,369 passed; Opc.Ua.Core.Tests JSON subset 83
passed; Opc.Ua.Aot.Tests 178 passed
- Opc.Ua.Encoders.Fuzz.Tests 8,112 passed; crash-asset tests in the
Certificates/Network/PubSub fuzz projects and Opc.Ua.Fuzzing.Tests
passed
- net48: `Opc.Ua.Encoders.Fuzz.Tests` builds with 0 warnings; net48
tests were not run locally

## Related Issues

- No tracking issue; found by triaging the fuzz crash-asset replay
output of the master pipeline.

## 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.
- [ ] 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.
- [ ] I have addressed **all** PR feedback received.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
2026-09-25 07:49:52 +02:00
Marc SchierandCopilot b17007bd55 Stop the encoder tests flooding the VSTest channel with per-test output (#4219)
# Description

The `Core.Encoders` test job of run
[`31249610490`](https://github.com/OPCFoundation/UA-.NETStandard/actions/runs/31249610490)
printed nothing for 57 minutes and was killed by the 60-minute job
timeout. Issue #4213 recorded that as "hung at or before the first test"
— that reading was wrong, and the evidence to disprove it was already in
the job's own artifact.

The blame collector *had* written a hang dump and it *was* uploaded,
inside the ordinary `dotnet-results-Core.Encoders-*` artifact (11 MB of
it was the dump), where nobody looked for it. Opening it names the cause
precisely.

## What the dump shows

522 threads (408 background) in a process that finishes in 25 seconds
when healthy. Every test had already run; NUnit was in `OneTimeTearDown`
waiting on its event pump:

- the **event pump thread** is blocked in
`NUnitEventListener.TestFinished` →
`TestRunEventsHandler.HandleLogMessage` →
`LengthPrefixCommunicationChannel.Send` → `Socket.Send`, on a socket
that never drained. That send holds the channel's lock;
- `TestRunCache.SendResults` is blocked on that channel lock, and it
holds the cache lock;
- `TestRunCache`'s **one-second timer** then fired repeatedly into that
held cache lock, and the thread pool injected a fresh worker for each —
**399 threads** parked in `Monitor.Enter_Slowpath` behind a single lock;
- the NUnit run thread sits in `EventPump.Stop()` → `Thread.Join()`, so
the run can never complete.

## What put the socket under that much pressure

Volume of test output. Every encoder case dumps its input, its encoded
payload and its decoded result, the NUnit adapter forwards each captured
line to the runner as an individual message over that socket, and the
suite has 2929 cases. This is not marginal:

| | Before | After |
| --- | --- | --- |
| `Core.Encoders` TRX, green local run | **496 MB** | **4 MB** |
| `Core.Encoders` published CI artifact | **156 MB** | ~3 MB |
| Next-largest test job's artifact, for scale | 15 MB | 15 MB |
| `Client.ComplexTypes` TRX | 74 MB | 40 MB |

So the suite was shipping half a gigabyte of text through a socket, one
message per line, on every run and on every OS — and paying for it in
artifact storage even when nothing went wrong.

## The change

`EncoderCommon` gains a buffered `TestOutput` writer. The dumps are
collected per test and handed to `TestContext.Out` in `TearDown` **only
when the test did not pass** — which is the only time anyone reads them.
Nothing is lost: a failing test still reports its full
input/encoded/decoded dump under *Standard Output Messages*, verified by
injecting a deliberate failure. Explicit opt-ins
(`PrettifyAndValidateJson(..., outputFormatted: true)`) and the static
helpers keep writing immediately.

The same base class is shared by the `Client.ComplexTypes` fixtures, so
those are moved over too rather than left with half-buffered,
half-immediate output.

Two supporting changes:

- **`.github/workflows/buildandtest.yml`** — `timeout-minutes: 30` on
the `Test` step. The blame collector did its job here (dump at +50 min,
once the run went quiet), but the run still had not unwound 7 minutes
later when the *job* timeout cancelled everything, so the step needs a
bound below the job's. `Upload test results` already runs `if:
always()`, so the dump is still published.
- **`docs/DeveloperGuide.md`** — a "Test output" convention under
*Running tests*, so the next data-driven fixture does not re-introduce
this.

## Notes for reviewers

- `[Parallelizable]` on these fixtures is `ParallelScope.Self`, i.e.
fixture-level; tests inside a fixture stay sequential, so the
per-fixture buffer is as safe as the existing per-fixture
`RandomSource`/`DataGenerator` state that is already reset in `SetUp`.
- I separately confirmed that `--blame-hang-timeout` *does* fire on a
hang before the first test (minimal NUnit repro, dump at exactly the
configured timeout), which is what rules out the original "hung during
startup/discovery" theory independently of the dump.
- Local verification was scoped to the two affected suites rather than
the whole solution: `Opc.Ua.Core.Encoders.Tests` 2929/2929 on
**net10.0** *and* **net48**, `Opc.Ua.Client.ComplexTypes.Tests`
2773/2773 on net10.0, all building with 0 warnings. The full `UA.slnx`
run is left to CI.

## Related Issues

- Fixes #4213

## Checklist

- [x] 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.
- [ ] I have addressed **all** PR feedback received.

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: da92151a-3da2-4caf-a39c-5556e56f0506
2026-08-09 21:49:05 +02:00
Marc SchierandCopilot 0383472ebc Fix PubSub occurrence publishing and JSON DataValue status encoding (#4106)
### Summary

Three related gaps mean no `PublishedDataSet` can produce a message that
is not a key frame or a derived delta frame.
`PubSubDataSetMessageType.Event` and the `ua-event` wire tag of Part 14
§7.2.5.4 are modelled but unreachable.

This PR also corrects the JSON `DataValue` status member to the Part 6
Table 42 name `Status`.

### 1. A dataset cannot declare what its sample is

`PublishedDataSetSnapshot` carries a metadata version, fields and a
timestamp. `WriterGroup.BuildDataSetMessageAsync` therefore decides the
message kind alone: `KeyFrameCount <= 1` forces `KeyFrame`, otherwise it
derives a delta by comparing the snapshot **positionally** against the
previous one.

That derivation assumes successive snapshots describe the same fields in
the same order, which holds only for a source returning its complete
current state every cycle. A source whose samples are *occurrences* — an
event, a condition snapshot, or a delta it has already computed — cannot
satisfy it: consecutive samples carry different fields, so field *i* of
one occurrence is compared against field *i* of an unrelated earlier
one, and `Math.Min` silently truncates when the counts differ.

`PublishedDataSetSnapshot` now carries an optional `MessageType`. When
set, the writer publishes exactly the supplied fields under the declared
type and leaves its key-frame state untouched. When null — the default,
and every existing caller — behaviour is unchanged.

### 2. The event writer was unreachable

`EventDataSetWriter` and `EventPublishedDataSet` implement the Part 14
§6.2.4 event model correctly, but nothing could use them:

- `EventPublishedDataSet` did not implement `IPublishedDataSet`, so
`PubSubApplication` could not register an event dataset;
- nothing in the library constructed `EventDataSetWriter`, and it was
not an `IDataSetWriter`;
- `WriterGroup` appends at most one message per writer per cycle,
whereas an event writer produces one per occurrence.

Now: `EventPublishedDataSet` is an `IPublishedDataSet` whose general
`SampleAsync` drains one occurrence per cycle declared as an event;
`EventDataSetWriter` is an `IDataSetWriter`; `PubSubApplication` builds
an event dataset when the configured `DataSetSource` is a
`PublishedEventsDataType` and the registered source is an
`IEventSampler`; and a writer group drains every pending occurrence in
one publish cycle. Event-writer sequence numbers now use the WriterGroup
checkpoint state, so failover and restart resume above the shared
checkpoint.

The rows-returning `EventPublishedDataSet.SampleAsync` is renamed
`SampleEventsAsync`, both to make room for the interface member and to
match `IEventSampler.SampleEventsAsync`.

### 3. A metadata announcement hides its own payload

`JsonMetaDataMessage` documents that the payload *"is exposed both on
the base `PubSubNetworkMessage.MetaData` property and on
`MetaDataPayload` so callers can use whichever accessor matches their
fluent style."* `MetaDataPublisher` set only `MetaDataPayload`, so the
documented contract was false: any consumer, transform or encoder hook
reading the base property saw an announcement as a message carrying
nothing. Both are now populated. Dataset subscriptions also use a
dispose-safe subscribe/recheck handshake, preventing a concurrent
shutdown from leaving a late handler attached.

### 4. JSON DataValue status used the wrong member name

Part 6 section 5.4.2.18 Table 42 names the `DataValue` status member
`Status`; `StatusCode` is its data type. `JsonEncoder` and `JsonDecoder`
instead shared the member name `StatusCode`, so stack-to-stack
round-trips passed while standards-conformant peers could not exchange
non-Good status values.

Both directions now use `Status`. The encoder regression test also
verifies that `StatusCode` is absent from the emitted `DataValue`
object.

Fixes #4111.

### Compatibility

The snapshot property is optional and defaults to null, so existing
PubSub sources and current-state sampling behavior remain unchanged. The
`EventPublishedDataSet.SampleAsync` rename is source-breaking on a type
that could not previously be reached from the runtime.

The JSON `DataValue` wire member intentionally changes from `StatusCode`
to the specification-defined `Status`. No legacy alias is retained: the
encoder emits `Status`, and the decoder expects `Status`.

### Testing
`Opc.Ua.PubSub.Tests` on `net10.0`: 1341 passed, 1 skipped, 0 failed.

New coverage:
- `WriterGroupDeclaredMessageTypeTests` — a declared type is honoured,
declared samples never become delta frames and are never suppressed as
unchanged, and an undeclared source keeps the existing key-frame
cadence.
-
`EventDataSetWriterTests.EventPublishedDataSet_SamplesOneOccurrencePerSnapshotAsync`
— the general contract drains one occurrence per call, declares it as an
event, and reports nothing when exhausted.
-
`WriterGroupEventWriterHotStandbyContinuesSequenceNumberFromSharedCheckpointAsync`
— event messages share the checkpointed sequence state and continue
monotonically after standby promotion.

Issue #4111 validation:
- `Opc.Ua.Types.Tests` JSON encoder/decoder fixtures on `net10.0`: 274
passed.
- `Opc.Ua.Core.Encoders.Tests` JSON conformance cases on `net10.0`: 604
passed.
- `Opc.Ua.PubSub.Tests` JSON fixtures on `net10.0`: 88 passed.
- Targeted `net48` regression tests: 2 passed; core JSON conformance
cases: 572 passed.
- Review-feedback fixtures on `net48`: 11 passed.
CI-fix validation:
- `Opc.Ua.Core.Tests` on `net10.0`: 4145 passed, 86 skipped.
- `Opc.Ua.Core.Tests` on `net48`: 4070 passed, 92 skipped.
- Patch coverage check passes after adding targeted metadata and event
publishing tests.

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 1a300bc8-8bde-4a82-8fc3-476ef837bc6a
Copilot-Session: 9f18bf87-795f-40a9-8494-b6e6334c4a97
2026-07-29 22:06:52 +02:00
84fa8e8906 CTT follow-up: monitored items, alarms, codecs, history, and model validation (#4002)
# Description

Follow-up to #3978 based on the latest OPC UA CTT aggregate, matrix,
Historical Access, Attribute, and Node Management runs.

## Aggregate run 16

- Ignore synthetic `BadBoundNotFound` raw-bound markers when feeding
aggregate calculators.
- Add direct and live historian regression coverage.

## Multi-dimensional Variant matrices

- Fix `Variant.Copy()` so rank-2 Variant matrices preserve their payload
and dimensions.
- Enforce valid Variant matrix dimensions consistently across Binary,
JSON, XML stream, and XML parser codecs.
- Encoders return `BadEncodingError`; decoders return `BadDecodingError`
for malformed matrix dimensions.
- Add cross-codec valid/null/empty/malformed tests and a batched
ReferenceServer Read regression.
- Correct the previous CTT issue attribution: the invalid
`Scalar_Static_Arrays2D_Variant` server encoding poisoned the batched
CTT Read.

## Node Management AddNodes

- Return `BadNodeIdRejected` for unsupported requested-NodeId
namespaces.
- Validate hierarchical ReferenceType source/target NodeClass
constraints.
- Document run-18 confirmation that all remaining 15 AddNodes errors are
CTT project/script configuration issues.

## Historical Access and Attribute Services run 18

- Reject whitespace in `NumericRange` syntax with
`BadIndexRangeInvalid`.
- Add HistoryRead tests for operation-level invalid syntax and
per-DataValue `BadIndexRangeNoData`.
- Document CTT defects:
- `012.js` expects `BadIndexRangeNoData` at the operation level instead
of on returned DataValues;
  - `Err-012.js` uses a non-historizing node for an access-denied test;
- generic Attribute array helpers omit NodeId[] and StatusCode[]
conversion paths.

## Base Information model validation run 19

- Run the CTT main session as `sysadmin` over Basic256Sha256
SignAndEncrypt so SecurityAdmin RolePermissions and EncryptionRequired
restrictions are both satisfied without weakening the server model.
- Preserve predefined well-known NodeIds while making generated dynamic
type and optional-child factories allocate compatible per-instance
NodeIds, remap internal references, and update references from the
owning subtree.
- Materialize dynamic RoleType properties through generated helpers and
keep role-child string NodeIds disjoint from numeric RoleManager root
IDs.
- Add net48/net10 regressions for generator output, NodeId
allocation/reference remapping, DI mandatory children, dynamic roles,
and low-privilege versus SecurityAdmin Browse visibility.
- Document CTT defects for permission-unaware mandatory-child
validation, the UA 1.04 reference model used against a UA 1.05 server,
stale ServerDiagnostics cache, and the `ConformanceUnits` scalar/array
mismatch.
## Related Issues

- Follow-up to #3960 and #3978.

## Checklist

- [x] I have signed the CLA and read the CONTRIBUTING document.
- [x] Tests prove the fixes and improve coverage.
- [x] Required CTT documentation is updated.
- [x] Focused builds/tests introduce no warnings.
- [x] Affected tests pass on .NET Framework and .NET 10.
- [ ] CI and CodeQL are green.
- [ ] All PR feedback is addressed.



## Latest CTT full run and A&C run 20

- Fix Monitor Value Change V2 `036.js`: an ambiguous
`DataValue(StatusCode)` call encoded `BadIndexRangeNoData` as a Good
value.
- Prove all 19 configured matrix monitored items notify when the
selected value actually changes; document the independent `042.js`
script defects.
- Rebase dynamic `CertificateExpired` / `TrustListOutOfDate` alarm roots
and descendants, restore standard type declarations and ModellingRules,
and expose the cross-node-manager references.
- Deliver method-triggered events to every authorized Session, fixing
the two-Session Confirm scenario.
- Preserve locale-only `LocalizedText` values consistently across
Binary, JSON, and XML and align Acknowledge/Confirm comment behavior and
tests.
- Document the remaining proven CTT defects for TransactionDiagnostics,
SemanticChange, History `013`, PolicyId scope, A&C, durable
subscriptions, and Subscription Minimum events.

## Base synchronization

Merged `master` through `007cfaf90` (`#4010`, repository layout
consolidation) and retained the matrix regression tests at their new
`tests/` locations.

## Latest validation

- Focused merged-tree tests pass on net10 and net48.
- `Opc.Ua.Server` and `Quickstarts.Servers` Release builds complete with
0 warnings and 0 errors on net10 and net48.
- The existing
`DisableEnableViaTypeAndInstanceMethodNoEventsDuringDisabledAsync`
timing test took its documented inconclusive timeout path after the
layout merge; no test failure was reported.

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>
Co-authored-by: romanett <romanett98@gmail.com>
2026-07-18 05:01:03 +02:00
Marc Schier 7cec2d614d Move repository files to the consolidated layout 2026-07-16 06:25:54 +02:00