From 5fcad593e350416c06fe7de5d187e9648aab470f Mon Sep 17 00:00:00 2001 From: helloween Date: Mon, 20 Nov 2017 20:18:22 +0300 Subject: [PATCH 1/2] safely handling closeable resources with try-with-resources --- .../apache/oltu/commons/encodedtoken/TokenReader.java | 11 ++--------- .../java/org/apache/oltu/jose/jwe/io/JWEReader.java | 11 ++--------- .../apache/oltu/oauth2/common/utils/OAuthUtils.java | 6 ++---- 3 files changed, 6 insertions(+), 22 deletions(-) diff --git a/commons/encodedtoken/src/main/java/org/apache/oltu/commons/encodedtoken/TokenReader.java b/commons/encodedtoken/src/main/java/org/apache/oltu/commons/encodedtoken/TokenReader.java index 7962934d..f206ef13 100644 --- a/commons/encodedtoken/src/main/java/org/apache/oltu/commons/encodedtoken/TokenReader.java +++ b/commons/encodedtoken/src/main/java/org/apache/oltu/commons/encodedtoken/TokenReader.java @@ -41,20 +41,13 @@ public T read(String base64String) { // TODO improve multi-line tokens StringBuilder buffer = new StringBuilder(); - BufferedReader reader = new BufferedReader(new StringReader(base64String)); String line = null; - try { + try (BufferedReader reader = new BufferedReader(new StringReader(base64String))) { while ((line = reader.readLine()) != null) { buffer.append(line.trim()); } } catch (IOException e) { - // it cannot happen - } finally { - try { - reader.close(); - } catch (IOException e) { - // swallow it - } + // swallow it } Matcher matcher = base64urlTokenPattern.matcher(buffer.toString()); diff --git a/jose/jwe/src/main/java/org/apache/oltu/jose/jwe/io/JWEReader.java b/jose/jwe/src/main/java/org/apache/oltu/jose/jwe/io/JWEReader.java index 070826d6..799ed70d 100644 --- a/jose/jwe/src/main/java/org/apache/oltu/jose/jwe/io/JWEReader.java +++ b/jose/jwe/src/main/java/org/apache/oltu/jose/jwe/io/JWEReader.java @@ -43,20 +43,13 @@ public JWE read(String base64String) { // TODO improve multi-line tokens StringBuilder buffer = new StringBuilder(); - BufferedReader reader = new BufferedReader(new StringReader(base64String)); String line = null; - try { + try (BufferedReader reader = new BufferedReader(new StringReader(base64String))) { while ((line = reader.readLine()) != null) { buffer.append(line.trim()); } } catch (IOException e) { - // it cannot happen - } finally { - try { - reader.close(); - } catch (IOException e) { - // swallow it - } + // swallow it } Matcher matcher = base64urlTokenPattern.matcher(buffer.toString()); diff --git a/oauth-2.0/common/src/main/java/org/apache/oltu/oauth2/common/utils/OAuthUtils.java b/oauth-2.0/common/src/main/java/org/apache/oltu/oauth2/common/utils/OAuthUtils.java index 2e705599..5f505c0c 100644 --- a/oauth-2.0/common/src/main/java/org/apache/oltu/oauth2/common/utils/OAuthUtils.java +++ b/oauth-2.0/common/src/main/java/org/apache/oltu/oauth2/common/utils/OAuthUtils.java @@ -143,16 +143,14 @@ public static String toString( if (charset == null) { charset = DEFAULT_CONTENT_CHARSET; } - Reader reader = new InputStreamReader(is, charset); + StringBuilder sb = new StringBuilder(); int l; - try { + try (Reader reader = new InputStreamReader(is, charset)) { char[] tmp = new char[4096]; while ((l = reader.read(tmp)) != -1) { sb.append(tmp, 0, l); } - } finally { - reader.close(); } return sb.toString(); } From 19e68229cede6f7bc8c48c4bc8b5ad6b8f2ed927 Mon Sep 17 00:00:00 2001 From: helloween Date: Fri, 26 Jan 2018 23:08:40 +0300 Subject: [PATCH 2/2] safely handling closeable resources with try-with-resources --- .../json/CustomizableEntityReader.java | 35 +++-- .../oltu/oauth2/common/utils/JSONUtils.java | 120 +++++++++--------- .../JSONHttpServletRequestWrapper.java | 55 ++++---- .../oltu/oauth2/utils/test/FileUtils.java | 14 +- 4 files changed, 110 insertions(+), 114 deletions(-) diff --git a/commons/json/src/main/java/org/apache/oltu/commons/json/CustomizableEntityReader.java b/commons/json/src/main/java/org/apache/oltu/commons/json/CustomizableEntityReader.java index 757a650c..525f6f9d 100644 --- a/commons/json/src/main/java/org/apache/oltu/commons/json/CustomizableEntityReader.java +++ b/commons/json/src/main/java/org/apache/oltu/commons/json/CustomizableEntityReader.java @@ -48,31 +48,30 @@ public void read(String jsonString) { } StringReader reader = new StringReader(jsonString); - JsonReader jsonReader = Json.createReader(reader); - JsonStructure structure = jsonReader.read(); + try (JsonReader jsonReader = Json.createReader(reader)) { + JsonStructure structure = jsonReader.read(); - if (structure == null || structure instanceof JsonArray) { - throw new IllegalArgumentException(format("String '%s' is not a valid JSON object representation", - jsonString)); - } + if (structure == null || structure instanceof JsonArray) { + throw new IllegalArgumentException(format("String '%s' is not a valid JSON object representation", + jsonString)); + } - JsonObject object = (JsonObject) structure; - for (Entry entry : object.entrySet()) { - String key = entry.getKey(); - JsonValue jsonValue = entry.getValue(); + JsonObject object = (JsonObject) structure; + for (Entry entry : object.entrySet()) { + String key = entry.getKey(); + JsonValue jsonValue = entry.getValue(); - // guard from null values - if (jsonValue != null) { - Object value = toJavaObject(jsonValue); + // guard from null values + if (jsonValue != null) { + Object value = toJavaObject(jsonValue); - // if the concrete implementation is not able to handle the property, set the custom field - if (!handleProperty(key, value)) { - builder.setCustomField(key, value); + // if the concrete implementation is not able to handle the property, set the custom field + if (!handleProperty(key, value)) { + builder.setCustomField(key, value); + } } } } - - jsonReader.close(); } private static Object toJavaObject(JsonValue jsonValue) { diff --git a/oauth-2.0/common/src/main/java/org/apache/oltu/oauth2/common/utils/JSONUtils.java b/oauth-2.0/common/src/main/java/org/apache/oltu/oauth2/common/utils/JSONUtils.java index 607138c3..0befb569 100644 --- a/oauth-2.0/common/src/main/java/org/apache/oltu/oauth2/common/utils/JSONUtils.java +++ b/oauth-2.0/common/src/main/java/org/apache/oltu/oauth2/common/utils/JSONUtils.java @@ -55,54 +55,53 @@ public final class JSONUtils { public static String buildJSON(Map params) { final StringWriter stringWriter = new StringWriter(); - final JsonGenerator generator = GENERATOR_FACTORY.createGenerator(stringWriter); - - generator.writeStartObject(); - - for (Map.Entry param : params.entrySet()) { - String key = param.getKey(); - Object value = param.getValue(); - if (key != null && value != null) { - if (value instanceof Boolean) { - generator.write(key, (Boolean) value); - } else if (value instanceof Double) { - generator.write(key, (Double) value); - } else if (value instanceof Integer) { - generator.write(key, (Integer) value); - } else if (value instanceof BigDecimal) { - generator.write(key, (BigDecimal) value); - } else if (value instanceof BigInteger) { - generator.write(key, (BigInteger) value); - } else if (value instanceof Long) { - generator.write(key, (Long) value); - } else if (value instanceof String) { - String string = (String) value; - if (!string.isEmpty()) { - generator.write(key, string); + try (final JsonGenerator generator = GENERATOR_FACTORY.createGenerator(stringWriter)) { + generator.writeStartObject(); + + for (Map.Entry param : params.entrySet()) { + String key = param.getKey(); + Object value = param.getValue(); + if (key != null && value != null) { + if (value instanceof Boolean) { + generator.write(key, (Boolean) value); + } else if (value instanceof Double) { + generator.write(key, (Double) value); + } else if (value instanceof Integer) { + generator.write(key, (Integer) value); + } else if (value instanceof BigDecimal) { + generator.write(key, (BigDecimal) value); + } else if (value instanceof BigInteger) { + generator.write(key, (BigInteger) value); + } else if (value instanceof Long) { + generator.write(key, (Long) value); + } else if (value instanceof String) { + String string = (String) value; + if (!string.isEmpty()) { + generator.write(key, string); + } + } else if (value.getClass().isArray()) { + generator.writeStartArray(key); + + for (int i = 0; i < Array.getLength(value); i++) { + witeItem(generator, Array.get(value, i)); + } + + generator.writeEnd(); + } else if (value instanceof Collection) { + generator.writeStartArray(key); + + Collection collection = (Collection) value; + for (Object item : collection) { + witeItem(generator, item); + } + + generator.writeEnd(); } - } else if (value.getClass().isArray()) { - generator.writeStartArray(key); - - for (int i = 0; i < Array.getLength(value); i++) { - witeItem(generator, Array.get(value, i)); - } - - generator.writeEnd(); - } else if (value instanceof Collection) { - generator.writeStartArray(key); - - Collection collection = (Collection) value; - for (Object item : collection) { - witeItem(generator, item); - } - - generator.writeEnd(); } } + generator.writeEnd(); } - generator.writeEnd().close(); - return stringWriter.toString(); } @@ -127,33 +126,32 @@ private static void witeItem(JsonGenerator generator, T item) { } public static Map parseJSON(String jsonBody) { - final Map params = new HashMap(); + final Map params = new HashMap<>(); StringReader reader = new StringReader(jsonBody); - JsonReader jsonReader = Json.createReader(reader); - JsonStructure structure = jsonReader.read(); + try (JsonReader jsonReader = Json.createReader(reader)) { + JsonStructure structure = jsonReader.read(); - if (structure == null || structure instanceof JsonArray) { - throw new IllegalArgumentException(format("String '%s' is not a valid JSON object representation", - jsonBody)); - } + if (structure == null || structure instanceof JsonArray) { + throw new IllegalArgumentException(format("String '%s' is not a valid JSON object representation", + jsonBody)); + } - JsonObject object = (JsonObject) structure; - for (Entry entry : object.entrySet()) { - String key = entry.getKey(); - if (key != null && !key.isEmpty()) { - JsonValue jsonValue = entry.getValue(); + JsonObject object = (JsonObject) structure; + for (Entry entry : object.entrySet()) { + String key = entry.getKey(); + if (key != null && !key.isEmpty()) { + JsonValue jsonValue = entry.getValue(); - // guard from null values - if (jsonValue != null) { - Object value = toJavaObject(jsonValue); + // guard from null values + if (jsonValue != null) { + Object value = toJavaObject(jsonValue); - params.put(key, value); + params.put(key, value); + } } } } - - jsonReader.close(); return params; } diff --git a/oauth-2.0/dynamicreg-server/src/main/java/org/apache/oltu/oauth2/ext/dynamicreg/server/request/JSONHttpServletRequestWrapper.java b/oauth-2.0/dynamicreg-server/src/main/java/org/apache/oltu/oauth2/ext/dynamicreg/server/request/JSONHttpServletRequestWrapper.java index 2a6875e7..5822be95 100644 --- a/oauth-2.0/dynamicreg-server/src/main/java/org/apache/oltu/oauth2/ext/dynamicreg/server/request/JSONHttpServletRequestWrapper.java +++ b/oauth-2.0/dynamicreg-server/src/main/java/org/apache/oltu/oauth2/ext/dynamicreg/server/request/JSONHttpServletRequestWrapper.java @@ -78,41 +78,40 @@ public Map getParameterMap() { String body = readJsonBody(); StringReader reader = new StringReader(body); - JsonReader jsonReader = Json.createReader(reader); - JsonStructure structure = jsonReader.read(); + try (JsonReader jsonReader = Json.createReader(reader)) { + JsonStructure structure = jsonReader.read(); - if (structure == null || structure instanceof JsonArray) { - throw new IllegalArgumentException(format("String '%s' is not a valid JSON object representation", - body)); - } + if (structure == null || structure instanceof JsonArray) { + throw new IllegalArgumentException(format("String '%s' is not a valid JSON object representation", + body)); + } - JsonObject object = (JsonObject) structure; - for (Entry entry : object.entrySet()) { - String key = entry.getKey(); - if (key != null) { - JsonValue jsonValue = entry.getValue(); - - // guard from null values - if (jsonValue != null) { - String[] values; - - if (ValueType.ARRAY == jsonValue.getValueType()) { - JsonArray array = (JsonArray) jsonValue; - values = new String[array.size()]; - for (int i = 0; i < array.size(); i++) { - JsonValue current = array.get(i); - values[i] = toJavaObject(current); + JsonObject object = (JsonObject) structure; + for (Entry entry : object.entrySet()) { + String key = entry.getKey(); + if (key != null) { + JsonValue jsonValue = entry.getValue(); + + // guard from null values + if (jsonValue != null) { + String[] values; + + if (ValueType.ARRAY == jsonValue.getValueType()) { + JsonArray array = (JsonArray) jsonValue; + values = new String[array.size()]; + for (int i = 0; i < array.size(); i++) { + JsonValue current = array.get(i); + values[i] = toJavaObject(current); + } + } else { + values = new String[]{toJavaObject(jsonValue)}; } - } else { - values = new String[]{ toJavaObject(jsonValue) }; - } - parameters.put(key, values); + parameters.put(key, values); + } } } } - - jsonReader.close(); } return Collections.unmodifiableMap(parameters); diff --git a/oauth-2.0/test-utils/src/main/java/org/apache/oltu/oauth2/utils/test/FileUtils.java b/oauth-2.0/test-utils/src/main/java/org/apache/oltu/oauth2/utils/test/FileUtils.java index 643a4952..63891170 100644 --- a/oauth-2.0/test-utils/src/main/java/org/apache/oltu/oauth2/utils/test/FileUtils.java +++ b/oauth-2.0/test-utils/src/main/java/org/apache/oltu/oauth2/utils/test/FileUtils.java @@ -34,13 +34,13 @@ private FileUtils() { public static String readTextFileAsString(String fileName) throws IOException { URL inputStream = ClassLoader.getSystemResource(fileName); - BufferedReader br = new BufferedReader(new InputStreamReader(inputStream.openStream())); - - String line; - StringBuffer buffer = new StringBuffer(); - while ((line = br.readLine()) != null) { - buffer.append(line); + try (BufferedReader br = new BufferedReader(new InputStreamReader(inputStream.openStream()))) { + String line; + StringBuffer buffer = new StringBuffer(); + while ((line = br.readLine()) != null) { + buffer.append(line); + } + return buffer.toString(); } - return buffer.toString(); } }