diff --git a/logstash-core/src/main/java/org/logstash/Accessors.java b/logstash-core/src/main/java/org/logstash/Accessors.java index 46ae39206..f7197d1f2 100644 --- a/logstash-core/src/main/java/org/logstash/Accessors.java +++ b/logstash-core/src/main/java/org/logstash/Accessors.java @@ -1,8 +1,8 @@ package org.logstash; import java.util.HashMap; -import java.util.Map; import java.util.List; +import java.util.Map; public class Accessors { @@ -21,9 +21,36 @@ public class Accessors { } public Object set(String reference, Object value) { - FieldReference field = PathCache.cache(reference); - Object target = findCreateTarget(field); - return store(target, field.getKey(), value); + final FieldReference field = PathCache.cache(reference); + final Object target = findCreateTarget(field); + final String key = field.getKey(); + if (target instanceof Map) { + ((Map) target).put(key, value); + } else if (target instanceof List) { + int i; + try { + i = Integer.parseInt(key); + } catch (NumberFormatException e) { + return null; + } + int size = ((List) target).size(); + if (i >= size) { + // grow array by adding trailing null items + // this strategy reflects legacy Ruby impl behaviour and is backed by specs + // TODO: (colin) this is potentially dangerous, and could produce OOM using arbitrary big numbers + // TODO: (colin) should be guard against this? + for (int j = size; j < i; j++) { + ((List) target).add(null); + } + ((List) target).add(value); + } else { + int offset = listIndex(i, ((List) target).size()); + ((List) target).set(offset, value); + } + } else { + throw newCollectionException(target); + } + return value; } public Object del(String reference) { @@ -48,18 +75,17 @@ public class Accessors { } public boolean includes(String reference) { - FieldReference field = PathCache.cache(reference); - Object target = findTarget(field); - if (target instanceof Map && foundInMap((Map) target, field.getKey())) { - return true; - } else if (target instanceof List) { - try { - int i = Integer.parseInt(field.getKey()); - return (foundInList((List) target, i) ? true : false); - } catch (NumberFormatException e) { - return false; - } - } else { + final FieldReference field = PathCache.cache(reference); + final Object target = findTarget(field); + final String key = field.getKey(); + return target instanceof Map && ((Map) target).containsKey(key) || + target instanceof List && foundInList(key, (List) target); + } + + private static boolean foundInList(final String key, final List target) { + try { + return foundInList(target, Integer.parseInt(key)); + } catch (NumberFormatException e) { return false; } } @@ -132,10 +158,6 @@ public class Accessors { } - private static boolean foundInMap(Map target, String key) { - return target.containsKey(key); - } - private static Object fetch(Object target, String key) { if (target instanceof Map) { Object result = ((Map) target).get(key); @@ -154,36 +176,6 @@ public class Accessors { } } - private static Object store(Object target, String key, Object value) { - if (target instanceof Map) { - ((Map) target).put(key, value); - } else if (target instanceof List) { - int i; - try { - i = Integer.parseInt(key); - } catch (NumberFormatException e) { - return null; - } - int size = ((List) target).size(); - if (i >= size) { - // grow array by adding trailing null items - // this strategy reflects legacy Ruby impl behaviour and is backed by specs - // TODO: (colin) this is potentially dangerous, and could produce OOM using arbitrary big numbers - // TODO: (colin) should be guard against this? - for (int j = size; j < i; j++) { - ((List) target).add(null); - } - ((List) target).add(value); - } else { - int offset = listIndex(i, ((List) target).size()); - ((List) target).set(offset, value); - } - } else { - throw newCollectionException(target); - } - return value; - } - private static boolean isCollection(Object target) { if (target == null) { return false;