Skip to content

fix(server): enforce published HistoryUpdate operation limits#4034

Merged
marcschier merged 2 commits into
OPCFoundation:masterfrom
MrAlaskan:fix/historyupdate-operation-limits
Jul 21, 2026
Merged

fix(server): enforce published HistoryUpdate operation limits#4034
marcschier merged 2 commits into
OPCFoundation:masterfrom
MrAlaskan:fix/historyupdate-operation-limits

Conversation

@MrAlaskan

Copy link
Copy Markdown
Contributor

Summary

This PR enforces the published HistoryUpdate operation limits in the server service entry point and adds regression coverage that verifies requests exceeding MaxNodesPerHistoryUpdateData or MaxNodesPerHistoryUpdateEvents are rejected with BadTooManyOperations.

Problem

The server already publishes MaxNodesPerHistoryUpdateData and MaxNodesPerHistoryUpdateEvents through ServerCapabilities.OperationLimits, but StandardServer.HistoryUpdateAsync() did not actually enforce either limit. The method only called the generic ValidateOperationLimits(historyUpdateDetails) overload without passing a concrete limit, which meant it only rejected empty requests with BadNothingToDo and never returned BadTooManyOperations for oversized HistoryUpdate requests.

The existing code already acknowledged this gap: HistoryUpdateAsync() contained a TODO comment stating that the history-update operation limits still needed to be checked elsewhere. In practice, neither StandardServer, MasterNodeManager, nor the downstream node-manager paths consumed MaxNodesPerHistoryUpdateData or MaxNodesPerHistoryUpdateEvents, so the advertised limits were never enforced at runtime.

Changes

  • Enforce HistoryUpdate operation limits in StandardServer.HistoryUpdateAsync() after the existing non-empty request validation.
  • Classify HistoryUpdateDetails into data-oriented vs event-oriented requests and apply MaxNodesPerHistoryUpdateData or MaxNodesPerHistoryUpdateEvents accordingly.
  • Add integration-style regression tests that verify oversized data-update and event-update requests are both rejected with BadTooManyOperations.

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

Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines

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

@codecov

codecov Bot commented Jul 20, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 84.61538% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.12%. Comparing base (84fa8e8) to head (8cd7f20).
⚠️ Report is 18 commits behind head on master.

Files with missing lines Patch % Lines
src/Opc.Ua.Server/Server/StandardServer.cs 84.61% 0 Missing and 2 partials ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4034      +/-   ##
==========================================
- Coverage   73.85%   73.12%   -0.74%     
==========================================
  Files        1345     1332      -13     
  Lines      179988   179102     -886     
  Branches    31668    31540     -128     
==========================================
- Hits       132938   130973    -1965     
- Misses      36299    37496    +1197     
+ Partials    10751    10633     -118     
Files with missing lines Coverage Δ
src/Opc.Ua.Server/Server/StandardServer.cs 73.49% <84.61%> (+0.09%) ⬆️

... and 91 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
marcschier merged commit d1f78e8 into OPCFoundation:master Jul 21, 2026
209 of 210 checks passed
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