Conversation
Implement OPC UA Reverse Connect at the stack and transport layers. Add ReverseHelloMessage encoding/decoding, client-side reverse connect transport with channel FSM, and server-side reverse connect transport that initiates connections to clients behind firewalls.
Extend OpcUaClient and DiscoveryClient with reverse connect factory methods that accept a pre-established transport. Add ReverseConnectManager and ReverseConnectHandle to OpcUaServer for managing outbound connections to clients. Update SessionFsm to support reverse connect lifecycle.
Add unit tests for ReverseHelloMessage encoding/decoding and ReverseConnectChannelFsm state transitions. Add integration tests for reverse connect client sessions and discovery. Include a Prosys reverse connect client example.
There was a problem hiding this comment.
Pull request overview
This PR adds OPC UA Reverse Connect support end-to-end (stack-core protocol message + transport implementations/FSMs + SDK server/client APIs), including documentation, examples, and a comprehensive set of unit/integration tests to validate the handshake, reconnection behavior, and discovery flows.
Changes:
- Introduces
ReverseHelloMessage(RHE) support in stack-core encoding/decoding and message type mapping. - Adds reverse-connect transports + FSMs for client (listening) and server (outbound connector/backoff + idle-socket invariant signaling).
- Adds SDK integration (
ReverseConnectManager,OpcUaServerAPIs,OpcUaClient.createReverseConnect()andDiscoveryClientoverloads) plus docs/examples/tests.
Reviewed changes
Copilot reviewed 37 out of 38 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| opc-ua-stack/transport/src/test/java/org/eclipse/milo/opcua/stack/transport/server/uasc/UascServerReverseHelloHandlerTest.java | Unit tests for server-side RHE handler behavior (RHE send, Hello/Ack, deadline). |
| opc-ua-stack/transport/src/test/java/org/eclipse/milo/opcua/stack/transport/server/tcp/ReverseConnectConnectionFsmTest.java | Unit tests for server reverse-connect connection FSM (backoff, stop, transitions). |
| opc-ua-stack/transport/src/test/java/org/eclipse/milo/opcua/stack/transport/server/tcp/OpcTcpReverseConnectServerTransportTest.java | Unit tests for outbound reverse-connect server transport connect success/failure. |
| opc-ua-stack/transport/src/test/java/org/eclipse/milo/opcua/stack/transport/client/uasc/UascClientReverseHelloHandlerTest.java | Unit tests for client-side RHE handler (whitelist, timeouts, error handling). |
| opc-ua-stack/transport/src/test/java/org/eclipse/milo/opcua/stack/transport/client/tcp/ReverseConnectChannelFsmTest.java | Unit tests for client reverse-connect channel FSM (connect/get/disconnect/reconnect). |
| opc-ua-stack/transport/src/main/java/org/eclipse/milo/opcua/stack/transport/server/uasc/UascServerReverseHelloHandler.java | Server Netty handler that sends ReverseHello on channel activation. |
| opc-ua-stack/transport/src/main/java/org/eclipse/milo/opcua/stack/transport/server/uasc/UascServerHelloHandler.java | Adjusts visibility to enable clean subclassing for reverse-connect handshake. |
| opc-ua-stack/transport/src/main/java/org/eclipse/milo/opcua/stack/transport/server/uasc/UascServerAsymmetricHandler.java | Fires a user event after OpenSecureChannelResponse to signal SecureChannel opened. |
| opc-ua-stack/transport/src/main/java/org/eclipse/milo/opcua/stack/transport/server/uasc/SecureChannelOpenedEvent.java | Defines the Netty user event used by reverse-connect server FSM. |
| opc-ua-stack/transport/src/main/java/org/eclipse/milo/opcua/stack/transport/server/tcp/ReverseConnectConnectionFsm.java | Server reverse-connect connection lifecycle FSM (connect/backoff/active/stop). |
| opc-ua-stack/transport/src/main/java/org/eclipse/milo/opcua/stack/transport/server/tcp/ReverseConnectConfigBuilder.java | Builder for server reverse-connect timing/backoff configuration. |
| opc-ua-stack/transport/src/main/java/org/eclipse/milo/opcua/stack/transport/server/tcp/ReverseConnectConfig.java | Interface for server reverse-connect config values. |
| opc-ua-stack/transport/src/main/java/org/eclipse/milo/opcua/stack/transport/server/tcp/OpcTcpReverseConnectServerTransport.java | Outbound Netty connector wiring server pipeline for reverse-connect sockets. |
| opc-ua-stack/transport/src/main/java/org/eclipse/milo/opcua/stack/transport/client/uasc/UascClientReverseHelloHandler.java | Client reverse-connect handshake handler (RHE → Hello → Ack → UASC handler install). |
| opc-ua-stack/transport/src/main/java/org/eclipse/milo/opcua/stack/transport/client/tcp/ReverseConnectChannelFsm.java | Client reverse-connect channel FSM (accepted connection → handshake → connected/reconnect). |
| opc-ua-stack/transport/src/main/java/org/eclipse/milo/opcua/stack/transport/client/tcp/OpcTcpReverseConnectTransportConfigBuilder.java | Builder for client reverse-connect transport config (listen addr, whitelist, timeouts). |
| opc-ua-stack/transport/src/main/java/org/eclipse/milo/opcua/stack/transport/client/tcp/OpcTcpReverseConnectTransportConfig.java | Client reverse-connect transport config interface. |
| opc-ua-stack/transport/src/main/java/org/eclipse/milo/opcua/stack/transport/client/tcp/OpcTcpReverseConnectTransport.java | Client transport that listens and accepts inbound server reverse-connect sockets. |
| opc-ua-stack/transport/src/main/java/org/eclipse/milo/opcua/stack/transport/client/tcp/OpcTcpClientTransport.java | Adds ChannelStateObservable support for forward-connect transport. |
| opc-ua-stack/transport/src/main/java/org/eclipse/milo/opcua/stack/transport/client/ChannelStateObservable.java | Transport-agnostic connection state observer interface for session reconnection. |
| opc-ua-stack/stack-core/src/test/java/org/eclipse/milo/opcua/stack/core/channel/messages/ReverseHelloMessageTest.java | Unit tests for ReverseHello message encode/decode and limits. |
| opc-ua-stack/stack-core/src/main/java/org/eclipse/milo/opcua/stack/core/channel/messages/TcpMessageEncoder.java | Adds encoding support for ReverseHello. |
| opc-ua-stack/stack-core/src/main/java/org/eclipse/milo/opcua/stack/core/channel/messages/TcpMessageDecoder.java | Adds decoding support for ReverseHello. |
| opc-ua-stack/stack-core/src/main/java/org/eclipse/milo/opcua/stack/core/channel/messages/ReverseHelloMessage.java | New protocol message type for reverse-connect initiation. |
| opc-ua-stack/stack-core/src/main/java/org/eclipse/milo/opcua/stack/core/channel/messages/MessageType.java | Adds ReverseHello message type mapping (“RHE”). |
| opc-ua-sdk/sdk-server/src/main/java/org/eclipse/milo/opcua/sdk/server/ReverseConnectManager.java | SDK manager enforcing idle socket invariant and dynamic registration lifecycle. |
| opc-ua-sdk/sdk-server/src/main/java/org/eclipse/milo/opcua/sdk/server/ReverseConnectHandle.java | Handle type for reverse-connect registrations. |
| opc-ua-sdk/sdk-server/src/main/java/org/eclipse/milo/opcua/sdk/server/OpcUaServer.java | Server integration: configure/start/stop reverse-connect manager + add/remove APIs. |
| opc-ua-sdk/sdk-client/src/main/java/org/eclipse/milo/opcua/sdk/client/session/SessionFsmFactory.java | Uses ChannelStateObservable for transport-agnostic connection loss detection. |
| opc-ua-sdk/sdk-client/src/main/java/org/eclipse/milo/opcua/sdk/client/session/SessionFsm.java | Stores transport listener with new ChannelStateObservable key. |
| opc-ua-sdk/sdk-client/src/main/java/org/eclipse/milo/opcua/sdk/client/OpcUaClient.java | Adds createReverseConnect() factory (two-pass discovery, keep listener open). |
| opc-ua-sdk/sdk-client/src/main/java/org/eclipse/milo/opcua/sdk/client/DiscoveryClient.java | Adds reverse-connect overloads for getEndpoints() and findServers(). |
| opc-ua-sdk/integration-tests/src/test/java/org/eclipse/milo/opcua/stack/transport/client/tcp/OpcTcpReverseConnectTransportTest.java | Transport integration tests for reverse-connect client transport against real server. |
| opc-ua-sdk/integration-tests/src/test/java/org/eclipse/milo/opcua/sdk/client/ReverseConnectTest.java | End-to-end SDK integration tests for reverse-connect sessions and reconnection. |
| opc-ua-sdk/integration-tests/src/test/java/org/eclipse/milo/opcua/sdk/client/ReverseConnectDiscoveryTest.java | Integration tests for reverse-connect discovery helpers on a fixed listen port. |
| milo-examples/client-examples/src/main/java/org/eclipse/milo/examples/client/ReverseConnectExampleProsys.java | Example showing reverse-connect client usage with Prosys Simulation Server. |
| docs/architecture/reverse-connect.md | Architecture/design document for reverse-connect layering, FSMs, and config. |
| .gitignore | Ignores .mcp.json. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
...ain/java/org/eclipse/milo/opcua/stack/transport/server/uasc/UascServerAsymmetricHandler.java
Outdated
Show resolved
Hide resolved
opc-ua-sdk/sdk-server/src/main/java/org/eclipse/milo/opcua/sdk/server/OpcUaServer.java
Outdated
Show resolved
Hide resolved
...re/src/main/java/org/eclipse/milo/opcua/stack/core/channel/messages/ReverseHelloMessage.java
Outdated
Show resolved
Hide resolved
...re/src/main/java/org/eclipse/milo/opcua/stack/core/channel/messages/ReverseHelloMessage.java
Outdated
Show resolved
Hide resolved
...core/src/main/java/org/eclipse/milo/opcua/stack/core/channel/messages/TcpMessageDecoder.java
Show resolved
Hide resolved
This comment was marked as resolved.
This comment was marked as resolved.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…tack/core/channel/messages/ReverseHelloMessage.java Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…tack/core/channel/messages/ReverseHelloMessage.java Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Make ReverseConnectManager thread-safe with a ReentrantLock that protects the connections and handlesByClientUrl maps, firing FSM events outside the lock to prevent lock-ordering deadlocks. Change OpcUaServer.shutdown() to stop the reverse connect manager non-blockingly so shutdown can be called from I/O or FSM executor threads without deadlocking. In ReverseConnectChannelFsm, introduce a ConnectFuture wrapper to eliminate raw-type warnings, execute pipeline operations on the channel's event loop, and pass the pre-decoded ReverseHelloMessage directly to the handler instead of re-encoding and replaying bytes. In OpcUaClient, use the FSM Disconnect event instead of closing the raw channel, and add an orTimeout to disconnectAsync's closeSession.
Add shutdownCompletesWithoutDeadlock integration test that verifies OpcUaServer.shutdown() completes when called from a single-threaded executor, catching the deadlock fixed in the previous commit. Add ReverseConnectChannelFsm tests for Disconnect event transitions. Add ReverseConnectManagerTest with unit tests for lifecycle, handle management, idle connection spawning, and concurrent stop safety.
Add a section to testing.md explaining -pl, -am, and -amd flags with examples and a rule of thumb for when to use each.
|
@kevinherron I've opened a new pull request, #1717, to work on those changes. Once the pull request is ready, I'll request review from you. |
|
@kevinherron I've opened a new pull request, #1718, to work on those changes. Once the pull request is ready, I'll request review from you. |
Corrects 6 discrepancies found by cross-referencing the doc against the current implementation: RHE dispatch mechanism (direct call, not pipeline re-encode), shutdown order, connect() signature, KEY_CF type, missing test entries, and missing test classes.
…reChannelResponse (#1717) * Initial plan * Fire SecureChannelOpenedEvent from writeAndFlush success listener Co-authored-by: kevinherron <340273+kevinherron@users.noreply.github.com> Agent-Logs-Url: https://github.com/eclipse-milo/milo/sessions/4b9455b2-5b0d-443e-a0b7-4cb32f655663 --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: kevinherron <340273+kevinherron@users.noreply.github.com>
* Initial plan * Replace assert with runtime UaException checks in TcpMessageDecoder Co-authored-by: kevinherron <340273+kevinherron@users.noreply.github.com> Agent-Logs-Url: https://github.com/eclipse-milo/milo/sessions/6f91137a-a9a1-424e-a62a-ebea7b8ddf6d --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: kevinherron <340273+kevinherron@users.noreply.github.com>
…nnect transport - Fail pending ConnectFuture on Disconnect in NotConnected state in ReverseConnectChannelFsm, preventing orphaned futures when disconnectAsync() is called before a server connects. - Reset serverBootstrap/serverChannel on bind failure in OpcTcpReverseConnectTransport.startListening() so subsequent connect() calls can retry instead of being permanently stuck. - Return a failed CompletableFuture from connect() on bind failure instead of throwing synchronously, ensuring downstream future chains (disconnect cleanup, timeouts) are properly constructed.
- Add configurable connectTimeout (default 60s) to OpcTcpReverseConnectTransportConfig for bounding one-shot discovery flows without affecting long-lived connections. - Apply .orTimeout() to connectAsync() in DiscoveryClient.getEndpoints, DiscoveryClient.findServers, and OpcUaClient.createReverseConnect so callers don't hang indefinitely waiting for a server to connect. - Chain disconnectAsync() to future completion in DiscoveryClient using handle/thenCompose instead of fire-and-forget whenComplete, ensuring the listening socket is fully closed before the result future completes.
- Move the !running check inside synchronized(pendingRegistrations) in addReverseConnect to close a TOCTOU race with start() that could orphan registrations. - Clean pendingRegistrations in removeReverseConnect so handles removed before start() don't get materialized into FSMs. - Rebuild stale FSMs on restart: snapshot handles from the prior start/stop cycle and replace their terminal-state FSMs with fresh ones, restoring connectivity without requiring re-registration.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 42 out of 43 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
opc-ua-sdk/sdk-client/src/main/java/org/eclipse/milo/opcua/sdk/client/OpcUaClient.java
Outdated
Show resolved
Hide resolved
...ua-sdk/sdk-server/src/main/java/org/eclipse/milo/opcua/sdk/server/ReverseConnectManager.java
Outdated
Show resolved
Hide resolved
...main/java/org/eclipse/milo/opcua/stack/transport/server/tcp/ReverseConnectConnectionFsm.java
Show resolved
Hide resolved
...n/java/org/eclipse/milo/opcua/stack/transport/client/uasc/UascClientReverseHelloHandler.java
Show resolved
Hide resolved
...in/java/org/eclipse/milo/opcua/stack/transport/client/tcp/OpcTcpReverseConnectTransport.java
Show resolved
Hide resolved
...ation-tests/src/test/java/org/eclipse/milo/opcua/sdk/client/ReverseConnectDiscoveryTest.java
Show resolved
Hide resolved
…/client/OpcUaClient.java Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…ack/transport/client/uasc/UascClientReverseHelloHandler.java Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…/server/ReverseConnectManager.java Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Clear KEY_RETRY_DELAY_MS in Disconnected state cleanup to avoid stale backoff state. Log serverChannel.localAddress() instead of config listen address so the actual bound port is visible when using ephemeral port 0.
Add an internal transition in the Handshaking state that closes channels from duplicate ConnectionAccepted events. Without this, a second inbound connection during handshake would be silently dropped by the FSM, leaving the child channel orphaned (never closed).
…r support Introduce a new transport model that allows multiple OPC UA servers to reverse-connect through a single client listening port, complementing the existing 1-1 transport where each server requires its own port. Stack transport layer: - OpcTcpMultiplexedReverseConnectTransport: per-server transport that receives channels from a shared listener instead of owning a socket - ChannelConsumerRegistry: dispatches inbound channels by ServerUri - InboundChannelTransport: abstraction for inbound channel delivery - EndpointResolver: on-demand endpoint discovery (1-shot and 2-shot) - Refactor ReverseConnectChannelFsm to accept a ChannelFsmConfig record, decoupling it from any specific transport type SDK client layer: - MultiplexedReverseConnectListener: owns the ServerBootstrap, decodes ReverseHello, and dispatches to per-server transports - ClientCustomizer/ClientListener: hooks for on-demand client creation - Config and builder classes for both listener and transport Tests and docs: - Unit tests for EndpointResolver, InboundChannelTransport, transport, and listener - Integration test for end-to-end multiplexed reverse connect - Example client for Prosys OPC UA Simulation Server - Update architecture doc with multiplexed transport model
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 59 out of 60 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
...n/java/org/eclipse/milo/opcua/stack/transport/client/uasc/UascClientReverseHelloHandler.java
Show resolved
Hide resolved
| var handler = | ||
| new UascClientReverseHelloHandler( | ||
| transportConfig, | ||
| applicationContext, | ||
| requestId::getAndIncrement, | ||
| hf, | ||
| Set.of(), // allowedServerUris: empty = accept all (already validated) | ||
| 0L); // reverseHelloTimeoutMs: 0 = no timeout (already received) | ||
|
|
There was a problem hiding this comment.
Passing reverseHelloTimeoutMs = 0L here relies on UascClientReverseHelloHandler interpreting 0 as "no timeout". Currently the handler schedules a ReverseHello timeout in channelActive()/handlerAdded(), so a 0ms timeout can race and fail the handshake before onReverseHello() cancels it. Either pass a positive timeout and/or update the handler to explicitly disable the ReverseHello timer when the ReverseHello is pre-decoded.
...dk/sdk-client/src/main/java/org/eclipse/milo/opcua/sdk/client/session/SessionFsmFactory.java
Show resolved
Hide resolved
...ation-tests/src/test/java/org/eclipse/milo/opcua/sdk/client/ReverseConnectDiscoveryTest.java
Show resolved
Hide resolved
AbstractRemoteAddressFilter removes itself from the pipeline during fireChannelRegistered(), racing with the test thread's assertion. Check for the handler inside the customizer callback instead, where initChannel() guarantees it is still present.
…p-alive close Skip scheduling the ReverseHello timeout when reverseHelloTimeoutMs <= 0 to prevent a racy 0ms wheel-timer task on channels where the ReverseHello was already pre-decoded. Add OpcTcpMultiplexedReverseConnectTransport to closeTransportChannel so keep-alive failure can force-close its channel.
Replace \[.*] with \[[^]]*] in ENDPOINT_URL_PATTERN to prevent polynomial backtracking on crafted inputs. The greedy .* could match ']' characters, creating ambiguity exploitable for denial of service (CodeQL java/polynomial-redos).
Add coverage for the ReverseHello timeout race guard (AtomicBoolean dual-path start), keep-alive channel close handling across all three transport types, and the single-argument createTransport overload.
The Handshaking entry action set KEY_CHANNEL to the raw TCP channel before the OPN handshake completed, allowing getChannel()'s fast path to return a partially initialized channel with no UASC message handler. During reconnection this caused ActivateSession write failures and retry storms that widened the window for the server-side session to be bound to a stale secure channel id, producing Bad_SecureChannelIdInvalid on the subsequent Read. Only set KEY_CHANNEL in the Connected entry action, after the handshake succeeds and the pipeline is fully initialized. During Handshaking, getChannel() now correctly falls through to the GetChannel event path which chains to KEY_CF and resolves when Connected is reached. Also replace the fragile random-port-with-retry allocation in TestServer.create() with ephemeral port 0 on the loopback address.
Closes #1715
Summary
ReverseHelloMessage), transport (client listener + server connector with FSMs), and SDK (serverReverseConnectManager, clientcreateReverseConnect()factory)ServerBootstrapto accept inbound connections, validatesReverseHelloagainst an optionalallowedServerUriswhitelist, then completes the standard Hello/Ack/OpenSecureChannel handshakeArchitecture
See docs/architecture/reverse-connect.md for the full design document covering protocol flow, layering decisions, FSM state diagrams, and configuration options.
Implementation details
Protocol (
stack-core)ReverseHelloMessage— encode/decode with 4096-byte field limits per specMessageType.ReverseHello(RHE) enum valueTransport layer
OpcTcpReverseConnectTransport,ReverseConnectChannelFsm(NotConnected → Handshaking → Connected → Reconnecting),UascClientReverseHelloHandlerOpcTcpReverseConnectServerTransport,ReverseConnectConnectionFsm(Idle → Connecting → RheSent → Active, with ConnectWait for backoff),UascServerReverseHelloHandlerChannelStateObservableinterface for transport-agnostic session reconnectionSDK layer
ReverseConnectManagerwith dynamic add/remove, idle socket invariant enforcement, and pre-start registration support;OpcUaServerintegration viasetReverseConnectManager()/addReverseConnect()/removeReverseConnect()OpcUaClient.createReverseConnect()with two-pass discovery;DiscoveryClientreverse connect overloadsTest plan
ReverseHelloMessageTest— encode/decode round-trip and validation edge casesUascClientReverseHelloHandlerTest— handshake sequence, server URI filtering, error handlingReverseConnectChannelFsmTest— client FSM state transitions and reconnectionUascServerReverseHelloHandlerTest— RHE send on channel active, Hello/Ack flowOpcTcpReverseConnectServerTransportTest— server outbound connection behaviorReverseConnectConnectionFsmTest— server FSM transitions, backoff, and idle socket invariantOpcTcpReverseConnectTransportTest— client transport end-to-end (listen, accept, handshake)ReverseConnectTest— SDK-level integration test (full client ↔ server reverse connect session)ReverseConnectDiscoveryTest— discovery via reverse connectmvn clean verifypasses