Skip to content

Fix unordered server endpoint validation - #4029

Merged
marcschier merged 1 commit into
OPCFoundation:masterfrom
MrAlaskan:fix/session-server-endpoints-set-comparison
Jul 20, 2026
Merged

Fix unordered server endpoint validation#4029
marcschier merged 1 commit into
OPCFoundation:masterfrom
MrAlaskan:fix/session-server-endpoints-set-comparison

Conversation

@MrAlaskan

Copy link
Copy Markdown
Contributor

Summary

This PR makes Session.ValidateServerEndpoints() compare ServerEndpoints and UserIdentityTokens as unordered sets and adds regression tests that verify OpenAsync() no longer rejects legal servers that return the same endpoint data in a different order.

Problem

According to OPC UA Part 4, the client validates the endpoint set returned by CreateSessionResponse.ServerEndpoints against the endpoint set observed during discovery. The specification describes these values as a set of endpoint descriptions filtered by the relevant transport profile, not as an ordered array that must preserve the exact discovery-time enumeration order.

Previously, Session.ValidateServerEndpoints() first checked the endpoint count and then compared m_discoveryServerEndpoints[ii] against serverEndpoints[ii] by index. It also compared UserIdentityTokens[jj] by index within each endpoint. As a result, a server that returned the same legal endpoints in a different order, or returned the same token policies in a different order within an endpoint, could be rejected with BadSecurityChecksFailed even though the endpoint data itself had not changed.

Changes

  • Replace index-based ServerEndpoints validation with unordered matching based on the same endpoint fields already validated by the client.
  • Compare UserIdentityTokens as an unordered multiset instead of requiring the server to preserve the original token enumeration order.
  • Extend the client session test scaffolding so discovery endpoints can be supplied explicitly.
  • Add regression tests that verify OpenAsync() accepts reordered ServerEndpoints and reordered UserIdentityTokens.

@marcschier marcschier added the ready Ready to merge once CI Passes label Jul 19, 2026
@codecov

codecov Bot commented Jul 19, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 67.44186% with 14 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.54%. Comparing base (84fa8e8) to head (a0241ff).
⚠️ Report is 4 commits behind head on master.

Files with missing lines Patch % Lines
src/Opc.Ua.Client/Session/Session.cs 67.44% 9 Missing and 5 partials ⚠️

❌ Your patch check has failed because the patch coverage (67.44%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4029      +/-   ##
==========================================
- Coverage   73.85%   73.54%   -0.32%     
==========================================
  Files        1345     1345              
  Lines      179988   180056      +68     
  Branches    31668    31681      +13     
==========================================
- Hits       132938   132415     -523     
- Misses      36299    36928     +629     
+ Partials    10751    10713      -38     
Files with missing lines Coverage Δ
src/Opc.Ua.Client/Session/Session.cs 72.81% <67.44%> (+0.88%) ⬆️

... and 47 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@marcschier

Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@marcschier
marcschier merged commit 8f40876 into OPCFoundation:master Jul 20, 2026
163 of 164 checks passed
marcschier pushed a commit that referenced this pull request Aug 18, 2026
…78] (#4269)

Backport of
8f40876
from `master` to `master378`.

### Summary
Makes `Session.ValidateServerEndpoints()` compare `ServerEndpoints` and
`UserIdentityTokens` as unordered sets and adds regression tests
verifying `OpenAsync()` no longer rejects legal servers that return the
same endpoint data in a different order.

### Problem
Per OPC UA Part 4, the client validates the endpoint set returned by
`CreateSessionResponse.ServerEndpoints` against the set observed during
discovery. The spec describes these as a *set* of endpoint descriptions
filtered by transport profile, not an ordered array. Previously the
validation compared `m_discoveryServerEndpoints[ii]` against
`serverEndpoints[ii]` by index, and `UserIdentityTokens[jj]` by index. A
server returning the same legal endpoints/token policies in a different
order could be wrongly rejected with `BadSecurityChecksFailed`.

### Changes
- Replace index-based `ServerEndpoints` validation with unordered
matching on the same endpoint fields already validated by the client.
- Compare `UserIdentityTokens` as an unordered multiset.
- Extend the client session test scaffolding so discovery endpoints can
be supplied explicitly.
- Add regression tests for reordered `ServerEndpoints` and reordered
`UserIdentityTokens`.

### Notes
Adapted to the `master378` API surface (`EndpointDescriptionCollection`
/ `UserTokenPolicyCollection` / `StringCollection` / `byte[]` server
nonce) and the `Libraries/`/`Tests/` layout. Client tests build and the
new tests pass.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready Ready to merge once CI Passes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants