Skip to content

Harden EventFilter validation and event delivery isolation - #1952

Merged
kevinherron merged 7 commits into
integration/1.2from
fix/event-filter-delivery-12
Sep 8, 2026
Merged

Harden EventFilter validation and event delivery isolation#1952
kevinherron merged 7 commits into
integration/1.2from
fix/event-filter-delivery-12

Conversation

@kevinherron

Copy link
Copy Markdown
Contributor

Several independent defects in the server-side EventFilter delivery path let a single monitored item's filter interfere with event delivery to other items, or let filters that Part 4 says are invalid through validation only to fail on every event afterwards.

EventContentFilter.decodeOperands cast each decoded operand to FilterOperand unchecked, so an operand that decodes to some other structure raised ClassCastException and one with an unknown encoding id raised UaSerializationException. Neither is a UaException, so both escaped MonitoredEventItem.onEvent. Related to that, installFilter only looked at select clause and element status codes when deciding whether to arm the filter, so an element whose operand had been reported as Bad_FilterOperandInvalid still armed. And ServerEventNotifier.fire notified listeners in a plain loop, so an exception from one listener stopped delivery to every listener registered after it and propagated to the caller.

Operands are now decoded explicitly and reported as Bad_FilterOperandInvalid, operand status codes participate in the arming decision, onEvent contains any exception from evaluation, and fire isolates each listener while leaving notifier-scope resolution and per-listener scope filtering unchanged. The item result for a filter that validates with errors is unchanged: the item is created with Good and the EventFilterResult carries the per-clause codes, as before.

Two further issues in ElementOperand handling are addressed. validate recorded Good for every ElementOperand; it now enforces Part 4 §7.7.4.2 (the index must be greater than the containing element's index and reference an existing element) so these filters are rejected at CreateMonitoredItems. And the cycle guard added in #1780 tracks only the current evaluation path, so an element referenced from several places in an acyclic filter was evaluated once per path, which is exponential for a chain of And elements that each reference the next element twice. DefaultOperatorContext now caches each element's result, or the UaException it raised, for the lifetime of one event evaluation. This is sound because element evaluation has no side effects within a single event.

On this branch DefaultConditionManager.deliverRefresh already isolated snapshot replay and RefreshEnd delivery per item but delivered RefreshStart in an unguarded loop; it now guards RefreshStart the same way, and tests pin the per-item isolation of the whole bracket and the release of the per-Subscription refresh guard. Tests also cover event items whose filter validates with errors through SubscriptionManager creation and the SetMonitoringMode registration toggle, and a throwing in-scope listener alongside scope filtering in ServerEventNotifier.fire. The onRefreshMarker, overflow, and onConditionEventOverflow paths were reviewed: they only reach select, which converts UaException to a null field, and their callers already guard with catch (Throwable) or catch (Exception), so no change was needed there.

Verification: spotless:apply, clean compile, and the full sdk-server test suite pass, along with the integration tests for event filter operators, event routing, ConditionRefresh, and the new event delivery isolation test. Each new unit test was also run against the unmodified production code to confirm it fails there.

The same fixes are opened separately against main, where the surrounding delivery code has diverged.

…FilterOperand

EventContentFilter.decodeOperands cast each decoded operand to
FilterOperand unchecked. An operand that decodes to some other structure
raised ClassCastException, and one with an unknown encoding id raised
UaSerializationException. Neither is a UaException, so both escaped the
per-event filter evaluation in MonitoredEventItem.

Decode each operand explicitly and report either failure as a UaException
with Bad_FilterOperandInvalid, which the event delivery path already
handles.
EventContentFilter.validate recorded Good for every ElementOperand. Part
4 §7.7.4.2 requires the index to be greater than the index of the element
it is part of and to reference an existing element. Filters that violated
this were accepted at CreateMonitoredItems and only failed later, per
event, when the operand was resolved.

Report such operands as Bad_FilterOperandInvalid in the
ContentFilterElementResult so the filter is rejected up front.
The ElementOperand guard in DefaultOperatorContext tracks the elements on
the current evaluation path, which detects cycles but does not prevent an
element referenced from several places in an acyclic filter from being
evaluated once per path. A chain of And elements that each reference the
next element twice costs 2^n evaluations, since And only short-circuits
on FALSE.

Element evaluation has no side effects within a single event, so cache
each element's result, or the UaException it raised, for the lifetime of
the operator context and reuse it on later references.
MonitoredEventItem.installFilter decided whether the filter could be
evaluated from the select clause and element status codes alone. An
element whose operator validated but whose operand was reported as
Bad_FilterOperandInvalid still armed the filter, and the operand failure
then surfaced on every event instead of in the EventFilterResult.

Treat a bad operand status code the same as a bad element status code:
the item is created with the EventFilterResult describing the error and
the filter is not evaluated.
MonitoredEventItem.onEvent caught UaException only. Any other exception
raised while evaluating the where clause or selecting event fields
propagated to the caller delivering the event.

Catch Exception so a failure evaluating one item's filter is logged
against that item and the delivery loop continues.
ServerEventNotifier.fire notified listeners in a plain loop. An
exception thrown by one listener stopped delivery to every listener
registered after it and propagated to the code firing the event.

Catch and log exceptions per listener so each registered listener is
notified independently of the others. Notifier-scope resolution and
per-listener scope filtering are unchanged.
DefaultConditionManager.deliverRefresh already isolated snapshot replay
and RefreshEnd delivery per item, but delivered the RefreshStart marker
in an unguarded loop. A failure delivering RefreshStart to one item
aborted the refresh before the remaining items received their marker.

Guard RefreshStart delivery per item the same way. An item whose
RefreshStart delivery fails is not treated as started and receives no
further refresh traffic; the other items receive a full bracket. Add
tests pinning the per-item isolation of replay and RefreshEnd delivery
and the release of the per-Subscription refresh guard.
@kevinherron
kevinherron merged commit 3ffa68b into integration/1.2 Sep 8, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant