From f39faec9980fa65602a216fbf34555b744139443 Mon Sep 17 00:00:00 2001 From: Elliotte Rusty Harold Date: Tue, 12 Jan 2021 23:19:38 +0000 Subject: [PATCH] fix: don't swallow exceptions in LocalServerReceiver (#595) * use modern I/O * PrintWriter is bug prone * declare guava dependency * format --- google-oauth-client-jetty/pom.xml | 5 +++ .../auth/oauth2/LocalServerReceiver.java | 32 +++++++++---------- .../auth/oauth2/LocalServerReceiverTest.java | 23 ++++++------- 3 files changed, 33 insertions(+), 27 deletions(-) diff --git a/google-oauth-client-jetty/pom.xml b/google-oauth-client-jetty/pom.xml index 44198f7ba..5acdfa8de 100644 --- a/google-oauth-client-jetty/pom.xml +++ b/google-oauth-client-jetty/pom.xml @@ -92,6 +92,11 @@ com.google.http-client google-http-client + + com.google.guava + guava + test + 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 cb0d07e05..fc7869667 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 @@ -26,9 +26,10 @@ import com.sun.net.httpserver.HttpServer; import java.io.IOException; import java.io.OutputStream; -import java.io.PrintWriter; +import java.io.OutputStreamWriter; import java.net.InetSocketAddress; import java.net.ServerSocket; +import java.nio.charset.StandardCharsets; import java.util.HashMap; import java.util.Map; import java.util.concurrent.Semaphore; @@ -138,9 +139,8 @@ public String getRedirectUri() throws IOException { } /* - *Copied from Jetty findFreePort() as referenced by: https://gist.github.com/vorburger/3429822 + * Copied from Jetty findFreePort() as referenced by: https://gist.github.com/vorburger/3429822 */ - private int findOpenPort() { try (ServerSocket socket = new ServerSocket(0)) { socket.setReuseAddress(true); @@ -315,19 +315,19 @@ private Map queryToMap(String query) { } 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 = new PrintWriter(os); - doc.println(""); - doc.println("OAuth 2.0 Authentication Token Received"); - doc.println(""); - doc.println("Received verification code. You may now close this window."); - doc.println(""); - doc.println(""); - doc.flush(); - os.close(); + try (OutputStream os = exchange.getResponseBody()) { + exchange.sendResponseHeaders(HTTP_OK, 0); + headers.add("ContentType", "text/html"); + + OutputStreamWriter doc = new OutputStreamWriter(os, StandardCharsets.UTF_8); + doc.write(""); + doc.write("OAuth 2.0 Authentication Token Received"); + doc.write(""); + doc.write("Received verification code. You may now close this window."); + doc.write(""); + doc.write("\n"); + doc.flush(); + } } } } diff --git a/google-oauth-client-jetty/src/test/java/com/google/api/client/extensions/jetty/auth/oauth2/LocalServerReceiverTest.java b/google-oauth-client-jetty/src/test/java/com/google/api/client/extensions/jetty/auth/oauth2/LocalServerReceiverTest.java index c6b82ab88..ec3c03eda 100644 --- a/google-oauth-client-jetty/src/test/java/com/google/api/client/extensions/jetty/auth/oauth2/LocalServerReceiverTest.java +++ b/google-oauth-client-jetty/src/test/java/com/google/api/client/extensions/jetty/auth/oauth2/LocalServerReceiverTest.java @@ -16,13 +16,17 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; +import com.google.common.io.CharStreams; import java.io.IOException; import java.io.InputStreamReader; +import java.io.Reader; import java.net.HttpURLConnection; import java.net.URL; +import java.nio.charset.StandardCharsets; import org.junit.Test; public class LocalServerReceiverTest { @@ -33,8 +37,8 @@ public void testActualPort() throws IOException { try { receiver.getRedirectUri(); - assertTrue(receiver.getPort() != 0); - assertTrue(receiver.getPort() != -1); + assertNotEquals(0, receiver.getPort()); + assertNotEquals(-1, receiver.getPort()); } finally { receiver.stop(); } @@ -268,12 +272,12 @@ private void verifyLoginSuccess() { } private void verifyLoginFailure() { - assertEquals(authCode, null); + assertNull(authCode); assertTrue(error.contains("some-error")); } private int responseCode; - private StringBuilder responseOutput = new StringBuilder(); + private String responseOutput; private String redirectedLandingPageUrl; @Test @@ -339,8 +343,8 @@ private void verifyRedirectedLandingPageUrl(String landingPageUrlMatch) { private void verifyDefaultLandingPage() { assertEquals(200, responseCode); assertNull(redirectedLandingPageUrl); - assertTrue(responseOutput.toString().contains("")); - assertTrue(responseOutput.toString().contains("")); + assertTrue(responseOutput.contains("")); + assertTrue(responseOutput.contains("")); } private void sendSuccessLoginResult(String serverEndpoint) throws IOException { @@ -362,11 +366,8 @@ private void sendLoginResult(final String serverEndpoint, final String parameter connection.setReadTimeout(2000 /* ms */); responseCode = connection.getResponseCode(); redirectedLandingPageUrl = connection.getHeaderField("Location"); - - InputStreamReader reader = new InputStreamReader(connection.getInputStream(), "UTF-8"); - for (int ch = reader.read(); ch != -1; ch = reader.read()) { - responseOutput.append((char) ch); - } + Reader reader = new InputStreamReader(connection.getInputStream(), StandardCharsets.UTF_8); + responseOutput = CharStreams.toString(reader); } finally { if (connection != null) { connection.disconnect();