Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: don't swallow exceptions in LocalServerReceiver #595

Merged
merged 4 commits into from Jan 12, 2021
Merged

fix: don't swallow exceptions in LocalServerReceiver #595

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2bf4ae2
use modern I/O
Jan 11, 2021
44e5f00
PrintWriter is bug prone
Jan 11, 2021
0958ffe
declare guava dependency
Jan 11, 2021
3482104
format
Jan 11, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions google-oauth-client-jetty/pom.xml
View file
Open in desktop
Expand Up @@ -92,6 +92,11 @@
<groupId>com.google.http-client</groupId>
<artifactId>google-http-client</artifactId>
</dependency>
<dependency>
<groupId>com.google.guava</groupId>
<artifactId>guava</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>junit</groupId>
<artifactId>junit</artifactId>
Expand Down
32 changes: 16 additions & 16 deletions ...src/main/java/com/google/api/client/extensions/jetty/auth/oauth2/LocalServerReceiver.java
View file
Open in desktop
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -315,19 +315,19 @@ private Map<String, String> 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("<html>");
doc.println("<head><title>OAuth 2.0 Authentication Token Received</title></head>");
doc.println("<body>");
doc.println("Received verification code. You may now close this window.");
doc.println("</body>");
doc.println("</html>");
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("<html>");
doc.write("<head><title>OAuth 2.0 Authentication Token Received</title></head>");
doc.write("<body>");
doc.write("Received verification code. You may now close this window.");
doc.write("</body>");
Comment on lines +323 to +327
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a huge deal as it's writing html, but do we want to preserve the newlines?

doc.write("</html>\n");
doc.flush();
}
}
}
}
23 changes: 12 additions & 11 deletions ...test/java/com/google/api/client/extensions/jetty/auth/oauth2/LocalServerReceiverTest.java
View file
Open in desktop
Expand Up @@ -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 {
Expand All @@ -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();
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -339,8 +343,8 @@ private void verifyRedirectedLandingPageUrl(String landingPageUrlMatch) {
private void verifyDefaultLandingPage() {
assertEquals(200, responseCode);
assertNull(redirectedLandingPageUrl);
assertTrue(responseOutput.toString().contains("<html>"));
assertTrue(responseOutput.toString().contains("</html>"));
assertTrue(responseOutput.contains("<html>"));
assertTrue(responseOutput.contains("</html>"));
}

private void sendSuccessLoginResult(String serverEndpoint) throws IOException {
Expand All @@ -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();
Expand Down