From fcb6115c1c2a8588df25e1929a60ba67c495dfd5 Mon Sep 17 00:00:00 2001 From: Jerome Louvel <374450+jlouvel@users.noreply.github.com> Date: Tue, 10 Nov 2026 12:10:36 -0500 Subject: [PATCH 1/8] Add an experimental OpenZiti extension on a Netty client transport SPI The Netty client connector gains the NettyClientTransport SPI, set with the "org.restlet.engine.netty.clientTransport" client context attribute: for each new connection it can return a route (a Netty channel factory and resolver) to the origin's host and port, used instead of TCP and the HTTP and SOCKS proxies. The org.restlet.ext.ziti extension (OpenZiti Java SDK 0.35.0) uses it to route the calls to the origins an OpenZiti identity has a service for over the Ziti overlay network, without resolving their host name with the system DNS. ZitiSocketChannel is a Netty channel over the SDK's asynchronous socket with Netty's semantics: one read at a time, non-blocking writes, every SDK completion run on the event loop. ziti-netty's ZitiChannel isn't used: it fires its read events off the event loop, loses its channel options and blocks on writes, and failed about one run in three under concurrent calls. Client side only. Co-Authored-By: Claude Opus 5.5 --- changes.md | 14 +- org.restlet.ext.ziti/pom.xml | 88 +++ .../src/main/java/module-info.java | 21 + .../restlet/ext/ziti/ZitiClientTransport.java | 83 +++ .../restlet/ext/ziti/ZitiSocketChannel.java | 455 ++++++++++++++ .../org/restlet/ext/ziti/package-info.java | 10 + .../ext/ziti/ZitiClientTransportTestCase.java | 410 +++++++++++++ .../restlet/ext/ziti/ZitiNetworkTestCase.java | 473 +++++++++++++++ .../ext/ziti/ZitiSocketChannelTestCase.java | 572 ++++++++++++++++++ .../engine/netty/NettyClientHelper.java | 50 +- .../engine/netty/NettyClientTransport.java | 56 ++ .../netty/NettyClientTransportTestCase.java | 186 ++++++ pom.xml | 2 + 13 files changed, 2406 insertions(+), 14 deletions(-) create mode 100644 org.restlet.ext.ziti/pom.xml create mode 100644 org.restlet.ext.ziti/src/main/java/module-info.java create mode 100644 org.restlet.ext.ziti/src/main/java/org/restlet/ext/ziti/ZitiClientTransport.java create mode 100644 org.restlet.ext.ziti/src/main/java/org/restlet/ext/ziti/ZitiSocketChannel.java create mode 100644 org.restlet.ext.ziti/src/main/java/org/restlet/ext/ziti/package-info.java create mode 100644 org.restlet.ext.ziti/src/test/java/org/restlet/ext/ziti/ZitiClientTransportTestCase.java create mode 100644 org.restlet.ext.ziti/src/test/java/org/restlet/ext/ziti/ZitiNetworkTestCase.java create mode 100644 org.restlet.ext.ziti/src/test/java/org/restlet/ext/ziti/ZitiSocketChannelTestCase.java create mode 100644 org.restlet/src/main/java/org/restlet/engine/netty/NettyClientTransport.java create mode 100644 org.restlet/src/test/java/org/restlet/engine/netty/NettyClientTransportTestCase.java diff --git a/changes.md b/changes.md index bc90ce64b1..88a4d56af2 100644 --- a/changes.md +++ b/changes.md @@ -8,7 +8,8 @@ Changes log Jetty 12. The connector class names are unchanged (`org.restlet.engine.connector.HttpServerHelper`, `HttpsServerHelper`, `HttpClientHelper`), and the Jetty dependencies are removed. No Netty type is exposed by - the public API: the module requires the Netty modules without `requires transitive`. + the public API, apart from the `NettyClientTransport` connector SPI: the module requires + the Netty modules without `requires transitive`. - Server calls run by default on virtual threads, one per call ("threadPool.virtual" parameter, `true` by default). Set it to `false` to use a pool of platform threads configured by the existing "threadPool.*" parameters. @@ -283,6 +284,17 @@ Changes log default), the session token of temporary credentials and an unsigned payload option are given as parameters of the `ChallengeResponse`. The `HTTP_AWS_S3` scheme still uses Signature Version 2, which AWS no longer accepts for buckets created since June 2020. + - Added the experimental OpenZiti extension (`org.restlet.ext.ziti`, OpenZiti Java SDK + 0.35.0): `ZitiClientTransport.install(client, zitiContext)` routes the calls of an HTTP + client to the origins an OpenZiti identity has a service for, such as + `crm.internal:443`, over the Ziti overlay network, without resolving their host name + with the system DNS, and the other calls over TCP. TLS, HTTP/1.1 and HTTP/2 run over it + as over TCP. Client side only. + - Added the `NettyClientTransport` SPI to the Netty client connector, set with the + "org.restlet.engine.netty.clientTransport" client context attribute: it can route the + connections to some origins through another Netty channel and resolver than TCP, the + HTTP and SOCKS proxies being then bypassed. It exposes Netty types and is meant for + extensions depending on Netty, such as the OpenZiti one. - Bugs fixed - The Netty connectors now flush each full output buffer of a request or response body, so a producer that pauses after filling one no longer holds back what it produced diff --git a/org.restlet.ext.ziti/pom.xml b/org.restlet.ext.ziti/pom.xml new file mode 100644 index 0000000000..3be8b34518 --- /dev/null +++ b/org.restlet.ext.ziti/pom.xml @@ -0,0 +1,88 @@ + + + 4.0.0 + + org.restlet + org.restlet.parent + 3.0.0-SNAPSHOT + ../pom.xml + + + org.restlet.ext.ziti + Restlet Framework - OpenZiti extension + Routes HTTP client calls over an OpenZiti overlay network. + + + + + + org.jetbrains.kotlin + kotlin-stdlib + 2.4.20 + + + org.jetbrains.kotlin + kotlin-reflect + 2.4.20 + + + org.jetbrains + annotations + 23.0.0 + + + com.google.code.gson + gson + ${lib-gson-version} + + + + + + + org.restlet + org.restlet + ${project.version} + + + + org.openziti + ziti + ${lib-ziti-version} + + + + org.slf4j + slf4j-simple + + + com.github.ajalt + clikt + + + + + + com.fasterxml.jackson.core + jackson-annotations + ${lib-jackson-annotations-version} + provided + + + + org.jetbrains.kotlin + kotlin-stdlib + provided + + + org.junit.jupiter + junit-jupiter-api + ${lib-junit-version} + test + + + diff --git a/org.restlet.ext.ziti/src/main/java/module-info.java b/org.restlet.ext.ziti/src/main/java/module-info.java new file mode 100644 index 0000000000..f18e811fbd --- /dev/null +++ b/org.restlet.ext.ziti/src/main/java/module-info.java @@ -0,0 +1,21 @@ +/** + * Copyright 2005-2026 Qlik + * + *

The content of this file is subject to the terms of the Apache 2.0 open source license + * available at https://www.opensource.org/licenses/apache-2.0 + * + *

Restlet is a registered trademark of QlikTech International AB. + */ +@SuppressWarnings({"requires-automatic", "requires-transitive-automatic"}) +module org.restlet.ext.ziti { + requires transitive org.restlet; + // The OpenZiti SDK jars are not modular and declare no Automatic-Module-Name. + requires transitive ziti; + requires edge.api; + requires io.netty.buffer; + requires io.netty.common; + requires io.netty.transport; + requires io.netty.resolver; + + exports org.restlet.ext.ziti; +} diff --git a/org.restlet.ext.ziti/src/main/java/org/restlet/ext/ziti/ZitiClientTransport.java b/org.restlet.ext.ziti/src/main/java/org/restlet/ext/ziti/ZitiClientTransport.java new file mode 100644 index 0000000000..e5ed5c6ae4 --- /dev/null +++ b/org.restlet.ext.ziti/src/main/java/org/restlet/ext/ziti/ZitiClientTransport.java @@ -0,0 +1,83 @@ +/** + * Copyright 2005-2026 Qlik + *

+ * The content of this file is subject to the terms of the Apache 2.0 open + * source license available at https://www.opensource.org/licenses/apache-2.0 + *

+ * Restlet is a registered trademark of QlikTech International AB. + */ +package org.restlet.ext.ziti; + +import io.netty.resolver.NoopAddressResolverGroup; +import java.net.InetSocketAddress; +import org.openziti.ZitiContext; +import org.restlet.Client; +import org.restlet.engine.netty.NettyClientTransport; + +/** + * Routes the HTTP client calls to the origins an OpenZiti identity can reach over the Ziti overlay + * network, and the others over TCP. + * + *

An origin is routed over Ziti when the context has a service intercepting its host and port + * ({@link ZitiContext#getService(InetSocketAddress)}), such as {@code crm.internal:443}. Its host + * name is then never resolved by the system DNS: the connection is a {@link ZitiSocketChannel} that + * dials that service. TLS, HTTP/1.1 and HTTP/2 run over it like over TCP: an {@code https} origin + * is encrypted end to end on top of the overlay's own encryption. + * + *

+ * ZitiContext ztx = Ziti.newContext(identityFile, new char[0]);
+ * Client client = new Client(Protocol.HTTPS);
+ * ZitiClientTransport.install(client, ztx);
+ * 
+ * + *

The context is not owned: destroy it after the client is stopped. + */ +public class ZitiClientTransport implements NettyClientTransport { + + /** The Ziti context. */ + private final ZitiContext context; + + /** The route over Ziti, shared by every intercepted origin. */ + private final Route route; + + /** + * Constructor. + * + * @param context The Ziti context of the identity used to dial the services. + */ + public ZitiClientTransport(ZitiContext context) { + this.context = context; + // The channel maps the host and port to the Ziti service itself. + this.route = + new Route(() -> new ZitiSocketChannel(context), NoopAddressResolverGroup.INSTANCE); + } + + /** + * Routes a client's calls to the origins the Ziti identity can reach over Ziti. + * + * @param client The client, before it is started. + * @param context The Ziti context of the identity used to dial the services. + * @return The transport set on the client. + */ + public static ZitiClientTransport install(Client client, ZitiContext context) { + ZitiClientTransport transport = new ZitiClientTransport(context); + client.getContext().getAttributes().put(NettyClientTransport.ATTRIBUTE, transport); + return transport; + } + + /** + * Returns the Ziti context. + * + * @return The Ziti context. + */ + public ZitiContext getContext() { + return context; + } + + @Override + public Route route(String host, int port) { + return (context.getService(InetSocketAddress.createUnresolved(host, port)) != null) + ? route + : null; + } +} diff --git a/org.restlet.ext.ziti/src/main/java/org/restlet/ext/ziti/ZitiSocketChannel.java b/org.restlet.ext.ziti/src/main/java/org/restlet/ext/ziti/ZitiSocketChannel.java new file mode 100644 index 0000000000..8234269936 --- /dev/null +++ b/org.restlet.ext.ziti/src/main/java/org/restlet/ext/ziti/ZitiSocketChannel.java @@ -0,0 +1,455 @@ +/** + * Copyright 2005-2026 Qlik + *

+ * The content of this file is subject to the terms of the Apache 2.0 open + * source license available at https://www.opensource.org/licenses/apache-2.0 + *

+ * Restlet is a registered trademark of QlikTech International AB. + */ +package org.restlet.ext.ziti; + +import io.netty.buffer.ByteBuf; +import io.netty.channel.AbstractChannel; +import io.netty.channel.ChannelConfig; +import io.netty.channel.ChannelMetadata; +import io.netty.channel.ChannelOutboundBuffer; +import io.netty.channel.ChannelPromise; +import io.netty.channel.ConnectTimeoutException; +import io.netty.channel.DefaultChannelConfig; +import io.netty.channel.EventLoop; +import java.io.IOException; +import java.net.ConnectException; +import java.net.InetSocketAddress; +import java.net.SocketAddress; +import java.nio.ByteBuffer; +import java.nio.channels.AsynchronousSocketChannel; +import java.nio.channels.ClosedChannelException; +import java.nio.channels.CompletionHandler; +import java.nio.channels.UnresolvedAddressException; +import java.util.concurrent.RejectedExecutionException; +import java.util.concurrent.ScheduledFuture; +import java.util.concurrent.TimeUnit; +import org.openziti.ZitiAddress; +import org.openziti.ZitiContext; +import org.openziti.edge.model.ServiceDetail; + +/** + * A Netty channel over a connection of the OpenZiti SDK ({@link ZitiContext#open()}), with the + * semantics of Netty's own socket channels. + * + *

+ * + *

It replaces {@code org.openziti.netty.ZitiChannel} of ziti-netty 0.35.0, which fires its read + * events from the thread completing the read: the SDK releases its pending read before calling the + * completion handler, so a read requested during the delivery starts at once, and completes + * concurrently on another thread or nested in the call, delivering bytes out of order. Its {@code + * config()} also returns a new configuration on each call, so channel options such as {@code + * AUTO_READ} are lost, and it blocks the event loop on every write. + * + *

The channel connects to an unresolved {@link InetSocketAddress} such as {@code + * crm.internal:443}: it dials the Ziti service that intercepts it, so the host name is never + * resolved by the system DNS. A {@link ZitiAddress} is dialed as is. + */ +final class ZitiSocketChannel extends AbstractChannel { + + private static final ChannelMetadata METADATA = new ChannelMetadata(false); + + /** The size of the buffer of a read, the payload size of a Ziti message. */ + private static final int READ_BUFFER_SIZE = 16 * 1024; + + /** The Ziti context dialing the services. */ + private final ZitiContext context; + + /** The socket of the Ziti connection. */ + private final AsynchronousSocketChannel socket; + + private final ChannelConfig config; + + /** Whether the connection is established. */ + private volatile boolean active; + + /** Whether a read is pending on the socket. Only used on the event loop. */ + private boolean readPending; + + /** Whether a write is in flight on the socket. Only used on the event loop. */ + private boolean writeInFlight; + + /** + * Constructor. + * + * @param context The Ziti context dialing the services. + */ + ZitiSocketChannel(ZitiContext context) { + super(null); + this.context = context; + this.socket = context.open(); + this.config = new DefaultChannelConfig(this); + } + + @Override + public ChannelConfig config() { + return config; + } + + @Override + public ChannelMetadata metadata() { + return METADATA; + } + + @Override + public boolean isOpen() { + return socket.isOpen(); + } + + @Override + public boolean isActive() { + return active && socket.isOpen(); + } + + @Override + protected boolean isCompatible(EventLoop loop) { + return true; + } + + @Override + protected SocketAddress localAddress0() { + try { + return socket.getLocalAddress(); + } catch (IOException e) { + return null; + } + } + + @Override + protected SocketAddress remoteAddress0() { + try { + return socket.getRemoteAddress(); + } catch (IOException e) { + return null; + } + } + + @Override + protected void doBind(SocketAddress localAddress) { + throw new UnsupportedOperationException("A Ziti connection can't be bound"); + } + + @Override + protected void doDisconnect() throws IOException { + doClose(); + } + + @Override + protected void doClose() throws IOException { + active = false; + socket.close(); + } + + @Override + protected Object filterOutboundMessage(Object msg) { + if (msg instanceof ByteBuf) { + return msg; + } + throw new UnsupportedOperationException( + "Unsupported message type: " + msg.getClass().getName()); + } + + /** Runs a completion of the SDK on the event loop; returns false if the loop refused it. */ + private boolean onEventLoop(Runnable task) { + try { + eventLoop().execute(task); + return true; + } catch (RejectedExecutionException e) { + // The event loop is shutting down, with the channel closed. + return false; + } + } + + @Override + protected void doBeginRead() { + if (readPending || !isActive()) { + return; + } + + readPending = true; + ByteBuf buffer = config.getAllocator().heapBuffer(READ_BUFFER_SIZE); + ByteBuffer target = buffer.nioBuffer(buffer.writerIndex(), buffer.writableBytes()); + + try { + socket.read(target, null, new ReadHandler(buffer)); + } catch (RuntimeException e) { + readPending = false; + buffer.release(); + throw e; + } + } + + /** Fires the bytes of a completed read, on the event loop. */ + private void readCompleted(ByteBuf buffer, int read) { + readPending = false; + + if (read < 0) { + buffer.release(); + pipeline().fireChannelReadComplete(); + unsafe().close(voidPromise()); + return; + } + + buffer.writerIndex(buffer.writerIndex() + read); + pipeline().fireChannelRead(buffer); + pipeline().fireChannelReadComplete(); + + if (config.isAutoRead()) { + read(); + } + } + + /** Reports a failed read, on the event loop. */ + private void readFailed(ByteBuf buffer, Throwable cause) { + readPending = false; + buffer.release(); + + if (isOpen()) { + pipeline().fireExceptionCaught(cause); + unsafe().close(voidPromise()); + } + } + + /** Completes a read on the event loop. */ + private final class ReadHandler implements CompletionHandler { + + private final ByteBuf buffer; + + ReadHandler(ByteBuf buffer) { + this.buffer = buffer; + } + + @Override + public void completed(Integer read, Void attachment) { + if (!onEventLoop(() -> readCompleted(buffer, read))) { + buffer.release(); + } + } + + @Override + public void failed(Throwable cause, Void attachment) { + if (!onEventLoop(() -> readFailed(buffer, cause))) { + buffer.release(); + } + } + } + + @Override + protected void doWrite(ChannelOutboundBuffer in) { + if (writeInFlight) { + // Resumed by the completion of the write in flight. + return; + } + + Object msg; + while ((msg = in.current()) != null) { + ByteBuf buffer = (ByteBuf) msg; + + if (!buffer.isReadable()) { + in.remove(); + continue; + } + + // Retained until the write completes: closing the channel meanwhile releases the + // pending messages while the SDK may still read this one. + writeInFlight = true; + buffer.retain(); + try { + socket.write(buffer.nioBuffer(), null, new WriteHandler(buffer)); + } catch (RuntimeException e) { + writeInFlight = false; + buffer.release(); + throw e; + } + return; + } + } + + /** Accounts for a completed write and starts the next one, on the event loop. */ + private void writeCompleted(int written) { + writeInFlight = false; + ChannelOutboundBuffer in = unsafe().outboundBuffer(); + + if (in != null) { + // Moves the reader index of the buffer written and releases it once fully written. + in.removeBytes(written); + unsafe().flush(); + } + } + + /** Fails the pending writes and closes the channel, on the event loop. */ + private void writeFailed(Throwable cause) { + writeInFlight = false; + + if (isOpen()) { + pipeline().fireExceptionCaught(cause); + } + + unsafe().close(voidPromise()); + } + + /** Completes a write on the event loop. */ + private final class WriteHandler implements CompletionHandler { + + /** The buffer written, retained for the write. */ + private final ByteBuf buffer; + + WriteHandler(ByteBuf buffer) { + this.buffer = buffer; + } + + @Override + public void completed(Integer written, Void attachment) { + buffer.release(); + onEventLoop(() -> writeCompleted(written)); + } + + @Override + public void failed(Throwable cause, Void attachment) { + buffer.release(); + onEventLoop(() -> writeFailed(cause)); + } + } + + /** + * Returns the Ziti address to dial for a remote address. + * + * @param remote An unresolved {@code host:port} or a Ziti address. + * @return The Ziti address. + */ + private SocketAddress dialAddress(SocketAddress remote) { + if (remote instanceof ZitiAddress) { + return remote; + } + + if (remote instanceof InetSocketAddress address) { + ServiceDetail service = context.getService(address); + + if (service != null) { + return new ZitiAddress.Dial(service.getName(), null, null, null); + } + + throw new UnresolvedAddressException(); + } + + throw new UnsupportedOperationException( + "Unsupported address type: " + remote.getClass().getName()); + } + + @Override + protected AbstractUnsafe newUnsafe() { + return new ZitiUnsafe(); + } + + /** Connects with the SDK's asynchronous connect, completed on the event loop. */ + private final class ZitiUnsafe extends AbstractUnsafe { + + @Override + public void connect( + SocketAddress remoteAddress, SocketAddress localAddress, ChannelPromise promise) { + if (!promise.setUncancellable() || !ensureOpen(promise)) { + return; + } + + SocketAddress dial; + try { + dial = dialAddress(remoteAddress); + } catch (RuntimeException e) { + promise.tryFailure(e); + closeIfClosed(); + return; + } + + int timeoutMillis = config.getConnectTimeoutMillis(); + ScheduledFuture timeout = + (timeoutMillis > 0) + ? eventLoop() + .schedule( + () -> { + if (promise.tryFailure( + new ConnectTimeoutException( + "Connection timed out after " + + timeoutMillis + + " ms: " + + remoteAddress))) { + close(voidPromise()); + } + }, + timeoutMillis, + TimeUnit.MILLISECONDS) + : null; + + try { + socket.connect( + dial, + null, + new CompletionHandler() { + @Override + public void completed(Void result, Void attachment) { + onEventLoop(() -> connected(promise, timeout)); + } + + @Override + public void failed(Throwable cause, Void attachment) { + onEventLoop( + () -> + connectFailed( + promise, timeout, remoteAddress, cause)); + } + }); + } catch (RuntimeException e) { + connectFailed(promise, timeout, remoteAddress, e); + } + } + + private void connected(ChannelPromise promise, ScheduledFuture timeout) { + if (timeout != null) { + timeout.cancel(false); + } + + active = true; + + if (promise.trySuccess()) { + pipeline().fireChannelActive(); + } else { + // Timed out or cancelled meanwhile. + close(voidPromise()); + } + } + + private void connectFailed( + ChannelPromise promise, + ScheduledFuture timeout, + SocketAddress remoteAddress, + Throwable cause) { + if (timeout != null) { + timeout.cancel(false); + } + + Throwable failure = cause; + if (cause instanceof IOException && !(cause instanceof ClosedChannelException)) { + ConnectException exception = + new ConnectException(cause.getMessage() + ": " + remoteAddress); + exception.initCause(cause); + failure = exception; + } + + promise.tryFailure(failure); + closeIfClosed(); + } + } +} diff --git a/org.restlet.ext.ziti/src/main/java/org/restlet/ext/ziti/package-info.java b/org.restlet.ext.ziti/src/main/java/org/restlet/ext/ziti/package-info.java new file mode 100644 index 0000000000..8643704ce4 --- /dev/null +++ b/org.restlet.ext.ziti/src/main/java/org/restlet/ext/ziti/package-info.java @@ -0,0 +1,10 @@ +/** + * Integration with OpenZiti, experimental. This extension routes the calls of the HTTP client + * connector to the origins an OpenZiti identity can reach over the Ziti overlay network, and the + * other calls over TCP, see {@link org.restlet.ext.ziti.ZitiClientTransport}. Only the client side + * is supported. + * + * @since Restlet 3.0 + * @see OpenZiti home + */ +package org.restlet.ext.ziti; diff --git a/org.restlet.ext.ziti/src/test/java/org/restlet/ext/ziti/ZitiClientTransportTestCase.java b/org.restlet.ext.ziti/src/test/java/org/restlet/ext/ziti/ZitiClientTransportTestCase.java new file mode 100644 index 0000000000..f918ba7ff8 --- /dev/null +++ b/org.restlet.ext.ziti/src/test/java/org/restlet/ext/ziti/ZitiClientTransportTestCase.java @@ -0,0 +1,410 @@ +/** + * Copyright 2005-2026 Qlik + *

+ * The content of this file is subject to the terms of the Apache 2.0 open + * source license available at https://www.opensource.org/licenses/apache-2.0 + *

+ * Restlet is a registered trademark of QlikTech International AB. + */ +package org.restlet.ext.ziti; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.io.IOException; +import java.io.InputStream; +import java.lang.reflect.Proxy; +import java.net.InetSocketAddress; +import java.net.SocketAddress; +import java.net.SocketOption; +import java.nio.ByteBuffer; +import java.nio.channels.AsynchronousSocketChannel; +import java.nio.channels.CompletionHandler; +import java.nio.file.Path; +import java.util.Arrays; +import java.util.Set; +import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; +import org.openziti.ZitiContext; +import org.openziti.edge.model.ServiceDetail; +import org.restlet.Client; +import org.restlet.Context; +import org.restlet.Request; +import org.restlet.Response; +import org.restlet.Restlet; +import org.restlet.Server; +import org.restlet.data.MediaType; +import org.restlet.data.Method; +import org.restlet.data.Protocol; +import org.restlet.data.Status; +import org.restlet.representation.ByteArrayRepresentation; + +/** + * Runs the Ziti socket channel under the Restlet client connector. Only the overlay network is + * replaced: the fake {@link ZitiContext} intercepts {@code crm.internal}, and its {@link + * ZitiContext#open()} returns an asynchronous socket that connects to a local Restlet server + * instead of dialing a Ziti service. + */ +class ZitiClientTransportTestCase { + + private static final String SERVICE_HOST = "crm.internal"; + + private static final int SERVICE_PORT = 8443; + + /** The local port the fake overlay connects the service to. */ + private int targetPort; + + /** The number of channels opened through the fake overlay. */ + private final AtomicInteger opened = new AtomicInteger(); + + private Server server; + + private Client client; + + @AfterEach + void stop() throws Exception { + if (client != null) { + client.stop(); + } + if (server != null) { + server.stop(); + } + } + + /** A Ziti context intercepting crm.internal:8443 and dialing the local server. */ + private ZitiContext fakeZiti() { + return (ZitiContext) + Proxy.newProxyInstance( + ZitiContext.class.getClassLoader(), + new Class[] {ZitiContext.class}, + (proxy, method, args) -> + switch (method.getName()) { + case "getService" -> + (args[0] instanceof InetSocketAddress a + && SERVICE_HOST.equals( + a.getHostString()) + && a.getPort() == SERVICE_PORT) + ? new ServiceDetail().name("crm") + : null; + case "open" -> { + opened.incrementAndGet(); + yield new OverlaySocket( + AsynchronousSocketChannel.open(), + new InetSocketAddress("127.0.0.1", targetPort)); + } + case "name", "toString" -> "fake-ziti"; + case "hashCode" -> System.identityHashCode(proxy); + case "equals" -> proxy == args[0]; + default -> + throw new UnsupportedOperationException( + method.getName()); + }); + } + + /** Echoes the path and the number of bytes received, then the received bytes if asked. */ + private static Restlet echo() { + return new Restlet() { + @Override + public void handle(Request request, Response response) { + byte[] body = new byte[0]; + try { + if (request.isEntityAvailable()) { + try (InputStream in = request.getEntity().getStream()) { + body = in.readAllBytes(); + } + } + } catch (IOException e) { + response.setStatus(Status.SERVER_ERROR_INTERNAL, e); + return; + } + if (request.getResourceRef().getPath().endsWith("/echo")) { + response.setEntity( + new ByteArrayRepresentation(body, MediaType.APPLICATION_OCTET_STREAM)); + } else { + response.setEntity( + request.getResourceRef().getPath() + " " + body.length, + MediaType.TEXT_PLAIN); + } + } + }; + } + + private void startHttp() throws Exception { + server = new Server(new Context(), Protocol.HTTP, 0, echo()); + server.start(); + targetPort = server.getActualPort(); + client = new Client(new Context(), Protocol.HTTP); + ZitiClientTransport.install(client, fakeZiti()); + client.start(); + } + + @Test + void routesAnInterceptedOriginOverZiti() throws Exception { + startHttp(); + + Response response = client.handle(new Request(Method.GET, "http://crm.internal:8443/hi")); + + assertEquals(Status.SUCCESS_OK, response.getStatus()); + assertEquals("/hi 0", response.getEntity().getText()); + assertEquals(1, opened.get(), "The connection should be dialed through Ziti"); + } + + @Test + void reusesTheZitiConnection() throws Exception { + startHttp(); + + for (int i = 0; i < 3; i++) { + Response response = + client.handle(new Request(Method.GET, "http://crm.internal:8443/hi/" + i)); + assertEquals("/hi/" + i + " 0", response.getEntity().getText()); + } + + assertEquals(1, opened.get(), "Keep-alive should reuse the Ziti connection"); + } + + @Test + void streamsLargeBodiesBothWays() throws Exception { + startHttp(); + byte[] body = new byte[2 * 1024 * 1024]; + for (int i = 0; i < body.length; i++) { + body[i] = (byte) (i * 31); + } + + Response response = + client.handle( + new Request( + Method.POST, + "http://crm.internal:8443/echo", + new ByteArrayRepresentation( + body, MediaType.APPLICATION_OCTET_STREAM))); + + assertEquals(Status.SUCCESS_OK, response.getStatus()); + byte[] received; + try (InputStream in = response.getEntity().getStream()) { + received = in.readAllBytes(); + } + assertTrue(Arrays.equals(body, received), "The 2 MiB body should come back intact"); + } + + @Test + void connectsToOtherOriginsOverTcp() throws Exception { + startHttp(); + + Response response = + client.handle( + new Request(Method.GET, "http://localhost:" + targetPort + "/direct")); + + assertEquals("/direct 0", response.getEntity().getText()); + assertEquals(0, opened.get(), "An origin Ziti doesn't intercept should use TCP"); + } + + @Test + void routesOnlyInterceptedOrigins() { + ZitiClientTransport transport = new ZitiClientTransport(fakeZiti()); + + assertTrue(transport.route(SERVICE_HOST, SERVICE_PORT) != null); + assertNull(transport.route(SERVICE_HOST, 80)); + assertNull(transport.route("example.org", SERVICE_PORT)); + } + + /** TLS runs end to end over the Ziti channel, with the host name of the service verified. */ + @Test + void runsTlsOverZiti(@TempDir Path dir) throws Exception { + Path keyStore = dir.resolve("crm.p12"); + Process keytool = + new ProcessBuilder( + Path.of(System.getProperty("java.home"), "bin", "keytool") + .toString(), + "-genkeypair", + "-alias", + "crm", + "-keyalg", + "EC", + "-groupname", + "secp256r1", + "-dname", + "CN=" + SERVICE_HOST, + "-ext", + "SAN=dns:" + SERVICE_HOST, + "-validity", + "2", + "-storetype", + "PKCS12", + "-keystore", + keyStore.toString(), + "-storepass", + "changeit", + "-keypass", + "changeit") + .redirectErrorStream(true) + .start(); + keytool.getInputStream().readAllBytes(); + assertTrue(keytool.waitFor(60, TimeUnit.SECONDS) && keytool.exitValue() == 0); + + Context serverContext = new Context(); + serverContext.getParameters().add("keyStorePath", keyStore.toString()); + serverContext.getParameters().add("keyStorePassword", "changeit"); + serverContext.getParameters().add("keyPassword", "changeit"); + serverContext.getParameters().add("keyStoreType", "PKCS12"); + server = new Server(serverContext, Protocol.HTTPS, 0, echo()); + server.start(); + targetPort = server.getActualPort(); + + Context clientContext = new Context(); + clientContext.getParameters().add("trustStorePath", keyStore.toString()); + clientContext.getParameters().add("trustStorePassword", "changeit"); + clientContext.getParameters().add("trustStoreType", "PKCS12"); + client = new Client(clientContext, Protocol.HTTPS); + ZitiClientTransport.install(client, fakeZiti()); + client.start(); + + Response response = + client.handle(new Request(Method.GET, "https://crm.internal:8443/secure")); + + assertEquals(Status.SUCCESS_OK, response.getStatus(), String.valueOf(response.getStatus())); + assertEquals("/secure 0", response.getEntity().getText()); + assertEquals(1, opened.get()); + } + + /** + * An asynchronous socket standing for a Ziti connection: it connects to the local server + * whatever unresolved service address it is given. + */ + private static final class OverlaySocket extends AsynchronousSocketChannel { + + private final AsynchronousSocketChannel socket; + + private final InetSocketAddress target; + + OverlaySocket(AsynchronousSocketChannel socket, InetSocketAddress target) { + super(socket.provider()); + this.socket = socket; + this.target = target; + } + + @Override + public AsynchronousSocketChannel bind(SocketAddress local) throws IOException { + socket.bind(local); + return this; + } + + @Override + public AsynchronousSocketChannel setOption(SocketOption name, T value) + throws IOException { + socket.setOption(name, value); + return this; + } + + @Override + public T getOption(SocketOption name) throws IOException { + return socket.getOption(name); + } + + @Override + public Set> supportedOptions() { + return socket.supportedOptions(); + } + + @Override + public AsynchronousSocketChannel shutdownInput() throws IOException { + socket.shutdownInput(); + return this; + } + + @Override + public AsynchronousSocketChannel shutdownOutput() throws IOException { + socket.shutdownOutput(); + return this; + } + + @Override + public SocketAddress getRemoteAddress() throws IOException { + return socket.getRemoteAddress(); + } + + @Override + public void connect( + SocketAddress remote, A attachment, CompletionHandler handler) { + socket.connect(target, attachment, handler); + } + + @Override + public Future connect(SocketAddress remote) { + return socket.connect(target); + } + + @Override + public void read( + ByteBuffer dst, + long timeout, + TimeUnit unit, + A attachment, + CompletionHandler handler) { + socket.read(dst, timeout, unit, attachment, handler); + } + + @Override + public Future read(ByteBuffer dst) { + return socket.read(dst); + } + + @Override + public void read( + ByteBuffer[] dsts, + int offset, + int length, + long timeout, + TimeUnit unit, + A attachment, + CompletionHandler handler) { + socket.read(dsts, offset, length, timeout, unit, attachment, handler); + } + + @Override + public void write( + ByteBuffer src, + long timeout, + TimeUnit unit, + A attachment, + CompletionHandler handler) { + socket.write(src, timeout, unit, attachment, handler); + } + + @Override + public Future write(ByteBuffer src) { + return socket.write(src); + } + + @Override + public void write( + ByteBuffer[] srcs, + int offset, + int length, + long timeout, + TimeUnit unit, + A attachment, + CompletionHandler handler) { + socket.write(srcs, offset, length, timeout, unit, attachment, handler); + } + + @Override + public SocketAddress getLocalAddress() throws IOException { + return socket.getLocalAddress(); + } + + @Override + public boolean isOpen() { + return socket.isOpen(); + } + + @Override + public void close() throws IOException { + socket.close(); + } + } +} diff --git a/org.restlet.ext.ziti/src/test/java/org/restlet/ext/ziti/ZitiNetworkTestCase.java b/org.restlet.ext.ziti/src/test/java/org/restlet/ext/ziti/ZitiNetworkTestCase.java new file mode 100644 index 0000000000..140ecae6bf --- /dev/null +++ b/org.restlet.ext.ziti/src/test/java/org/restlet/ext/ziti/ZitiNetworkTestCase.java @@ -0,0 +1,473 @@ +/** + * Copyright 2005-2026 Qlik + *

+ * The content of this file is subject to the terms of the Apache 2.0 open + * source license available at https://www.opensource.org/licenses/apache-2.0 + *

+ * Restlet is a registered trademark of QlikTech International AB. + */ +package org.restlet.ext.ziti; + +import static org.junit.jupiter.api.Assertions.assertArrayEquals; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.io.File; +import java.io.IOException; +import java.io.InputStream; +import java.net.InetAddress; +import java.net.InetSocketAddress; +import java.net.UnknownHostException; +import java.nio.ByteBuffer; +import java.nio.channels.AsynchronousServerSocketChannel; +import java.nio.channels.AsynchronousSocketChannel; +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.Callable; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.condition.EnabledIfSystemProperty; +import org.junit.jupiter.api.io.TempDir; +import org.openziti.Ziti; +import org.openziti.ZitiAddress; +import org.openziti.ZitiContext; +import org.restlet.Client; +import org.restlet.Context; +import org.restlet.Request; +import org.restlet.Response; +import org.restlet.Restlet; +import org.restlet.Server; +import org.restlet.data.MediaType; +import org.restlet.data.Method; +import org.restlet.data.Protocol; +import org.restlet.data.Status; +import org.restlet.representation.ByteArrayRepresentation; + +/** + * Calls a Restlet server over a real OpenZiti network with {@link ZitiClientTransport}. + * + *

Runs only when given two enrolled identities: {@code -Dziti.client.identity=client.json}, + * which may dial the {@code crm} service, intercepted as {@code crm.internal:8443}, and {@code + * -Dziti.server.identity=server.json}, which may bind it. The server identity binds the service + * with the SDK's server API and pipes each Ziti connection to a local Restlet server, so a request + * goes: Restlet client, Ziti socket channel, edge router, Ziti server socket, Restlet server. + */ +@EnabledIfSystemProperty(named = "ziti.client.identity", matches = ".+") +class ZitiNetworkTestCase { + + private static final String SERVICE = "crm"; + + private static final String HOST = "crm.internal"; + + private static ZitiContext clientZiti; + + private static ZitiContext serverZiti; + + private static AsynchronousServerSocketChannel binding; + + private static ExecutorService bridge; + + /** The local port the bridge pipes the Ziti connections to. */ + private static volatile int targetPort; + + private static Server server; + + @BeforeAll + static void start() throws Exception { + clientZiti = + Ziti.newContext(new File(System.getProperty("ziti.client.identity")), new char[0]); + serverZiti = + Ziti.newContext(new File(System.getProperty("ziti.server.identity")), new char[0]); + + // A context learns its services asynchronously after authenticating: binding before the + // server identity has loaded the service is refused as "no permission to bind". + long bindDeadline = System.nanoTime() + TimeUnit.SECONDS.toNanos(60); + while (serverZiti.getService(SERVICE) == null) { + assertTrue(System.nanoTime() < bindDeadline, "The server never saw the crm service"); + Thread.sleep(250); + } + + // Host the service: accept the Ziti connections and pipe them to the local server. + binding = serverZiti.openServer(); + binding.bind(new ZitiAddress.Bind(SERVICE, null, false), 64); + bridge = Executors.newVirtualThreadPerTaskExecutor(); + bridge.submit(ZitiNetworkTestCase::acceptLoop); + + // The client learns its services asynchronously after authenticating. + long deadline = System.nanoTime() + TimeUnit.SECONDS.toNanos(60); + while (clientZiti.getService(InetSocketAddress.createUnresolved(HOST, 8443)) == null) { + assertTrue(System.nanoTime() < deadline, "The client never saw the crm service"); + Thread.sleep(250); + } + + // The bind registers a terminator asynchronously: dialing before gives "no terminators". + while (clientZiti + .getServiceTerminators( + clientZiti.getService(InetSocketAddress.createUnresolved(HOST, 8443))) + .isEmpty()) { + assertTrue(System.nanoTime() < deadline, "The crm service never got a terminator"); + Thread.sleep(250); + } + } + + @AfterAll + static void stop() throws Exception { + if (server != null) { + server.stop(); + } + if (binding != null) { + binding.close(); + } + if (bridge != null) { + bridge.shutdownNow(); + } + if (clientZiti != null) { + clientZiti.destroy(); + } + if (serverZiti != null) { + serverZiti.destroy(); + } + } + + private static Void acceptLoop() throws Exception { + while (binding.isOpen()) { + AsynchronousSocketChannel ziti = binding.accept().get(); + bridge.submit(() -> pipe(ziti)); + } + return null; + } + + /** Pipes one Ziti connection to the local server, both ways. */ + private static Void pipe(AsynchronousSocketChannel ziti) throws Exception { + try (ziti; + AsynchronousSocketChannel local = AsynchronousSocketChannel.open()) { + local.connect(new InetSocketAddress(InetAddress.getLoopbackAddress(), targetPort)) + .get(); + Future up = bridge.submit(copy(ziti, local)); + copy(local, ziti).call(); + up.get(); + } + return null; + } + + private static Callable copy( + AsynchronousSocketChannel from, AsynchronousSocketChannel to) { + return () -> { + ByteBuffer buffer = ByteBuffer.allocate(64 * 1024); + try { + while (from.read(buffer).get() >= 0) { + buffer.flip(); + while (buffer.hasRemaining()) { + to.write(buffer).get(); + } + buffer.clear(); + } + to.shutdownOutput(); + } catch (Exception e) { + // The other side closed: the pipe ends. + } + return null; + }; + } + + /** Echoes the path and the size of the body, or the body itself on /echo. */ + private static Restlet echo() { + return new Restlet() { + @Override + public void handle(Request request, Response response) { + byte[] body = new byte[0]; + try { + if (request.isEntityAvailable()) { + try (InputStream in = request.getEntity().getStream()) { + body = in.readAllBytes(); + } + } + } catch (IOException e) { + response.setStatus(Status.SERVER_ERROR_INTERNAL, e); + return; + } + if (request.getResourceRef().getPath().endsWith("/echo")) { + response.setEntity( + new ByteArrayRepresentation(body, MediaType.APPLICATION_OCTET_STREAM)); + } else { + response.setEntity( + request.getResourceRef().getPath() + " " + body.length, + MediaType.TEXT_PLAIN); + } + } + }; + } + + private static void startServer(Protocol protocol, Context context) throws Exception { + if (server != null) { + server.stop(); + } + server = new Server(context, protocol, "127.0.0.1", 0, echo()); + server.start(); + targetPort = server.getActualPort(); + } + + private static Client zitiClient(Protocol protocol, Context context) throws Exception { + Client client = new Client(context, protocol); + ZitiClientTransport.install(client, clientZiti); + client.start(); + return client; + } + + @Test + void theServiceHostIsNotInTheDns() { + assertThrows(UnknownHostException.class, () -> InetAddress.getByName(HOST)); + } + + @Test + void callsOverZitiWithKeepAliveAndLargeBodies() throws Exception { + startServer(Protocol.HTTP, new Context()); + Client client = zitiClient(Protocol.HTTP, new Context()); + try { + for (int i = 0; i < 3; i++) { + Response response = + client.handle(new Request(Method.GET, "http://crm.internal:8443/hi/" + i)); + assertEquals(Status.SUCCESS_OK, response.getStatus(), "call " + i); + assertEquals("/hi/" + i + " 0", response.getEntity().getText()); + } + + byte[] body = new byte[4 * 1024 * 1024]; + for (int i = 0; i < body.length; i++) { + body[i] = (byte) (i * 31 + 7); + } + Response response = + client.handle( + new Request( + Method.POST, + "http://crm.internal:8443/echo", + new ByteArrayRepresentation( + body, MediaType.APPLICATION_OCTET_STREAM))); + assertEquals(Status.SUCCESS_OK, response.getStatus()); + try (InputStream in = response.getEntity().getStream()) { + assertArrayEquals(body, in.readAllBytes(), "4 MiB should come back intact"); + } + } finally { + client.stop(); + } + } + + @Test + void callsConcurrentlyOverZiti() throws Exception { + startServer(Protocol.HTTP, new Context()); + Client client = zitiClient(Protocol.HTTP, new Context()); + try (ExecutorService callers = Executors.newFixedThreadPool(8)) { + List> results = new ArrayList<>(); + for (int i = 0; i < 32; i++) { + String path = "/concurrent/" + i; + results.add( + callers.submit( + () -> { + Response response = + client.handle( + new Request( + Method.GET, + "http://crm.internal:8443" + path)); + if (response.getEntity() == null) { + Throwable cause = response.getStatus().getThrowable(); + return response.getStatus() + + (cause == null ? "" : " caused by " + cause); + } + return response.getEntity().getText(); + })); + } + for (int i = 0; i < results.size(); i++) { + assertEquals("/concurrent/" + i + " 0", results.get(i).get(60, TimeUnit.SECONDS)); + } + } finally { + client.stop(); + } + } + + /** + * The same concurrent calls to the {@code crm2} service, intercepted as {@code + * crm2.internal:8443} and hosted by the edge router itself (a {@code host.v1} config forwarding + * to {@code 127.0.0.1:}), so that no Java SDK hosts the service: only the + * client side of the connections is the Restlet transport. + */ + @Test + @EnabledIfSystemProperty(named = "ziti.router.port", matches = "\\d+") + void callsConcurrentlyARouterHostedService() throws Exception { + InetSocketAddress service = InetSocketAddress.createUnresolved("crm2.internal", 8443); + long deadline = System.nanoTime() + TimeUnit.SECONDS.toNanos(60); + while (clientZiti.getService(service) == null + || clientZiti.getServiceTerminators(clientZiti.getService(service)).isEmpty()) { + assertTrue(System.nanoTime() < deadline, "The crm2 service is not reachable"); + Thread.sleep(250); + } + + Server routerTarget = + new Server( + new Context(), + Protocol.HTTP, + "127.0.0.1", + Integer.parseInt(System.getProperty("ziti.router.port")), + echo()); + routerTarget.start(); + Client client = zitiClient(Protocol.HTTP, new Context()); + try (ExecutorService callers = Executors.newFixedThreadPool(8)) { + List> results = new ArrayList<>(); + for (int i = 0; i < 32; i++) { + String path = "/router/" + i; + results.add( + callers.submit( + () -> { + Response response = + client.handle( + new Request( + Method.GET, + "http://crm2.internal:8443" + path)); + if (response.getEntity() == null) { + Throwable cause = response.getStatus().getThrowable(); + return response.getStatus() + + (cause == null ? "" : " caused by " + cause); + } + return response.getEntity().getText(); + })); + } + for (int i = 0; i < results.size(); i++) { + assertEquals("/router/" + i + " 0", results.get(i).get(60, TimeUnit.SECONDS)); + } + } finally { + client.stop(); + routerTarget.stop(); + } + } + + /** + * Control experiment: the same 32 concurrent calls to the router-hosted {@code crm2} service + * with the plain SDK ({@link ZitiContext#connect(String, int)} sockets and raw HTTP), without + * Netty nor Restlet on the client side. + */ + @Test + @EnabledIfSystemProperty(named = "ziti.router.port", matches = "\\d+") + void callsConcurrentlyWithThePlainSdk() throws Exception { + Server routerTarget = + new Server( + new Context(), + Protocol.HTTP, + "127.0.0.1", + Integer.parseInt(System.getProperty("ziti.router.port")), + echo()); + routerTarget.start(); + try (ExecutorService callers = Executors.newFixedThreadPool(8)) { + List> results = new ArrayList<>(); + for (int i = 0; i < 32; i++) { + String path = "/raw/" + i; + results.add( + callers.submit( + () -> { + try (java.net.Socket socket = + clientZiti.connect("crm2.internal", 8443)) { + socket.setSoTimeout(30_000); + socket.getOutputStream() + .write( + ("GET " + + path + + " HTTP/1.1\r\n" + + "Host: crm2.internal\r\n" + + "Connection: close\r\n\r\n") + .getBytes( + java.nio.charset + .StandardCharsets + .US_ASCII)); + socket.getOutputStream().flush(); + return new String( + socket.getInputStream().readAllBytes(), + java.nio.charset.StandardCharsets.US_ASCII); + } catch (Exception e) { + return "failed: " + e; + } + })); + } + int failed = 0; + StringBuilder failures = new StringBuilder(); + for (int i = 0; i < results.size(); i++) { + String response = results.get(i).get(90, TimeUnit.SECONDS); + if (!response.endsWith("/raw/" + i + " 0")) { + failed++; + failures.append("\n ") + .append(i) + .append(": ") + .append( + response.length() > 160 + ? response.substring(0, 160) + : response); + } + } + assertEquals(0, failed, "Plain SDK calls failed:" + failures); + } finally { + routerTarget.stop(); + } + } + + /** TLS runs end to end over Ziti, the server certificate checked against crm.internal. */ + @Test + void callsHttpsOverZiti(@TempDir Path dir) throws Exception { + Path keyStore = dir.resolve("crm.p12"); + Process keytool = + new ProcessBuilder( + Path.of(System.getProperty("java.home"), "bin", "keytool") + .toString(), + "-genkeypair", + "-alias", + "crm", + "-keyalg", + "EC", + "-groupname", + "secp256r1", + "-dname", + "CN=" + HOST, + "-ext", + "SAN=dns:" + HOST, + "-validity", + "2", + "-storetype", + "PKCS12", + "-keystore", + keyStore.toString(), + "-storepass", + "changeit", + "-keypass", + "changeit") + .redirectErrorStream(true) + .start(); + keytool.getInputStream().readAllBytes(); + assertTrue(keytool.waitFor(60, TimeUnit.SECONDS) && keytool.exitValue() == 0); + + Context serverContext = new Context(); + serverContext.getParameters().add("keyStorePath", keyStore.toString()); + serverContext.getParameters().add("keyStorePassword", "changeit"); + serverContext.getParameters().add("keyPassword", "changeit"); + serverContext.getParameters().add("keyStoreType", "PKCS12"); + startServer(Protocol.HTTPS, serverContext); + + Context clientContext = new Context(); + clientContext.getParameters().add("trustStorePath", keyStore.toString()); + clientContext.getParameters().add("trustStorePassword", "changeit"); + clientContext.getParameters().add("trustStoreType", "PKCS12"); + Client client = zitiClient(Protocol.HTTPS, clientContext); + try { + Response response = + client.handle(new Request(Method.GET, "https://crm.internal:8443/secure")); + assertEquals( + Status.SUCCESS_OK, response.getStatus(), String.valueOf(response.getStatus())); + assertNotNull(response.getEntity()); + assertEquals("/secure 0", response.getEntity().getText()); + } finally { + client.stop(); + } + } +} diff --git a/org.restlet.ext.ziti/src/test/java/org/restlet/ext/ziti/ZitiSocketChannelTestCase.java b/org.restlet.ext.ziti/src/test/java/org/restlet/ext/ziti/ZitiSocketChannelTestCase.java new file mode 100644 index 0000000000..d47ae31c80 --- /dev/null +++ b/org.restlet.ext.ziti/src/test/java/org/restlet/ext/ziti/ZitiSocketChannelTestCase.java @@ -0,0 +1,572 @@ +/** + * Copyright 2005-2026 Qlik + *

+ * The content of this file is subject to the terms of the Apache 2.0 open + * source license available at https://www.opensource.org/licenses/apache-2.0 + *

+ * Restlet is a registered trademark of QlikTech International AB. + */ +package org.restlet.ext.ziti; + +import static org.junit.jupiter.api.Assertions.assertArrayEquals; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import io.netty.buffer.ByteBuf; +import io.netty.buffer.Unpooled; +import io.netty.channel.ChannelFuture; +import io.netty.channel.ChannelHandlerContext; +import io.netty.channel.ChannelInboundHandlerAdapter; +import io.netty.channel.ChannelOption; +import io.netty.channel.ConnectTimeoutException; +import io.netty.channel.EventLoopGroup; +import io.netty.channel.MultiThreadIoEventLoopGroup; +import io.netty.channel.nio.NioIoHandler; +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.lang.reflect.Proxy; +import java.net.ConnectException; +import java.net.InetSocketAddress; +import java.net.SocketAddress; +import java.net.SocketOption; +import java.nio.ByteBuffer; +import java.nio.channels.AsynchronousSocketChannel; +import java.nio.channels.CompletionHandler; +import java.nio.channels.UnresolvedAddressException; +import java.nio.channels.spi.AsynchronousChannelProvider; +import java.nio.charset.StandardCharsets; +import java.util.ArrayDeque; +import java.util.Queue; +import java.util.Set; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.CopyOnWriteArrayList; +import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.function.Consumer; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.openziti.ZitiAddress; +import org.openziti.ZitiContext; +import org.openziti.edge.model.ServiceDetail; + +/** + * Checks that {@link ZitiSocketChannel} gives a Ziti connection the semantics of Netty's own socket + * channels, over a scripted SDK socket. + */ +class ZitiSocketChannelTestCase { + + private static final InetSocketAddress SERVICE = + InetSocketAddress.createUnresolved("crm.internal", 443); + + private EventLoopGroup group; + + private ScriptedSocket socket; + + private ZitiSocketChannel channel; + + /** What the pipeline received. */ + private final Recorder recorder = new Recorder(); + + @BeforeEach + void start() throws Exception { + group = new MultiThreadIoEventLoopGroup(1, NioIoHandler.newFactory()); + socket = new ScriptedSocket(); + channel = new ZitiSocketChannel(fakeZiti(socket)); + channel.config().setAutoRead(false); + channel.pipeline().addLast(recorder); + group.register(channel).sync(); + } + + @AfterEach + void stop() throws Exception { + channel.close().sync(); + group.shutdownGracefully(0, 1, TimeUnit.SECONDS).sync(); + } + + /** A Ziti context intercepting crm.internal:443 with the "crm" service. */ + private static ZitiContext fakeZiti(AsynchronousSocketChannel socket) { + return (ZitiContext) + Proxy.newProxyInstance( + ZitiContext.class.getClassLoader(), + new Class[] {ZitiContext.class}, + (proxy, method, args) -> + switch (method.getName()) { + case "getService" -> + SERVICE.equals(args[0]) + ? new ServiceDetail().name("crm") + : null; + case "open" -> socket; + case "toString" -> "fake-ziti"; + case "hashCode" -> System.identityHashCode(proxy); + case "equals" -> proxy == args[0]; + default -> + throw new UnsupportedOperationException( + method.getName()); + }); + } + + /** Runs a task on the event loop and waits for it. */ + private void onEventLoop(Runnable task) throws Exception { + channel.eventLoop().submit(task).get(5, TimeUnit.SECONDS); + } + + private void connect() throws Exception { + channel.connect(SERVICE).sync(); + } + + @Test + void dialsTheServiceInterceptingTheOrigin() throws Exception { + connect(); + + assertTrue(channel.isActive()); + ZitiAddress.Dial dialed = assertInstanceOf(ZitiAddress.Dial.class, socket.dialed); + assertEquals("crm", dialed.getService()); + assertTrue(recorder.active.get()); + } + + @Test + void dialsAZitiAddressAsIs() throws Exception { + ZitiAddress.Dial address = new ZitiAddress.Dial("billing", null, null, null); + + channel.connect(address).sync(); + + assertSame(address, socket.dialed); + } + + @Test + void failsToConnectToAnOriginWithoutService() { + ChannelFuture future = + channel.connect(InetSocketAddress.createUnresolved("other.internal", 443)) + .awaitUninterruptibly(); + + assertInstanceOf(UnresolvedAddressException.class, future.cause()); + assertEquals(0, socket.connects.get()); + } + + @Test + void reportsAConnectFailure() { + socket.connectFailure = new IOException("no terminators"); + + ChannelFuture future = channel.connect(SERVICE).awaitUninterruptibly(); + + ConnectException failure = assertInstanceOf(ConnectException.class, future.cause()); + assertSame(socket.connectFailure, failure.getCause()); + assertFalse(channel.isActive()); + } + + @Test + void timesOutAConnect() { + socket.connectHangs = true; + channel.config().setOption(ChannelOption.CONNECT_TIMEOUT_MILLIS, 100); + + ChannelFuture future = channel.connect(SERVICE).awaitUninterruptibly(); + + assertInstanceOf(ConnectTimeoutException.class, future.cause()); + assertTrue(channel.closeFuture().awaitUninterruptibly(5, TimeUnit.SECONDS)); + } + + @Test + void refusesToBind() { + ChannelFuture future = channel.bind(new InetSocketAddress(0)).awaitUninterruptibly(); + + assertInstanceOf(UnsupportedOperationException.class, future.cause()); + } + + @Test + void readsOneAtATime() throws Exception { + connect(); + + onEventLoop( + () -> { + channel.read(); + channel.read(); + channel.read(); + }); + + assertEquals(1, socket.reads.get(), "A read requested while one is pending is ignored"); + + // Completed by an SDK thread: the bytes are fired on the event loop. + socket.completePendingRead("hello"); + recorder.received.get(5, TimeUnit.SECONDS); + + assertEquals("hello", recorder.text()); + assertTrue(recorder.allOnEventLoop.get()); + + onEventLoop(channel::read); + assertEquals(2, socket.reads.get(), "A read is accepted once the pending one completed"); + } + + @Test + void neverFiresAReadNestedInTheReadRequest() throws Exception { + connect(); + // The SDK completes the read inside read(), the data being buffered already. + socket.bufferedReads.add("buffered"); + AtomicBoolean inRead = new AtomicBoolean(); + recorder.onRead = bytes -> recorder.nested.compareAndSet(false, inRead.get()); + + onEventLoop( + () -> { + inRead.set(true); + channel.read(); + inRead.set(false); + }); + recorder.received.get(5, TimeUnit.SECONDS); + + assertEquals("buffered", recorder.text()); + assertFalse(recorder.nested.get(), "The read was fired nested in Channel.read()"); + } + + @Test + void readsOnItsOwnWithAutoRead() throws Exception { + connect(); + socket.bufferedReads.add("first"); + socket.bufferedReads.add("second"); + + channel.config().setAutoRead(true); + recorder.awaitText("firstsecond"); + + assertEquals(3, socket.reads.get(), "A read is started after each one"); + } + + @Test + void closesAtTheEndOfTheStream() throws Exception { + connect(); + socket.bufferedReads.add(-1); + + onEventLoop(channel::read); + + assertTrue(channel.closeFuture().await(5, TimeUnit.SECONDS)); + assertFalse(recorder.active.get()); + } + + @Test + void reportsAReadFailure() throws Exception { + connect(); + IOException failure = new IOException("connection reset"); + socket.bufferedReads.add(failure); + + onEventLoop(channel::read); + + assertTrue(channel.closeFuture().await(5, TimeUnit.SECONDS)); + assertSame(failure, recorder.failures.get(0)); + } + + @Test + void writesInOrderWithoutBlocking() throws Exception { + connect(); + // The SDK accepts 3 bytes per write, completing the write inside the call. + socket.maxWrite = 3; + + channel.write(Unpooled.copiedBuffer("hello ", StandardCharsets.UTF_8)); + channel.write(Unpooled.EMPTY_BUFFER); + ChannelFuture last = + channel.writeAndFlush(Unpooled.copiedBuffer("over ziti", StandardCharsets.UTF_8)); + + assertTrue(last.await(5, TimeUnit.SECONDS)); + assertTrue(last.isSuccess()); + assertArrayEquals( + "hello over ziti".getBytes(StandardCharsets.UTF_8), socket.written.toByteArray()); + assertEquals(1, socket.maxWritesInFlight.get(), "One write is in flight at a time"); + } + + @Test + void reportsAWriteFailure() throws Exception { + connect(); + socket.writeFailure = new IOException("broken pipe"); + + ChannelFuture future = + channel.writeAndFlush(Unpooled.copiedBuffer("lost", StandardCharsets.UTF_8)) + .awaitUninterruptibly(); + + assertFalse(future.isSuccess()); + assertTrue(channel.closeFuture().await(5, TimeUnit.SECONDS)); + assertSame(socket.writeFailure, recorder.failures.get(0)); + } + + @Test + void rejectsMessagesOtherThanBuffers() throws Exception { + connect(); + + ChannelFuture future = channel.writeAndFlush("text").awaitUninterruptibly(); + + assertInstanceOf(UnsupportedOperationException.class, future.cause()); + } + + /** Records what reaches the end of the pipeline. */ + private final class Recorder extends ChannelInboundHandlerAdapter { + + private final ByteArrayOutputStream bytes = new ByteArrayOutputStream(); + + final CompletableFuture received = new CompletableFuture<>(); + + final AtomicBoolean active = new AtomicBoolean(); + + final AtomicBoolean allOnEventLoop = new AtomicBoolean(true); + + final AtomicBoolean nested = new AtomicBoolean(); + + final CopyOnWriteArrayList failures = new CopyOnWriteArrayList<>(); + + volatile Consumer onRead = b -> {}; + + synchronized String text() { + return bytes.toString(StandardCharsets.UTF_8); + } + + void awaitText(String expected) throws InterruptedException { + long deadline = System.nanoTime() + TimeUnit.SECONDS.toNanos(5); + while (!text().equals(expected) && System.nanoTime() < deadline) { + Thread.sleep(10); + } + assertEquals(expected, text()); + } + + @Override + public void channelActive(ChannelHandlerContext ctx) { + active.set(true); + } + + @Override + public void channelInactive(ChannelHandlerContext ctx) { + active.set(false); + } + + @Override + public void channelRead(ChannelHandlerContext ctx, Object msg) { + allOnEventLoop.compareAndSet(true, ctx.channel().eventLoop().inEventLoop()); + ByteBuf buffer = (ByteBuf) msg; + byte[] read = new byte[buffer.readableBytes()]; + buffer.readBytes(read); + buffer.release(); + synchronized (this) { + bytes.writeBytes(read); + } + onRead.accept(read); + received.complete(null); + } + + @Override + public void exceptionCaught(ChannelHandlerContext ctx, Throwable cause) { + failures.add(cause); + } + } + + /** + * A socket of the SDK, scripted by the test. A read completes inside the call with the next + * buffered result, or stays pending until {@link #completePendingRead(String)}. A write + * completes inside the call. + */ + private static final class ScriptedSocket extends AsynchronousSocketChannel { + + volatile boolean open = true; + + volatile SocketAddress dialed; + + final AtomicInteger connects = new AtomicInteger(); + + volatile IOException connectFailure; + + volatile boolean connectHangs; + + /** Read results: a String read, an Integer -1 for the end of stream, or a Throwable. */ + final Queue bufferedReads = new ArrayDeque<>(); + + final AtomicInteger reads = new AtomicInteger(); + + private ByteBuffer pendingTarget; + + private CompletionHandler pendingHandler; + + final ByteArrayOutputStream written = new ByteArrayOutputStream(); + + volatile int maxWrite = Integer.MAX_VALUE; + + volatile IOException writeFailure; + + private final AtomicInteger writesInFlight = new AtomicInteger(); + + final AtomicInteger maxWritesInFlight = new AtomicInteger(); + + ScriptedSocket() { + super(AsynchronousChannelProvider.provider()); + } + + @Override + public void connect( + SocketAddress remote, A attachment, CompletionHandler handler) { + connects.incrementAndGet(); + dialed = remote; + if (connectHangs) { + return; + } + if (connectFailure != null) { + handler.failed(connectFailure, attachment); + } else { + handler.completed(null, attachment); + } + } + + @Override + @SuppressWarnings("unchecked") + public void read( + ByteBuffer dst, + long timeout, + TimeUnit unit, + A attachment, + CompletionHandler handler) { + reads.incrementAndGet(); + Object result; + synchronized (this) { + result = bufferedReads.poll(); + if (result == null) { + pendingTarget = dst; + pendingHandler = (CompletionHandler) handler; + return; + } + } + complete(dst, (CompletionHandler) handler, result); + } + + void completePendingRead(String text) { + ByteBuffer target; + CompletionHandler handler; + synchronized (this) { + target = pendingTarget; + handler = pendingHandler; + pendingHandler = null; + } + complete(target, handler, text); + } + + private static void complete( + ByteBuffer dst, CompletionHandler handler, Object result) { + if (result instanceof Throwable failure) { + handler.failed(failure, null); + } else if (result instanceof Integer end) { + handler.completed(end, null); + } else { + byte[] bytes = ((String) result).getBytes(StandardCharsets.UTF_8); + dst.put(bytes); + handler.completed(bytes.length, null); + } + } + + @Override + public void write( + ByteBuffer src, + long timeout, + TimeUnit unit, + A attachment, + CompletionHandler handler) { + int inFlight = writesInFlight.incrementAndGet(); + maxWritesInFlight.accumulateAndGet(inFlight, Math::max); + if (writeFailure != null) { + writesInFlight.decrementAndGet(); + handler.failed(writeFailure, attachment); + return; + } + byte[] bytes = new byte[Math.min(maxWrite, src.remaining())]; + src.get(bytes); + synchronized (written) { + written.writeBytes(bytes); + } + writesInFlight.decrementAndGet(); + handler.completed(bytes.length, attachment); + } + + @Override + public boolean isOpen() { + return open; + } + + @Override + public void close() { + open = false; + } + + @Override + public SocketAddress getRemoteAddress() { + return dialed; + } + + @Override + public SocketAddress getLocalAddress() throws IOException { + throw new IOException("No local address"); + } + + @Override + public AsynchronousSocketChannel bind(SocketAddress local) { + throw new UnsupportedOperationException(); + } + + @Override + public AsynchronousSocketChannel setOption(SocketOption name, T value) { + return this; + } + + @Override + public T getOption(SocketOption name) { + return null; + } + + @Override + public Set> supportedOptions() { + return Set.of(); + } + + @Override + public AsynchronousSocketChannel shutdownInput() { + return this; + } + + @Override + public AsynchronousSocketChannel shutdownOutput() { + return this; + } + + @Override + public Future connect(SocketAddress remote) { + throw new UnsupportedOperationException(); + } + + @Override + public Future read(ByteBuffer dst) { + throw new UnsupportedOperationException(); + } + + @Override + public void read( + ByteBuffer[] dsts, + int offset, + int length, + long timeout, + TimeUnit unit, + A attachment, + CompletionHandler handler) { + throw new UnsupportedOperationException(); + } + + @Override + public Future write(ByteBuffer src) { + throw new UnsupportedOperationException(); + } + + @Override + public void write( + ByteBuffer[] srcs, + int offset, + int length, + long timeout, + TimeUnit unit, + A attachment, + CompletionHandler handler) { + throw new UnsupportedOperationException(); + } + } +} diff --git a/org.restlet/src/main/java/org/restlet/engine/netty/NettyClientHelper.java b/org.restlet/src/main/java/org/restlet/engine/netty/NettyClientHelper.java index 2113b0c52d..d30804a010 100644 --- a/org.restlet/src/main/java/org/restlet/engine/netty/NettyClientHelper.java +++ b/org.restlet/src/main/java/org/restlet/engine/netty/NettyClientHelper.java @@ -21,7 +21,6 @@ import io.netty.channel.group.ChannelGroup; import io.netty.channel.group.DefaultChannelGroup; import io.netty.channel.nio.NioIoHandler; -import io.netty.channel.socket.SocketChannel; import io.netty.channel.socket.nio.NioSocketChannel; import io.netty.handler.codec.http.DefaultHttpHeaders; import io.netty.handler.codec.http.HttpClientCodec; @@ -715,23 +714,24 @@ private Channel connect(final OriginPool pool, final String proxyAuthorization) boolean acquired = true; try { final SslContext ssl = pool.secure ? getSslContext() : null; - final InetSocketAddress proxy = resolve(getProxy(pool.secure, pool.host)); + // A transport route (e.g. an overlay network) replaces TCP and the proxies. + final NettyClientTransport.Route route = getRoute(pool); + final InetSocketAddress proxy = + (route == null) ? resolve(getProxy(pool.secure, pool.host)) : null; final InetSocketAddress socksProxy = - (proxy == null) ? resolve(getSocksProxy(pool.host)) : null; + (route == null && proxy == null) ? resolve(getSocksProxy(pool.host)) : null; Bootstrap bootstrap = new Bootstrap() .group(group) - .channel(NioSocketChannel.class) .option(ChannelOption.AUTO_READ, false) - .option(ChannelOption.TCP_NODELAY, true) .option( ChannelOption.CONNECT_TIMEOUT_MILLIS, (int) Math.min(Integer.MAX_VALUE, timeout)) .handler( - new ChannelInitializer() { + new ChannelInitializer() { @Override - protected void initChannel(SocketChannel ch) { + protected void initChannel(Channel ch) { initConnection( ch, pool, @@ -742,13 +742,21 @@ protected void initChannel(SocketChannel ch) { } }); - SocketAddress bindAddress = getBindAddress(); - if (bindAddress != null) { - bootstrap.localAddress(bindAddress); + if (route != null) { + bootstrap.channelFactory(route.channelFactory()).resolver(route.resolver()); + } else { + bootstrap.channel(NioSocketChannel.class).option(ChannelOption.TCP_NODELAY, true); + SocketAddress bindAddress = getBindAddress(); + if (bindAddress != null) { + bootstrap.localAddress(bindAddress); + } } SocketAddress remote; - if ((proxy != null && pool.secure) || (socksProxy != null)) { + if (route != null) { + // The route's resolver maps the host name, never the system DNS. + remote = InetSocketAddress.createUnresolved(pool.host, pool.port); + } else if ((proxy != null && pool.secure) || (socksProxy != null)) { // Tunnel with CONNECT or SOCKS, the proxy resolves the host name. bootstrap.resolver(NoopAddressResolverGroup.INSTANCE); remote = InetSocketAddress.createUnresolved(pool.host, pool.port); @@ -828,6 +836,22 @@ private boolean acquirePermit(OriginPool pool, long timeout) throws InterruptedE return pool.permits.tryAcquire(timeout, TimeUnit.MILLISECONDS); } + /** + * Returns the transport route to an origin, from the {@link NettyClientTransport} set in the + * {@value NettyClientTransport#ATTRIBUTE} attribute of the client context, or null to connect + * over TCP. + * + * @param pool The connection pool of the origin. + * @return The route, or null. + */ + private NettyClientTransport.Route getRoute(OriginPool pool) { + Object transport = + (getContext() == null) + ? null + : getContext().getAttributes().get(NettyClientTransport.ATTRIBUTE); + return (transport instanceof NettyClientTransport t) ? t.route(pool.host, pool.port) : null; + } + /** * Releases a connection of a pool, closed or not opened. A pool specific to proxy credentials * is dropped once it has no connection left, so that such pools do not accumulate. A call @@ -846,7 +870,7 @@ private void releaseConnection(OriginPool pool, boolean permit) { } private void initConnection( - SocketChannel ch, + Channel ch, OriginPool pool, SslContext ssl, InetSocketAddress proxy, @@ -918,7 +942,7 @@ public void exceptionCaught(ChannelHandlerContext ctx, Throwable cause) { * @param forwardProxy True for a cleartext connection to a forward proxy. */ private void initProtocols( - SocketChannel ch, + Channel ch, OriginPool pool, SslContext ssl, Promise ready, diff --git a/org.restlet/src/main/java/org/restlet/engine/netty/NettyClientTransport.java b/org.restlet/src/main/java/org/restlet/engine/netty/NettyClientTransport.java new file mode 100644 index 0000000000..a603a15c98 --- /dev/null +++ b/org.restlet/src/main/java/org/restlet/engine/netty/NettyClientTransport.java @@ -0,0 +1,56 @@ +/** + * Copyright 2005-2026 Qlik + *

+ * The content of this file is subject to the terms of the Apache 2.0 open + * source license available at https://www.opensource.org/licenses/apache-2.0 + *

+ * Restlet is a registered trademark of QlikTech International AB. + */ +package org.restlet.engine.netty; + +import io.netty.channel.Channel; +import io.netty.channel.ChannelFactory; +import io.netty.resolver.AddressResolverGroup; +import java.net.SocketAddress; + +/** + * Chooses how the Netty HTTP client connector reaches an origin, so that an extension can route + * some origins over another network, such as an overlay network (OpenZiti), instead of TCP. + * + *

Set it on a client with the {@value #ATTRIBUTE} attribute of its context. For each new + * connection the connector asks it for a {@link Route} to the origin's host and port: with a route, + * the connection is made with the route's channel factory and resolver, to the unresolved host and + * port, and the HTTP and HTTPS proxies are not used; without one, the connector connects over TCP + * as usual. TLS, HTTP/1.1 and HTTP/2 run over the channel either way. Connections are pooled per + * origin, so an origin is always reached the same way. + * + *

This is a connector SPI: it exposes Netty types and is meant for extensions that depend on + * Netty themselves. The {@code org.restlet} module still requires Netty without {@code requires + * transitive}, so applications that don't use it see no Netty type. + */ +@SuppressWarnings("exports") +public interface NettyClientTransport { + + /** The name of the client context attribute holding the transport. */ + String ATTRIBUTE = "org.restlet.engine.netty.clientTransport"; + + /** + * How to connect to an origin. + * + * @param channelFactory Creates the channel of each connection. Its channels must accept any + * event loop of the connector, which runs on NIO. + * @param resolver Resolves the unresolved host and port given to the channel. + */ + record Route( + ChannelFactory channelFactory, + AddressResolverGroup resolver) {} + + /** + * Returns the route to an origin, or null to connect to it over TCP. + * + * @param host The host name of the origin, unresolved. + * @param port The port of the origin. + * @return The route, or null. + */ + Route route(String host, int port); +} diff --git a/org.restlet/src/test/java/org/restlet/engine/netty/NettyClientTransportTestCase.java b/org.restlet/src/test/java/org/restlet/engine/netty/NettyClientTransportTestCase.java new file mode 100644 index 0000000000..ec39e32296 --- /dev/null +++ b/org.restlet/src/test/java/org/restlet/engine/netty/NettyClientTransportTestCase.java @@ -0,0 +1,186 @@ +/** + * Copyright 2005-2026 Qlik + *

+ * The content of this file is subject to the terms of the Apache 2.0 open + * source license available at https://www.opensource.org/licenses/apache-2.0 + *

+ * Restlet is a registered trademark of QlikTech International AB. + */ +package org.restlet.engine.netty; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +import com.sun.net.httpserver.HttpServer; +import io.netty.channel.socket.nio.NioSocketChannel; +import io.netty.resolver.AbstractAddressResolver; +import io.netty.resolver.AddressResolver; +import io.netty.resolver.AddressResolverGroup; +import io.netty.util.concurrent.EventExecutor; +import io.netty.util.concurrent.Promise; +import java.io.OutputStream; +import java.net.InetSocketAddress; +import java.net.ServerSocket; +import java.nio.charset.StandardCharsets; +import java.util.List; +import java.util.concurrent.CopyOnWriteArrayList; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.restlet.Client; +import org.restlet.Context; +import org.restlet.Request; +import org.restlet.Response; +import org.restlet.data.Method; +import org.restlet.data.Protocol; +import org.restlet.data.Status; +import org.restlet.engine.Engine; +import org.restlet.engine.connector.HttpClientHelper; + +/** Tests the routes of a {@link NettyClientTransport} set on a client. */ +class NettyClientTransportTestCase { + + /** The origin routed by the transport: not in the DNS. */ + private static final String ROUTED_HOST = "overlay.invalid"; + + private HttpServer peer; + + private Client client; + + /** The origins the transport was asked for, as host:port. */ + private final List asked = new CopyOnWriteArrayList<>(); + + /** The addresses the route's resolver resolved. */ + private final List resolved = new CopyOnWriteArrayList<>(); + + @BeforeEach + void start() throws Exception { + Engine.clearThreadLocalVariables(); + Engine.register(false); + Engine.getInstance().getRegisteredClients().add(new HttpClientHelper(null)); + peer = HttpServer.create(new InetSocketAddress("127.0.0.1", 0), 0); + peer.createContext( + "/", + exchange -> { + byte[] body = + exchange.getRequestHeaders() + .getFirst("Host") + .getBytes(StandardCharsets.US_ASCII); + exchange.sendResponseHeaders(200, body.length); + try (OutputStream out = exchange.getResponseBody()) { + out.write(body); + } + }); + peer.start(); + } + + @AfterEach + void stop() throws Exception { + if (client != null) { + client.stop(); + } + peer.stop(0); + Engine.clearThreadLocalVariables(); + } + + /** Routes ROUTED_HOST over TCP to the peer, with a resolver that never asks the DNS. */ + private NettyClientTransport transport() { + int peerPort = peer.getAddress().getPort(); + AddressResolverGroup resolver = + new AddressResolverGroup<>() { + @Override + protected AddressResolver newResolver( + EventExecutor executor) { + return new AbstractAddressResolver<>(executor) { + @Override + protected boolean doIsResolved(InetSocketAddress address) { + return !address.isUnresolved(); + } + + @Override + protected void doResolve( + InetSocketAddress address, Promise promise) { + resolved.add(address.getHostString() + ":" + address.getPort()); + promise.setSuccess(new InetSocketAddress("127.0.0.1", peerPort)); + } + + @Override + protected void doResolveAll( + InetSocketAddress address, + Promise> promise) { + promise.setSuccess( + List.of(new InetSocketAddress("127.0.0.1", peerPort))); + } + }; + } + }; + NettyClientTransport.Route route = + new NettyClientTransport.Route(NioSocketChannel::new, resolver); + return (host, port) -> { + asked.add(host + ":" + port); + return ROUTED_HOST.equals(host) ? route : null; + }; + } + + private void startClient(Context context) throws Exception { + context.getAttributes().put(NettyClientTransport.ATTRIBUTE, transport()); + client = new Client(context, Protocol.HTTP); + client.start(); + } + + @Test + void connectsToARoutedOriginThroughTheRoute() throws Exception { + startClient(new Context()); + + Response response = + client.handle(new Request(Method.GET, "http://" + ROUTED_HOST + ":8080/a")); + + assertEquals(Status.SUCCESS_OK, response.getStatus(), String.valueOf(response.getStatus())); + assertEquals(ROUTED_HOST + ":8080", response.getEntity().getText()); + assertEquals(List.of(ROUTED_HOST + ":8080"), asked); + assertEquals(List.of(ROUTED_HOST + ":8080"), resolved); + } + + @Test + void reusesTheConnectionOfARoutedOrigin() throws Exception { + startClient(new Context()); + + for (int i = 0; i < 3; i++) { + Response response = + client.handle(new Request(Method.GET, "http://" + ROUTED_HOST + "/b" + i)); + assertEquals(Status.SUCCESS_OK, response.getStatus()); + response.getEntity().exhaust(); + } + + assertEquals(1, asked.size(), "The route is asked once per connection"); + } + + @Test + void doesNotUseTheProxyForARoutedOrigin() throws Exception { + int closedPort; + try (ServerSocket socket = new ServerSocket(0)) { + closedPort = socket.getLocalPort(); + } + Context context = new Context(); + context.getParameters().add("proxyHost", "127.0.0.1"); + context.getParameters().add("proxyPort", Integer.toString(closedPort)); + startClient(context); + + Response response = client.handle(new Request(Method.GET, "http://" + ROUTED_HOST + "/c")); + + assertEquals(Status.SUCCESS_OK, response.getStatus(), String.valueOf(response.getStatus())); + assertEquals(ROUTED_HOST, response.getEntity().getText()); + } + + @Test + void connectsToOtherOriginsOverTcp() throws Exception { + startClient(new Context()); + String origin = "127.0.0.1:" + peer.getAddress().getPort(); + + Response response = client.handle(new Request(Method.GET, "http://" + origin + "/d")); + + assertEquals(Status.SUCCESS_OK, response.getStatus(), String.valueOf(response.getStatus())); + assertEquals(origin, response.getEntity().getText()); + assertEquals(List.of(origin), asked); + assertEquals(List.of(), resolved); + } +} diff --git a/pom.xml b/pom.xml index 73d7a638d4..dabc01d58f 100644 --- a/pom.xml +++ b/pom.xml @@ -70,6 +70,7 @@ 2.1.48 3.1.5.RELEASE 2.4.1 + 0.35.0 3.21.0 @@ -96,6 +97,7 @@ org.restlet.ext.velocity org.restlet.ext.websocket org.restlet.ext.xml + org.restlet.ext.ziti From 8b4bd7e702d58f01a3c7a9930aed325cc2c60c1b Mon Sep 17 00:00:00 2001 From: Jerome Louvel <374450+jlouvel@users.noreply.github.com> Date: Tue, 10 Nov 2026 14:13:03 -0500 Subject: [PATCH 2/8] Fix the end-of-stream test race and the SonarCloud issues of the Ziti extension - ZitiSocketChannelTestCase: wait for channelInactive, which Netty fires after completing the close future, instead of reading the flag right away (failed in CI). The recorder now waits for its text with wait/notifyAll. - ZitiNetworkTestCase: poll the network with a scheduled executor instead of Thread.sleep loops. - ZitiSocketChannel: move the read, write and dial helpers into the inner classes that call them, and use unnamed catch variables. - NettyClientHelper: extract the transport and remote address selection out of connect() to reduce its cognitive complexity. Co-Authored-By: Claude Opus 5.5 --- .../restlet/ext/ziti/ZitiSocketChannel.java | 154 +++++++++--------- .../ext/ziti/ZitiClientTransportTestCase.java | 3 +- .../restlet/ext/ziti/ZitiNetworkTestCase.java | 81 ++++++--- .../ext/ziti/ZitiSocketChannelTestCase.java | 14 +- .../engine/netty/NettyClientHelper.java | 68 +++++--- 5 files changed, 190 insertions(+), 130 deletions(-) diff --git a/org.restlet.ext.ziti/src/main/java/org/restlet/ext/ziti/ZitiSocketChannel.java b/org.restlet.ext.ziti/src/main/java/org/restlet/ext/ziti/ZitiSocketChannel.java index 8234269936..cfd1af313f 100644 --- a/org.restlet.ext.ziti/src/main/java/org/restlet/ext/ziti/ZitiSocketChannel.java +++ b/org.restlet.ext.ziti/src/main/java/org/restlet/ext/ziti/ZitiSocketChannel.java @@ -124,7 +124,7 @@ protected boolean isCompatible(EventLoop loop) { protected SocketAddress localAddress0() { try { return socket.getLocalAddress(); - } catch (IOException e) { + } catch (IOException _) { return null; } } @@ -133,7 +133,7 @@ protected SocketAddress localAddress0() { protected SocketAddress remoteAddress0() { try { return socket.getRemoteAddress(); - } catch (IOException e) { + } catch (IOException _) { return null; } } @@ -168,7 +168,7 @@ private boolean onEventLoop(Runnable task) { try { eventLoop().execute(task); return true; - } catch (RejectedExecutionException e) { + } catch (RejectedExecutionException _) { // The event loop is shutting down, with the channel closed. return false; } @@ -193,37 +193,6 @@ protected void doBeginRead() { } } - /** Fires the bytes of a completed read, on the event loop. */ - private void readCompleted(ByteBuf buffer, int read) { - readPending = false; - - if (read < 0) { - buffer.release(); - pipeline().fireChannelReadComplete(); - unsafe().close(voidPromise()); - return; - } - - buffer.writerIndex(buffer.writerIndex() + read); - pipeline().fireChannelRead(buffer); - pipeline().fireChannelReadComplete(); - - if (config.isAutoRead()) { - read(); - } - } - - /** Reports a failed read, on the event loop. */ - private void readFailed(ByteBuf buffer, Throwable cause) { - readPending = false; - buffer.release(); - - if (isOpen()) { - pipeline().fireExceptionCaught(cause); - unsafe().close(voidPromise()); - } - } - /** Completes a read on the event loop. */ private final class ReadHandler implements CompletionHandler { @@ -235,17 +204,48 @@ private final class ReadHandler implements CompletionHandler { @Override public void completed(Integer read, Void attachment) { - if (!onEventLoop(() -> readCompleted(buffer, read))) { + if (!onEventLoop(() -> readCompleted(read))) { buffer.release(); } } @Override public void failed(Throwable cause, Void attachment) { - if (!onEventLoop(() -> readFailed(buffer, cause))) { + if (!onEventLoop(() -> readFailed(cause))) { buffer.release(); } } + + /** Fires the bytes of a completed read, on the event loop. */ + private void readCompleted(int read) { + readPending = false; + + if (read < 0) { + buffer.release(); + pipeline().fireChannelReadComplete(); + unsafe().close(voidPromise()); + return; + } + + buffer.writerIndex(buffer.writerIndex() + read); + pipeline().fireChannelRead(buffer); + pipeline().fireChannelReadComplete(); + + if (config.isAutoRead()) { + read(); + } + } + + /** Reports a failed read, on the event loop. */ + private void readFailed(Throwable cause) { + readPending = false; + buffer.release(); + + if (isOpen()) { + pipeline().fireExceptionCaught(cause); + unsafe().close(voidPromise()); + } + } } @Override @@ -279,29 +279,6 @@ protected void doWrite(ChannelOutboundBuffer in) { } } - /** Accounts for a completed write and starts the next one, on the event loop. */ - private void writeCompleted(int written) { - writeInFlight = false; - ChannelOutboundBuffer in = unsafe().outboundBuffer(); - - if (in != null) { - // Moves the reader index of the buffer written and releases it once fully written. - in.removeBytes(written); - unsafe().flush(); - } - } - - /** Fails the pending writes and closes the channel, on the event loop. */ - private void writeFailed(Throwable cause) { - writeInFlight = false; - - if (isOpen()) { - pipeline().fireExceptionCaught(cause); - } - - unsafe().close(voidPromise()); - } - /** Completes a write on the event loop. */ private final class WriteHandler implements CompletionHandler { @@ -323,31 +300,29 @@ public void failed(Throwable cause, Void attachment) { buffer.release(); onEventLoop(() -> writeFailed(cause)); } - } - /** - * Returns the Ziti address to dial for a remote address. - * - * @param remote An unresolved {@code host:port} or a Ziti address. - * @return The Ziti address. - */ - private SocketAddress dialAddress(SocketAddress remote) { - if (remote instanceof ZitiAddress) { - return remote; + /** Accounts for a completed write and starts the next one, on the event loop. */ + private void writeCompleted(int written) { + writeInFlight = false; + ChannelOutboundBuffer in = unsafe().outboundBuffer(); + + if (in != null) { + // Moves the reader index of the buffer written and releases it once fully written. + in.removeBytes(written); + unsafe().flush(); + } } - if (remote instanceof InetSocketAddress address) { - ServiceDetail service = context.getService(address); + /** Fails the pending writes and closes the channel, on the event loop. */ + private void writeFailed(Throwable cause) { + writeInFlight = false; - if (service != null) { - return new ZitiAddress.Dial(service.getName(), null, null, null); + if (isOpen()) { + pipeline().fireExceptionCaught(cause); } - throw new UnresolvedAddressException(); + unsafe().close(voidPromise()); } - - throw new UnsupportedOperationException( - "Unsupported address type: " + remote.getClass().getName()); } @Override @@ -358,6 +333,31 @@ protected AbstractUnsafe newUnsafe() { /** Connects with the SDK's asynchronous connect, completed on the event loop. */ private final class ZitiUnsafe extends AbstractUnsafe { + /** + * Returns the Ziti address to dial for a remote address. + * + * @param remote An unresolved {@code host:port} or a Ziti address. + * @return The Ziti address. + */ + private SocketAddress dialAddress(SocketAddress remote) { + if (remote instanceof ZitiAddress) { + return remote; + } + + if (remote instanceof InetSocketAddress address) { + ServiceDetail service = context.getService(address); + + if (service != null) { + return new ZitiAddress.Dial(service.getName(), null, null, null); + } + + throw new UnresolvedAddressException(); + } + + throw new UnsupportedOperationException( + "Unsupported address type: " + remote.getClass().getName()); + } + @Override public void connect( SocketAddress remoteAddress, SocketAddress localAddress, ChannelPromise promise) { diff --git a/org.restlet.ext.ziti/src/test/java/org/restlet/ext/ziti/ZitiClientTransportTestCase.java b/org.restlet.ext.ziti/src/test/java/org/restlet/ext/ziti/ZitiClientTransportTestCase.java index f918ba7ff8..36d9cefde1 100644 --- a/org.restlet.ext.ziti/src/test/java/org/restlet/ext/ziti/ZitiClientTransportTestCase.java +++ b/org.restlet.ext.ziti/src/test/java/org/restlet/ext/ziti/ZitiClientTransportTestCase.java @@ -9,6 +9,7 @@ package org.restlet.ext.ziti; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -207,7 +208,7 @@ void connectsToOtherOriginsOverTcp() throws Exception { void routesOnlyInterceptedOrigins() { ZitiClientTransport transport = new ZitiClientTransport(fakeZiti()); - assertTrue(transport.route(SERVICE_HOST, SERVICE_PORT) != null); + assertNotNull(transport.route(SERVICE_HOST, SERVICE_PORT)); assertNull(transport.route(SERVICE_HOST, 80)); assertNull(transport.route("example.org", SERVICE_PORT)); } diff --git a/org.restlet.ext.ziti/src/test/java/org/restlet/ext/ziti/ZitiNetworkTestCase.java b/org.restlet.ext.ziti/src/test/java/org/restlet/ext/ziti/ZitiNetworkTestCase.java index 140ecae6bf..839695bed8 100644 --- a/org.restlet.ext.ziti/src/test/java/org/restlet/ext/ziti/ZitiNetworkTestCase.java +++ b/org.restlet.ext.ziti/src/test/java/org/restlet/ext/ziti/ZitiNetworkTestCase.java @@ -13,6 +13,7 @@ import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.fail; import java.io.File; import java.io.IOException; @@ -27,10 +28,14 @@ import java.util.ArrayList; import java.util.List; import java.util.concurrent.Callable; +import java.util.concurrent.CompletableFuture; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.Future; +import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; +import java.util.function.BooleanSupplier; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; @@ -89,11 +94,9 @@ static void start() throws Exception { // A context learns its services asynchronously after authenticating: binding before the // server identity has loaded the service is refused as "no permission to bind". - long bindDeadline = System.nanoTime() + TimeUnit.SECONDS.toNanos(60); - while (serverZiti.getService(SERVICE) == null) { - assertTrue(System.nanoTime() < bindDeadline, "The server never saw the crm service"); - Thread.sleep(250); - } + awaitUntil( + () -> serverZiti.getService(SERVICE) != null, + "The server never saw the crm service"); // Host the service: accept the Ziti connections and pipe them to the local server. binding = serverZiti.openServer(); @@ -101,20 +104,53 @@ static void start() throws Exception { bridge = Executors.newVirtualThreadPerTaskExecutor(); bridge.submit(ZitiNetworkTestCase::acceptLoop); - // The client learns its services asynchronously after authenticating. - long deadline = System.nanoTime() + TimeUnit.SECONDS.toNanos(60); - while (clientZiti.getService(InetSocketAddress.createUnresolved(HOST, 8443)) == null) { - assertTrue(System.nanoTime() < deadline, "The client never saw the crm service"); - Thread.sleep(250); - } + // The client learns its services asynchronously after authenticating, and the bind + // registers a terminator asynchronously: dialing before gives "no terminators". + awaitReachable(InetSocketAddress.createUnresolved(HOST, 8443)); + } + + /** + * Waits until the client sees a service intercepting an address, with a terminator. + * + * @param address The intercepted address. + */ + private static void awaitReachable(InetSocketAddress address) throws Exception { + awaitUntil( + () -> + clientZiti.getService(address) != null + && !clientZiti + .getServiceTerminators(clientZiti.getService(address)) + .isEmpty(), + "The service of " + address + " is not reachable"); + } - // The bind registers a terminator asynchronously: dialing before gives "no terminators". - while (clientZiti - .getServiceTerminators( - clientZiti.getService(InetSocketAddress.createUnresolved(HOST, 8443))) - .isEmpty()) { - assertTrue(System.nanoTime() < deadline, "The crm service never got a terminator"); - Thread.sleep(250); + /** + * Waits up to a minute for a condition on the network, checked every 250 ms. + * + * @param condition The condition. + * @param message The failure message. + */ + private static void awaitUntil(BooleanSupplier condition, String message) throws Exception { + CompletableFuture met = new CompletableFuture<>(); + try (ScheduledExecutorService poller = Executors.newSingleThreadScheduledExecutor()) { + poller.scheduleWithFixedDelay( + () -> { + try { + if (condition.getAsBoolean()) { + met.complete(null); + } + } catch (RuntimeException e) { + met.completeExceptionally(e); + } + }, + 0, + 250, + TimeUnit.MILLISECONDS); + met.get(60, TimeUnit.SECONDS); + } catch (TimeoutException _) { + fail(message); + } finally { + met.cancel(false); } } @@ -171,7 +207,7 @@ private static Callable copy( buffer.clear(); } to.shutdownOutput(); - } catch (Exception e) { + } catch (Exception _) { // The other side closed: the pipe ends. } return null; @@ -301,12 +337,7 @@ void callsConcurrentlyOverZiti() throws Exception { @EnabledIfSystemProperty(named = "ziti.router.port", matches = "\\d+") void callsConcurrentlyARouterHostedService() throws Exception { InetSocketAddress service = InetSocketAddress.createUnresolved("crm2.internal", 8443); - long deadline = System.nanoTime() + TimeUnit.SECONDS.toNanos(60); - while (clientZiti.getService(service) == null - || clientZiti.getServiceTerminators(clientZiti.getService(service)).isEmpty()) { - assertTrue(System.nanoTime() < deadline, "The crm2 service is not reachable"); - Thread.sleep(250); - } + awaitReachable(service); Server routerTarget = new Server( diff --git a/org.restlet.ext.ziti/src/test/java/org/restlet/ext/ziti/ZitiSocketChannelTestCase.java b/org.restlet.ext.ziti/src/test/java/org/restlet/ext/ziti/ZitiSocketChannelTestCase.java index d47ae31c80..a0278099a8 100644 --- a/org.restlet.ext.ziti/src/test/java/org/restlet/ext/ziti/ZitiSocketChannelTestCase.java +++ b/org.restlet.ext.ziti/src/test/java/org/restlet/ext/ziti/ZitiSocketChannelTestCase.java @@ -43,6 +43,7 @@ import java.util.Set; import java.util.concurrent.CompletableFuture; import java.util.concurrent.CopyOnWriteArrayList; +import java.util.concurrent.CountDownLatch; import java.util.concurrent.Future; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; @@ -242,6 +243,8 @@ void closesAtTheEndOfTheStream() throws Exception { onEventLoop(channel::read); assertTrue(channel.closeFuture().await(5, TimeUnit.SECONDS)); + // Netty fires channelInactive after completing the close future. + assertTrue(recorder.inactive.await(5, TimeUnit.SECONDS)); assertFalse(recorder.active.get()); } @@ -307,6 +310,8 @@ private final class Recorder extends ChannelInboundHandlerAdapter { final AtomicBoolean active = new AtomicBoolean(); + final CountDownLatch inactive = new CountDownLatch(1); + final AtomicBoolean allOnEventLoop = new AtomicBoolean(true); final AtomicBoolean nested = new AtomicBoolean(); @@ -319,10 +324,11 @@ synchronized String text() { return bytes.toString(StandardCharsets.UTF_8); } - void awaitText(String expected) throws InterruptedException { + synchronized void awaitText(String expected) throws InterruptedException { long deadline = System.nanoTime() + TimeUnit.SECONDS.toNanos(5); - while (!text().equals(expected) && System.nanoTime() < deadline) { - Thread.sleep(10); + long left; + while (!text().equals(expected) && (left = deadline - System.nanoTime()) > 0) { + TimeUnit.NANOSECONDS.timedWait(this, left); } assertEquals(expected, text()); } @@ -335,6 +341,7 @@ public void channelActive(ChannelHandlerContext ctx) { @Override public void channelInactive(ChannelHandlerContext ctx) { active.set(false); + inactive.countDown(); } @Override @@ -346,6 +353,7 @@ public void channelRead(ChannelHandlerContext ctx, Object msg) { buffer.release(); synchronized (this) { bytes.writeBytes(read); + notifyAll(); } onRead.accept(read); received.complete(null); diff --git a/org.restlet/src/main/java/org/restlet/engine/netty/NettyClientHelper.java b/org.restlet/src/main/java/org/restlet/engine/netty/NettyClientHelper.java index d30804a010..deda3a6810 100644 --- a/org.restlet/src/main/java/org/restlet/engine/netty/NettyClientHelper.java +++ b/org.restlet/src/main/java/org/restlet/engine/netty/NettyClientHelper.java @@ -742,30 +742,7 @@ protected void initChannel(Channel ch) { } }); - if (route != null) { - bootstrap.channelFactory(route.channelFactory()).resolver(route.resolver()); - } else { - bootstrap.channel(NioSocketChannel.class).option(ChannelOption.TCP_NODELAY, true); - SocketAddress bindAddress = getBindAddress(); - if (bindAddress != null) { - bootstrap.localAddress(bindAddress); - } - } - - SocketAddress remote; - if (route != null) { - // The route's resolver maps the host name, never the system DNS. - remote = InetSocketAddress.createUnresolved(pool.host, pool.port); - } else if ((proxy != null && pool.secure) || (socksProxy != null)) { - // Tunnel with CONNECT or SOCKS, the proxy resolves the host name. - bootstrap.resolver(NoopAddressResolverGroup.INSTANCE); - remote = InetSocketAddress.createUnresolved(pool.host, pool.port); - } else if (proxy != null) { - // Forward proxy: absolute request URIs are sent to the proxy. - remote = proxy; - } else { - remote = InetSocketAddress.createUnresolved(pool.host, pool.port); - } + SocketAddress remote = configureTransport(bootstrap, pool, route, proxy, socksProxy); ChannelFuture connectFuture = bootstrap.connect(remote); Channel channel = @@ -798,6 +775,49 @@ protected void initChannel(Channel ch) { } } + /** + * Sets the transport of a connection on its bootstrap: the route's channel and resolver, or NIO + * over TCP. Returns the address to connect to. + * + * @param bootstrap The bootstrap of the connection. + * @param pool The pool of the origin. + * @param route The transport route of the origin, or null for TCP. + * @param proxy The HTTP proxy, or null. + * @param socksProxy The SOCKS proxy, or null. + * @return The address to connect to. + */ + private SocketAddress configureTransport( + Bootstrap bootstrap, + OriginPool pool, + NettyClientTransport.Route route, + InetSocketAddress proxy, + InetSocketAddress socksProxy) { + if (route != null) { + // The route's resolver maps the host name, never the system DNS. + bootstrap.channelFactory(route.channelFactory()).resolver(route.resolver()); + return InetSocketAddress.createUnresolved(pool.host, pool.port); + } + + bootstrap.channel(NioSocketChannel.class).option(ChannelOption.TCP_NODELAY, true); + SocketAddress bindAddress = getBindAddress(); + if (bindAddress != null) { + bootstrap.localAddress(bindAddress); + } + + if ((proxy != null && pool.secure) || (socksProxy != null)) { + // Tunnel with CONNECT or SOCKS, the proxy resolves the host name. + bootstrap.resolver(NoopAddressResolverGroup.INSTANCE); + return InetSocketAddress.createUnresolved(pool.host, pool.port); + } + + if (proxy != null) { + // Forward proxy: absolute request URIs are sent to the proxy. + return proxy; + } + + return InetSocketAddress.createUnresolved(pool.host, pool.port); + } + /** * Returns the connection permits of an origin, shared by its pools. * From 61c3087490bf586a6e3a962faee5582bf6421dfb Mon Sep 17 00:00:00 2001 From: Jerome Louvel <374450+jlouvel@users.noreply.github.com> Date: Tue, 10 Nov 2026 18:08:05 -0500 Subject: [PATCH 3/8] Address the Copilot review: route per request, and fix the Ziti transport and channel Core (Netty client connector): - Choose the route once per request, in create(): a routed request is sent in origin form, its Proxy-Authorization header is removed, it never uses HTTP/3 (which resolved the host with the system DNS and bypassed the route), and its connections are pooled apart from the TCP ones. OpenZiti extension: - ZitiClientTransport no longer calls ZitiContext.getService(InetSocketAddress), which waits for a service event, forever for an identity without services. It waits, up to a timeout (10 s by default), for the context to load its services, then looks the origin up without waiting, caching the lookups until a service event. - install() sets a context on a client without one. - ZitiSocketChannel hands the unresolved address to the SDK, which dials the intercepting service with its dial data and intercept dial options (such as the terminator identity) instead of a service-only Dial. - A completed write resumes the flushed writes only (flush0()), instead of flushing the messages written since. Co-Authored-By: Claude Opus 5.5 --- changes.md | 4 +- .../restlet/ext/ziti/ZitiClientTransport.java | 144 ++++++++++++++++-- .../restlet/ext/ziti/ZitiSocketChannel.java | 56 ++----- .../ext/ziti/ZitiClientTransportTestCase.java | 95 +++++++++++- .../ext/ziti/ZitiSocketChannelTestCase.java | 67 +++++++- .../restlet/engine/netty/NettyClientCall.java | 48 +++++- .../engine/netty/NettyClientHelper.java | 49 +++++- .../engine/netty/NettyClientTransport.java | 16 +- .../netty/NettyClientTransportTestCase.java | 71 +++++++-- 9 files changed, 454 insertions(+), 96 deletions(-) diff --git a/changes.md b/changes.md index 88a4d56af2..670564ea89 100644 --- a/changes.md +++ b/changes.md @@ -292,8 +292,8 @@ Changes log as over TCP. Client side only. - Added the `NettyClientTransport` SPI to the Netty client connector, set with the "org.restlet.engine.netty.clientTransport" client context attribute: it can route the - connections to some origins through another Netty channel and resolver than TCP, the - HTTP and SOCKS proxies being then bypassed. It exposes Netty types and is meant for + requests to some origins through another Netty channel and resolver than TCP, the HTTP + and SOCKS proxies and HTTP/3 being then bypassed, and the proxy credentials removed. It exposes Netty types and is meant for extensions depending on Netty, such as the OpenZiti one. - Bugs fixed - The Netty connectors now flush each full output buffer of a request or response body, diff --git a/org.restlet.ext.ziti/src/main/java/org/restlet/ext/ziti/ZitiClientTransport.java b/org.restlet.ext.ziti/src/main/java/org/restlet/ext/ziti/ZitiClientTransport.java index e5ed5c6ae4..698d8b2805 100644 --- a/org.restlet.ext.ziti/src/main/java/org/restlet/ext/ziti/ZitiClientTransport.java +++ b/org.restlet.ext.ziti/src/main/java/org/restlet/ext/ziti/ZitiClientTransport.java @@ -10,8 +10,19 @@ import io.netty.resolver.NoopAddressResolverGroup; import java.net.InetSocketAddress; +import java.time.Duration; +import java.util.Map; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; +import java.util.concurrent.atomic.AtomicLong; +import java.util.logging.Level; +import java.util.logging.Logger; import org.openziti.ZitiContext; import org.restlet.Client; +import org.restlet.Context; import org.restlet.engine.netty.NettyClientTransport; /** @@ -19,10 +30,17 @@ * network, and the others over TCP. * *

An origin is routed over Ziti when the context has a service intercepting its host and port - * ({@link ZitiContext#getService(InetSocketAddress)}), such as {@code crm.internal:443}. Its host - * name is then never resolved by the system DNS: the connection is a {@link ZitiSocketChannel} that - * dials that service. TLS, HTTP/1.1 and HTTP/2 run over it like over TCP: an {@code https} origin - * is encrypted end to end on top of the overlay's own encryption. + * ({@link ZitiContext#getService(InetSocketAddress, long)}), such as {@code crm.internal:443}. Its + * host name is then never resolved by the system DNS: the connection is a {@link ZitiSocketChannel} + * that dials that service. Every other origin is reached over TCP. + * + *

The context loads its services after authenticating. Until it has (its status is {@link + * ZitiContext.Status.Active}), routing waits, up to a timeout counted from the creation of the + * transport (10 seconds by default). Past it, the origins are reached over TCP until the services + * are loaded: create the transport early, or wait for the context to be active, so that no + * intercepted host name goes to the DNS. The lookups of the origins are cached, and the cache is + * cleared on each service event of the context. TLS, HTTP/1.1 and HTTP/2 run over it like over TCP: + * an {@code https} origin is encrypted end to end on top of the overlay's own encryption. * *

  * ZitiContext ztx = Ziti.newContext(identityFile, new char[0]);
@@ -34,33 +52,79 @@
  */
 public class ZitiClientTransport implements NettyClientTransport {
 
+    private static final Logger LOGGER = Logger.getLogger(ZitiClientTransport.class.getName());
+
+    /** The default time to wait for the services of the context. */
+    private static final Duration DEFAULT_SERVICES_TIMEOUT = Duration.ofSeconds(10);
+
+    /** The maximum number of origins whose lookup is cached. */
+    private static final int MAX_CACHED_ORIGINS = 1024;
+
     /** The Ziti context. */
     private final ZitiContext context;
 
     /** The route over Ziti, shared by every intercepted origin. */
     private final Route route;
 
+    /** Completed once the context has loaded its services. */
+    private final CompletableFuture servicesLoaded = new CompletableFuture<>();
+
+    /** The time, in {@link System#nanoTime()}, past which routing no longer waits for them. */
+    private final long servicesDeadline;
+
+    /** Whether each origin, as host:port, is intercepted by a service. */
+    private final Map intercepted = new ConcurrentHashMap<>();
+
+    /** Counts the service events, so that a lookup made meanwhile isn't cached. */
+    private final AtomicLong serviceEvents = new AtomicLong();
+
     /**
-     * Constructor.
+     * Constructor, waiting up to 10 seconds for the services of the context.
      *
      * @param context The Ziti context of the identity used to dial the services.
      */
     public ZitiClientTransport(ZitiContext context) {
+        this(context, DEFAULT_SERVICES_TIMEOUT);
+    }
+
+    /**
+     * Constructor.
+     *
+     * @param context The Ziti context of the identity used to dial the services.
+     * @param servicesTimeout The time to wait for the context to load its services, from now.
+     */
+    public ZitiClientTransport(ZitiContext context, Duration servicesTimeout) {
         this.context = context;
         // The channel maps the host and port to the Ziti service itself.
         this.route =
                 new Route(() -> new ZitiSocketChannel(context), NoopAddressResolverGroup.INSTANCE);
+        this.servicesDeadline = System.nanoTime() + servicesTimeout.toNanos();
+        // Both run as long as the context. The status is active once the services are loaded.
+        context.onStatus(
+                status -> {
+                    if (status instanceof ZitiContext.Status.Active) {
+                        servicesLoaded.complete(null);
+                    }
+                });
+        context.onServiceEvent(
+                event -> {
+                    serviceEvents.incrementAndGet();
+                    intercepted.clear();
+                });
     }
 
     /**
      * Routes a client's calls to the origins the Ziti identity can reach over Ziti.
      *
-     * @param client The client, before it is started.
+     * @param client The client, before it is started. A context is set on it if it has none.
      * @param context The Ziti context of the identity used to dial the services.
      * @return The transport set on the client.
      */
     public static ZitiClientTransport install(Client client, ZitiContext context) {
         ZitiClientTransport transport = new ZitiClientTransport(context);
+        if (client.getContext() == null) {
+            client.setContext(new Context());
+        }
         client.getContext().getAttributes().put(NettyClientTransport.ATTRIBUTE, transport);
         return transport;
     }
@@ -76,8 +140,70 @@ public ZitiContext getContext() {
 
     @Override
     public Route route(String host, int port) {
-        return (context.getService(InetSocketAddress.createUnresolved(host, port)) != null)
-                ? route
-                : null;
+        if (!awaitServices()) {
+            return null;
+        }
+
+        String origin = host + ":" + port;
+        Boolean routed = intercepted.get(origin);
+        if (routed == null) {
+            long events = serviceEvents.get();
+            routed = isIntercepted(host, port);
+            if ((serviceEvents.get() == events) && (intercepted.size() < MAX_CACHED_ORIGINS)) {
+                intercepted.put(origin, routed);
+            }
+        }
+        return routed ? route : null;
+    }
+
+    /**
+     * Waits for the context to load its services, until the deadline.
+     *
+     * @return True if they are loaded.
+     */
+    private boolean awaitServices() {
+        if (servicesLoaded.isDone()) {
+            return true;
+        }
+
+        long left = servicesDeadline - System.nanoTime();
+        try {
+            if (left > 0) {
+                servicesLoaded.get(left, TimeUnit.NANOSECONDS);
+                return true;
+            }
+        } catch (InterruptedException _) {
+            Thread.currentThread().interrupt();
+        } catch (TimeoutException | ExecutionException _) {
+            // Not loaded in time.
+        }
+
+        LOGGER.log(
+                Level.WARNING,
+                "The Ziti context {0} has not loaded its services: connecting over TCP",
+                context.name());
+        return false;
+    }
+
+    /**
+     * Indicates whether a service of the context intercepts an origin, without waiting for a
+     * service event: {@link ZitiContext#getService(InetSocketAddress)} waits for one, forever if
+     * the identity has no service.
+     *
+     * @param host The host name of the origin.
+     * @param port The port of the origin.
+     * @return True if a service intercepts it.
+     */
+    private boolean isIntercepted(String host, int port) {
+        try {
+            // Looks the loaded services up, then times out at once.
+            return context.getService(InetSocketAddress.createUnresolved(host, port), 0L) != null;
+        } catch (Exception e) {
+            // Kotlin doesn't declare the TimeoutException thrown when no service intercepts it.
+            if (e instanceof TimeoutException) {
+                return false;
+            }
+            throw e;
+        }
     }
 }
diff --git a/org.restlet.ext.ziti/src/main/java/org/restlet/ext/ziti/ZitiSocketChannel.java b/org.restlet.ext.ziti/src/main/java/org/restlet/ext/ziti/ZitiSocketChannel.java
index cfd1af313f..6e39f15f0b 100644
--- a/org.restlet.ext.ziti/src/main/java/org/restlet/ext/ziti/ZitiSocketChannel.java
+++ b/org.restlet.ext.ziti/src/main/java/org/restlet/ext/ziti/ZitiSocketChannel.java
@@ -25,13 +25,11 @@
 import java.nio.channels.AsynchronousSocketChannel;
 import java.nio.channels.ClosedChannelException;
 import java.nio.channels.CompletionHandler;
-import java.nio.channels.UnresolvedAddressException;
 import java.util.concurrent.RejectedExecutionException;
 import java.util.concurrent.ScheduledFuture;
 import java.util.concurrent.TimeUnit;
 import org.openziti.ZitiAddress;
 import org.openziti.ZitiContext;
-import org.openziti.edge.model.ServiceDetail;
 
 /**
  * A Netty channel over a connection of the OpenZiti SDK ({@link ZitiContext#open()}), with the
@@ -56,8 +54,11 @@
  * AUTO_READ} are lost, and it blocks the event loop on every write.
  *
  * 

The channel connects to an unresolved {@link InetSocketAddress} such as {@code - * crm.internal:443}: it dials the Ziti service that intercepts it, so the host name is never - * resolved by the system DNS. A {@link ZitiAddress} is dialed as is. + * crm.internal:443}, or to a {@link ZitiAddress}, both handed to the SDK as is. The SDK dials the + * Ziti service intercepting the address, with the destination data and the dial options of its + * intercept configuration (such as the identity of the terminator), and fails with an {@link + * java.nio.channels.UnresolvedAddressException} if none does. It never resolves a host name with + * the system DNS. */ final class ZitiSocketChannel extends AbstractChannel { @@ -66,9 +67,6 @@ final class ZitiSocketChannel extends AbstractChannel { /** The size of the buffer of a read, the payload size of a Ziti message. */ private static final int READ_BUFFER_SIZE = 16 * 1024; - /** The Ziti context dialing the services. */ - private final ZitiContext context; - /** The socket of the Ziti connection. */ private final AsynchronousSocketChannel socket; @@ -90,7 +88,6 @@ final class ZitiSocketChannel extends AbstractChannel { */ ZitiSocketChannel(ZitiContext context) { super(null); - this.context = context; this.socket = context.open(); this.config = new DefaultChannelConfig(this); } @@ -309,7 +306,9 @@ private void writeCompleted(int written) { if (in != null) { // Moves the reader index of the buffer written and releases it once fully written. in.removeBytes(written); - unsafe().flush(); + // Resumes the flushed writes only: the messages written since, without a flush, + // wait for one. + ((ZitiUnsafe) unsafe()).resumeWrites(); } } @@ -333,29 +332,10 @@ protected AbstractUnsafe newUnsafe() { /** Connects with the SDK's asynchronous connect, completed on the event loop. */ private final class ZitiUnsafe extends AbstractUnsafe { - /** - * Returns the Ziti address to dial for a remote address. - * - * @param remote An unresolved {@code host:port} or a Ziti address. - * @return The Ziti address. - */ - private SocketAddress dialAddress(SocketAddress remote) { - if (remote instanceof ZitiAddress) { - return remote; - } - - if (remote instanceof InetSocketAddress address) { - ServiceDetail service = context.getService(address); - - if (service != null) { - return new ZitiAddress.Dial(service.getName(), null, null, null); - } - - throw new UnresolvedAddressException(); - } - - throw new UnsupportedOperationException( - "Unsupported address type: " + remote.getClass().getName()); + /** Writes the flushed messages left, after a write completed. */ + void resumeWrites() { + // Unlike flush(), doesn't flush the messages written since the last flush. + flush0(); } @Override @@ -365,15 +345,6 @@ public void connect( return; } - SocketAddress dial; - try { - dial = dialAddress(remoteAddress); - } catch (RuntimeException e) { - promise.tryFailure(e); - closeIfClosed(); - return; - } - int timeoutMillis = config.getConnectTimeoutMillis(); ScheduledFuture timeout = (timeoutMillis > 0) @@ -394,8 +365,9 @@ public void connect( : null; try { + // The SDK maps an InetSocketAddress to its Ziti dial address itself. socket.connect( - dial, + remoteAddress, null, new CompletionHandler() { @Override diff --git a/org.restlet.ext.ziti/src/test/java/org/restlet/ext/ziti/ZitiClientTransportTestCase.java b/org.restlet.ext.ziti/src/test/java/org/restlet/ext/ziti/ZitiClientTransportTestCase.java index 36d9cefde1..0297578d17 100644 --- a/org.restlet.ext.ziti/src/test/java/org/restlet/ext/ziti/ZitiClientTransportTestCase.java +++ b/org.restlet.ext.ziti/src/test/java/org/restlet/ext/ziti/ZitiClientTransportTestCase.java @@ -11,6 +11,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertSame; import static org.junit.jupiter.api.Assertions.assertTrue; import java.io.IOException; @@ -23,11 +24,14 @@ import java.nio.channels.AsynchronousSocketChannel; import java.nio.channels.CompletionHandler; import java.nio.file.Path; +import java.time.Duration; import java.util.Arrays; import java.util.Set; +import java.util.concurrent.CompletableFuture; import java.util.concurrent.Future; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; +import java.util.function.Consumer; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; @@ -43,6 +47,7 @@ import org.restlet.data.Method; import org.restlet.data.Protocol; import org.restlet.data.Status; +import org.restlet.engine.netty.NettyClientTransport; import org.restlet.representation.ByteArrayRepresentation; /** @@ -63,6 +68,18 @@ class ZitiClientTransportTestCase { /** The number of channels opened through the fake overlay. */ private final AtomicInteger opened = new AtomicInteger(); + /** Whether the fake context has loaded its services when the transport is created. */ + private volatile boolean activeAtStart = true; + + /** The number of service lookups made on the fake context. */ + private final AtomicInteger lookups = new AtomicInteger(); + + /** The status listener registered on the fake context. */ + private volatile Consumer statusListener; + + /** The service event listener registered on the fake context. */ + private volatile Consumer serviceListener; + private Server server; private Client client; @@ -77,7 +94,11 @@ void stop() throws Exception { } } - /** A Ziti context intercepting crm.internal:8443 and dialing the local server. */ + /** + * A Ziti context intercepting crm.internal:8443 and dialing the local server. Like the SDK's, + * its {@code getService(InetSocketAddress)} waits for a service event: here it fails instead. + */ + @SuppressWarnings("unchecked") private ZitiContext fakeZiti() { return (ZitiContext) Proxy.newProxyInstance( @@ -85,13 +106,31 @@ private ZitiContext fakeZiti() { new Class[] {ZitiContext.class}, (proxy, method, args) -> switch (method.getName()) { - case "getService" -> - (args[0] instanceof InetSocketAddress a - && SERVICE_HOST.equals( - a.getHostString()) - && a.getPort() == SERVICE_PORT) - ? new ServiceDetail().name("crm") - : null; + case "getService" -> { + if (args.length == 1) { + throw new AssertionError( + "Waits for a service event, forever if none"); + } + lookups.incrementAndGet(); + yield (args[0] instanceof InetSocketAddress a + && SERVICE_HOST.equals(a.getHostString()) + && a.getPort() == SERVICE_PORT) + ? new ServiceDetail().name("crm") + : null; + } + case "onStatus" -> { + statusListener = (Consumer) args[0]; + if (activeAtStart) { + statusListener.accept( + ZitiContext.Status.Active.INSTANCE); + } + yield new CompletableFuture<>(); + } + case "onServiceEvent" -> { + serviceListener = + (Consumer) args[0]; + yield new CompletableFuture<>(); + } case "open" -> { opened.incrementAndGet(); yield new OverlaySocket( @@ -209,10 +248,50 @@ void routesOnlyInterceptedOrigins() { ZitiClientTransport transport = new ZitiClientTransport(fakeZiti()); assertNotNull(transport.route(SERVICE_HOST, SERVICE_PORT)); + // Without waiting for a service event, which may never come. assertNull(transport.route(SERVICE_HOST, 80)); assertNull(transport.route("example.org", SERVICE_PORT)); } + @Test + void cachesTheLookupsUntilAServiceEvent() { + ZitiClientTransport transport = new ZitiClientTransport(fakeZiti()); + + transport.route("example.org", 443); + transport.route("example.org", 443); + assertEquals(1, lookups.get(), "The lookup is cached"); + + serviceListener.accept( + new ZitiContext.ServiceEvent( + new ServiceDetail().name("crm"), ZitiContext.ServiceUpdate.Available)); + transport.route("example.org", 443); + assertEquals(2, lookups.get(), "A service event clears the cache"); + } + + @Test + void waitsForTheServicesUpToTheTimeout() { + activeAtStart = false; + ZitiClientTransport transport = new ZitiClientTransport(fakeZiti(), Duration.ofMillis(200)); + + long start = System.nanoTime(); + assertNull(transport.route(SERVICE_HOST, SERVICE_PORT), "Not loaded: over TCP"); + assertTrue(System.nanoTime() - start >= Duration.ofMillis(150).toNanos()); + assertEquals(0, lookups.get()); + + statusListener.accept(ZitiContext.Status.Active.INSTANCE); + assertNotNull(transport.route(SERVICE_HOST, SERVICE_PORT), "Loaded: over Ziti"); + } + + @Test + void installsOnAClientWithoutContext() { + client = new Client(Protocol.HTTP); + + ZitiClientTransport transport = ZitiClientTransport.install(client, fakeZiti()); + + assertSame( + transport, client.getContext().getAttributes().get(NettyClientTransport.ATTRIBUTE)); + } + /** TLS runs end to end over the Ziti channel, with the host name of the service verified. */ @Test void runsTlsOverZiti(@TempDir Path dir) throws Exception { diff --git a/org.restlet.ext.ziti/src/test/java/org/restlet/ext/ziti/ZitiSocketChannelTestCase.java b/org.restlet.ext.ziti/src/test/java/org/restlet/ext/ziti/ZitiSocketChannelTestCase.java index a0278099a8..6de950cdea 100644 --- a/org.restlet.ext.ziti/src/test/java/org/restlet/ext/ziti/ZitiSocketChannelTestCase.java +++ b/org.restlet.ext.ziti/src/test/java/org/restlet/ext/ziti/ZitiSocketChannelTestCase.java @@ -122,12 +122,12 @@ private void connect() throws Exception { } @Test - void dialsTheServiceInterceptingTheOrigin() throws Exception { + void letsTheSdkDialTheServiceInterceptingTheOrigin() throws Exception { connect(); assertTrue(channel.isActive()); - ZitiAddress.Dial dialed = assertInstanceOf(ZitiAddress.Dial.class, socket.dialed); - assertEquals("crm", dialed.getService()); + // The SDK maps it to the service, with the dial data and options of its intercept. + assertSame(SERVICE, socket.dialed); assertTrue(recorder.active.get()); } @@ -147,7 +147,7 @@ void failsToConnectToAnOriginWithoutService() { .awaitUninterruptibly(); assertInstanceOf(UnresolvedAddressException.class, future.cause()); - assertEquals(0, socket.connects.get()); + assertFalse(channel.isActive()); } @Test @@ -278,6 +278,30 @@ void writesInOrderWithoutBlocking() throws Exception { assertEquals(1, socket.maxWritesInFlight.get(), "One write is in flight at a time"); } + @Test + void resumesOnlyTheFlushedWrites() throws Exception { + connect(); + socket.holdWrites = true; + + ChannelFuture first = + channel.writeAndFlush(Unpooled.copiedBuffer("first", StandardCharsets.UTF_8)); + ChannelFuture second = + channel.write(Unpooled.copiedBuffer("second", StandardCharsets.UTF_8)); + // Both writes reached the event loop: the first is in flight, the second queued. + onEventLoop(() -> {}); + socket.holdWrites = false; + socket.completeHeldWrite(); + + assertTrue(first.await(5, TimeUnit.SECONDS)); + onEventLoop(() -> {}); + assertFalse(second.isDone(), "A write not flushed was sent"); + assertEquals("first", socket.writtenText()); + + channel.flush(); + assertTrue(second.await(5, TimeUnit.SECONDS)); + assertEquals("firstsecond", socket.writtenText()); + } + @Test void reportsAWriteFailure() throws Exception { connect(); @@ -368,7 +392,8 @@ public void exceptionCaught(ChannelHandlerContext ctx, Throwable cause) { /** * A socket of the SDK, scripted by the test. A read completes inside the call with the next * buffered result, or stays pending until {@link #completePendingRead(String)}. A write - * completes inside the call. + * completes inside the call, or when held, at {@link #completeHeldWrite()}. A connect to an + * address other than the service's fails as with the SDK. */ private static final class ScriptedSocket extends AsynchronousSocketChannel { @@ -399,6 +424,12 @@ private static final class ScriptedSocket extends AsynchronousSocketChannel { private final AtomicInteger writesInFlight = new AtomicInteger(); + /** Whether the next write is held until {@link #completeHeldWrite()}. */ + volatile boolean holdWrites; + + /** Completes the held write, if any. */ + private volatile Runnable heldWrite; + final AtomicInteger maxWritesInFlight = new AtomicInteger(); ScriptedSocket() { @@ -409,6 +440,10 @@ private static final class ScriptedSocket extends AsynchronousSocketChannel { public void connect( SocketAddress remote, A attachment, CompletionHandler handler) { connects.incrementAndGet(); + if (remote instanceof InetSocketAddress address && !SERVICE.equals(address)) { + // The SDK finds no service intercepting it. + throw new UnresolvedAddressException(); + } dialed = remote; if (connectHangs) { return; @@ -484,8 +519,26 @@ public void write( synchronized (written) { written.writeBytes(bytes); } - writesInFlight.decrementAndGet(); - handler.completed(bytes.length, attachment); + Runnable completion = + () -> { + writesInFlight.decrementAndGet(); + handler.completed(bytes.length, attachment); + }; + if (holdWrites) { + heldWrite = completion; + } else { + completion.run(); + } + } + + void completeHeldWrite() { + heldWrite.run(); + } + + String writtenText() { + synchronized (written) { + return written.toString(StandardCharsets.UTF_8); + } } @Override diff --git a/org.restlet/src/main/java/org/restlet/engine/netty/NettyClientCall.java b/org.restlet/src/main/java/org/restlet/engine/netty/NettyClientCall.java index 87f78b06be..0f2604fa83 100644 --- a/org.restlet/src/main/java/org/restlet/engine/netty/NettyClientCall.java +++ b/org.restlet/src/main/java/org/restlet/engine/netty/NettyClientCall.java @@ -90,6 +90,9 @@ class NettyClientCall { /** Indicates whether the exchange is secured with TLS. */ private final boolean secure; + /** The transport route to the origin, or null for TCP and the configured proxies. */ + private final NettyClientTransport.Route route; + /** The "Proxy-Authorization" header value for the CONNECT request of a tunnel, if any. */ private volatile String tunnelProxyAuthorization; @@ -209,6 +212,26 @@ class NettyClientCall { Reference resourceRef, String target, boolean secure) { + this(helper, method, resourceRef, target, secure, null); + } + + /** + * Constructor. + * + * @param helper The parent HTTP client helper. + * @param method The method name. + * @param resourceRef The absolute target resource reference. + * @param target The request target sent on the wire. + * @param secure True for HTTPS. + * @param route The transport route to the origin, or null for TCP and the proxies. + */ + NettyClientCall( + NettyClientHelper helper, + String method, + Reference resourceRef, + String target, + boolean secure, + NettyClientTransport.Route route) { this.helper = helper; this.method = method; this.target = target; @@ -217,6 +240,7 @@ class NettyClientCall { int defaultPort = secure ? 443 : 80; this.targetPort = (port == -1) ? defaultPort : port; this.secure = secure; + this.route = route; } /** @@ -240,6 +264,15 @@ boolean isConfidential() { return secure; } + /** + * Returns the transport route to the origin, chosen when the exchange was created. + * + * @return The route, or null for TCP and the configured proxies. + */ + NettyClientTransport.Route getRoute() { + return route; + } + /** * Returns the "Proxy-Authorization" header value to send with the CONNECT request of a new * tunnel, if any. @@ -259,14 +292,19 @@ String getTunnelProxyIdentity() { return tunnelProxyIdentity; } - /** Indicates whether the exchange goes through a CONNECT tunnel of the HTTP proxy. */ + /** + * Indicates whether the exchange goes through a CONNECT tunnel of the HTTP proxy. A routed + * exchange never does. + */ private boolean isTunneled() { - return secure && (helper.getProxy(true, targetHost) != null); + return (route == null) && secure && (helper.getProxy(true, targetHost) != null); } - /** Indicates whether the exchange is sent to an HTTP forward proxy. */ + /** + * Indicates whether the exchange is sent to an HTTP forward proxy. A routed exchange never is. + */ private boolean isForwardProxied() { - return !secure && (helper.getProxy(false, targetHost) != null); + return (route == null) && !secure && (helper.getProxy(false, targetHost) != null); } /** @@ -376,7 +414,7 @@ void handle(org.restlet.Request request, org.restlet.Response response) { if (isTunneled()) { moveTunnelProxyAuthorization(request); } else if (!isForwardProxied()) { - // Direct or SOCKS connection: the proxy credentials must not reach the origin server. + // Direct, SOCKS or routed connection: the proxy credentials must not reach the origin. requestHeaders.removeAll(HeaderConstants.HEADER_PROXY_AUTHORIZATION, true); } diff --git a/org.restlet/src/main/java/org/restlet/engine/netty/NettyClientHelper.java b/org.restlet/src/main/java/org/restlet/engine/netty/NettyClientHelper.java index deda3a6810..e214784e65 100644 --- a/org.restlet/src/main/java/org/restlet/engine/netty/NettyClientHelper.java +++ b/org.restlet/src/main/java/org/restlet/engine/netty/NettyClientHelper.java @@ -280,6 +280,9 @@ static final class OriginPool { final int port; + /** The transport route of the connections, or null for TCP and the proxies. */ + final NettyClientTransport.Route route; + final Deque idle = new ConcurrentLinkedDeque<>(); /** The connection permits of the origin, shared by its pools. */ @@ -302,12 +305,14 @@ static final class OriginPool { boolean secure, String host, int port, + NettyClientTransport.Route route, Semaphore permits) { this.key = key; this.transientPool = transientPool; this.secure = secure; this.host = host; this.port = port; + this.route = route; this.permits = permits; } } @@ -357,9 +362,17 @@ NettyClientCall create(Request request) { } boolean secure = "https".equalsIgnoreCase(scheme); - boolean forwardProxy = !secure && (getProxy(false, ref.getHostDomain()) != null); + // The route is chosen once per exchange: the request target, the proxy credentials and + // the connection all follow from it. + int port = ref.getHostPort(); + int defaultPort = secure ? 443 : 80; + NettyClientTransport.Route route = + getRoute(ref.getHostDomain(), (port == -1) ? defaultPort : port); + boolean forwardProxy = + (route == null) && !secure && (getProxy(false, ref.getHostDomain()) != null); String target = ReferenceUtils.format(request.getResourceRef(), forwardProxy, request); - return new NettyClientCall(this, request.getMethod().toString(), ref, target, secure); + return new NettyClientCall( + this, request.getMethod().toString(), ref, target, secure, route); } @Override @@ -510,14 +523,18 @@ private static void resetResponse(Response response, Request next) { * @throws IOException If no connection could be established. */ Channel acquire(NettyClientCall call) throws IOException { + // A routed origin is reached over its route's channel, never over QUIC. if (call.isConfidential() + && call.getRoute() == null && getTransportMode() == TransportMode.HTTP3 && getProxy(true, call.getTargetHost()) == null && getSocksProxy(call.getTargetHost()) == null) { return acquireHttp3(call); } - // Tunnels opened with different proxy credentials are kept apart. + // Tunnels opened with different proxy credentials are kept apart, and so are the routed + // connections from the TCP ones, should the route of an origin change. + NettyClientTransport.Route route = call.getRoute(); String origin = (call.isConfidential() ? "https://" : "http://") + call.getTargetHost() @@ -526,7 +543,7 @@ && getSocksProxy(call.getTargetHost()) == null) { String proxyIdentity = call.getTunnelProxyIdentity(); OriginPool pool = pools.computeIfAbsent( - (proxyIdentity == null) ? origin : origin + "#" + proxyIdentity, + poolKey(origin, proxyIdentity, route), k -> new OriginPool( k, @@ -534,6 +551,7 @@ && getSocksProxy(call.getTargetHost()) == null) { call.isConfidential(), call.getTargetHost(), call.getTargetPort(), + route, getPermits(origin))); Channel http2 = pool.http2; @@ -715,7 +733,7 @@ private Channel connect(final OriginPool pool, final String proxyAuthorization) try { final SslContext ssl = pool.secure ? getSslContext() : null; // A transport route (e.g. an overlay network) replaces TCP and the proxies. - final NettyClientTransport.Route route = getRoute(pool); + final NettyClientTransport.Route route = pool.route; final InetSocketAddress proxy = (route == null) ? resolve(getProxy(pool.secure, pool.host)) : null; final InetSocketAddress socksProxy = @@ -861,15 +879,30 @@ private boolean acquirePermit(OriginPool pool, long timeout) throws InterruptedE * {@value NettyClientTransport#ATTRIBUTE} attribute of the client context, or null to connect * over TCP. * - * @param pool The connection pool of the origin. + * @param host The host name of the origin. + * @param port The port of the origin. * @return The route, or null. */ - private NettyClientTransport.Route getRoute(OriginPool pool) { + private NettyClientTransport.Route getRoute(String host, int port) { Object transport = (getContext() == null) ? null : getContext().getAttributes().get(NettyClientTransport.ATTRIBUTE); - return (transport instanceof NettyClientTransport t) ? t.route(pool.host, pool.port) : null; + return (transport instanceof NettyClientTransport t) ? t.route(host, port) : null; + } + + /** + * Returns the key of the connection pool of an origin. + * + * @param origin The origin. + * @param proxyIdentity The identity of the tunnel proxy credentials, or null. + * @param route The transport route, or null. + * @return The key. + */ + private static String poolKey( + String origin, String proxyIdentity, NettyClientTransport.Route route) { + String key = (proxyIdentity == null) ? origin : origin + "#" + proxyIdentity; + return (route == null) ? key : key + "#route@" + System.identityHashCode(route); } /** diff --git a/org.restlet/src/main/java/org/restlet/engine/netty/NettyClientTransport.java b/org.restlet/src/main/java/org/restlet/engine/netty/NettyClientTransport.java index a603a15c98..052f1a8849 100644 --- a/org.restlet/src/main/java/org/restlet/engine/netty/NettyClientTransport.java +++ b/org.restlet/src/main/java/org/restlet/engine/netty/NettyClientTransport.java @@ -17,12 +17,16 @@ * Chooses how the Netty HTTP client connector reaches an origin, so that an extension can route * some origins over another network, such as an overlay network (OpenZiti), instead of TCP. * - *

Set it on a client with the {@value #ATTRIBUTE} attribute of its context. For each new - * connection the connector asks it for a {@link Route} to the origin's host and port: with a route, - * the connection is made with the route's channel factory and resolver, to the unresolved host and - * port, and the HTTP and HTTPS proxies are not used; without one, the connector connects over TCP - * as usual. TLS, HTTP/1.1 and HTTP/2 run over the channel either way. Connections are pooled per - * origin, so an origin is always reached the same way. + *

Set it on a client with the {@value #ATTRIBUTE} attribute of its context. For each request the + * connector asks it for a {@link Route} to the origin's host and port. With a route, the request is + * sent in origin form without the proxy credentials, over a connection made with the route's + * channel factory and resolver to the unresolved host and port: the HTTP, HTTPS and SOCKS proxies + * and HTTP/3 are not used. Without one, the connector connects over TCP as usual. TLS, HTTP/1.1 and + * HTTP/2 run over the channel either way. Routed and TCP connections are pooled apart, so a + * connection is only reused for requests routed the same way. + * + *

Since it is asked for each request, {@link #route(String, int)} must be fast. It may only + * block for a bounded time, for example while an overlay network loads its services. * *

This is a connector SPI: it exposes Netty types and is meant for extensions that depend on * Netty themselves. The {@code org.restlet} module still requires Netty without {@code requires diff --git a/org.restlet/src/test/java/org/restlet/engine/netty/NettyClientTransportTestCase.java b/org.restlet/src/test/java/org/restlet/engine/netty/NettyClientTransportTestCase.java index ec39e32296..2012048391 100644 --- a/org.restlet/src/test/java/org/restlet/engine/netty/NettyClientTransportTestCase.java +++ b/org.restlet/src/test/java/org/restlet/engine/netty/NettyClientTransportTestCase.java @@ -9,9 +9,13 @@ package org.restlet.engine.netty; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assumptions.assumeTrue; import com.sun.net.httpserver.HttpServer; import io.netty.channel.socket.nio.NioSocketChannel; +import io.netty.handler.codec.quic.Quic; import io.netty.resolver.AbstractAddressResolver; import io.netty.resolver.AddressResolver; import io.netty.resolver.AddressResolverGroup; @@ -23,6 +27,7 @@ import java.nio.charset.StandardCharsets; import java.util.List; import java.util.concurrent.CopyOnWriteArrayList; +import java.util.concurrent.atomic.AtomicInteger; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -30,6 +35,8 @@ import org.restlet.Context; import org.restlet.Request; import org.restlet.Response; +import org.restlet.data.ChallengeResponse; +import org.restlet.data.ChallengeScheme; import org.restlet.data.Method; import org.restlet.data.Protocol; import org.restlet.data.Status; @@ -52,6 +59,15 @@ class NettyClientTransportTestCase { /** The addresses the route's resolver resolved. */ private final List resolved = new CopyOnWriteArrayList<>(); + /** The channels the route created, one per routed connection. */ + private final AtomicInteger routedChannels = new AtomicInteger(); + + /** The request targets the peer received. */ + private final List targets = new CopyOnWriteArrayList<>(); + + /** The "Proxy-Authorization" headers the peer received, null when absent. */ + private final List proxyAuthorizations = new CopyOnWriteArrayList<>(); + @BeforeEach void start() throws Exception { Engine.clearThreadLocalVariables(); @@ -61,6 +77,9 @@ void start() throws Exception { peer.createContext( "/", exchange -> { + targets.add(exchange.getRequestURI().toString()); + proxyAuthorizations.add( + exchange.getRequestHeaders().getFirst("Proxy-Authorization")); byte[] body = exchange.getRequestHeaders() .getFirst("Host") @@ -114,7 +133,12 @@ protected void doResolveAll( } }; NettyClientTransport.Route route = - new NettyClientTransport.Route(NioSocketChannel::new, resolver); + new NettyClientTransport.Route( + () -> { + routedChannels.incrementAndGet(); + return new NioSocketChannel(); + }, + resolver); return (host, port) -> { asked.add(host + ":" + port); return ROUTED_HOST.equals(host) ? route : null; @@ -122,8 +146,12 @@ protected void doResolveAll( } private void startClient(Context context) throws Exception { + startClient(context, Protocol.HTTP); + } + + private void startClient(Context context, Protocol protocol) throws Exception { context.getAttributes().put(NettyClientTransport.ATTRIBUTE, transport()); - client = new Client(context, Protocol.HTTP); + client = new Client(context, protocol); client.start(); } @@ -151,11 +179,41 @@ void reusesTheConnectionOfARoutedOrigin() throws Exception { response.getEntity().exhaust(); } - assertEquals(1, asked.size(), "The route is asked once per connection"); + assertEquals(3, asked.size(), "The route is asked for each request"); + assertEquals(1, routedChannels.get(), "The routed connection is reused"); } @Test void doesNotUseTheProxyForARoutedOrigin() throws Exception { + startClient(contextWithClosedProxy()); + Request request = new Request(Method.GET, "http://" + ROUTED_HOST + "/c"); + request.setProxyChallengeResponse( + new ChallengeResponse(ChallengeScheme.HTTP_BASIC, "scott", "tiger")); + + Response response = client.handle(request); + + assertEquals(Status.SUCCESS_OK, response.getStatus(), String.valueOf(response.getStatus())); + assertEquals(ROUTED_HOST, response.getEntity().getText()); + assertEquals(List.of("/c"), targets, "The request target is in origin form"); + assertNull(proxyAuthorizations.get(0), "The proxy credentials reached the origin"); + } + + @Test + void doesNotUseHttp3ForARoutedOrigin() throws Exception { + assumeTrue(Quic.isAvailable(), "The native QUIC library is not available"); + Context context = new Context(); + context.getParameters().add("httpClientTransportMode", "HTTP3"); + startClient(context, Protocol.HTTPS); + + // The peer doesn't speak TLS: the call fails, but over the route, not over QUIC. + client.handle(new Request(Method.GET, "https://" + ROUTED_HOST + "/e")).release(); + + assertEquals(List.of(ROUTED_HOST + ":443"), resolved); + assertFalse(routedChannels.get() == 0, "The route was not used"); + } + + /** Returns a client context whose HTTP proxy is a closed port. */ + private static Context contextWithClosedProxy() throws Exception { int closedPort; try (ServerSocket socket = new ServerSocket(0)) { closedPort = socket.getLocalPort(); @@ -163,12 +221,7 @@ void doesNotUseTheProxyForARoutedOrigin() throws Exception { Context context = new Context(); context.getParameters().add("proxyHost", "127.0.0.1"); context.getParameters().add("proxyPort", Integer.toString(closedPort)); - startClient(context); - - Response response = client.handle(new Request(Method.GET, "http://" + ROUTED_HOST + "/c")); - - assertEquals(Status.SUCCESS_OK, response.getStatus(), String.valueOf(response.getStatus())); - assertEquals(ROUTED_HOST, response.getEntity().getText()); + return context; } @Test From 3211bb2f862c1168aef90ec169151bd819236369 Mon Sep 17 00:00:00 2001 From: Jerome Louvel <374450+jlouvel@users.noreply.github.com> Date: Tue, 10 Nov 2026 18:17:38 -0500 Subject: [PATCH 4/8] Address the SonarCloud issues of the review fixes - Build the warning on missing Ziti services lazily. - Catch the TimeoutException of the service lookup through a lookup interface declaring it, instead of an instanceof test. - Use assertNotEquals in the HTTP/3 routing test. Co-Authored-By: Claude Opus 5.5 --- .../restlet/ext/ziti/ZitiClientTransport.java | 27 ++++++++++++------- .../netty/NettyClientTransportTestCase.java | 4 +-- 2 files changed, 19 insertions(+), 12 deletions(-) diff --git a/org.restlet.ext.ziti/src/main/java/org/restlet/ext/ziti/ZitiClientTransport.java b/org.restlet.ext.ziti/src/main/java/org/restlet/ext/ziti/ZitiClientTransport.java index 698d8b2805..2d49ca0222 100644 --- a/org.restlet.ext.ziti/src/main/java/org/restlet/ext/ziti/ZitiClientTransport.java +++ b/org.restlet.ext.ziti/src/main/java/org/restlet/ext/ziti/ZitiClientTransport.java @@ -180,8 +180,10 @@ private boolean awaitServices() { LOGGER.log( Level.WARNING, - "The Ziti context {0} has not loaded its services: connecting over TCP", - context.name()); + () -> + "The Ziti context " + + context.name() + + " has not loaded its services: connecting over TCP"); return false; } @@ -195,15 +197,20 @@ private boolean awaitServices() { * @return True if a service intercepts it. */ private boolean isIntercepted(String host, int port) { + // Looks the loaded services up, then times out at once. Kotlin doesn't declare the + // TimeoutException thrown when no service intercepts it: the lookup declares it. + ServiceLookup lookup = + () -> context.getService(InetSocketAddress.createUnresolved(host, port), 0L); try { - // Looks the loaded services up, then times out at once. - return context.getService(InetSocketAddress.createUnresolved(host, port), 0L) != null; - } catch (Exception e) { - // Kotlin doesn't declare the TimeoutException thrown when no service intercepts it. - if (e instanceof TimeoutException) { - return false; - } - throw e; + return lookup.get() != null; + } catch (TimeoutException _) { + return false; } } + + /** A service lookup declaring the TimeoutException that the SDK throws undeclared. */ + @FunctionalInterface + private interface ServiceLookup { + Object get() throws TimeoutException; + } } diff --git a/org.restlet/src/test/java/org/restlet/engine/netty/NettyClientTransportTestCase.java b/org.restlet/src/test/java/org/restlet/engine/netty/NettyClientTransportTestCase.java index 2012048391..7c67312587 100644 --- a/org.restlet/src/test/java/org/restlet/engine/netty/NettyClientTransportTestCase.java +++ b/org.restlet/src/test/java/org/restlet/engine/netty/NettyClientTransportTestCase.java @@ -9,7 +9,7 @@ package org.restlet.engine.netty; import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotEquals; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assumptions.assumeTrue; @@ -209,7 +209,7 @@ void doesNotUseHttp3ForARoutedOrigin() throws Exception { client.handle(new Request(Method.GET, "https://" + ROUTED_HOST + "/e")).release(); assertEquals(List.of(ROUTED_HOST + ":443"), resolved); - assertFalse(routedChannels.get() == 0, "The route was not used"); + assertNotEquals(0, routedChannels.get(), "The route was not used"); } /** Returns a client context whose HTTP proxy is a closed port. */ From fbc149d97eabd65756ae633eb4ac10b366bd1d1d Mon Sep 17 00:00:00 2001 From: Jerome Louvel <374450+jlouvel@users.noreply.github.com> Date: Tue, 10 Nov 2026 19:03:57 -0500 Subject: [PATCH 5/8] Key the routed pools on the route itself, drop them once unused, and fix a cache race - NettyClientHelper: key the connection pools with a PoolKey record holding the route, instead of the route's identity hash code, which two routes can share. Routed pools are transient like the tunnel ones: dropped once they have no connection left, so retired routes don't accumulate. - ZitiClientTransport: uncache a lookup when a service event was counted while it was published, so that a pre-event result isn't left in the cache. Co-Authored-By: Claude Opus 5.5 --- .../restlet/ext/ziti/ZitiClientTransport.java | 6 ++++ .../engine/netty/NettyClientHelper.java | 30 ++++++++-------- .../netty/NettyClientTransportTestCase.java | 36 +++++++++++++++++++ 3 files changed, 57 insertions(+), 15 deletions(-) diff --git a/org.restlet.ext.ziti/src/main/java/org/restlet/ext/ziti/ZitiClientTransport.java b/org.restlet.ext.ziti/src/main/java/org/restlet/ext/ziti/ZitiClientTransport.java index 2d49ca0222..64d68b6865 100644 --- a/org.restlet.ext.ziti/src/main/java/org/restlet/ext/ziti/ZitiClientTransport.java +++ b/org.restlet.ext.ziti/src/main/java/org/restlet/ext/ziti/ZitiClientTransport.java @@ -151,6 +151,12 @@ public Route route(String host, int port) { routed = isIntercepted(host, port); if ((serviceEvents.get() == events) && (intercepted.size() < MAX_CACHED_ORIGINS)) { intercepted.put(origin, routed); + // A service event counted after the check above may have cleared the cache + // before the put: the result may be stale, so it is uncached. One counted after + // this check clears it afterwards. + if (serviceEvents.get() != events) { + intercepted.remove(origin, routed); + } } } return routed ? route : null; diff --git a/org.restlet/src/main/java/org/restlet/engine/netty/NettyClientHelper.java b/org.restlet/src/main/java/org/restlet/engine/netty/NettyClientHelper.java index e214784e65..e87f86cc2e 100644 --- a/org.restlet/src/main/java/org/restlet/engine/netty/NettyClientHelper.java +++ b/org.restlet/src/main/java/org/restlet/engine/netty/NettyClientHelper.java @@ -269,9 +269,12 @@ protected enum TransportMode { static final class OriginPool { /** The key of the pool. */ - final String key; + final PoolKey key; - /** Indicates whether the pool is specific to proxy credentials, and dropped once unused. */ + /** + * Indicates whether the pool is specific to proxy credentials or to a route, and dropped + * once unused. + */ final boolean transientPool; final boolean secure; @@ -300,7 +303,7 @@ static final class OriginPool { volatile Channel http2; OriginPool( - String key, + PoolKey key, boolean transientPool, boolean secure, String host, @@ -333,7 +336,7 @@ static final class OriginPool { private final Map http3Connections = new ConcurrentHashMap<>(); /** The connection pools, per origin, and per proxy credentials for tunnels. */ - private final Map pools = new ConcurrentHashMap<>(); + private final Map pools = new ConcurrentHashMap<>(); /** The connection permits, per origin. */ private final Map permits = new ConcurrentHashMap<>(); @@ -543,11 +546,11 @@ && getSocksProxy(call.getTargetHost()) == null) { String proxyIdentity = call.getTunnelProxyIdentity(); OriginPool pool = pools.computeIfAbsent( - poolKey(origin, proxyIdentity, route), + new PoolKey(origin, proxyIdentity, route), k -> new OriginPool( k, - proxyIdentity != null, + (proxyIdentity != null) || (route != null), call.isConfidential(), call.getTargetHost(), call.getTargetPort(), @@ -892,23 +895,20 @@ private NettyClientTransport.Route getRoute(String host, int port) { } /** - * Returns the key of the connection pool of an origin. + * The key of the connection pool of an origin. Equal routes (same channel factory and resolver) + * share a pool, distinct ones never do. * * @param origin The origin. * @param proxyIdentity The identity of the tunnel proxy credentials, or null. * @param route The transport route, or null. - * @return The key. */ - private static String poolKey( - String origin, String proxyIdentity, NettyClientTransport.Route route) { - String key = (proxyIdentity == null) ? origin : origin + "#" + proxyIdentity; - return (route == null) ? key : key + "#route@" + System.identityHashCode(route); - } + record PoolKey(String origin, String proxyIdentity, NettyClientTransport.Route route) {} /** * Releases a connection of a pool, closed or not opened. A pool specific to proxy credentials - * is dropped once it has no connection left, so that such pools do not accumulate. A call - * holding it meanwhile still works, its connection is simply not reused by the next calls. + * or to a route is dropped once it has no connection left, so that such pools do not + * accumulate, for example when a transport returns a new route over time. A call holding it + * meanwhile still works, its connection is simply not reused by the next calls. * * @param pool The pool. * @param permit True if the connection held a permit. diff --git a/org.restlet/src/test/java/org/restlet/engine/netty/NettyClientTransportTestCase.java b/org.restlet/src/test/java/org/restlet/engine/netty/NettyClientTransportTestCase.java index 7c67312587..fad2c12d91 100644 --- a/org.restlet/src/test/java/org/restlet/engine/netty/NettyClientTransportTestCase.java +++ b/org.restlet/src/test/java/org/restlet/engine/netty/NettyClientTransportTestCase.java @@ -183,6 +183,42 @@ void reusesTheConnectionOfARoutedOrigin() throws Exception { assertEquals(1, routedChannels.get(), "The routed connection is reused"); } + @Test + void usesTheConnectionsOfTheCurrentRoute() throws Exception { + AtomicInteger otherChannels = new AtomicInteger(); + NettyClientTransport first = transport(); + NettyClientTransport.Route other = + new NettyClientTransport.Route( + () -> { + otherChannels.incrementAndGet(); + return new NioSocketChannel(); + }, + first.route(ROUTED_HOST, 80).resolver()); + AtomicInteger requests = new AtomicInteger(); + Context context = new Context(); + // The route of the origin changes after the first request. + context.getAttributes() + .put( + NettyClientTransport.ATTRIBUTE, + (NettyClientTransport) + (host, port) -> + (requests.getAndIncrement() == 0) + ? first.route(host, port) + : other); + client = new Client(context, Protocol.HTTP); + client.start(); + + for (int i = 0; i < 2; i++) { + Response response = + client.handle(new Request(Method.GET, "http://" + ROUTED_HOST + "/f" + i)); + assertEquals(Status.SUCCESS_OK, response.getStatus()); + response.getEntity().exhaust(); + } + + assertEquals(1, routedChannels.get(), "The first route connected once"); + assertEquals(1, otherChannels.get(), "The new route didn't reuse the old connection"); + } + @Test void doesNotUseTheProxyForARoutedOrigin() throws Exception { startClient(contextWithClosedProxy()); From d156d3230ac8de013cc88d1aa3aedb22c4965cea Mon Sep 17 00:00:00 2001 From: Jerome Louvel <374450+jlouvel@users.noreply.github.com> Date: Tue, 10 Nov 2026 19:16:32 -0500 Subject: [PATCH 6/8] Extract the pool lookup out of NettyClientHelper.acquire() Reduces its cognitive complexity below SonarCloud's limit. Co-Authored-By: Claude Opus 5.5 --- .../engine/netty/NettyClientHelper.java | 50 +++++++++++-------- 1 file changed, 29 insertions(+), 21 deletions(-) diff --git a/org.restlet/src/main/java/org/restlet/engine/netty/NettyClientHelper.java b/org.restlet/src/main/java/org/restlet/engine/netty/NettyClientHelper.java index e87f86cc2e..bd173b3bdf 100644 --- a/org.restlet/src/main/java/org/restlet/engine/netty/NettyClientHelper.java +++ b/org.restlet/src/main/java/org/restlet/engine/netty/NettyClientHelper.java @@ -535,27 +535,7 @@ && getSocksProxy(call.getTargetHost()) == null) { return acquireHttp3(call); } - // Tunnels opened with different proxy credentials are kept apart, and so are the routed - // connections from the TCP ones, should the route of an origin change. - NettyClientTransport.Route route = call.getRoute(); - String origin = - (call.isConfidential() ? "https://" : "http://") - + call.getTargetHost() - + ":" - + call.getTargetPort(); - String proxyIdentity = call.getTunnelProxyIdentity(); - OriginPool pool = - pools.computeIfAbsent( - new PoolKey(origin, proxyIdentity, route), - k -> - new OriginPool( - k, - (proxyIdentity != null) || (route != null), - call.isConfidential(), - call.getTargetHost(), - call.getTargetPort(), - route, - getPermits(origin))); + OriginPool pool = getPool(call); Channel http2 = pool.http2; if (http2 != null && http2.isActive()) { @@ -839,6 +819,34 @@ private SocketAddress configureTransport( return InetSocketAddress.createUnresolved(pool.host, pool.port); } + /** + * Returns the connection pool of a call. Tunnels opened with different proxy credentials are + * kept apart, and so are the routed connections from the TCP ones, and per route. + * + * @param call The call. + * @return The pool. + */ + private OriginPool getPool(NettyClientCall call) { + NettyClientTransport.Route route = call.getRoute(); + String origin = + (call.isConfidential() ? "https://" : "http://") + + call.getTargetHost() + + ":" + + call.getTargetPort(); + String proxyIdentity = call.getTunnelProxyIdentity(); + return pools.computeIfAbsent( + new PoolKey(origin, proxyIdentity, route), + k -> + new OriginPool( + k, + (proxyIdentity != null) || (route != null), + call.isConfidential(), + call.getTargetHost(), + call.getTargetPort(), + route, + getPermits(origin))); + } + /** * Returns the connection permits of an origin, shared by its pools. * From 113c4b28c74227f78cd3e15fb469384e928e1512 Mon Sep 17 00:00:00 2001 From: Jerome Louvel <374450+jlouvel@users.noreply.github.com> Date: Wed, 11 Nov 2026 06:15:52 -0500 Subject: [PATCH 7/8] Reserve routed pool connections atomically, reclaim idle HTTP/2 permits, and close Ziti transports Address the third Copilot review: - Count a new connection in a routed pool and drop an empty one under the pool map's lock of their key, so a connection is never opened in a pool already dropped, where it couldn't be reused nor its permit reclaimed. A call whose pool was dropped takes the current one. - Count the streams of each HTTP/2 connection. Waiting for a permit, a call closes an idle connection of another pool of the origin: an HTTP/1.1 one, or now an HTTP/2 one without streams, retired first so that no stream is opened on it. It looks again while it waits, until one closes, since a connection becomes idle only once its last stream has closed. - ZitiClientTransport keeps its subscriptions to the context's events and cancels them on close(), without destroying the context; install() closes the Ziti transport it replaces. - The network test looks the services up without waiting, and interrupts a check still running when it gives up. Co-Authored-By: Claude Opus 5.5 --- changes.md | 3 +- .../restlet/ext/ziti/ZitiClientTransport.java | 67 ++++-- .../ext/ziti/ZitiClientTransportTestCase.java | 46 +++- .../restlet/ext/ziti/ZitiNetworkTestCase.java | 38 +++- .../engine/netty/NettyClientHelper.java | 201 ++++++++++++++++-- .../netty/NettyClientTransportTestCase.java | 76 ++++++- 6 files changed, 384 insertions(+), 47 deletions(-) diff --git a/changes.md b/changes.md index 670564ea89..d32ae9ed23 100644 --- a/changes.md +++ b/changes.md @@ -289,7 +289,8 @@ Changes log client to the origins an OpenZiti identity has a service for, such as `crm.internal:443`, over the Ziti overlay network, without resolving their host name with the system DNS, and the other calls over TCP. TLS, HTTP/1.1 and HTTP/2 run over it - as over TCP. Client side only. + as over TCP. Client side only. The transport listens to the context's events until + closed: close it once the client is stopped, or install another one, which closes it. - Added the `NettyClientTransport` SPI to the Netty client connector, set with the "org.restlet.engine.netty.clientTransport" client context attribute: it can route the requests to some origins through another Netty channel and resolver than TCP, the HTTP diff --git a/org.restlet.ext.ziti/src/main/java/org/restlet/ext/ziti/ZitiClientTransport.java b/org.restlet.ext.ziti/src/main/java/org/restlet/ext/ziti/ZitiClientTransport.java index 64d68b6865..4b1ebbd02e 100644 --- a/org.restlet.ext.ziti/src/main/java/org/restlet/ext/ziti/ZitiClientTransport.java +++ b/org.restlet.ext.ziti/src/main/java/org/restlet/ext/ziti/ZitiClientTransport.java @@ -15,6 +15,7 @@ import java.util.concurrent.CompletableFuture; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ExecutionException; +import java.util.concurrent.Future; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; import java.util.concurrent.atomic.AtomicLong; @@ -48,9 +49,11 @@ * ZitiClientTransport.install(client, ztx); *

* - *

The context is not owned: destroy it after the client is stopped. + *

The context is not owned: destroy it after the client is stopped. The transport listens to the + * events of the context: close it once the client is stopped, to stop listening while the context + * lives on. Installing a new transport on a client closes the one it replaces. */ -public class ZitiClientTransport implements NettyClientTransport { +public class ZitiClientTransport implements NettyClientTransport, AutoCloseable { private static final Logger LOGGER = Logger.getLogger(ZitiClientTransport.class.getName()); @@ -78,6 +81,15 @@ public class ZitiClientTransport implements NettyClientTransport { /** Counts the service events, so that a lookup made meanwhile isn't cached. */ private final AtomicLong serviceEvents = new AtomicLong(); + /** The subscription to the status of the context. */ + private final Future statusSubscription; + + /** The subscription to the service events of the context. */ + private final Future serviceSubscription; + + /** Whether the transport is closed: it then no longer caches the lookups. */ + private volatile boolean closed; + /** * Constructor, waiting up to 10 seconds for the services of the context. * @@ -99,22 +111,25 @@ public ZitiClientTransport(ZitiContext context, Duration servicesTimeout) { this.route = new Route(() -> new ZitiSocketChannel(context), NoopAddressResolverGroup.INSTANCE); this.servicesDeadline = System.nanoTime() + servicesTimeout.toNanos(); - // Both run as long as the context. The status is active once the services are loaded. - context.onStatus( - status -> { - if (status instanceof ZitiContext.Status.Active) { - servicesLoaded.complete(null); - } - }); - context.onServiceEvent( - event -> { - serviceEvents.incrementAndGet(); - intercepted.clear(); - }); + // Both run until closed. The status is active once the services are loaded. + this.statusSubscription = + context.onStatus( + status -> { + if (status instanceof ZitiContext.Status.Active) { + servicesLoaded.complete(null); + } + }); + this.serviceSubscription = + context.onServiceEvent( + event -> { + serviceEvents.incrementAndGet(); + intercepted.clear(); + }); } /** - * Routes a client's calls to the origins the Ziti identity can reach over Ziti. + * Routes a client's calls to the origins the Ziti identity can reach over Ziti. A Ziti + * transport installed before on the client is closed. * * @param client The client, before it is started. A context is set on it if it has none. * @param context The Ziti context of the identity used to dial the services. @@ -125,10 +140,26 @@ public static ZitiClientTransport install(Client client, ZitiContext context) { if (client.getContext() == null) { client.setContext(new Context()); } - client.getContext().getAttributes().put(NettyClientTransport.ATTRIBUTE, transport); + Object replaced = + client.getContext().getAttributes().put(NettyClientTransport.ATTRIBUTE, transport); + if (replaced instanceof ZitiClientTransport previous) { + previous.close(); + } return transport; } + /** + * Stops listening to the events of the context, without destroying it. A closed transport still + * routes, without caching the lookups since it no longer sees the service events. + */ + @Override + public void close() { + closed = true; + statusSubscription.cancel(true); + serviceSubscription.cancel(true); + intercepted.clear(); + } + /** * Returns the Ziti context. * @@ -149,7 +180,9 @@ public Route route(String host, int port) { if (routed == null) { long events = serviceEvents.get(); routed = isIntercepted(host, port); - if ((serviceEvents.get() == events) && (intercepted.size() < MAX_CACHED_ORIGINS)) { + if (!closed + && (serviceEvents.get() == events) + && (intercepted.size() < MAX_CACHED_ORIGINS)) { intercepted.put(origin, routed); // A service event counted after the check above may have cleared the cache // before the put: the result may be stale, so it is uncached. One counted after diff --git a/org.restlet.ext.ziti/src/test/java/org/restlet/ext/ziti/ZitiClientTransportTestCase.java b/org.restlet.ext.ziti/src/test/java/org/restlet/ext/ziti/ZitiClientTransportTestCase.java index 0297578d17..b5d7a32411 100644 --- a/org.restlet.ext.ziti/src/test/java/org/restlet/ext/ziti/ZitiClientTransportTestCase.java +++ b/org.restlet.ext.ziti/src/test/java/org/restlet/ext/ziti/ZitiClientTransportTestCase.java @@ -26,8 +26,10 @@ import java.nio.file.Path; import java.time.Duration; import java.util.Arrays; +import java.util.List; import java.util.Set; import java.util.concurrent.CompletableFuture; +import java.util.concurrent.CopyOnWriteArrayList; import java.util.concurrent.Future; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; @@ -80,6 +82,9 @@ class ZitiClientTransportTestCase { /** The service event listener registered on the fake context. */ private volatile Consumer serviceListener; + /** The subscriptions made to the fake context, cancelled when ended. */ + private final List> subscriptions = new CopyOnWriteArrayList<>(); + private Server server; private Client client; @@ -124,12 +129,12 @@ private ZitiContext fakeZiti() { statusListener.accept( ZitiContext.Status.Active.INSTANCE); } - yield new CompletableFuture<>(); + yield subscription(); } case "onServiceEvent" -> { serviceListener = (Consumer) args[0]; - yield new CompletableFuture<>(); + yield subscription(); } case "open" -> { opened.incrementAndGet(); @@ -146,6 +151,13 @@ yield new OverlaySocket( }); } + /** A subscription to the fake context, running until cancelled. */ + private CompletableFuture subscription() { + CompletableFuture subscription = new CompletableFuture<>(); + subscriptions.add(subscription); + return subscription; + } + /** Echoes the path and the number of bytes received, then the received bytes if asked. */ private static Restlet echo() { return new Restlet() { @@ -292,6 +304,36 @@ void installsOnAClientWithoutContext() { transport, client.getContext().getAttributes().get(NettyClientTransport.ATTRIBUTE)); } + /** Closing stops listening to the context, which lives on, and stops caching the lookups. */ + @Test + void closingStopsListeningToTheContext() { + ZitiClientTransport transport = new ZitiClientTransport(fakeZiti()); + assertEquals(2, subscriptions.size()); + assertTrue(subscriptions.stream().noneMatch(CompletableFuture::isCancelled)); + + transport.close(); + + assertTrue(subscriptions.stream().allMatch(CompletableFuture::isCancelled)); + assertNotNull(transport.route(SERVICE_HOST, SERVICE_PORT), "It still routes"); + transport.route(SERVICE_HOST, SERVICE_PORT); + assertEquals(2, lookups.get(), "Without the service events, no lookup is cached"); + } + + /** Installing a transport on a client closes the one it replaces. */ + @Test + void installingClosesTheReplacedTransport() { + client = new Client(new Context(), Protocol.HTTP); + ZitiContext ziti = fakeZiti(); + ZitiClientTransport.install(client, ziti); + List> first = List.copyOf(subscriptions); + + ZitiClientTransport.install(client, ziti); + + assertTrue(first.stream().allMatch(CompletableFuture::isCancelled)); + assertEquals(4, subscriptions.size()); + assertTrue(subscriptions.subList(2, 4).stream().noneMatch(CompletableFuture::isCancelled)); + } + /** TLS runs end to end over the Ziti channel, with the host name of the service verified. */ @Test void runsTlsOverZiti(@TempDir Path dir) throws Exception { diff --git a/org.restlet.ext.ziti/src/test/java/org/restlet/ext/ziti/ZitiNetworkTestCase.java b/org.restlet.ext.ziti/src/test/java/org/restlet/ext/ziti/ZitiNetworkTestCase.java index 839695bed8..5d640936b2 100644 --- a/org.restlet.ext.ziti/src/test/java/org/restlet/ext/ziti/ZitiNetworkTestCase.java +++ b/org.restlet.ext.ziti/src/test/java/org/restlet/ext/ziti/ZitiNetworkTestCase.java @@ -44,6 +44,7 @@ import org.openziti.Ziti; import org.openziti.ZitiAddress; import org.openziti.ZitiContext; +import org.openziti.edge.model.ServiceDetail; import org.restlet.Client; import org.restlet.Context; import org.restlet.Request; @@ -95,7 +96,7 @@ static void start() throws Exception { // A context learns its services asynchronously after authenticating: binding before the // server identity has loaded the service is refused as "no permission to bind". awaitUntil( - () -> serverZiti.getService(SERVICE) != null, + () -> loaded(() -> serverZiti.getService(SERVICE, 0L)) != null, "The server never saw the crm service"); // Host the service: accept the Ziti connections and pipe them to the local server. @@ -116,14 +117,34 @@ static void start() throws Exception { */ private static void awaitReachable(InetSocketAddress address) throws Exception { awaitUntil( - () -> - clientZiti.getService(address) != null - && !clientZiti - .getServiceTerminators(clientZiti.getService(address)) - .isEmpty(), + () -> { + ServiceDetail service = loaded(() -> clientZiti.getService(address, 0L)); + return service != null && !clientZiti.getServiceTerminators(service).isEmpty(); + }, "The service of " + address + " is not reachable"); } + /** + * Looks a service up among the loaded ones, without waiting: the overloads without timeout wait + * for a service event, forever if the identity has none. + * + * @param lookup A lookup with a zero timeout. + * @return The service, or null if not loaded. + */ + private static ServiceDetail loaded(ServiceLookup lookup) { + try { + return lookup.get(); + } catch (TimeoutException _) { + return null; + } + } + + /** A service lookup declaring the TimeoutException that the SDK throws undeclared. */ + @FunctionalInterface + private interface ServiceLookup { + ServiceDetail get() throws TimeoutException; + } + /** * Waits up to a minute for a condition on the network, checked every 250 ms. * @@ -132,7 +153,8 @@ private static void awaitReachable(InetSocketAddress address) throws Exception { */ private static void awaitUntil(BooleanSupplier condition, String message) throws Exception { CompletableFuture met = new CompletableFuture<>(); - try (ScheduledExecutorService poller = Executors.newSingleThreadScheduledExecutor()) { + ScheduledExecutorService poller = Executors.newSingleThreadScheduledExecutor(); + try { poller.scheduleWithFixedDelay( () -> { try { @@ -151,6 +173,8 @@ private static void awaitUntil(BooleanSupplier condition, String message) throws fail(message); } finally { met.cancel(false); + // Interrupts a check still running, rather than waiting for it. + poller.shutdownNow(); } } diff --git a/org.restlet/src/main/java/org/restlet/engine/netty/NettyClientHelper.java b/org.restlet/src/main/java/org/restlet/engine/netty/NettyClientHelper.java index bd173b3bdf..45a7bdadf8 100644 --- a/org.restlet/src/main/java/org/restlet/engine/netty/NettyClientHelper.java +++ b/org.restlet/src/main/java/org/restlet/engine/netty/NettyClientHelper.java @@ -254,6 +254,16 @@ public abstract class NettyClientHelper extends ClientHelper { private static final AttributeKey POOL = AttributeKey.valueOf(NettyClientHelper.class, "pool"); + /** + * The streams of an HTTP/2 connection, open or reserved, or -1 once it is retired to free its + * permit. + */ + private static final AttributeKey STREAMS = + AttributeKey.valueOf(NettyClientHelper.class, "streams"); + + /** How often a call waiting for a connection permit looks for an idle connection to close. */ + private static final long PERMIT_POLL_NANOS = TimeUnit.MILLISECONDS.toNanos(50); + /** Supported transport modes. */ protected enum TransportMode { HTTP1_1, @@ -535,10 +545,26 @@ && getSocksProxy(call.getTargetHost()) == null) { return acquireHttp3(call); } - OriginPool pool = getPool(call); + Channel channel; + do { + // Null when the pool was dropped before it could connect: the next one is taken. + channel = acquire(getPool(call), call); + } while (channel == null); + return channel; + } + /** + * Acquires a channel of a pool: an idle or new HTTP/1.1 connection, or a new stream of its + * HTTP/2 connection. + * + * @param pool The pool. + * @param call The call that will use the channel. + * @return The channel, or null if the pool was dropped before it could open a connection. + * @throws IOException If no connection could be established. + */ + private Channel acquire(OriginPool pool, NettyClientCall call) throws IOException { Channel http2 = pool.http2; - if (http2 != null && http2.isActive()) { + if (http2 != null && http2.isActive() && reserveStream(http2)) { return openStream(http2, call); } @@ -560,11 +586,13 @@ && getSocksProxy(call.getTargetHost()) == null) { pool.connectLock.lock(); try { http2 = pool.http2; - if (http2 != null && http2.isActive()) { + if (http2 != null && http2.isActive() && reserveStream(http2)) { return openStream(http2, call); } channel = connect(pool, call.getTunnelProxyAuthorization()); - if (channel.pipeline().get(Http2MultiplexHandler.class) != null) { + if (channel != null + && channel.pipeline().get(Http2MultiplexHandler.class) != null) { + channel.attr(STREAMS).set(new AtomicInteger(1)); pool.http2 = channel; return openStream(channel, call); } @@ -573,10 +601,56 @@ && getSocksProxy(call.getTargetHost()) == null) { } } - bindHttp1(channel, call); + if (channel != null) { + bindHttp1(channel, call); + } return channel; } + /** + * Reserves a stream on an HTTP/2 connection, unless it was retired to free its permit. + * + * @param parent The HTTP/2 connection. + * @return True if reserved: the stream must then be opened, or the reservation released. + */ + private static boolean reserveStream(Channel parent) { + AtomicInteger streams = parent.attr(STREAMS).get(); + if (streams == null) { + return false; + } + int count; + do { + count = streams.get(); + if (count < 0) { + return false; + } + } while (!streams.compareAndSet(count, count + 1)); + return true; + } + + /** + * Releases a stream reserved on an HTTP/2 connection. + * + * @param parent The HTTP/2 connection. + */ + private static void releaseStream(Channel parent) { + AtomicInteger streams = parent.attr(STREAMS).get(); + if (streams != null) { + streams.decrementAndGet(); + } + } + + /** + * Retires an HTTP/2 connection without streams, so that no stream is opened on it any more. + * + * @param parent The HTTP/2 connection. + * @return True if retired: it can then be closed. + */ + private static boolean retireIdle(Channel parent) { + AtomicInteger streams = parent.attr(STREAMS).get(); + return (streams != null) && streams.compareAndSet(0, -1); + } + /** * Opens an HTTP/3 request stream, on the shared QUIC connection of the origin. * @@ -674,7 +748,23 @@ private void bindHttp1(Channel channel, NettyClientCall call) throws IOException handler.bind(call); } + /** + * Opens a stream reserved on an HTTP/2 connection. The reservation is released when the stream + * closes, or at once if it can't be opened. + */ private Channel openStream(Channel parent, NettyClientCall call) throws IOException { + Channel stream; + try { + stream = openReservedStream(parent, call); + } catch (IOException | RuntimeException e) { + releaseStream(parent); + throw e; + } + stream.closeFuture().addListener(f -> releaseStream(parent)); + return stream; + } + + private Channel openReservedStream(Channel parent, NettyClientCall call) throws IOException { final NettyClientHandler handler = new NettyClientHandler(); handler.bind(call); call.setStreamKind(StreamKind.HTTP2); @@ -699,7 +789,9 @@ protected void initChannel(Channel stream) { private Channel connect(final OriginPool pool, final String proxyAuthorization) throws IOException { long timeout = getConnectTimeout(); - pool.connections.incrementAndGet(); + if (!reserveConnection(pool)) { + return null; + } try { if (!acquirePermit(pool, timeout)) { releaseConnection(pool, false); @@ -860,8 +952,8 @@ private Semaphore getPermits(String origin) { /** * Acquires a connection permit of the origin of a pool. When they are all taken, an idle - * connection of another pool of the origin (opened with other proxy credentials) is closed to - * free one. + * connection of another pool of the origin (opened with other proxy credentials, or over + * another route) is closed to free one: an HTTP/1.1 one, or an HTTP/2 one without streams. * * @param pool The pool. * @param timeout The max time to wait, in milliseconds. @@ -869,20 +961,57 @@ private Semaphore getPermits(String origin) { * @throws InterruptedException If interrupted while waiting. */ private boolean acquirePermit(OriginPool pool, long timeout) throws InterruptedException { - if (pool.permits.tryAcquire()) { - return true; + long deadline = System.nanoTime() + TimeUnit.MILLISECONDS.toNanos(timeout); + boolean closed = false; + while (!pool.permits.tryAcquire()) { + // A connection becomes idle once its exchange or its last stream ends: until one is + // closed, look again while waiting. + closed = closed || closeIdleOfOtherPool(pool); + long left = deadline - System.nanoTime(); + if (left <= 0) { + return false; + } + long wait = closed ? left : Math.min(left, PERMIT_POLL_NANOS); + if (pool.permits.tryAcquire(wait, TimeUnit.NANOSECONDS)) { + return true; + } } + return true; + } + /** + * Closes an idle connection of another pool of the origin of a pool. + * + * @param pool The pool. + * @return True if a connection was closed. + */ + private boolean closeIdleOfOtherPool(OriginPool pool) { for (OriginPool other : pools.values()) { - if ((other != pool) && (other.permits == pool.permits)) { - Channel idle = other.idle.pollLast(); - if (idle != null) { - idle.close(); - break; - } + if ((other != pool) && (other.permits == pool.permits) && closeIdle(other)) { + return true; } } - return pool.permits.tryAcquire(timeout, TimeUnit.MILLISECONDS); + return false; + } + + /** + * Closes an idle connection of a pool, to free its permit: an HTTP/1.1 one not carrying an + * exchange, or its HTTP/2 one without streams. + * + * @param pool The pool. + * @return True if a connection was closed. + */ + private static boolean closeIdle(OriginPool pool) { + Channel idle = pool.idle.pollLast(); + if (idle == null) { + Channel http2 = pool.http2; + idle = (http2 != null && retireIdle(http2)) ? http2 : null; + } + if (idle == null) { + return false; + } + idle.close(); + return true; } /** @@ -925,9 +1054,43 @@ private void releaseConnection(OriginPool pool, boolean permit) { if (permit) { pool.permits.release(); } - if ((pool.connections.decrementAndGet() == 0) && pool.transientPool) { - pools.remove(pool.key, pool); + if (!pool.transientPool) { + pool.connections.decrementAndGet(); + return; + } + // Under the lock of the key, so that no connection is reserved meanwhile. + pools.compute( + pool.key, + (k, current) -> + ((pool.connections.decrementAndGet() == 0) && (current == pool)) + ? null + : current); + } + + /** + * Counts a connection about to be opened in a pool, unless the pool was dropped: a connection + * of a dropped pool could not be reused nor have its permit reclaimed. + * + * @param pool The pool. + * @return True if counted, false if the pool was dropped. + */ + private boolean reserveConnection(OriginPool pool) { + if (!pool.transientPool) { + pool.connections.incrementAndGet(); + return true; } + // Under the lock of the key, so that the pool isn't dropped meanwhile. + boolean[] reserved = {false}; + pools.computeIfPresent( + pool.key, + (k, current) -> { + if (current == pool) { + pool.connections.incrementAndGet(); + reserved[0] = true; + } + return current; + }); + return reserved[0]; } private void initConnection( diff --git a/org.restlet/src/test/java/org/restlet/engine/netty/NettyClientTransportTestCase.java b/org.restlet/src/test/java/org/restlet/engine/netty/NettyClientTransportTestCase.java index fad2c12d91..67e3a14c75 100644 --- a/org.restlet/src/test/java/org/restlet/engine/netty/NettyClientTransportTestCase.java +++ b/org.restlet/src/test/java/org/restlet/engine/netty/NettyClientTransportTestCase.java @@ -35,13 +35,17 @@ import org.restlet.Context; import org.restlet.Request; import org.restlet.Response; +import org.restlet.Restlet; +import org.restlet.Server; import org.restlet.data.ChallengeResponse; import org.restlet.data.ChallengeScheme; +import org.restlet.data.MediaType; import org.restlet.data.Method; import org.restlet.data.Protocol; import org.restlet.data.Status; import org.restlet.engine.Engine; import org.restlet.engine.connector.HttpClientHelper; +import org.restlet.engine.connector.HttpServerHelper; /** Tests the routes of a {@link NettyClientTransport} set on a client. */ class NettyClientTransportTestCase { @@ -103,7 +107,11 @@ void stop() throws Exception { /** Routes ROUTED_HOST over TCP to the peer, with a resolver that never asks the DNS. */ private NettyClientTransport transport() { - int peerPort = peer.getAddress().getPort(); + return transport(peer.getAddress().getPort()); + } + + /** Routes ROUTED_HOST over TCP to a local port, with a resolver that never asks the DNS. */ + private NettyClientTransport transport(int peerPort) { AddressResolverGroup resolver = new AddressResolverGroup<>() { @Override @@ -219,6 +227,72 @@ void usesTheConnectionsOfTheCurrentRoute() throws Exception { assertEquals(1, otherChannels.get(), "The new route didn't reuse the old connection"); } + /** + * With one connection per origin, the HTTP/2 connection of the old route, without streams once + * its call ended, is closed to let the new route connect. + */ + @Test + void reclaimsTheIdleHttp2ConnectionOfAnotherRoute() throws Exception { + Engine.getInstance().getRegisteredServers().add(new HttpServerHelper(null)); + Context serverContext = new Context(); + serverContext.getParameters().add("http.transport.protocol", "HTTP2"); + Server h2 = + new Server( + serverContext, + Protocol.HTTP, + 0, + new Restlet() { + @Override + public void handle(Request request, Response response) { + response.setEntity("ok", MediaType.TEXT_PLAIN); + } + }); + h2.start(); + try { + NettyClientTransport first = transport(h2.getActualPort()); + AtomicInteger otherChannels = new AtomicInteger(); + NettyClientTransport.Route other = + new NettyClientTransport.Route( + () -> { + otherChannels.incrementAndGet(); + return new NioSocketChannel(); + }, + first.route(ROUTED_HOST, 80).resolver()); + AtomicInteger requests = new AtomicInteger(); + Context context = new Context(); + context.getParameters().add("httpClientTransportMode", "HTTP2"); + context.getParameters().add("maxConnectionsPerDestination", "1"); + context.getParameters().add("connectTimeout", "5000"); + // The route of the origin changes after the first request. + context.getAttributes() + .put( + NettyClientTransport.ATTRIBUTE, + (NettyClientTransport) + (host, port) -> + (requests.getAndIncrement() == 0) + ? first.route(host, port) + : other); + client = new Client(context, Protocol.HTTP); + client.start(); + + for (int i = 0; i < 2; i++) { + Response response = + client.handle(new Request(Method.GET, "http://" + ROUTED_HOST + "/g" + i)); + assertEquals( + Status.SUCCESS_OK, + response.getStatus(), + "Call " + i + ": " + response.getStatus()); + assertEquals("2.0", response.getProtocol().getVersion()); + assertEquals("ok", response.getEntity().getText()); + } + + assertEquals(1, routedChannels.get(), "The first route connected once"); + assertEquals(1, otherChannels.get(), "The new route connected once"); + } finally { + h2.stop(); + } + } + @Test void doesNotUseTheProxyForARoutedOrigin() throws Exception { startClient(contextWithClosedProxy()); From fa5a5b739e5b2973abb8496a72285d754d0b8afa Mon Sep 17 00:00:00 2001 From: Jerome Louvel <374450+jlouvel@users.noreply.github.com> Date: Wed, 11 Nov 2026 09:06:07 -0500 Subject: [PATCH 8/8] Keep the HTTP/2 response a server sends before the end of the request A server may answer an HTTP/2 request as soon as it has its headers, for example with an immediate 401, then close the stream. The client's last, empty write of the request then failed at once, and the call reported a communication error instead of the response it had received. When the stream ended that way before the request was marked sent, the request was also reported as disconnected. Once the response is complete, a failed last write is no longer an error, and a later failure of the exchange no longer reports a disconnect. This made HttpDisconnectTestCase's HTTP/2 case fail now and then, 2 runs in 10 before, none in 15 after. Co-Authored-By: Claude Opus 5.5 --- changes.md | 3 +++ .../restlet/engine/netty/NettyClientCall.java | 25 +++++++++++++++++-- 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/changes.md b/changes.md index d32ae9ed23..319ae43ed7 100644 --- a/changes.md +++ b/changes.md @@ -297,6 +297,9 @@ Changes log and SOCKS proxies and HTTP/3 being then bypassed, and the proxy credentials removed. It exposes Netty types and is meant for extensions depending on Netty, such as the OpenZiti one. - Bugs fixed + - The Netty HTTP/2 client no longer turns a complete response into a communication error, + nor reports the request as disconnected, when the server answers before the end of the + request, such as with an immediate error, then closes the stream. - The Netty connectors now flush each full output buffer of a request or response body, so a producer that pauses after filling one no longer holds back what it produced until it resumes or ends. Issue #1632. diff --git a/org.restlet/src/main/java/org/restlet/engine/netty/NettyClientCall.java b/org.restlet/src/main/java/org/restlet/engine/netty/NettyClientCall.java index 0f2604fa83..3abfd74c35 100644 --- a/org.restlet/src/main/java/org/restlet/engine/netty/NettyClientCall.java +++ b/org.restlet/src/main/java/org/restlet/engine/netty/NettyClientCall.java @@ -893,7 +893,15 @@ private void writeRequest(Representation entity) throws IOException { } lastContent = withTrailers; } - check(ch.writeAndFlush(lastContent)); + ChannelFuture sent = ch.writeAndFlush(lastContent); + if (sent.isDone() && !sent.isSuccess() && isResponseComplete()) { + // The server answered without waiting for the end of the request, then closed the + // stream, as HTTP/2 allows: the exchange ended with its response. + helper.getLogger() + .fine(() -> "The response came before the end of the request on " + ch); + } else { + check(sent); + } helper.getLogger().fine(() -> "Sent the request " + method + " " + target + " on " + ch); lock.lock(); @@ -975,6 +983,15 @@ private static boolean requiresContentLength(String method) { return "POST".equals(method) || "PUT".equals(method) || "PATCH".equals(method); } + private boolean isResponseComplete() { + lock.lock(); + try { + return responseComplete; + } finally { + lock.unlock(); + } + } + private static void check(ChannelFuture future) throws IOException { if (future.isDone() && !future.isSuccess()) { throw new IOException("Unable to write the request", future.cause()); @@ -1120,6 +1137,7 @@ void onResponseComplete(io.netty.handler.codec.http.HttpHeaders trailers) { void onFailure(Throwable cause) { boolean notifyResponseFailure = false; boolean responseHeadReceived; + boolean answered; lock.lock(); try { if (!requestSent) { @@ -1129,6 +1147,9 @@ void onFailure(Throwable cause) { if (responseComplete && requestSent) { return; } + // A server may answer before the request is marked sent, such as an error refusing + // it at once: the exchange then ended with its response, not with this failure. + answered = responseComplete; reusable = false; if (!head) { failure = cause; @@ -1144,7 +1165,7 @@ void onFailure(Throwable cause) { helper.logFailure("The exchange failed on " + channel, cause); ChannelInputStream stream = this.body; org.restlet.Request request = currentRequest; - if (request != null) { + if (request != null && !answered) { request.notifyDisconnect(cause); } if (stream != null) {