Skip to content

Commit f39faec

Browse files
elharoelharo
authored
fix: don't swallow exceptions in LocalServerReceiver (#595)
* use modern I/O * PrintWriter is bug prone * declare guava dependency * format
1 parent 3fbd4d5 commit f39faec

File tree

3 files changed

+33
-27
lines changed

3 files changed

+33
-27
lines changed

‎google-oauth-client-jetty/pom.xml‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,11 @@
9292
<groupId>com.google.http-client</groupId>
9393
<artifactId>google-http-client</artifactId>
9494
</dependency>
95+
<dependency>
96+
<groupId>com.google.guava</groupId>
97+
<artifactId>guava</artifactId>
98+
<scope>test</scope>
99+
</dependency>
95100
<dependency>
96101
<groupId>junit</groupId>
97102
<artifactId>junit</artifactId>

‎google-oauth-client-jetty/src/main/java/com/google/api/client/extensions/jetty/auth/oauth2/LocalServerReceiver.java‎

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,10 @@
2626
import com.sun.net.httpserver.HttpServer;
2727
import java.io.IOException;
2828
import java.io.OutputStream;
29-
import java.io.PrintWriter;
29+
import java.io.OutputStreamWriter;
3030
import java.net.InetSocketAddress;
3131
import java.net.ServerSocket;
32+
import java.nio.charset.StandardCharsets;
3233
import java.util.HashMap;
3334
import java.util.Map;
3435
import java.util.concurrent.Semaphore;
@@ -138,9 +139,8 @@ public String getRedirectUri() throws IOException {
138139
}
139140

140141
/*
141-
*Copied from Jetty findFreePort() as referenced by: https://gist.github.com/vorburger/3429822
142+
* Copied from Jetty findFreePort() as referenced by: https://gist.github.com/vorburger/3429822
142143
*/
143-
144144
private int findOpenPort() {
145145
try (ServerSocket socket = new ServerSocket(0)) {
146146
socket.setReuseAddress(true);
@@ -315,19 +315,19 @@ private Map<String, String> queryToMap(String query) {
315315
}
316316

317317
private void writeLandingHtml(HttpExchange exchange, Headers headers) throws IOException {
318-
OutputStream os = exchange.getResponseBody();
319-
exchange.sendResponseHeaders(HTTP_OK, 0);
320-
headers.add("ContentType", "text/html");
321-
322-
PrintWriter doc = new PrintWriter(os);
323-
doc.println("<html>");
324-
doc.println("<head><title>OAuth 2.0 Authentication Token Received</title></head>");
325-
doc.println("<body>");
326-
doc.println("Received verification code. You may now close this window.");
327-
doc.println("</body>");
328-
doc.println("</html>");
329-
doc.flush();
330-
os.close();
318+
try (OutputStream os = exchange.getResponseBody()) {
319+
exchange.sendResponseHeaders(HTTP_OK, 0);
320+
headers.add("ContentType", "text/html");
321+
322+
OutputStreamWriter doc = new OutputStreamWriter(os, StandardCharsets.UTF_8);
323+
doc.write("<html>");
324+
doc.write("<head><title>OAuth 2.0 Authentication Token Received</title></head>");
325+
doc.write("<body>");
326+
doc.write("Received verification code. You may now close this window.");
327+
doc.write("</body>");
328+
doc.write("</html>\n");
329+
doc.flush();
330+
}
331331
}
332332
}
333333
}

‎google-oauth-client-jetty/src/test/java/com/google/api/client/extensions/jetty/auth/oauth2/LocalServerReceiverTest.java‎

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -16,13 +16,17 @@
1616

1717
import static org.junit.Assert.assertEquals;
1818
import static org.junit.Assert.assertFalse;
19+
import static org.junit.Assert.assertNotEquals;
1920
import static org.junit.Assert.assertNull;
2021
import static org.junit.Assert.assertTrue;
2122

23+
import com.google.common.io.CharStreams;
2224
import java.io.IOException;
2325
import java.io.InputStreamReader;
26+
import java.io.Reader;
2427
import java.net.HttpURLConnection;
2528
import java.net.URL;
29+
import java.nio.charset.StandardCharsets;
2630
import org.junit.Test;
2731

2832
public class LocalServerReceiverTest {
@@ -33,8 +37,8 @@ public void testActualPort() throws IOException {
3337

3438
try {
3539
receiver.getRedirectUri();
36-
assertTrue(receiver.getPort() != 0);
37-
assertTrue(receiver.getPort() != -1);
40+
assertNotEquals(0, receiver.getPort());
41+
assertNotEquals(-1, receiver.getPort());
3842
} finally {
3943
receiver.stop();
4044
}
@@ -268,12 +272,12 @@ private void verifyLoginSuccess() {
268272
}
269273

270274
private void verifyLoginFailure() {
271-
assertEquals(authCode, null);
275+
assertNull(authCode);
272276
assertTrue(error.contains("some-error"));
273277
}
274278

275279
private int responseCode;
276-
private StringBuilder responseOutput = new StringBuilder();
280+
private String responseOutput;
277281
private String redirectedLandingPageUrl;
278282

279283
@Test
@@ -339,8 +343,8 @@ private void verifyRedirectedLandingPageUrl(String landingPageUrlMatch) {
339343
private void verifyDefaultLandingPage() {
340344
assertEquals(200, responseCode);
341345
assertNull(redirectedLandingPageUrl);
342-
assertTrue(responseOutput.toString().contains("<html>"));
343-
assertTrue(responseOutput.toString().contains("</html>"));
346+
assertTrue(responseOutput.contains("<html>"));
347+
assertTrue(responseOutput.contains("</html>"));
344348
}
345349

346350
private void sendSuccessLoginResult(String serverEndpoint) throws IOException {
@@ -362,11 +366,8 @@ private void sendLoginResult(final String serverEndpoint, final String parameter
362366
connection.setReadTimeout(2000 /* ms */);
363367
responseCode = connection.getResponseCode();
364368
redirectedLandingPageUrl = connection.getHeaderField("Location");
365-
366-
InputStreamReader reader = new InputStreamReader(connection.getInputStream(), "UTF-8");
367-
for (int ch = reader.read(); ch != -1; ch = reader.read()) {
368-
responseOutput.append((char) ch);
369-
}
369+
Reader reader = new InputStreamReader(connection.getInputStream(), StandardCharsets.UTF_8);
370+
responseOutput = CharStreams.toString(reader);
370371
} finally {
371372
if (connection != null) {
372373
connection.disconnect();

0 commit comments

Comments
 (0)