Client: never select an OpenAPI endpoint in SelectEndpoint, and keep a closed Session from sending requests without authenticationToken - #4646
Merged
marcschier merged 3 commits intoOct 4, 2026
Conversation
An HTTPS listener with the REST binding announces its OpenAPI endpoint (OPC 10000-6 §G.3) with the same URL and message mode None as its binary endpoint. CoreClientUtils.SelectEndpoint matched on the URL scheme and the message mode only, so the first of the two in the GetEndpoints result won, both in the main pass and in the HTTPS fallback for useSecurity. A caller that connects with the binary transport then got the REST endpoint: with the default channel bindings the Session could not be created on it, and with the REST channel registered it ran over REST instead. Following OPC 10000-4 §5.5.4 (select by TransportProfileUri), both passes now skip endpoints with the HTTPS or WSS OpenAPI transport profile. Endpoints without a TransportProfileUri are selected as before.
A request with a null authenticationToken is a session-less invocation (OPC 10000-4 §6.3.1). After CloseAsync the Session's token is null, so a request sent through the same object afterwards, with the channel still open, reached the server as a session-less invocation. A server that supports those processed it instead of rejecting it. Session now records that a session was created (SessionCreated with a non-null cookie, which OpenAsync now calls through the override). From then on UpdateRequestHeader fails a request without token locally with Bad_SessionIdInvalid; CreateSession and ActivateSession are exempt, so reopening the Session and reactivating it keep working. A Session that was never created behaves as before. ClientTest closes the session and then the channel; the read that follows now fails with Bad_SessionIdInvalid before it reaches the closed channel, so the test expects that code instead of the channel's.
marcschier
approved these changes
Oct 4, 2026
Contributor
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The fixes are narrowly scoped, consistent with the stated requirements, and comprehensively covered by regression tests.
Review effort: Balanced
Findings: None
What changed in this PR
Prevents invalid OpenAPI endpoint selection and blocks unauthenticated requests from previously closed sessions.
Changes:
- Excludes HTTPS/WSS OpenAPI profiles during endpoint selection.
- Rejects tokenless post-close requests with
BadSessionIdInvalid. - Adds focused unit and integration regression coverage.
| File | Description |
|---|---|
src/Opc.Ua.Client/CoreClientUtils.cs |
Filters OpenAPI endpoints in both selection passes. |
src/Opc.Ua.Client/Session/Session.cs |
Tracks prior session creation and rejects invalid tokenless requests. |
tests/Opc.Ua.Client.Tests/CoreClientUtilsSelectEndpointTests.cs |
Covers OpenAPI filtering and compatibility behavior. |
tests/Opc.Ua.Client.Tests/Session/SessionTests.cs |
Covers closed, reopened, and not-yet-created session behavior. |
tests/Opc.Ua.Sessions.Tests/ClientTest.cs |
Updates the expected post-close error. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Draft
4 of 7 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Two client fixes, one commit each.
1.
CoreClientUtils.SelectEndpointskips OpenAPI endpoints.SelectEndpointmatched on the URL scheme and the message mode only, so the first of the two in the GetEndpoints result won, in the main pass and in the HTTPS fallback foruseSecurity.ClientChannelManagermaps it toopc.https+webapi), so a Session cannot be created on it; with the REST channel registered, the Session silently runs over REST.TransportProfileUri), both passes now skip endpoints with the HTTPS or WSS OpenAPI transport profile. Endpoints without aTransportProfileUriare selected as before.2. A closed Session does not send requests without authenticationToken.
authenticationTokenis a session-less invocation (OPC 10000-4 §6.3.1).CloseAsync(closeChannel: false), a request sent through the sameSessionobject went out that way. A server that supports session-less invocation processed it as one instead of rejecting it.Sessionnow records that a session was created (SessionCreatedwith a non-null cookie;OpenAsyncnow calls it through the override instead ofbase.SessionCreated). From then onUpdateRequestHeaderfails a request without token locally withBad_SessionIdInvalid, and the request is never sent. CreateSession and ActivateSession are exempt, so reopening and reactivating keep working. A Session that was never created behaves as before.Changes
src/Opc.Ua.Client/CoreClientUtils.cs:IsOpenApiEndpoint, used in both selection passes; the<returns>documentation says that an OpenAPI endpoint is never returned.src/Opc.Ua.Client/Session/Session.cs: overrides ofSessionCreatedandUpdateRequestHeader(both virtual inSessionClient/ClientBase), onevolatile boolfield.Tests
Opc.Ua.Client.Tests,CoreClientUtilsSelectEndpointTests):SelectEndpointSkipsTheOpenApiEndpointListedFirst,SelectEndpointWithSecurityFallbackSkipsTheOpenApiEndpoint,SelectEndpointSkipsTheWssOpenApiEndpoint,SelectEndpointReturnsNullWhenOnlyOpenApiEndpointsMatch(with and withoutuseSecurity),SelectEndpointKeepsTheFirstBinaryEndpointWithoutTransportProfile.Opc.Ua.Client.Tests,SessionTestswith the channel mock):RequestAfterCloseFailsLocallyWithBadSessionIdInvalidAsync(the Read never reaches the channel),ActivateSessionIsStillSentAfterCloseAsync,RequestAfterANewSessionIsCreatedCarriesItsTokenAsync,RequestBeforeASessionIsCreatedIsNotRefusedAsync.Opc.Ua.Sessions.TestsClientTest.ReconnectSessionOnAlternateChannelAsynccloses the Session and then the channel. The read that follows now fails withBad_SessionIdInvalidbefore it reaches the closed channel, and the test expects that code instead of the codes of the closed channel (on master it getsBad_NotConnected).CoreClientUtils.csandSession.csfrom master, all five SelectEndpoint tests that involve an OpenAPI endpoint (6 cases) fail, as doesRequestAfterCloseFailsLocallyWithBadSessionIdInvalidAsync, andReconnectSessionOnAlternateChannelAsyncfails withBad_NotConnected.SelectEndpointKeepsTheFirstBinaryEndpointWithoutTransportProfileand the three other Session tests pass there by design: they pin behaviour that must not change.Opc.Ua.Client.Tests2743/2743 andOpc.Ua.Sessions.Tests672 passed / 287 skipped / 0 failed on net10.0 and net8.0 (macOS).Opc.Ua.Clientbuilds for net48, net8.0, net9.0 and net10.0 with 0 warnings;dotnet format(whitespace, style, analyzers) reports nothing on the changed lines; changed lines covered 26/26 (100 %) with the two suites above.Opc.Ua.Interop.TestsLegacyClientExtendedCheckAsync("ComplexTypes")/LoadAndDecodeLegacyComplexTypesAsync("1 of N structures were not decoded"), which came with Add 1.5.378 <-> 2.0 interop tests; return BadAttributeIdInvalid for unset optional security attributes #4630 and fails the same way on other open pull requests; this change does not touch ComplexTypes.Notes for review
Bad_SessionIdInvalidis raised locally instead of whatever the server or the closed channel answers. Reconnect paths are not affected, because they use ActivateSession or a new Session object. If you prefer to keep part 2 out, its commit can be dropped without affecting part 1.ClientFixture) is left as it is; the upstream test servers do not need the change.Related Issues
Checklist