From 4314c70ada39303c98a3e9114907ad58c8623449 Mon Sep 17 00:00:00 2001 From: Luciano Balmaceda Date: Tue, 14 Feb 2017 12:50:04 -0300 Subject: [PATCH 1/4] ensure closing the response body after it was parsed --- .../android/request/internal/BaseRequest.java | 9 ++++++++- .../android/request/internal/SimpleRequest.java | 17 +++++++++++++++-- 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/auth0/src/main/java/com/auth0/android/request/internal/BaseRequest.java b/auth0/src/main/java/com/auth0/android/request/internal/BaseRequest.java index 4f31b5907..6c14c4801 100755 --- a/auth0/src/main/java/com/auth0/android/request/internal/BaseRequest.java +++ b/auth0/src/main/java/com/auth0/android/request/internal/BaseRequest.java @@ -43,6 +43,7 @@ import com.squareup.okhttp.Request; import com.squareup.okhttp.RequestBody; import com.squareup.okhttp.Response; +import com.squareup.okhttp.ResponseBody; import java.io.IOException; import java.lang.reflect.Type; @@ -125,8 +126,9 @@ protected RequestBody buildBody() throws RequestBodyBuildException { protected U parseUnsuccessfulResponse(Response response) { String stringPayload = null; + ResponseBody body = response.body(); try { - stringPayload = response.body().string(); + stringPayload = body.string(); Type mapType = new TypeToken>() { }.getType(); Map mapPayload = gson.fromJson(stringPayload, mapType); @@ -136,6 +138,11 @@ protected U parseUnsuccessfulResponse(Response response) { } catch (IOException e) { final Auth0Exception auth0Exception = new Auth0Exception("Error parsing the server response", e); return errorBuilder.from("Request to " + url.toString() + " failed", auth0Exception); + } finally { + try { + body.close(); + } catch (Exception ignored) { + } } } diff --git a/auth0/src/main/java/com/auth0/android/request/internal/SimpleRequest.java b/auth0/src/main/java/com/auth0/android/request/internal/SimpleRequest.java index 67d2b4088..348de4e86 100755 --- a/auth0/src/main/java/com/auth0/android/request/internal/SimpleRequest.java +++ b/auth0/src/main/java/com/auth0/android/request/internal/SimpleRequest.java @@ -35,6 +35,7 @@ import com.squareup.okhttp.OkHttpClient; import com.squareup.okhttp.Request; import com.squareup.okhttp.Response; +import com.squareup.okhttp.ResponseBody; import java.io.IOException; import java.io.Reader; @@ -66,13 +67,19 @@ public void onResponse(Response response) throws IOException { return; } + ResponseBody body = response.body(); try { - Reader charStream = response.body().charStream(); + Reader charStream = body.charStream(); T payload = getAdapter().fromJson(charStream); postOnSuccess(payload); } catch (IOException e) { final Auth0Exception auth0Exception = new Auth0Exception("Failed to parse response to request to " + url, e); postOnFailure(getErrorBuilder().from("Failed to parse a successful response", auth0Exception)); + } finally { + try { + body.close(); + } catch (Exception ignored) { + } } } @@ -99,11 +106,17 @@ public T execute() throws Auth0Exception { throw parseUnsuccessfulResponse(response); } + ResponseBody body = response.body(); try { - Reader charStream = response.body().charStream(); + Reader charStream = body.charStream(); return getAdapter().fromJson(charStream); } catch (IOException e) { throw new Auth0Exception("Failed to parse response to request to " + url, e); + } finally { + try { + body.close(); + } catch (Exception ignored) { + } } } } From 36aca42fe2174e1002281b01e7de2d77baa42021 Mon Sep 17 00:00:00 2001 From: Luciano Balmaceda Date: Mon, 20 Feb 2017 15:47:26 -0300 Subject: [PATCH 2/4] only catch IOException when closing the response body --- .../java/com/auth0/android/request/internal/BaseRequest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/auth0/src/main/java/com/auth0/android/request/internal/BaseRequest.java b/auth0/src/main/java/com/auth0/android/request/internal/BaseRequest.java index 6c14c4801..5b79a95c9 100755 --- a/auth0/src/main/java/com/auth0/android/request/internal/BaseRequest.java +++ b/auth0/src/main/java/com/auth0/android/request/internal/BaseRequest.java @@ -131,7 +131,7 @@ protected U parseUnsuccessfulResponse(Response response) { stringPayload = body.string(); Type mapType = new TypeToken>() { }.getType(); - Map mapPayload = gson.fromJson(stringPayload, mapType); + Map mapPayload = gson.fromJson(body.charStream(), mapType); return errorBuilder.from(mapPayload); } catch (JsonSyntaxException e) { return errorBuilder.from(stringPayload, response.code()); @@ -141,7 +141,7 @@ protected U parseUnsuccessfulResponse(Response response) { } finally { try { body.close(); - } catch (Exception ignored) { + } catch (IOException ignored) { } } } From 43fc54e4c1e5ac46572c677592116097c982d4b9 Mon Sep 17 00:00:00 2001 From: Luciano Balmaceda Date: Mon, 20 Feb 2017 16:23:04 -0300 Subject: [PATCH 3/4] fix invalid JSON parse source --- .../java/com/auth0/android/request/internal/BaseRequest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/auth0/src/main/java/com/auth0/android/request/internal/BaseRequest.java b/auth0/src/main/java/com/auth0/android/request/internal/BaseRequest.java index 5b79a95c9..ba59eda41 100755 --- a/auth0/src/main/java/com/auth0/android/request/internal/BaseRequest.java +++ b/auth0/src/main/java/com/auth0/android/request/internal/BaseRequest.java @@ -131,7 +131,7 @@ protected U parseUnsuccessfulResponse(Response response) { stringPayload = body.string(); Type mapType = new TypeToken>() { }.getType(); - Map mapPayload = gson.fromJson(body.charStream(), mapType); + Map mapPayload = gson.fromJson(stringPayload, mapType); return errorBuilder.from(mapPayload); } catch (JsonSyntaxException e) { return errorBuilder.from(stringPayload, response.code()); From 4cd830e592227796b905a2f2dc36360cb9ee824e Mon Sep 17 00:00:00 2001 From: Luciano Balmaceda Date: Wed, 1 Mar 2017 12:46:55 -0300 Subject: [PATCH 4/4] extract close body method to another class --- .../android/request/internal/BaseRequest.java | 7 +++--- .../request/internal/ResponseUtils.java | 19 +++++++++++++++ .../request/internal/SimpleRequest.java | 12 ++++------ .../request/internal/ResponseUtilsTest.java | 24 +++++++++++++++++++ 4 files changed, 50 insertions(+), 12 deletions(-) create mode 100644 auth0/src/main/java/com/auth0/android/request/internal/ResponseUtils.java create mode 100644 auth0/src/test/java/com/auth0/android/request/internal/ResponseUtilsTest.java diff --git a/auth0/src/main/java/com/auth0/android/request/internal/BaseRequest.java b/auth0/src/main/java/com/auth0/android/request/internal/BaseRequest.java index ba59eda41..e2c3e0782 100755 --- a/auth0/src/main/java/com/auth0/android/request/internal/BaseRequest.java +++ b/auth0/src/main/java/com/auth0/android/request/internal/BaseRequest.java @@ -50,6 +50,8 @@ import java.util.HashMap; import java.util.Map; +import static com.auth0.android.request.internal.ResponseUtils.closeStream; + abstract class BaseRequest implements ParameterizableRequest, AuthorizableRequest, Callback { private final Map headers; @@ -139,10 +141,7 @@ protected U parseUnsuccessfulResponse(Response response) { final Auth0Exception auth0Exception = new Auth0Exception("Error parsing the server response", e); return errorBuilder.from("Request to " + url.toString() + " failed", auth0Exception); } finally { - try { - body.close(); - } catch (IOException ignored) { - } + closeStream(body); } } diff --git a/auth0/src/main/java/com/auth0/android/request/internal/ResponseUtils.java b/auth0/src/main/java/com/auth0/android/request/internal/ResponseUtils.java new file mode 100644 index 000000000..6b9ae5e1a --- /dev/null +++ b/auth0/src/main/java/com/auth0/android/request/internal/ResponseUtils.java @@ -0,0 +1,19 @@ +package com.auth0.android.request.internal; + +import java.io.Closeable; +import java.io.IOException; + +class ResponseUtils { + + /** + * Attempts to close a stream. No exception will be thrown if an IOException was raised. + * + * @param closeable the stream to close + */ + static void closeStream(Closeable closeable) { + try { + closeable.close(); + } catch (IOException ignored) { + } + } +} diff --git a/auth0/src/main/java/com/auth0/android/request/internal/SimpleRequest.java b/auth0/src/main/java/com/auth0/android/request/internal/SimpleRequest.java index 348de4e86..4eefe1c05 100755 --- a/auth0/src/main/java/com/auth0/android/request/internal/SimpleRequest.java +++ b/auth0/src/main/java/com/auth0/android/request/internal/SimpleRequest.java @@ -40,6 +40,8 @@ import java.io.IOException; import java.io.Reader; +import static com.auth0.android.request.internal.ResponseUtils.closeStream; + class SimpleRequest extends BaseRequest implements ParameterizableRequest, Callback { private final String method; @@ -76,10 +78,7 @@ public void onResponse(Response response) throws IOException { final Auth0Exception auth0Exception = new Auth0Exception("Failed to parse response to request to " + url, e); postOnFailure(getErrorBuilder().from("Failed to parse a successful response", auth0Exception)); } finally { - try { - body.close(); - } catch (Exception ignored) { - } + closeStream(body); } } @@ -113,10 +112,7 @@ public T execute() throws Auth0Exception { } catch (IOException e) { throw new Auth0Exception("Failed to parse response to request to " + url, e); } finally { - try { - body.close(); - } catch (Exception ignored) { - } + closeStream(body); } } } diff --git a/auth0/src/test/java/com/auth0/android/request/internal/ResponseUtilsTest.java b/auth0/src/test/java/com/auth0/android/request/internal/ResponseUtilsTest.java new file mode 100644 index 000000000..cb8db7e46 --- /dev/null +++ b/auth0/src/test/java/com/auth0/android/request/internal/ResponseUtilsTest.java @@ -0,0 +1,24 @@ +package com.auth0.android.request.internal; + +import com.squareup.okhttp.ResponseBody; + +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.ExpectedException; + +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; + +public class ResponseUtilsTest { + + @Rule + public ExpectedException exception = ExpectedException.none(); + + @Test + public void shouldCloseBody() throws Exception { + ResponseBody body = mock(ResponseBody.class); + ResponseUtils.closeStream(body); + + verify(body).close(); + } +} \ No newline at end of file