WebSocket interception API enhancement (#108)

Closes #103

Signed-off-by: Thomas Segismont <tsegismont@gmail.com>

Author: tsegismont

src/main/asciidoc/index.adoc

@@ -120,8 +120,6 @@ Likewise with the proxy response
 {@link examples.HttpProxyExamples#outboundInterceptor}
 ----
 
-Interceptors will not apply to WebSocket handshake packets by default. While in some use cases (e.g. changing the WebSocket request path), you can overwrite {@link io.vertx.httpproxy.ProxyInterceptor#allowApplyToWebSocket} method to allow interceptors apply to WebSocket.
-
 ==== Body filtering
 
 You can filter body by simply replacing the original {@link io.vertx.httpproxy.Body} with a new one
@@ -190,9 +188,11 @@ You can use body interceptor to create body transformations for common data type
 
 Please check the {@link io.vertx.httpproxy.interceptors.BodyTransformer} for other supported transformations.
 
-==== WebSocket interceptor
+==== Interception and WebSocket upgrades
+
+By default, interceptors are not invoked during WebSocket upgrades.
 
-You can use WebSocket interceptor to wrap an interceptor to let it allow WebSocket handling:
+To make an interceptor available during the WebSocket handshake, use {@link io.vertx.httpproxy.HttpProxy#addInterceptor(io.vertx.httpproxy.ProxyInterceptor, boolean):
 
 [source,java]
 ----

src/main/java/examples/HttpProxyExamples.java

@@ -13,7 +13,9 @@
 import io.vertx.core.net.SocketAddress;
 import io.vertx.httpproxy.*;
 import io.vertx.httpproxy.cache.CacheOptions;
-import io.vertx.httpproxy.interceptors.*;
+import io.vertx.httpproxy.interceptors.BodyInterceptor;
+import io.vertx.httpproxy.interceptors.BodyTransformer;
+import io.vertx.httpproxy.interceptors.HeadInterceptor;
 
 import java.util.Set;
 
@@ -127,9 +129,10 @@ public void bodyInterceptorJson(HttpProxy proxy) {
   }
 
   public void webSocketInterceptorPath(HttpProxy proxy) {
-    proxy.addInterceptor(
-      WebSocketInterceptor.allow(HeadInterceptor.builder().addingPathPrefix("/api").build())
-    );
+    HeadInterceptor interceptor = HeadInterceptor.builder()
+      .addingPathPrefix("/api")
+      .build();
+    proxy.addInterceptor(interceptor, true);
   }
 
   public void immediateResponse(HttpProxy proxy) {

src/main/java/io/vertx/httpproxy/HttpProxy.java

@@ -102,12 +102,29 @@ default HttpProxy originSelector(Function<HttpServerRequest, Future<SocketAddres
 
   /**
    * Add an interceptor to the interceptor chain.
+   * <p>
+   * Interceptors are invoked in order of configuration.
+   * When added with this method, it is considered the interceptor doesn't support WebSocket upgrades.
    *
-   * @param interceptor
+   * @param interceptor the {@link ProxyInterceptor} to add
    * @return a reference to this, so the API can be used fluently
    */
   @Fluent
-  HttpProxy addInterceptor(ProxyInterceptor interceptor);
+  default HttpProxy addInterceptor(ProxyInterceptor interceptor) {
+    return addInterceptor(interceptor, false);
+  }
+
+  /**
+   * Add an interceptor to the interceptor chain.
+   * <p>
+   * Interceptors are invoked in order of configuration.
+   *
+   * @param interceptor the {@link ProxyInterceptor} to add
+   * @param supportsWebSocketUpgrade whether the interceptor supports WebSocket upgrades
+   * @return a reference to this, so the API can be used fluently
+   */
+  @Fluent
+  HttpProxy addInterceptor(ProxyInterceptor interceptor, boolean supportsWebSocketUpgrade);
 
   /**
    * Handle the <i><b>outbound</b></i> {@code HttpServerRequest}.

src/main/java/io/vertx/httpproxy/ProxyInterceptor.java

@@ -28,13 +28,4 @@ default Future<ProxyResponse> handleProxyRequest(ProxyContext context) {
   default Future<Void> handleProxyResponse(ProxyContext context) {
     return context.sendResponse();
   }
-
-  /**
-   * Used to set whether to apply the interceptor to the WebSocket
-   * handshake packet. The default value is false.
-   * @return the boolean value
-   */
-  default boolean allowApplyToWebSocket() {
-    return false;
-  }
 }

src/main/java/io/vertx/httpproxy/impl/ReverseProxy.java

@@ -33,7 +33,7 @@ public class ReverseProxy implements HttpProxy {
   private final HttpClient client;
   private final boolean supportWebSocket;
   private BiFunction<HttpServerRequest, HttpClient, Future<HttpClientRequest>> selector = (req, client) -> Future.failedFuture("No origin available");
-  private final List<ProxyInterceptor> interceptors = new ArrayList<>();
+  private final List<ProxyInterceptorEntry> interceptors = new ArrayList<>();
 
   public ReverseProxy(ProxyOptions options, HttpClient client) {
     CacheOptions cacheOptions = options.getCacheOptions();
@@ -52,12 +52,11 @@ public HttpProxy originRequestProvider(BiFunction<HttpServerRequest, HttpClient,
   }
 
   @Override
-  public HttpProxy addInterceptor(ProxyInterceptor interceptor) {
-    interceptors.add(interceptor);
+  public HttpProxy addInterceptor(ProxyInterceptor interceptor, boolean supportsWebSocketUpgrade) {
+    interceptors.add(new ProxyInterceptorEntry(Objects.requireNonNull(interceptor), supportsWebSocketUpgrade));
     return this;
   }
 
-
   @Override
   public void handle(HttpServerRequest request) {
     ProxyRequest proxyRequest = ProxyRequest.reverseProxy(request);
@@ -71,7 +70,6 @@ public void handle(HttpServerRequest request) {
 
     boolean isWebSocket = supportWebSocket && request.canUpgradeToWebSocket();
     Proxy proxy = new Proxy(proxyRequest, isWebSocket);
-    proxy.filters = interceptors.listIterator();
     proxy.sendRequest()
       .recover(throwable -> {
         log.trace("Error in sending the request", throwable);
@@ -103,12 +101,13 @@ private class Proxy implements ProxyContext {
     private final ProxyRequest request;
     private ProxyResponse response;
     private final Map<String, Object> attachments = new HashMap<>();
-    private ListIterator<ProxyInterceptor> filters;
+    private final ListIterator<ProxyInterceptorEntry> filters;
     private final boolean isWebSocket;
 
     private Proxy(ProxyRequest request, boolean isWebSocket) {
       this.request = request;
       this.isWebSocket = isWebSocket;
+      this.filters = interceptors.listIterator();
     }
 
     @Override
@@ -135,11 +134,11 @@ public ProxyRequest request() {
     @Override
     public Future<ProxyResponse> sendRequest() {
       if (filters.hasNext()) {
-        ProxyInterceptor next = filters.next();
-        if (isWebSocket && !next.allowApplyToWebSocket()) {
+        ProxyInterceptorEntry next = filters.next();
+        if (isWebSocket && !next.supportsWebSocketUpgrade) {
           return sendRequest();
         }
-        return next.handleProxyRequest(this);
+        return next.interceptor.handleProxyRequest(this);
       } else {
         if (isWebSocket) {
           HttpServerRequest proxiedRequest = request().proxiedRequest();
@@ -171,11 +170,11 @@ public ProxyResponse response() {
     @Override
     public Future<Void> sendResponse() {
       if (filters.hasPrevious()) {
-        ProxyInterceptor filter = filters.previous();
-        if (isWebSocket && !filter.allowApplyToWebSocket()) {
+        ProxyInterceptorEntry previous = filters.previous();
+        if (isWebSocket && !previous.supportsWebSocketUpgrade) {
           return sendResponse();
         }
-        return filter.handleProxyResponse(this);
+        return previous.interceptor.handleProxyResponse(this);
       } else {
         if (isWebSocket) {
           HttpClientResponse proxiedResponse = response().proxiedResponse();
@@ -227,4 +226,15 @@ private Future<Void> sendProxyResponse(ProxyResponse response) {
       return sendResponse();
     }
   }
+
+  private static class ProxyInterceptorEntry {
+
+    final ProxyInterceptor interceptor;
+    final boolean supportsWebSocketUpgrade;
+
+    ProxyInterceptorEntry(ProxyInterceptor interceptor, boolean supportsWebSocketUpgrade) {
+      this.interceptor = interceptor;
+      this.supportsWebSocketUpgrade = supportsWebSocketUpgrade;
+    }
+  }
 }

src/main/java/io/vertx/httpproxy/interceptors/WebSocketInterceptor.java

@@ -6,14 +6,14 @@
 import io.vertx.core.net.SocketAddress;
 import io.vertx.ext.unit.Async;
 import io.vertx.ext.unit.TestContext;
-import io.vertx.httpproxy.*;
+import io.vertx.httpproxy.ProxyContext;
+import io.vertx.httpproxy.ProxyInterceptor;
+import io.vertx.httpproxy.ProxyOptions;
+import io.vertx.httpproxy.ProxyResponse;
 import io.vertx.httpproxy.interceptors.HeadInterceptor;
-import io.vertx.httpproxy.interceptors.WebSocketInterceptor;
 import io.vertx.tests.ProxyTestBase;
 import org.junit.Test;
 
-
-
 public class WebSocketInterceptorTest extends ProxyTestBase {
 
   public WebSocketInterceptorTest(ProxyOptions options) {
@@ -53,7 +53,7 @@ private void testWithInterceptor(TestContext ctx, ProxyInterceptor interceptor,
     startProxy(proxy -> {
       proxy.origin(backend);
       if (interceptor != null) {
-        proxy.addInterceptor(interceptor);
+        proxy.addInterceptor(interceptor, wsHit);
       };
     });
 
@@ -93,7 +93,7 @@ public void testNotApplySocket(TestContext ctx) {
   @Test
   public void testWithSocketInterceptor(TestContext ctx) {
     // this interceptor applies to both regular HTTP traffic and WebSocket handshake
-    ProxyInterceptor interceptor = WebSocketInterceptor.allow(HeadInterceptor.builder().updatingPath(x -> x + "/updated").build());
+    ProxyInterceptor interceptor = HeadInterceptor.builder().updatingPath(x -> x + "/updated").build();
     testWithInterceptor(ctx, interceptor, true, true);
   }
 
@@ -108,11 +108,6 @@ public Future<ProxyResponse> handleProxyRequest(ProxyContext context) {
         }
         return context.sendRequest();
       }
-
-      @Override
-      public boolean allowApplyToWebSocket() {
-        return true;
-      }
     };
     testWithInterceptor(ctx, interceptor, false, true);
   }