Minimal backports to the 1.5.378 line of three findings from the codec
audit behind #4599 / #4602 (B1-1, X1-1, P1-1). All three can be reached
from untrusted input. The changes are kept small, and the performance of
legitimate messages is unchanged.
## Changes
**B1-1: nested array preallocation (`BinaryDecoder`)**
Variant, DataValue, ExtensionObject and encodeable arrays were allocated
at the full length prefix before any element was read. Arrays nested in
the first element of their parent all see the same remaining message, so
a chain of them multiplied the allocation: about 1 KB of input in
`RequestHeader.AdditionalHeader` preallocated about 200 MB before
authentication.
- These arrays now use the #4602 budget. An array is allocated at full
length only while all such arrays still being read take at most 8 bytes
per remaining message byte. Otherwise it starts at 16 KB and grows as
elements are read.
- The reservation is released in a `finally`.
- Leaf arrays (numbers, strings, NodeIds, …) are unchanged and stay
bounded by MaxArrayLength.
**X1-1: XML element depth (`XmlDecoder`, `BinaryDecoder`,
`JsonDecoder`)**
The depth of XmlElement values and XML ExtensionObject bodies was
unbounded, and the `InnerXml` setter and `CloneNode(true)` recurse once
per level. A known-type XML body of about 200 KB inside a binary message
terminated the process with a stack overflow.
- Before any network XML is loaded into a DOM, it is now checked with a
streaming reader. XML nested deeper than `MaxEncodingNestingLevels`
(default 200, the same limit master uses) is rejected with
`BadEncodingLimitsExceeded`.
- Master's node-by-node copier is not ported.
**P1-1: inbound DataSetMetaData (`UaPubSubConnection`)**
Received metadata is not authenticated, yet it replaced the metadata of
every reader with a matching DataSetWriterId, whichever publisher sent
it.
- It now only updates readers whose PublisherId filter accepts the
sender, using the same rule as data messages. A reader without a
PublisherId still accepts any publisher.
- Master's registry cap is not needed, because 378 has no registry.
## Verification (net10.0 unless noted)
| Check | Result |
|---|---|
| Nested Variant-array chain, 995 bytes | 199.2 MB → 3.3 MB allocated |
| Known-type XML body 30 000 levels deep inside binary | process crash →
`BadEncodingLimitsExceeded` |
| `Opc.Ua.Types.Tests` (incl. new `DecoderNestingLimitTests`) | 327 /
327; new tests also 9 / 9 on net48 |
| `Opc.Ua.Encoders.Fuzz.Tests` (crash/timeout/slow corpus) | 150 / 150 |
| `Opc.Ua.Core.Tests` (Encoders + Types) | 6 327 passed, 0 failed |
| `Opc.Ua.PubSub.Tests` (without broker-dependent MQTT/UDP) + new
`MetaDataPublisherFilterTests` | 617 + 4 passed, 13 skipped |
| Types / Core / PubSub build, all TFMs | 0 warnings |
**Performance vs. db39629d2:**
- Test cases: decoding `Variant[100000]`, `Variant(Variant[])`,
`DataValue[10000]`, `ReadValueId[20000]` and `ExtensionObject[20000]`.
- Allocations are byte-identical to the baseline.
- Median times are within run-to-run noise over two interleaved rounds.
- The only new cost is one extra streaming parse for XmlElement values.
## Notes for review
- If a deployment raises `MaxEncodingNestingLevels` well above the
default, the allowed XML depth rises with it. Master behaves the same
way.
- No public API changes. The XML depth helper is internal to
`Opc.Ua.Types`, and `JsonDecoder` in `Opc.Ua.Core` has a private copy.
🤖 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: romanett <7413710+romanett@users.noreply.github.com>
Fixed timer leaks in ChannelAsyncOperation.EndAsync
## Proposed changes
- Added linked cancellation token source for delay task (runtimes <=
.NET 6.0)
- Added delay task cancellation (runtimes <= .NET 6.0)
## Related Issues
- Fixes#3668
## Types of changes
- [x] Bugfix (non-breaking change which fixes an issue)
- [ ] Enhancement (non-breaking change which adds functionality)
- [ ] Test enhancement (non-breaking change to increase test coverage)
- [ ] Breaking change (fix or feature that would cause existing
functionality to not work as expected, requires version increase of
Nuget packages)
- [ ] Documentation Update (if none of the other choices apply)
## Checklist
- [x] I have read the
[CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md)
doc.
- [x] I have signed the
[CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf).
- [ ] I ran tests locally with my changes, all passed.
- [ ] I fixed all failing tests in the CI pipelines.
- [ ] I fixed all introduced issues with CodeQL and LGTM.
- [ ] I have added tests that prove my fix is effective or that my
feature works and increased code coverage.
- [ ] I have added necessary documentation (if appropriate).
- [ ] Any dependent changes have been merged and published in downstream
modules.
## Further comments
Fix already accepted in master branch, please add it to latest 1.5.378
version
---------
Co-authored-by: Suciu Mircea Adrian <Mircea-Adrian.Suciu@softing.com>
Co-authored-by: Marc Schier <marcschier@users.noreply.github.com>
This reverts commit 00519fa6dd.
# Description
Due to longer investigation time needed on ModelCompiler the pruning of
the extra decoded children that should stay on the type definition only
is reverted
## Related Issues
_Reference all GitHub issues this PR addresses. If there is no issue
yet, open one and link it here._
_If this is a relatively large or complex change, a design must have
been discussed in the related tracking issue and signed off (which
becomes the Architectural Decision Record (ADR))._
- Fixes #github-issue-number, ...
## 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](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.
- [ ] 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.
# Description
`ConditionState.m_branches` (a plain `Dictionary`) was read and written
from multiple threads without synchronization. A state change on a
background thread could mutate the dictionary while readers
(`GetRetainState`/`UpdateRetainState`, `ConditionRefresh`, `GetBranch`)
enumerated it, yielding torn entries and native access violations
(observed as `IsBranch` on a null `this`). This ports the master378 fix
to `master`, where the type lives at
`Stack/Opc.Ua.Core.Types/State/ConditionState.cs` and uses
`ByteString`/nullable refs.
- **Lock all branch access**: added a private `m_branchesLock` guarding
every read/write of `m_branches` (`CreateBranch`, `GetBranch`,
`ReplaceBranchEvent`, `RemoveBranchEvent`, `ClearBranches`,
`GetBranchCount`).
- **Snapshot enumeration**: `GetBranches()` now returns a copy taken
under the lock, so internal readers and
`AlarmHolder.GetBranchesForConditionRefresh` iterate a stable collection
while other threads mutate the live one. Public signature is unchanged.
- **Test**: added `ConcurrentBranchAccessIsThreadSafe`, stressing
concurrent create/clear writers against enumerate/count readers.
`lock` is used rather than `SemaphoreSlim` because all affected methods
are synchronous. Each `ConditionState` holds its own lock, so the
recursive `UpdateRetainState` traversal across branches neither contends
nor deadlocks. `ConcurrentDictionary` was rejected: it would change the
public return type and still leave compound read-modify-write sequences
(e.g. `ReplaceBranchEvent`) unprotected.
## Related Issues
## 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](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.
---------
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: Marc Schier <marcschier@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Suciu Mircea Adrian <Mircea-Adrian.Suciu@softing.com>
Keep the generated children that belong on instances, and prune the
extra decoded children that should stay on the type definition only,
such as Optional/Mandatory placeholders and ones that do not have
Mandatory/Optional ModellingRules.
# Description
Currently child nodes that are type definition detailes such as ones
that do not have Mandatory/Optional ModellingRules are instantiated in
the runtime instance of such type definitions. This PR proposes to
correct this behaviour during NodeState creation as an alternative from
it being fixed by the ModelCompiler.
## Related Issues
- Fixes#3770
## 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](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.
- [ ] 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.
* Test that OrderedListState does not instantiate OrderedObjectPlaceholder
* Add global NodeState placeholder scan with assembly context
* Prune optional placeholder children during instance creation while leaving type-definition trees untouched.
* test(numeric-range): add failing test
* fix(numeric-range): include sub ranges in equality check
* test(numeric-range): add tests for coverage
(cherry picked from commit 56486efcdb)
* test(type-info): add failing test
* fix(type-info): require IEnumerable implementation before treating generic type as array
* test(type-info): add test for coverage
(cherry picked from commit 25a4882145)
This pull request modernizes the OPC UA .NET codebase and tests, removes .NET Standard 2.0 support from certificate/security code, and updates test code to use C# 12 and .NET 8+ features for clarity and maintainability.
- Certificate and BouncyCastle interop files (e.g., CertificateBuilder.cs, X509Extensions.cs, X509SignatureFactory.cs) are refactored to drop NETSTANDARD2_0 support and use modern .NET APIs and conditional compilation.
- Test code across the codebase is updated to use default, strongly-typed Parse methods, collection expressions, and improved async/await patterns.
- Exception handling and assertions in tests are modernized, and test method naming is made consistent with Async suffixes.
- The project and build files are updated to clarify Roslynator analyzer usage and remove obsolete netstandard tags.
- General code and test cleanup includes improved null safety, type safety, and removal of deprecated or unused code.
* Initial plan
* Fix parent-child references when importing NodeSet2.xml
- Added LinkParentChildRelationships method to establish parent-child links after import
- Sets Parent property and adds child to parent's children collection
- Added test to verify parent-child references work correctly after import
- All existing tests continue to pass
Co-authored-by: romanett <7413710+romanett@users.noreply.github.com>
* Make parent-child linking optional with default false
- Added linkParentChild parameter to Import method (defaults to false)
- Maintains backward compatibility by not establishing parent-child relationships by default
- Added test to verify backward compatibility (default behavior)
- Updated ParentChildReferencesTest to explicitly enable the feature
- All 7 tests pass
Co-authored-by: marcschier <11168470+marcschier@users.noreply.github.com>
---------
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: romanett <7413710+romanett@users.noreply.github.com>
Co-authored-by: marcschier <11168470+marcschier@users.noreply.github.com>
* Initial plan
* Add support for private encodeable factory in server initialization
- Add new ServiceMessageContext constructor that accepts a custom IEncodeableFactory
- Add CreateMessageContext(IEncodeableFactory) overload to ApplicationConfiguration
- Mark CreateMessageContext(bool) as obsolete since the clonedFactory parameter was never used
- Add PrivateEncodeableFactory property to ServerBase for specifying a private factory
- Update all internal calls to use the new parameterless CreateMessageContext()
Co-authored-by: romanett <7413710+romanett@users.noreply.github.com>
* Add unit tests for ServiceMessageContext private factory support
Co-authored-by: romanett <7413710+romanett@users.noreply.github.com>
* Improve documentation for ServiceMessageContext constructors
Co-authored-by: romanett <7413710+romanett@users.noreply.github.com>
---------
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: romanett <7413710+romanett@users.noreply.github.com>
* Initial plan
* Fix ByteString vs Byte array distinction in WrappedValue
Modified BaseVariableState.WrappedValue to use DataType and ValueRank
when creating Variant to correctly distinguish between byte[] (Byte array)
and ByteString. Added tests to verify the fix.
Co-authored-by: romanett <7413710+romanett@users.noreply.github.com>
* Address code review feedback
- Remove redundant null check for NodeId (NodeId.IsNull handles null)
- Update comment in test to be more specific about ByteString testing
Co-authored-by: romanett <7413710+romanett@users.noreply.github.com>
---------
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: romanett <7413710+romanett@users.noreply.github.com>
* Initial plan
* Add enhanced error messages for XML decoding failures
- Updated CreateBadDecodingError to optionally include failed value
- Modified SafeXmlConvert to pass XML value to error handler
- Updated ReadFloat, ReadDouble, ReadDateTime, ReadGuid, ReadNodeId, ReadExpandedNodeId, and ReadEnumerated to include values in error messages
- Added comprehensive tests to validate enhanced error messages
Co-authored-by: romanett <7413710+romanett@users.noreply.github.com>
* Fix parameter order consistency in SafeXmlConvert
Use named parameter for value argument to maintain consistency with other calls
Co-authored-by: romanett <7413710+romanett@users.noreply.github.com>
* Use named parameters consistently in SafeXmlConvert
Use named parameters for both functionName and value for better code clarity
Co-authored-by: romanett <7413710+romanett@users.noreply.github.com>
---------
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: romanett <7413710+romanett@users.noreply.github.com>
Modified TypeInfo.GetDataTypeId(Type) to check if IEncodeable types have a specific TypeId and return it instead of the generic Structure (i=22) DataTypeId. Added comprehensive tests for EUInformation and other well-known IEncodeable types.
Co-authored-by: marcschier <11168470+marcschier@users.noreply.github.com>
* Simplify GetDataTypeId fix based on code review feedback
Removed unnecessary static property check and simplified the instance creation logic. Caught specific exceptions (MissingMethodException, TargetInvocationException) instead of using bare catch.
Co-authored-by: marcschier <11168470+marcschier@users.noreply.github.com>
* Add additional exception handling for GetDataTypeId
Added handling for MethodAccessException and NotSupportedException to provide more comprehensive error handling when creating instances via Activator.CreateInstance.
Co-authored-by: marcschier <11168470+marcschier@users.noreply.github.com>
---------
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: marcschier <11168470+marcschier@users.noreply.github.com>
This change adds a **new Opc.Ua.Types** nuget that contains core types that do not rely on generated code. Why the name? Because almost all code that was in the Types subfolder is part of it, and because we do not want to change the name of the Opc.Ua.Core nuget/assembly because code relies on it. Any other name like base or bootstrap felt out of place due to the current structure already defining the name "types". Key design is to depend on only a small set of libraries with limited transitive dependencies, that includes hashcode, logging.abstraction, and for non standard20 builds System.Text.Json (needed for localizedText mul parsing). Also, key is to support .netstd 2.0 to support source generators and to allow removal of EEC_SUPPORT defines by removing std2.0 from core and all libraries.
The changes are mostly restructuring, and thus small but contain some key changes:
- **Status code** struct now comes with a Symbol field, which is used to carry symbols in case the code is not generated. Currently (until generated) the status code struct must be manually created with a symbol or the ToSymbolicId extension must be called in the core project. Future will generate Status code constants just like NodeId that contain the symbol which also removes the reflection-based lookup. Several tests needed to be adjusted as they checked the status code symbol strings instead of the code.
- **Core identifiers** are copied as they were needed for code to compile, that includes attributes, a subset of status codes and node identifiers. These will never change so no need to generate them.
- The types folder now contains **encoders**, except for json which is still in core. Once the json encoder is ported to system.text.json it will also be included and Newtonsoft.Json dropped as dependency (-> #3329).
- Since types relies on **IOperationContext** and **ISystemContext**, and these export IUserIdentity which relies on crypto, these were split and extended in core with ISessionOperationContext and ISessionSystemContext, which mean changes to ServerSystemContext and introduction of SessionOperationContext and SessionSystemContext classes that extend OperationContext and SystemContext. Some runtime checks in the server are therefore needed to get at the session and auth info, however, those are anyway optional fields and are not on the critical paths, and thus a type check is in order (and was done in the past already, but against OperationContext class).
- **Manifest resources** are now split between Core (nodeset zip, some generated resoures) and Types (built in bsd/xsds). This needs to be revisited later if the bsd/xsd need to be regenerated during type generation. Note that the embedded resource names changed as a result. Since these are only used internally and do not constitute public API this feels reasonable vs duplication of resources in 2 assemblies.
- Reflection however now needs to load not just the built in **encodeables** from Opc.Ua.Types assembly, but also the generated encodeables from Opc.Ua.Core assembly. For this a new helper Utils method was added to get the Core assembly when linked against Opc.Ua.Core otherwise null.
- Whatever utils methods needed to be split out into the new Opc.Ua.Types assembly, they are now in **CoreUtils** static class in Opc.Ua.Types, and Utils.xxxx forwards to them to ensure minimal changes. Later we would want to obsolete them in favor of CoreUtils, although it might make sense to split them again into domain specific utilities, such as networking, files, etc
romanett