From cb0a581ca828e211116b15f14b9e17e00d6245fd Mon Sep 17 00:00:00 2001 From: eraskin Date: Tue, 18 Feb 2020 10:45:01 -0500 Subject: [PATCH 1/4] feat: Replace Jetty with HttpServer. Remove Jetty v8.2 and replace with com.sun.net.httpserver.HttpServer. Jetty has had breaking changes since 8.2 (specifically with version 9.4). This causes with conflicts with newer code using later versions of Jetty. Removing dependency on Jetty resolves this issue. Fixes #397. --- google-oauth-client-jetty/pom.xml | 4 - .../auth/oauth2/LocalServerReceiver.java | 198 +++++++++++++----- pom.xml | 8 +- 3 files changed, 150 insertions(+), 60 deletions(-) diff --git a/google-oauth-client-jetty/pom.xml b/google-oauth-client-jetty/pom.xml index d37ca4e60..1bcfbc6b7 100644 --- a/google-oauth-client-jetty/pom.xml +++ b/google-oauth-client-jetty/pom.xml @@ -86,10 +86,6 @@ com.google.oauth-client google-oauth-client-java6 - - org.eclipse.jetty - jetty-server - junit junit diff --git a/google-oauth-client-jetty/src/main/java/com/google/api/client/extensions/jetty/auth/oauth2/LocalServerReceiver.java b/google-oauth-client-jetty/src/main/java/com/google/api/client/extensions/jetty/auth/oauth2/LocalServerReceiver.java index 06f26f873..1fbdb453e 100644 --- a/google-oauth-client-jetty/src/main/java/com/google/api/client/extensions/jetty/auth/oauth2/LocalServerReceiver.java +++ b/google-oauth-client-jetty/src/main/java/com/google/api/client/extensions/jetty/auth/oauth2/LocalServerReceiver.java @@ -16,15 +16,19 @@ import com.google.api.client.extensions.java6.auth.oauth2.VerificationCodeReceiver; import com.google.api.client.util.Throwables; +import com.sun.net.httpserver.*; + import java.io.IOException; +import java.io.OutputStream; import java.io.PrintWriter; +import java.net.InetSocketAddress; +import java.net.ServerSocket; +import java.util.HashMap; +import java.util.Map; import java.util.concurrent.Semaphore; -import javax.servlet.http.HttpServletRequest; -import javax.servlet.http.HttpServletResponse; -import org.eclipse.jetty.server.Connector; -import org.eclipse.jetty.server.Request; -import org.eclipse.jetty.server.Server; -import org.eclipse.jetty.server.handler.AbstractHandler; + +import static java.net.HttpURLConnection.HTTP_MOVED_TEMP; +import static java.net.HttpURLConnection.HTTP_OK; /** * OAuth 2.0 verification code receiver that runs a Jetty server on a free port, waiting for a @@ -34,8 +38,8 @@ * Implementation is thread-safe. *

* - * @since 1.11 * @author Yaniv Inbar + * @since 1.11 */ public final class LocalServerReceiver implements VerificationCodeReceiver { @@ -43,25 +47,39 @@ public final class LocalServerReceiver implements VerificationCodeReceiver { private static final String CALLBACK_PATH = "/Callback"; - /** Server or {@code null} before {@link #getRedirectUri()}. */ - private Server server; + /** + * Server or {@code null} before {@link #getRedirectUri()}. + */ + private HttpServer server; - /** Verification code or {@code null} for none. */ + /** + * Verification code or {@code null} for none. + */ String code; - /** Error code or {@code null} for none. */ + /** + * Error code or {@code null} for none. + */ String error; - /** To block until receiving an authorization response or stop() is called. */ + /** + * To block until receiving an authorization response or stop() is called. + */ final Semaphore waitUnlessSignaled = new Semaphore(0 /* initially zero permit */); - /** Port to use or {@code -1} to select an unused port in {@link #getRedirectUri()}. */ + /** + * Port to use or {@code -1} to select an unused port in {@link #getRedirectUri()}. + */ private int port; - /** Host name to use. */ + /** + * Host name to use. + */ private final String host; - /** Callback path of redirect_uri */ + /** + * Callback path of redirect_uri. + */ private final String callbackPath; /** @@ -115,18 +133,53 @@ public LocalServerReceiver() { @Override public String getRedirectUri() throws IOException { + + server = HttpServer.create(new InetSocketAddress(port != -1 ? port : findOpenPort()), 0); + HttpContext context = server.createContext(callbackPath, new CallbackHandler()); + server.setExecutor(null); +/* server = new Server(port != -1 ? port : 0); - Connector connector = server.getConnectors()[0]; + ServerConnector connector = new ServerConnector(server); connector.setHost(host); + server.setConnectors(new Connector[] { connector } ); server.setHandler(new CallbackHandler()); +*/ try { server.start(); - port = connector.getLocalPort(); + port = server.getAddress().getPort(); } catch (Exception e) { Throwables.propagateIfPossible(e); throw new IOException(e); } - return "http://" + connector.getHost() + ":" + port + callbackPath; + return "http://" + this.getHost() + ":" + port + callbackPath; + } + + /* + *Copied from Jetty findFreePort() as referenced by: https://gist.github.com/vorburger/3429822 + */ + + private int findOpenPort() { + ServerSocket socket = null; + try { + socket = new ServerSocket(0); + socket.setReuseAddress(true); + int port = socket.getLocalPort(); + try { + socket.close(); + } catch (IOException e) { + // Ignore IOException on close() + } + return port; + } catch (IOException e) { + } finally { + if (socket != null) { + try { + socket.close(); + } catch (IOException e) { + } + } + } + throw new IllegalStateException("No free TCP/IP port to start embedded HTTP Server on"); } /** @@ -134,9 +187,9 @@ public String getRedirectUri() throws IOException { * by {@link #stop()}, to return an authorization code. * * @return authorization code if login succeeds; may return {@code null} if the server - * is stopped by {@link #stop()} + * is stopped by {@link #stop()} * @throws IOException if the server receives an error code (through an HTTP request - * parameter {@code error}) + * parameter {@code error}) */ @Override public String waitForCode() throws IOException { @@ -152,7 +205,7 @@ public void stop() throws IOException { waitUnlessSignaled.release(); if (server != null) { try { - server.stop(); + server.stop(0); } catch (Exception e) { Throwables.propagateIfPossible(e); throw new IOException(e); @@ -161,7 +214,9 @@ public void stop() throws IOException { } } - /** Returns the host name to use. */ + /** + * Returns the host name to use. + */ public String getHost() { return host; } @@ -189,10 +244,14 @@ public String getCallbackPath() { */ public static final class Builder { - /** Host name to use. */ + /** + * Host name to use. + */ private String host = LOCALHOST; - /** Port to use or {@code -1} to select an unused port. */ + /** + * Port to use or {@code -1} to select an unused port. + */ private int port = -1; private String successLandingPageUrl; @@ -200,40 +259,54 @@ public static final class Builder { private String callbackPath = CALLBACK_PATH; - /** Builds the {@link LocalServerReceiver}. */ + /** + * Builds the {@link LocalServerReceiver}. + */ public LocalServerReceiver build() { return new LocalServerReceiver(host, port, callbackPath, successLandingPageUrl, failureLandingPageUrl); } - /** Returns the host name to use. */ + /** + * Returns the host name to use. + */ public String getHost() { return host; } - /** Sets the host name to use. */ + /** + * Sets the host name to use. + */ public Builder setHost(String host) { this.host = host; return this; } - /** Returns the port to use or {@code -1} to select an unused port. */ + /** + * Returns the port to use or {@code -1} to select an unused port. + */ public int getPort() { return port; } - /** Sets the port to use or {@code -1} to select an unused port. */ + /** + * Sets the port to use or {@code -1} to select an unused port. + */ public Builder setPort(int port) { this.port = port; return this; } - /** Returns the callback path of redirect_uri */ + /** + * Returns the callback path of redirect_uri. + */ public String getCallbackPath() { return callbackPath; } - /** Set the callback path of redirect_uri */ + /** + * Set the callback path of redirect_uri. + */ public Builder setCallbackPath(String callbackPath) { this.callbackPath = callbackPath; return this; @@ -247,44 +320,64 @@ public Builder setLandingPages(String successLandingPageUrl, String failureLandi } /** - * Jetty handler that takes the verifier token passed over from the OAuth provider and stashes it + * HttpServer handler that takes the verifier token passed over + * from the OAuth provider and stashes it * where {@link #waitForCode} will find it. */ - class CallbackHandler extends AbstractHandler { + class CallbackHandler implements HttpHandler { @Override - public void handle( - String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response - ) - throws IOException { - if (!callbackPath.equals(target)) { + public void handle(HttpExchange httpExchange) throws IOException { + + if (!callbackPath.equals(httpExchange.getRequestURI().getPath())) { return; } + StringBuilder body = new StringBuilder(); + try { - ((Request) request).setHandled(true); - error = request.getParameter("error"); - code = request.getParameter("code"); + Map parms = + this.queryToMap(httpExchange.getRequestURI().getQuery()); + error = parms.get("error"); + code = parms.get("code"); + Headers respHeaders = httpExchange.getResponseHeaders(); if (error == null && successLandingPageUrl != null) { - response.sendRedirect(successLandingPageUrl); + respHeaders.add("Location", successLandingPageUrl); + httpExchange.sendResponseHeaders(HTTP_MOVED_TEMP, -1); } else if (error != null && failureLandingPageUrl != null) { - response.sendRedirect(failureLandingPageUrl); + respHeaders.add("Location", failureLandingPageUrl); + httpExchange.sendResponseHeaders(HTTP_MOVED_TEMP, -1); } else { - writeLandingHtml(response); + writeLandingHtml(httpExchange, respHeaders); } - response.flushBuffer(); - } - finally { + httpExchange.close(); + } finally { waitUnlessSignaled.release(); } } - private void writeLandingHtml(HttpServletResponse response) throws IOException { - response.setStatus(HttpServletResponse.SC_OK); - response.setContentType("text/html"); + private Map queryToMap(String query) { + Map result = new HashMap(); + if (query != null) { + for (String param : query.split("&")) { + String pair[] = param.split("="); + if (pair.length > 1) { + result.put(pair[0], pair[1]); + } else { + result.put(pair[0], ""); + } + } + } + return result; + } + + private void writeLandingHtml(HttpExchange exchange, Headers headers) throws IOException { + OutputStream os = exchange.getResponseBody(); + exchange.sendResponseHeaders(HTTP_OK, 0); + headers.add("ContentType", "text/html"); - PrintWriter doc = response.getWriter(); + PrintWriter doc = new PrintWriter(os); doc.println(""); doc.println("OAuth 2.0 Authentication Token Received"); doc.println(""); @@ -292,6 +385,9 @@ private void writeLandingHtml(HttpServletResponse response) throws IOException { doc.println(""); doc.println(""); doc.flush(); + os.close(); } + } + } diff --git a/pom.xml b/pom.xml index 4e1799a26..8eed258a1 100644 --- a/pom.xml +++ b/pom.xml @@ -150,11 +150,6 @@ google-oauth-client-jetty ${project.version} - - org.eclipse.jetty - jetty-server - ${project.jetty.version} - org.datanucleus datanucleus-core @@ -399,6 +394,9 @@ java17 1.0 + + com.sun.net.httpserver.* + From 48fb3f261562151c047a3f0fb9b1add1c152e635 Mon Sep 17 00:00:00 2001 From: eraskin Date: Wed, 19 Feb 2020 16:20:52 -0500 Subject: [PATCH 2/4] feat: Rewrite findOpenSocket using try with resources --- .../auth/oauth2/LocalServerReceiver.java | 23 ++++--------------- 1 file changed, 4 insertions(+), 19 deletions(-) diff --git a/google-oauth-client-jetty/src/main/java/com/google/api/client/extensions/jetty/auth/oauth2/LocalServerReceiver.java b/google-oauth-client-jetty/src/main/java/com/google/api/client/extensions/jetty/auth/oauth2/LocalServerReceiver.java index 1fbdb453e..b578605b9 100644 --- a/google-oauth-client-jetty/src/main/java/com/google/api/client/extensions/jetty/auth/oauth2/LocalServerReceiver.java +++ b/google-oauth-client-jetty/src/main/java/com/google/api/client/extensions/jetty/auth/oauth2/LocalServerReceiver.java @@ -159,27 +159,12 @@ public String getRedirectUri() throws IOException { */ private int findOpenPort() { - ServerSocket socket = null; - try { - socket = new ServerSocket(0); + try (ServerSocket socket = new ServerSocket(0)) { socket.setReuseAddress(true); - int port = socket.getLocalPort(); - try { - socket.close(); - } catch (IOException e) { - // Ignore IOException on close() - } - return port; - } catch (IOException e) { - } finally { - if (socket != null) { - try { - socket.close(); - } catch (IOException e) { - } - } + return socket.getLocalPort(); + } catch(IOException e) { + throw new IllegalStateException("No free TCP/IP port to start embedded HTTP Server on"); } - throw new IllegalStateException("No free TCP/IP port to start embedded HTTP Server on"); } /** From 84c457e48e3992cf61433041bff7fc30587f0a35 Mon Sep 17 00:00:00 2001 From: eraskin Date: Wed, 19 Feb 2020 17:03:56 -0500 Subject: [PATCH 3/4] fix: Remove commented out Jetty 9.4 code since we are using HttpServer now. --- .../extensions/jetty/auth/oauth2/LocalServerReceiver.java | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/google-oauth-client-jetty/src/main/java/com/google/api/client/extensions/jetty/auth/oauth2/LocalServerReceiver.java b/google-oauth-client-jetty/src/main/java/com/google/api/client/extensions/jetty/auth/oauth2/LocalServerReceiver.java index b578605b9..bb6fd2161 100644 --- a/google-oauth-client-jetty/src/main/java/com/google/api/client/extensions/jetty/auth/oauth2/LocalServerReceiver.java +++ b/google-oauth-client-jetty/src/main/java/com/google/api/client/extensions/jetty/auth/oauth2/LocalServerReceiver.java @@ -137,13 +137,7 @@ public String getRedirectUri() throws IOException { server = HttpServer.create(new InetSocketAddress(port != -1 ? port : findOpenPort()), 0); HttpContext context = server.createContext(callbackPath, new CallbackHandler()); server.setExecutor(null); -/* - server = new Server(port != -1 ? port : 0); - ServerConnector connector = new ServerConnector(server); - connector.setHost(host); - server.setConnectors(new Connector[] { connector } ); - server.setHandler(new CallbackHandler()); -*/ + try { server.start(); port = server.getAddress().getPort(); From 27540687ab67210c489a3e518ed328bedd5a1623 Mon Sep 17 00:00:00 2001 From: eraskin Date: Thu, 20 Feb 2020 08:25:52 -0500 Subject: [PATCH 4/4] fix: Enforce Google coding style. --- .../auth/oauth2/LocalServerReceiver.java | 44 ++++++++++--------- 1 file changed, 23 insertions(+), 21 deletions(-) diff --git a/google-oauth-client-jetty/src/main/java/com/google/api/client/extensions/jetty/auth/oauth2/LocalServerReceiver.java b/google-oauth-client-jetty/src/main/java/com/google/api/client/extensions/jetty/auth/oauth2/LocalServerReceiver.java index bb6fd2161..341661a10 100644 --- a/google-oauth-client-jetty/src/main/java/com/google/api/client/extensions/jetty/auth/oauth2/LocalServerReceiver.java +++ b/google-oauth-client-jetty/src/main/java/com/google/api/client/extensions/jetty/auth/oauth2/LocalServerReceiver.java @@ -14,10 +14,16 @@ package com.google.api.client.extensions.jetty.auth.oauth2; +import static java.net.HttpURLConnection.HTTP_MOVED_TEMP; +import static java.net.HttpURLConnection.HTTP_OK; + import com.google.api.client.extensions.java6.auth.oauth2.VerificationCodeReceiver; import com.google.api.client.util.Throwables; -import com.sun.net.httpserver.*; - +import com.sun.net.httpserver.Headers; +import com.sun.net.httpserver.HttpContext; +import com.sun.net.httpserver.HttpExchange; +import com.sun.net.httpserver.HttpHandler; +import com.sun.net.httpserver.HttpServer; import java.io.IOException; import java.io.OutputStream; import java.io.PrintWriter; @@ -27,9 +33,6 @@ import java.util.Map; import java.util.concurrent.Semaphore; -import static java.net.HttpURLConnection.HTTP_MOVED_TEMP; -import static java.net.HttpURLConnection.HTTP_OK; - /** * OAuth 2.0 verification code receiver that runs a Jetty server on a free port, waiting for a * redirect with the verification code. @@ -89,8 +92,8 @@ public final class LocalServerReceiver implements VerificationCodeReceiver { private String successLandingPageUrl; /** - * URL to an HTML page to be shown (via redirect) after failed login. If null, a canned - * default landing page will be shown (via direct response). + * URL to an HTML page to be shown (via redirect) after failed login. If null, a canned default + * landing page will be shown (via direct response). */ private String failureLandingPageUrl; @@ -112,7 +115,7 @@ public LocalServerReceiver() { * @param port Port to use or {@code -1} to select an unused port */ LocalServerReceiver(String host, int port, - String successLandingPageUrl, String failureLandingPageUrl) { + String successLandingPageUrl, String failureLandingPageUrl) { this(host, port, CALLBACK_PATH, successLandingPageUrl, failureLandingPageUrl); } @@ -123,7 +126,7 @@ public LocalServerReceiver() { * @param port Port to use or {@code -1} to select an unused port */ LocalServerReceiver(String host, int port, String callbackPath, - String successLandingPageUrl, String failureLandingPageUrl) { + String successLandingPageUrl, String failureLandingPageUrl) { this.host = host; this.port = port; this.callbackPath = callbackPath; @@ -156,19 +159,19 @@ private int findOpenPort() { try (ServerSocket socket = new ServerSocket(0)) { socket.setReuseAddress(true); return socket.getLocalPort(); - } catch(IOException e) { + } catch (IOException e) { throw new IllegalStateException("No free TCP/IP port to start embedded HTTP Server on"); } } /** - * Blocks until the server receives a login result, or the server is stopped - * by {@link #stop()}, to return an authorization code. + * Blocks until the server receives a login result, or the server is stopped by {@link #stop()}, + * to return an authorization code. * - * @return authorization code if login succeeds; may return {@code null} if the server - * is stopped by {@link #stop()} - * @throws IOException if the server receives an error code (through an HTTP request - * parameter {@code error}) + * @return authorization code if login succeeds; may return {@code null} if the server is stopped + * by {@link #stop()} + * @throws IOException if the server receives an error code (through an HTTP request parameter + * {@code error}) */ @Override public String waitForCode() throws IOException { @@ -243,7 +246,7 @@ public static final class Builder { */ public LocalServerReceiver build() { return new LocalServerReceiver(host, port, callbackPath, - successLandingPageUrl, failureLandingPageUrl); + successLandingPageUrl, failureLandingPageUrl); } /** @@ -299,9 +302,8 @@ public Builder setLandingPages(String successLandingPageUrl, String failureLandi } /** - * HttpServer handler that takes the verifier token passed over - * from the OAuth provider and stashes it - * where {@link #waitForCode} will find it. + * HttpServer handler that takes the verifier token passed over from the OAuth provider and + * stashes it where {@link #waitForCode} will find it. */ class CallbackHandler implements HttpHandler { @@ -316,7 +318,7 @@ public void handle(HttpExchange httpExchange) throws IOException { try { Map parms = - this.queryToMap(httpExchange.getRequestURI().getQuery()); + this.queryToMap(httpExchange.getRequestURI().getQuery()); error = parms.get("error"); code = parms.get("code");