diff options
6 files changed, 136 insertions, 20 deletions
diff --git a/android/guava-tests/test/com/google/common/util/concurrent/AbstractFutureTest.java b/android/guava-tests/test/com/google/common/util/concurrent/AbstractFutureTest.java index 1ef7cbbe8..3c210ce7e 100644 --- a/android/guava-tests/test/com/google/common/util/concurrent/AbstractFutureTest.java +++ b/android/guava-tests/test/com/google/common/util/concurrent/AbstractFutureTest.java @@ -25,6 +25,7 @@ import com.google.common.collect.Range; import com.google.common.collect.Sets; import com.google.common.util.concurrent.internal.InternalFutureFailureAccess; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; import java.util.List; import java.util.Set; @@ -213,6 +214,44 @@ public class AbstractFutureTest extends TestCase { assertThat(SettableFuture.create().toString()).isNotEqualTo(SettableFuture.create().toString()); } + public void testToString_oom() throws Exception { + SettableFuture<Object> future = SettableFuture.create(); + future.set( + new Object() { + @Override + public String toString() { + throw new OutOfMemoryError(); + } + + @Override + public int hashCode() { + throw new OutOfMemoryError(); + } + }); + + String unused = future.toString(); + + SettableFuture<Object> future2 = SettableFuture.create(); + + // A more organic OOM from a toString implementation + Object object = + new Object() { + @Override + public String toString() { + return new String(new char[50_000]); + } + }; + List<Object> list = Collections.singletonList(object); + for (int i = 0; i < 10; i++) { + Object[] array = new Object[500]; + Arrays.fill(array, list); + list = Arrays.asList(array); + } + future2.set(list); + + unused = future.toString(); + } + public void testToString_notDone() throws Exception { AbstractFuture<Object> testFuture = new AbstractFuture<Object>() { @@ -243,7 +282,8 @@ public class AbstractFutureTest extends TestCase { return "cause=[Because this test isn't done]"; } }; - assertThat(testFuture.toString()).matches("[^\\[]+\\[status=SUCCESS, result=\\[true\\]\\]"); + assertThat(testFuture.toString()) + .matches("[^\\[]+\\[status=SUCCESS, result=\\[java.lang.Boolean@\\w+\\]\\]"); } /** @@ -308,7 +348,7 @@ public class AbstractFutureTest extends TestCase { + " info=\\[cause=\\[Someday...]]]]]"); testFuture2.set("result string"); assertThat(testFuture3.toString()) - .matches("[^\\[]+\\[status=SUCCESS, result=\\[result string\\]\\]"); + .matches("[^\\[]+\\[status=SUCCESS, result=\\[java.lang.String@\\w+\\]\\]"); } public void testToString_cancelled() throws Exception { @@ -944,11 +984,11 @@ public class AbstractFutureTest extends TestCase { public void testSetIndirectSelf_toString() { final SettableFuture<Object> orig = SettableFuture.create(); // unlike the above this indirection defeats the trivial cycle detection and causes a SOE - orig.set( - new Object() { + orig.setFuture( + new ForwardingListenableFuture<Object>() { @Override - public String toString() { - return orig.toString(); + protected ListenableFuture<Object> delegate() { + return orig; } }); assertThat(orig.toString()) diff --git a/android/guava-tests/test/com/google/common/util/concurrent/FuturesTest.java b/android/guava-tests/test/com/google/common/util/concurrent/FuturesTest.java index 4445aab46..797ee4882 100644 --- a/android/guava-tests/test/com/google/common/util/concurrent/FuturesTest.java +++ b/android/guava-tests/test/com/google/common/util/concurrent/FuturesTest.java @@ -2627,8 +2627,8 @@ public class FuturesTest extends TestCase { assertThat(futureResult.toString()) .matches( "CombinedFuture@\\w+\\[status=PENDING," - + " info=\\[futures=\\[SettableFuture@\\w+\\[status=SUCCESS, result=\\[1]]," - + " SettableFuture@\\w+\\[status=PENDING]]]]"); + + " info=\\[futures=\\[SettableFuture@\\w+\\[status=SUCCESS," + + " result=\\[java.lang.Integer@\\w+]], SettableFuture@\\w+\\[status=PENDING]]]]"); // Backing futures complete Boolean booleanPartial = true; @@ -2649,7 +2649,7 @@ public class FuturesTest extends TestCase { String expectedResult = createCombinedResult(integerPartial, booleanPartial); assertEquals(expectedResult, futureResult.get()); assertThat(futureResult.toString()) - .matches("CombinedFuture@\\w+\\[status=SUCCESS, result=\\[" + expectedResult + "]]"); + .matches("CombinedFuture@\\w+\\[status=SUCCESS, result=\\[java.lang.String@\\w+]]"); } public void testWhenAllComplete_asyncError() throws Exception { diff --git a/android/guava/src/com/google/common/util/concurrent/AbstractFuture.java b/android/guava/src/com/google/common/util/concurrent/AbstractFuture.java index 2e77ab7cc..e7992415c 100644 --- a/android/guava/src/com/google/common/util/concurrent/AbstractFuture.java +++ b/android/guava/src/com/google/common/util/concurrent/AbstractFuture.java @@ -1158,7 +1158,7 @@ public abstract class AbstractFuture<V> extends InternalFutureFailureAccess try { V value = getUninterruptibly(this); builder.append("SUCCESS, result=["); - appendUserObject(builder, value); + appendResultObject(builder, value); builder.append("]"); } catch (ExecutionException e) { builder.append("FAILURE, cause=[").append(e.getCause()).append("]"); @@ -1169,6 +1169,24 @@ public abstract class AbstractFuture<V> extends InternalFutureFailureAccess } } + /** + * Any object can be the result of a Future, and not every object has a reasonable toString() + * implementation. Using a reconstruction of the default Object.toString() prevents OOMs and stack + * overflows, and helps avoid sensitive data inadvertently ending up in exception messages. + */ + private void appendResultObject(StringBuilder builder, Object o) { + if (o == null) { + builder.append("null"); + } else if (o == this) { + builder.append("this future"); + } else { + builder + .append(o.getClass().getName()) + .append("@") + .append(Integer.toHexString(System.identityHashCode(o))); + } + } + /** Helper for printing user supplied objects into our toString method. */ private void appendUserObject(StringBuilder builder, Object o) { // This is some basic recursion detection for when people create cycles via set/setFuture or diff --git a/guava-tests/test/com/google/common/util/concurrent/AbstractFutureTest.java b/guava-tests/test/com/google/common/util/concurrent/AbstractFutureTest.java index 1ef7cbbe8..3c210ce7e 100644 --- a/guava-tests/test/com/google/common/util/concurrent/AbstractFutureTest.java +++ b/guava-tests/test/com/google/common/util/concurrent/AbstractFutureTest.java @@ -25,6 +25,7 @@ import com.google.common.collect.Range; import com.google.common.collect.Sets; import com.google.common.util.concurrent.internal.InternalFutureFailureAccess; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; import java.util.List; import java.util.Set; @@ -213,6 +214,44 @@ public class AbstractFutureTest extends TestCase { assertThat(SettableFuture.create().toString()).isNotEqualTo(SettableFuture.create().toString()); } + public void testToString_oom() throws Exception { + SettableFuture<Object> future = SettableFuture.create(); + future.set( + new Object() { + @Override + public String toString() { + throw new OutOfMemoryError(); + } + + @Override + public int hashCode() { + throw new OutOfMemoryError(); + } + }); + + String unused = future.toString(); + + SettableFuture<Object> future2 = SettableFuture.create(); + + // A more organic OOM from a toString implementation + Object object = + new Object() { + @Override + public String toString() { + return new String(new char[50_000]); + } + }; + List<Object> list = Collections.singletonList(object); + for (int i = 0; i < 10; i++) { + Object[] array = new Object[500]; + Arrays.fill(array, list); + list = Arrays.asList(array); + } + future2.set(list); + + unused = future.toString(); + } + public void testToString_notDone() throws Exception { AbstractFuture<Object> testFuture = new AbstractFuture<Object>() { @@ -243,7 +282,8 @@ public class AbstractFutureTest extends TestCase { return "cause=[Because this test isn't done]"; } }; - assertThat(testFuture.toString()).matches("[^\\[]+\\[status=SUCCESS, result=\\[true\\]\\]"); + assertThat(testFuture.toString()) + .matches("[^\\[]+\\[status=SUCCESS, result=\\[java.lang.Boolean@\\w+\\]\\]"); } /** @@ -308,7 +348,7 @@ public class AbstractFutureTest extends TestCase { + " info=\\[cause=\\[Someday...]]]]]"); testFuture2.set("result string"); assertThat(testFuture3.toString()) - .matches("[^\\[]+\\[status=SUCCESS, result=\\[result string\\]\\]"); + .matches("[^\\[]+\\[status=SUCCESS, result=\\[java.lang.String@\\w+\\]\\]"); } public void testToString_cancelled() throws Exception { @@ -944,11 +984,11 @@ public class AbstractFutureTest extends TestCase { public void testSetIndirectSelf_toString() { final SettableFuture<Object> orig = SettableFuture.create(); // unlike the above this indirection defeats the trivial cycle detection and causes a SOE - orig.set( - new Object() { + orig.setFuture( + new ForwardingListenableFuture<Object>() { @Override - public String toString() { - return orig.toString(); + protected ListenableFuture<Object> delegate() { + return orig; } }); assertThat(orig.toString()) diff --git a/guava-tests/test/com/google/common/util/concurrent/FuturesTest.java b/guava-tests/test/com/google/common/util/concurrent/FuturesTest.java index 6c834db14..9586a65a1 100644 --- a/guava-tests/test/com/google/common/util/concurrent/FuturesTest.java +++ b/guava-tests/test/com/google/common/util/concurrent/FuturesTest.java @@ -2627,8 +2627,8 @@ public class FuturesTest extends TestCase { assertThat(futureResult.toString()) .matches( "CombinedFuture@\\w+\\[status=PENDING," - + " info=\\[futures=\\[SettableFuture@\\w+\\[status=SUCCESS, result=\\[1]]," - + " SettableFuture@\\w+\\[status=PENDING]]]]"); + + " info=\\[futures=\\[SettableFuture@\\w+\\[status=SUCCESS," + + " result=\\[java.lang.Integer@\\w+]], SettableFuture@\\w+\\[status=PENDING]]]]"); // Backing futures complete Boolean booleanPartial = true; @@ -2649,7 +2649,7 @@ public class FuturesTest extends TestCase { String expectedResult = createCombinedResult(integerPartial, booleanPartial); assertEquals(expectedResult, futureResult.get()); assertThat(futureResult.toString()) - .matches("CombinedFuture@\\w+\\[status=SUCCESS, result=\\[" + expectedResult + "]]"); + .matches("CombinedFuture@\\w+\\[status=SUCCESS, result=\\[java.lang.String@\\w+]]"); } public void testWhenAllComplete_asyncError() throws Exception { diff --git a/guava/src/com/google/common/util/concurrent/AbstractFuture.java b/guava/src/com/google/common/util/concurrent/AbstractFuture.java index dd863df05..06feb5a21 100644 --- a/guava/src/com/google/common/util/concurrent/AbstractFuture.java +++ b/guava/src/com/google/common/util/concurrent/AbstractFuture.java @@ -1156,7 +1156,7 @@ public abstract class AbstractFuture<V> extends InternalFutureFailureAccess try { V value = getUninterruptibly(this); builder.append("SUCCESS, result=["); - appendUserObject(builder, value); + appendResultObject(builder, value); builder.append("]"); } catch (ExecutionException e) { builder.append("FAILURE, cause=[").append(e.getCause()).append("]"); @@ -1167,6 +1167,24 @@ public abstract class AbstractFuture<V> extends InternalFutureFailureAccess } } + /** + * Any object can be the result of a Future, and not every object has a reasonable toString() + * implementation. Using a reconstruction of the default Object.toString() prevents OOMs and stack + * overflows, and helps avoid sensitive data inadvertently ending up in exception messages. + */ + private void appendResultObject(StringBuilder builder, Object o) { + if (o == null) { + builder.append("null"); + } else if (o == this) { + builder.append("this future"); + } else { + builder + .append(o.getClass().getName()) + .append("@") + .append(Integer.toHexString(System.identityHashCode(o))); + } + } + /** Helper for printing user supplied objects into our toString method. */ private void appendUserObject(StringBuilder builder, Object o) { // This is some basic recursion detection for when people create cycles via set/setFuture or |