Repository navigation
Let servers start in globalization-invariant mode - #4716
Conversation
A server did not start in globalization-invariant mode, which Native AOT
and container images often enable with InvariantGlobalization. When the
server loaded its default status texts, ResourceManager created
CultureInfo("en-US"), and the runtime throws CultureNotFoundException for
every culture but the invariant one in this mode.
ResourceManager now keeps the locale id of each translation table as a
string, and falls back to its language subtag to find another region of
the same language. When the runtime cannot create the culture of a
well-formed locale id, the id is parsed into the casing of its culture
instead, and still rejected if it names a language without a region. A
malformed locale id is rejected as before. The resolved locale ids are
cached, so loading the default texts creates each culture once.
The tests reproduce both invariant-mode behaviors of the runtime: no
culture at all, and cultures that carry invariant data. Verified by
starting MinimalBoilerServer with DOTNET_SYSTEM_GLOBALIZATION_INVARIANT=1
and connecting MinimalClient (browse, read, subscribe).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
@copilot resolve the merge conflicts in this pull request |
There was a problem hiding this comment.
🟡 Changes recommended
The fallback parser accepts malformed RFC 5646 locale identifiers in invariant mode.
2 open findings
What changed in this PR
Enables OPC UA servers to start under globalization-invariant mode.
Changes:
- Stores and resolves translations using locale identifiers.
- Adds invariant-globalization regression tests.
- Updates Native AOT guidance.
| File | Description |
|---|---|
ResourceManager.cs |
Adds culture-independent locale handling and fallback. |
ResourceManagerInvariantGlobalizationTests.cs |
Tests invariant-mode translation behavior. |
NativeAoT.md |
Documents invariant-mode server support. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
@copilot resolve the merge conflicts in this pull request |
…t-globalization # Conflicts: # src/Opc.Ua.Server/NodeManager/ResourceManager.cs Co-authored-by: marcschier <11168470+marcschier@users.noreply.github.com>
Resolved locally in merge commit |
|
|
Resolved and pushed in merge commit
Resolved and pushed in |
Review feedback: - TryParseLocaleId accepted tags that are not well-formed, such as en-US-x (a private use singleton without a subtag) and en-US-ab (a two-character variant), and registered them in globalization-invariant mode. It now follows the grammar of RFC 5646 section 2.1: the language, up to three extended language subtags, the script, the region, variants, extensions with their subtags, and a private use sequence, in this order. A tag that does not match is rejected; grandfathered tags and tags that consist of a private use sequence only stay rejected. - The test fixture is sealed. New invariant-mode cases: ten malformed tags are rejected (the old parser accepted six of them); five well-formed tags with extended language, variant, extension, or private use subtags are registered in the recommended casing (the old parser read zh-cmn-hans-cn as neutral); and two tags without a region but with a variant or an extension are rejected as neutral. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Code coverage✅ Coverage gate passed.
Uncovered changed lines
Coverage is above the recorded baseline - consider ratcheting Thresholds live in |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #4716 +/- ##
=======================================
Coverage 82.83% 82.83%
=======================================
Files 2267 2267
Lines 345803 345892 +89
Branches 60271 60297 +26
=======================================
+ Hits 286437 286511 +74
- Misses 39272 39277 +5
- Partials 20094 20104 +10
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
…est (#4719) # Description `dotnet test tests/Opc.Ua.Server.Tests` exits 1 even when every test passes. The assembly-level `LeakDetectionSetup` teardown reports `Certificate leak detected: 3 instance(s) created but not disposed`. Cause: `CloseAndUpdateRejectsRetainedCrlOfRemovedCaAsync` and `CloseAndUpdateAcceptsVersion3CertificateWithoutExtensionsAsync` stage TrustList updates through `CloseAndUpdate` and finish without `ApplyChanges` or `CancelChanges`. `TearDown` disposed the TrustLists but not the staged operations, so the `PushConfigurationTransactionCoordinator` kept the staged and original `CertificateCollection`s undisposed. CI did not catch it: `.github/scripts/get-test-verdict.ps1` accepts a nonzero exit when the TRX counters show no failures, and reports it only as an "at-exit stall" warning. Fix: `TearDown` calls `m_coordinator.Reset()`, which discards the staged operations and disposes their collections. This is a test-only change; in production, a session's staged operations are applied, cancelled, or discarded when the session closes. ### Validation | Command | Result | | --- | --- | | `dotnet test tests/Opc.Ua.Server.Tests -f net10.0 -p:CustomTestTarget=net10.0 --filter "FullyQualifiedName~PushComplianceRegressionTests"`, before the fix | 15 passed, but exit 1: `TearDown failed for test fixture Opc.Ua.Server.Tests.LeakDetectionSetup`, `Certificate leak detected: 3 instance(s) created but not disposed (created=49, disposed=46)` | | Same, after the fix, 3 runs | 15 passed, exit 0, no leak report | | Same with `-f net48 -p:CustomTestTarget=net48` | 15 passed, exit 0 | | `dotnet test tests/Opc.Ua.Server.Tests -f net10.0 -p:CustomTestTarget=net10.0` | 8386 passed, 0 failed, 5 skipped; exit 0 | ## Related Issues - No tracking issue. Found while explaining the exit code 1 of a full `Opc.Ua.Server.Tests` run for #4716. ## 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. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…4712) # Description `MetaDataPublisher` encoded the DataSetMetaData announcement itself and called `IPubSubTransport.SendAsync` directly. That bypassed the connection's security wrapper and its fail-closed check. A WriterGroup configured for `SignAndEncrypt` therefore announced its DataSet layout (field names, types, version) unsigned and unencrypted. A secured subscriber dropped the announcement (`Dropping unsecured inbound frame ... requiring SignAndEncrypt`, documented as an expected warning in docs/PubSub.md), and any listener could read it. OPC 10000-14 §6.2.5.2 applies the WriterGroup's SecurityMode to the NetworkMessages it publishes, and §7.2.4.6.3 carries the security of a discovery announcement in its SecurityHeader. Confirmed on the wire: the documented `udp-uadp` exchange sent secured DataSetMessages plus one DiscoveryResponse (announcement type 2, DataSetMetaData) with ExtendedFlags1 `0x81`, meaning the SecurityHeader bit was clear. Review feedback widened the scope to every UADP discovery message of a secured connection, and to the diagnostics of secured messages. Fix: - `MetaDataPublisher` builds the announcement and hands it to the connection (`PubSubConnection.SendMetaDataAnnouncementAsync`). - One encode-and-secure step (`PubSubConnection.EncodeOutboundAsync`) now serves data messages, discovery requests and responses, and the announcements that UDP and Ethernet send to their transport-specific discovery destination (ApplicationInformation, PublisherEndpoints, PubSubConnection). On a connection configured for message security, it applies the SecurityMode (`Sign`, or `SignAndEncrypt`) to UADP messages and refuses to send any message it cannot secure. - Before, a secured connection dropped its own discovery requests, so a secured subscriber could not ask for metadata, writer configuration, or endpoints. It also refused metadata and writer-configuration probe responses, and sent the transport-specific announcements in plaintext. - A discovery response or announcement that cannot be secured or sent is logged (`DiscoveryMessageNotSent`, Warning) and dropped. Securing announcements makes them depend on a current key; without this, a key provider with no key yet, for example before a Security Key Service delivers the first key, made `StartAsync` fail, and a probe response that could not be secured ended the receive loop. Requests sent through `RequestDiscoveryAsync` still throw to the caller. - Secured messages are counted in `SentNetworkMessages` and `SentDataSetMessages` like unsecured ones. Before, they bypassed the encoder that counts them, so secured data messages were never counted. - `UadpDiscoveryCoder` can encode a discovery message with the SecurityHeader bit set, and reports the security boundary after the PublisherId (and DataSetClassId), which is where `UadpDecoder.TryReadOuterPrefix` ends the prefix. `UadpEncoder.EncodeWithSecurityBoundary` accepts discovery messages. - The receive path is unchanged: it already unwraps secured discovery frames and registers authenticated metadata. - Not changed: the MQTT last-will status message is registered with the broker when the connection opens and published later, so it cannot carry a current SecurityHeader; it stays unsecured. - docs/PubSub.md no longer lists the unsecured-frame warning. It describes how the SecurityMode applies to announcements and discovery (`Sign` authenticates; `SignAndEncrypt` also encrypts) and the last-will exception. Tests: - `MetaDataPublisherTests.UadpPathSecuresTheAnnouncementOfASecuredWriterGroupAsync`: the announcement of a SignAndEncrypt WriterGroup carries a SecurityHeader, does not contain the DataSet name in cleartext, and unwraps and decodes with the subscriber's keys. Before the fix, the announcement had no SecurityHeader. - `MetaDataPublisherTests.SecuredNetworkMessagesAreCountedAsSentAsync`: before the fix, `SentNetworkMessages` read 0 on a secured connection. - `PubSubDiscoveryTests.SecuredSubscriberDiscoversSecuredPublisherWithoutPlaintextFramesAsync`: a secured subscriber and a secured publisher, connected through an in-memory transport that also sends announcements to a separate destination, discover metadata, writer configuration, and endpoints, and no frame lacks a SecurityHeader. Before the fix, the metadata request was dropped. With only the announcement change reverted, the PublisherEndpoints response was dropped. - `PubSubDiscoveryTests.StartupAnnouncementsWithoutACurrentKeyDoNotFailTheStartAsync` and `ReceiveLoopSurvivesADiscoveryResponseThatCannotBeSecuredAsync`: both failed before the fail-soft change. - `UadpDiscoveryTests.SecuredDiscoveryMessagesSplitWhereTheReceiverExpects`: for requests and responses, with and without a DataSetClassId, the secured encoding differs from the unsecured one only by the SecurityHeader flag, and its boundary equals the receiver's prefix length. - `UadpDiscoveryTests.EncodeWithSecurityBoundaryRejectsAMessageItCannotSecure`. ### Validation | Command | Result | | --- | --- | | `dotnet test tests/Opc.Ua.PubSub.Tests -f net10.0 -p:CustomTestTarget=net10.0` | 1498 passed, 1 skipped (the UDP multicast loopback discovery test, which this host skips) | | `dotnet test tests/Opc.Ua.PubSub.Tests -f net48 -p:CustomTestTarget=net48` | 1498 passed, 1 skipped | | `dotnet test` on net10.0 for `Opc.Ua.PubSub.Udp.Tests`, `.Eth.Tests`, `.Mqtt.Tests`, `.Server.Tests`, `fuzzing/Opc.Ua.PubSub.Fuzz.Tests` | 234, 55, 157, 177, 149 passed | | `dotnet test` on net10.0 for `Opc.Ua.PubSub.Kafka.Tests`, `.Adapter.Tests`, `.Diagnostics.Tests`, `Opc.Ua.Redundancy.PubSub.Tests` (on 9417ccb) | 92 (1 skipped), 152, 53 (1 skipped), 45 passed | | `tests\Opc.Ua.Aot.Tests\bin\Debug\net10.0\Opc.Ua.Aot.Tests.exe --treenode-filter "/*/*/PubSubAotTests/*"` (JIT; the AOT job failed on 94d4391) | 7 of 7 passed, including `StartsAndStopsPublisher_UdpUadp` | | `dotnet build src/Opc.Ua.PubSub -c Release` (all target frameworks) | 0 warnings | | The docs/PubSub.md first exchange, run verbatim, with #4710 applied (before the review changes) | The announcement is secured on the wire (ExtendedFlags1 `0x91`, 268 bytes); 14 DataSets; no unsecured-frame warning; the subscriber logs `Registered inbound metadata for writer 1` | | After merging master (#4710, #4711, #4716), on 83567ce: `dotnet build src/Opc.Ua.PubSub -c Release` | 0 warnings | | Same merge: `dotnet test tests/Opc.Ua.PubSub.Tests -f net10.0`; the changed fixtures on net48; `dotnet test tests/Opc.Ua.PubSub.Udp.Tests -f net10.0` | 1498 passed, 1 skipped; 132 passed, 1 skipped; 237 passed | | Same merge: `Opc.Ua.Aot.Tests.exe --treenode-filter "/*/*/PubSubAotTests/*"` (JIT) | 8 of 8 passed, including #4710's `SamplePublisherDeliversSecuredDataSetToSampleSubscriberAsync` | | Same merge: the docs/PubSub.md first exchange, run verbatim on `opc.udp://127.0.0.1:4840`, then the publisher alone against a UDP listener | 9 DataSets in 10 seconds and no warning on either side; the listener saw 9 DataSetMessages and 1 DiscoveryResponse (the startup announcement), all with the SecurityHeader bit set | ## Related Issues - No tracking issue. Found while verifying the PubSub first-exchange documentation (follow-up to #4555). ## 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. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>


Description
A server did not start in globalization-invariant mode (
InvariantGlobalization, orDOTNET_SYSTEM_GLOBALIZATION_INVARIANT=1), which Native AOT and container images often enable. When the server loaded its default status texts,ResourceManagercreatedCultureInfo("en-US"). In this mode the runtime throwsCultureNotFoundExceptionfor every culture except the invariant one, so the server crashed inResourceManager.LoadDefaultText.ResourceManagernow:docs/NativeAoT.mdno longer warns against invariant mode for servers.The new tests (
ResourceManagerInvariantGlobalizationTests) reproduce both invariant-mode behaviors of the runtime: no cultures at all, and cultures that carry invariant data. They also cover malformed tags, and well-formed tags with extended language, variant, extension, and private use subtags.The branch includes a merge of master: #4707 moved translation lookups to a lock-free snapshot of the tables, and the snapshot now copies the locale-id-based tables.
Validation
dotnet test tests/Opc.Ua.Server.Tests -f net10.0 -p:CustomTestTarget=net10.0 --filter "FullyQualifiedName~ResourceManager"dotnet test tests/Opc.Ua.Server.Tests -f net48 -p:CustomTestTarget=net48 --filter "FullyQualifiedName~ResourceManager"dotnet test tests/Opc.Ua.Server.Tests -f net10.0 -p:CustomTestTarget=net10.0, on 80e5163 (before the master merge)PushComplianceRegressionTests, which is unrelated to this change and fixed in #4719dotnet build src/Opc.Ua.Server -c Release(net10.0, net9.0, net8.0, net48)DOTNET_SYSTEM_GLOBALIZATION_INVARIANT=1, then MinimalClient (on 27c8478)CultureNotFoundException. After: it starts, and the client browses, reads, and receives a subscription notificationRelated Issues
Checklist