From 4660bb37397fa448be318d43b10f9b5bbfb041b9 Mon Sep 17 00:00:00 2001 From: Tomas Slusny Date: Fri, 22 Dec 2017 02:05:28 +0100 Subject: [PATCH] Fix OkHttp connection leaks on connection error When request fails, entire response needs to be wrapped in try with resources in order to close the connection properly and not only response body. Signed-off-by: Tomas Slusny --- .../http/api/account/AccountClient.java | 11 ++--- .../http/api/config/ConfigClient.java | 7 +-- .../http/api/hiscore/HiscoreClient.java | 25 +++++------ .../runelite/http/api/item/ItemClient.java | 45 +++++++++---------- .../runelite/http/api/xtea/XteaClient.java | 13 ++---- .../http/service/hiscore/HiscoreService.java | 41 +++++++++-------- .../http/service/item/ItemService.java | 29 +++++------- .../http/service/worlds/WorldsService.java | 6 +-- .../net/runelite/client/ConfigLoader.java | 4 +- 9 files changed, 77 insertions(+), 104 deletions(-) diff --git a/http-api/src/main/java/net/runelite/http/api/account/AccountClient.java b/http-api/src/main/java/net/runelite/http/api/account/AccountClient.java index f620c13395..21aa5683a0 100644 --- a/http-api/src/main/java/net/runelite/http/api/account/AccountClient.java +++ b/http-api/src/main/java/net/runelite/http/api/account/AccountClient.java @@ -33,7 +33,6 @@ import net.runelite.http.api.RuneliteAPI; import okhttp3.HttpUrl; import okhttp3.Request; import okhttp3.Response; -import okhttp3.ResponseBody; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -65,11 +64,9 @@ public class AccountClient .url(url) .build(); - Response response = RuneliteAPI.CLIENT.newCall(request).execute(); - - try (ResponseBody body = response.body()) + try (Response response = RuneliteAPI.CLIENT.newCall(request).execute()) { - InputStream in = body.byteStream(); + InputStream in = response.body().byteStream(); return RuneliteAPI.GSON.fromJson(new InputStreamReader(in), OAuthResponse.class); } catch (JsonParseException ex) @@ -92,9 +89,7 @@ public class AccountClient .url(url) .build(); - Response response = RuneliteAPI.CLIENT.newCall(request).execute(); - - try (ResponseBody body = response.body()) + try (Response response = RuneliteAPI.CLIENT.newCall(request).execute()) { logger.debug("Sent logout request"); } diff --git a/http-api/src/main/java/net/runelite/http/api/config/ConfigClient.java b/http-api/src/main/java/net/runelite/http/api/config/ConfigClient.java index 753515d43e..36f62ed7a9 100644 --- a/http-api/src/main/java/net/runelite/http/api/config/ConfigClient.java +++ b/http-api/src/main/java/net/runelite/http/api/config/ConfigClient.java @@ -35,7 +35,6 @@ import okhttp3.MediaType; import okhttp3.Request; import okhttp3.RequestBody; import okhttp3.Response; -import okhttp3.ResponseBody; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -65,11 +64,9 @@ public class ConfigClient .url(url) .build(); - Response response = RuneliteAPI.CLIENT.newCall(request).execute(); - - try (ResponseBody body = response.body()) + try (Response response = RuneliteAPI.CLIENT.newCall(request).execute()) { - InputStream in = body.byteStream(); + InputStream in = response.body().byteStream(); return RuneliteAPI.GSON.fromJson(new InputStreamReader(in), Configuration.class); } catch (JsonParseException ex) diff --git a/http-api/src/main/java/net/runelite/http/api/hiscore/HiscoreClient.java b/http-api/src/main/java/net/runelite/http/api/hiscore/HiscoreClient.java index bf5499d297..c0843a5fed 100644 --- a/http-api/src/main/java/net/runelite/http/api/hiscore/HiscoreClient.java +++ b/http-api/src/main/java/net/runelite/http/api/hiscore/HiscoreClient.java @@ -32,7 +32,6 @@ import net.runelite.http.api.RuneliteAPI; import okhttp3.HttpUrl; import okhttp3.Request; import okhttp3.Response; -import okhttp3.ResponseBody; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -55,11 +54,9 @@ public class HiscoreClient .url(url) .build(); - Response response = RuneliteAPI.CLIENT.newCall(request).execute(); - - try (ResponseBody body = response.body()) + try (Response response = RuneliteAPI.CLIENT.newCall(request).execute()) { - InputStream in = body.byteStream(); + InputStream in = response.body().byteStream(); return RuneliteAPI.GSON.fromJson(new InputStreamReader(in), HiscoreResult.class); } catch (JsonParseException ex) @@ -76,24 +73,22 @@ public class HiscoreClient public SingleHiscoreSkillResult lookup(String username, HiscoreSkill skill, HiscoreEndpoint endpoint) throws IOException { HttpUrl.Builder builder = RuneliteAPI.getApiBase().newBuilder() - .addPathSegment("hiscore") - .addPathSegment(endpoint.name()) - .addPathSegment(skill.toString().toLowerCase()) - .addQueryParameter("username", username); + .addPathSegment("hiscore") + .addPathSegment(endpoint.name()) + .addPathSegment(skill.toString().toLowerCase()) + .addQueryParameter("username", username); HttpUrl url = builder.build(); logger.debug("Built URI: {}", url); Request request = new Request.Builder() - .url(url) - .build(); + .url(url) + .build(); - Response response = RuneliteAPI.CLIENT.newCall(request).execute(); - - try (ResponseBody body = response.body()) + try (Response response = RuneliteAPI.CLIENT.newCall(request).execute()) { - InputStream in = body.byteStream(); + InputStream in = response.body().byteStream(); return RuneliteAPI.GSON.fromJson(new InputStreamReader(in), SingleHiscoreSkillResult.class); } catch (JsonParseException ex) diff --git a/http-api/src/main/java/net/runelite/http/api/item/ItemClient.java b/http-api/src/main/java/net/runelite/http/api/item/ItemClient.java index a601de2517..62cdb0404f 100644 --- a/http-api/src/main/java/net/runelite/http/api/item/ItemClient.java +++ b/http-api/src/main/java/net/runelite/http/api/item/ItemClient.java @@ -32,7 +32,6 @@ import net.runelite.http.api.RuneliteAPI; import okhttp3.HttpUrl; import okhttp3.Request; import okhttp3.Response; -import okhttp3.ResponseBody; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -54,17 +53,15 @@ public class ItemClient .url(url) .build(); - Response response = RuneliteAPI.CLIENT.newCall(request).execute(); - - if (!response.isSuccessful()) + try (Response response = RuneliteAPI.CLIENT.newCall(request).execute()) { - logger.debug("Error looking up item {}: {}", itemId, response.message()); - return null; - } + if (!response.isSuccessful()) + { + logger.debug("Error looking up item {}: {}", itemId, response.message()); + return null; + } - try (ResponseBody body = response.body()) - { - InputStream in = body.byteStream(); + InputStream in = response.body().byteStream(); return RuneliteAPI.GSON.fromJson(new InputStreamReader(in), ItemPrice.class); } catch (JsonParseException ex) @@ -76,28 +73,26 @@ public class ItemClient public SearchResult search(String itemName) throws IOException { HttpUrl url = RuneliteAPI.getApiBase().newBuilder() - .addPathSegment("item") - .addPathSegment("search") - .addQueryParameter("query", itemName) - .build(); + .addPathSegment("item") + .addPathSegment("search") + .addQueryParameter("query", itemName) + .build(); logger.debug("Built URI: {}", url); Request request = new Request.Builder() - .url(url) - .build(); + .url(url) + .build(); - Response response = RuneliteAPI.CLIENT.newCall(request).execute(); - - if (!response.isSuccessful()) + try (Response response = RuneliteAPI.CLIENT.newCall(request).execute()) { - logger.debug("Error looking up item {}: {}", itemName, response.message()); - return null; - } + if (!response.isSuccessful()) + { + logger.debug("Error looking up item {}: {}", itemName, response.message()); + return null; + } - try (ResponseBody body = response.body()) - { - InputStream in = body.byteStream(); + InputStream in = response.body().byteStream(); return RuneliteAPI.GSON.fromJson(new InputStreamReader(in), SearchResult.class); } catch (JsonParseException ex) diff --git a/http-api/src/main/java/net/runelite/http/api/xtea/XteaClient.java b/http-api/src/main/java/net/runelite/http/api/xtea/XteaClient.java index 8c4894a76a..76aed2e0a0 100644 --- a/http-api/src/main/java/net/runelite/http/api/xtea/XteaClient.java +++ b/http-api/src/main/java/net/runelite/http/api/xtea/XteaClient.java @@ -36,7 +36,6 @@ import okhttp3.MediaType; import okhttp3.Request; import okhttp3.RequestBody; import okhttp3.Response; -import okhttp3.ResponseBody; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -83,11 +82,9 @@ public class XteaClient .url(url) .build(); - Response response = RuneliteAPI.CLIENT.newCall(request).execute(); - - try (ResponseBody body = response.body()) + try (Response response = RuneliteAPI.CLIENT.newCall(request).execute()) { - InputStream in = body.byteStream(); + InputStream in = response.body().byteStream(); // CHECKSTYLE:OFF return RuneliteAPI.GSON.fromJson(new InputStreamReader(in), new TypeToken>() { }.getType()); // CHECKSTYLE:ON @@ -109,11 +106,9 @@ public class XteaClient .url(url) .build(); - Response response = RuneliteAPI.CLIENT.newCall(request).execute(); - - try (ResponseBody body = response.body()) + try (Response response = RuneliteAPI.CLIENT.newCall(request).execute()) { - InputStream in = body.byteStream(); + InputStream in = response.body().byteStream(); return RuneliteAPI.GSON.fromJson(new InputStreamReader(in), XteaKey.class); } catch (JsonParseException ex) diff --git a/http-service/src/main/java/net/runelite/http/service/hiscore/HiscoreService.java b/http-service/src/main/java/net/runelite/http/service/hiscore/HiscoreService.java index 107e9d17f3..20fe4546dc 100644 --- a/http-service/src/main/java/net/runelite/http/service/hiscore/HiscoreService.java +++ b/http-service/src/main/java/net/runelite/http/service/hiscore/HiscoreService.java @@ -26,14 +26,17 @@ package net.runelite.http.service.hiscore; import java.io.IOException; import net.runelite.http.api.RuneliteAPI; -import net.runelite.http.api.hiscore.*; +import net.runelite.http.api.hiscore.HiscoreEndpoint; +import net.runelite.http.api.hiscore.HiscoreResult; +import net.runelite.http.api.hiscore.HiscoreSkill; +import net.runelite.http.api.hiscore.SingleHiscoreSkillResult; +import net.runelite.http.api.hiscore.Skill; import net.runelite.http.service.util.HiscoreEndpointEditor; import net.runelite.http.service.util.exception.InternalServerErrorException; import net.runelite.http.service.util.exception.NotFoundException; import okhttp3.HttpUrl; import okhttp3.Request; import okhttp3.Response; -import okhttp3.ResponseBody; import org.apache.commons.csv.CSVFormat; import org.apache.commons.csv.CSVParser; import org.apache.commons.csv.CSVRecord; @@ -41,7 +44,11 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.http.HttpStatus; import org.springframework.web.bind.WebDataBinder; -import org.springframework.web.bind.annotation.*; +import org.springframework.web.bind.annotation.InitBinder; +import org.springframework.web.bind.annotation.PathVariable; +import org.springframework.web.bind.annotation.RequestMapping; +import org.springframework.web.bind.annotation.RequestParam; +import org.springframework.web.bind.annotation.RestController; @RestController @RequestMapping("/hiscore") @@ -66,24 +73,22 @@ public class HiscoreService .url(url) .build(); - Response okresponse = RuneliteAPI.CLIENT.newCall(okrequest).execute(); - - if (!okresponse.isSuccessful()) - { - switch (HttpStatus.valueOf(okresponse.code())) - { - case NOT_FOUND: - throw new NotFoundException(); - default: - throw new InternalServerErrorException("Error retrieving data from Jagex Hiscores: " + okresponse.message()); - } - } - String responseStr; - try (ResponseBody body = okresponse.body()) + try (Response okresponse = RuneliteAPI.CLIENT.newCall(okrequest).execute()) { - responseStr = body.string(); + if (!okresponse.isSuccessful()) + { + switch (HttpStatus.valueOf(okresponse.code())) + { + case NOT_FOUND: + throw new NotFoundException(); + default: + throw new InternalServerErrorException("Error retrieving data from Jagex Hiscores: " + okresponse.message()); + } + } + + responseStr = okresponse.body().string(); } CSVParser parser = CSVParser.parse(responseStr, CSVFormat.DEFAULT); diff --git a/http-service/src/main/java/net/runelite/http/service/item/ItemService.java b/http-service/src/main/java/net/runelite/http/service/item/ItemService.java index b404478b8d..13edad19cd 100644 --- a/http-service/src/main/java/net/runelite/http/service/item/ItemService.java +++ b/http-service/src/main/java/net/runelite/http/service/item/ItemService.java @@ -45,7 +45,6 @@ import net.runelite.http.api.item.SearchResult; import okhttp3.HttpUrl; import okhttp3.Request; import okhttp3.Response; -import okhttp3.ResponseBody; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Autowired; @@ -487,16 +486,14 @@ public class ItemService private T fetchJson(Request request, Class clazz) throws IOException { - Response response = RuneliteAPI.CLIENT.newCall(request).execute(); - - if (!response.isSuccessful()) + try (Response response = RuneliteAPI.CLIENT.newCall(request).execute()) { - throw new IOException("Unsuccessful http response: " + response.message()); - } + if (!response.isSuccessful()) + { + throw new IOException("Unsuccessful http response: " + response.message()); + } - try (ResponseBody body = response.body()) - { - InputStream in = body.byteStream(); + InputStream in = response.body().byteStream(); return RuneliteAPI.GSON.fromJson(new InputStreamReader(in), clazz); } catch (JsonParseException ex) @@ -513,16 +510,14 @@ public class ItemService .url(httpUrl) .build(); - Response response = RuneliteAPI.CLIENT.newCall(request).execute(); - - if (!response.isSuccessful()) + try (Response response = RuneliteAPI.CLIENT.newCall(request).execute()) { - throw new IOException("Unsuccessful http response: " + response.message()); - } + if (!response.isSuccessful()) + { + throw new IOException("Unsuccessful http response: " + response.message()); + } - try (ResponseBody body = response.body()) - { - return body.bytes(); + return response.body().bytes(); } } } diff --git a/http-service/src/main/java/net/runelite/http/service/worlds/WorldsService.java b/http-service/src/main/java/net/runelite/http/service/worlds/WorldsService.java index 9029c9b18d..e896357769 100644 --- a/http-service/src/main/java/net/runelite/http/service/worlds/WorldsService.java +++ b/http-service/src/main/java/net/runelite/http/service/worlds/WorldsService.java @@ -34,7 +34,6 @@ import net.runelite.http.api.worlds.WorldResult; import okhttp3.HttpUrl; import okhttp3.Request; import okhttp3.Response; -import okhttp3.ResponseBody; import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.RestController; @@ -53,12 +52,11 @@ public class WorldsService .url(url) .build(); - Response okresponse = RuneliteAPI.CLIENT.newCall(okrequest).execute(); byte[] b; - try (ResponseBody body = okresponse.body()) + try (Response okresponse = RuneliteAPI.CLIENT.newCall(okrequest).execute()) { - b = body.bytes(); + b = okresponse.body().bytes(); } List worlds = new ArrayList<>(); diff --git a/runelite-client/src/main/java/net/runelite/client/ConfigLoader.java b/runelite-client/src/main/java/net/runelite/client/ConfigLoader.java index 9abf41888b..a7897760dc 100644 --- a/runelite-client/src/main/java/net/runelite/client/ConfigLoader.java +++ b/runelite-client/src/main/java/net/runelite/client/ConfigLoader.java @@ -31,13 +31,11 @@ import java.util.HashMap; import java.util.Map; import net.runelite.http.api.RuneliteAPI; import okhttp3.HttpUrl; -import okhttp3.OkHttpClient; import okhttp3.Request; import okhttp3.Response; public class ConfigLoader { - private static final OkHttpClient CLIENT = RuneliteAPI.CLIENT; private static final HttpUrl CONFIG_URL = HttpUrl.parse("http://oldschool.runescape.com/jav_config.ws"); // https redirects us to rs3 public static final String CODEBASE = "codebase"; @@ -53,7 +51,7 @@ public class ConfigLoader .url(CONFIG_URL) .build(); - try (Response response = CLIENT.newCall(request).execute(); + try (Response response = RuneliteAPI.CLIENT.newCall(request).execute(); BufferedReader in = new BufferedReader(new InputStreamReader(response.body().byteStream()))) { String str;