Skip to content

Commit 2ebf27f

Browse files
clmcpovirk
clm
authored and
cpovirk
committed
Don't call toString() on the results of successful futures.
RELNOTES=AbstractFuture.toString() no longer includes the toString() of the result. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=336911024
1 parent 1b20a37 commit 2ebf27f

File tree

6 files changed

+136
-20
lines changed

6 files changed

+136
-20
lines changed

‎android/guava-tests/test/com/google/common/util/concurrent/AbstractFutureTest.java

Lines changed: 46 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import com.google.common.collect.Sets;
2626
import com.google.common.util.concurrent.internal.InternalFutureFailureAccess;
2727
import java.util.ArrayList;
28+
import java.util.Arrays;
2829
import java.util.Collections;
2930
import java.util.List;
3031
import java.util.Set;
@@ -213,6 +214,44 @@ public void testToString_allUnique() throws Exception {
213214
assertThat(SettableFuture.create().toString()).isNotEqualTo(SettableFuture.create().toString());
214215
}
215216

217+
public void testToString_oom() throws Exception {
218+
SettableFuture<Object> future = SettableFuture.create();
219+
future.set(
220+
new Object() {
221+
@Override
222+
public String toString() {
223+
throw new OutOfMemoryError();
224+
}
225+
226+
@Override
227+
public int hashCode() {
228+
throw new OutOfMemoryError();
229+
}
230+
});
231+
232+
String unused = future.toString();
233+
234+
SettableFuture<Object> future2 = SettableFuture.create();
235+
236+
// A more organic OOM from a toString implementation
237+
Object object =
238+
new Object() {
239+
@Override
240+
public String toString() {
241+
return new String(new char[50_000]);
242+
}
243+
};
244+
List<Object> list = Collections.singletonList(object);
245+
for (int i = 0; i < 10; i++) {
246+
Object[] array = new Object[500];
247+
Arrays.fill(array, list);
248+
list = Arrays.asList(array);
249+
}
250+
future2.set(list);
251+
252+
unused = future.toString();
253+
}
254+
216255
public void testToString_notDone() throws Exception {
217256
AbstractFuture<Object> testFuture =
218257
new AbstractFuture<Object>() {
@@ -243,7 +282,8 @@ public String pendingToString() {
243282
return "cause=[Because this test isn't done]";
244283
}
245284
};
246-
assertThat(testFuture.toString()).matches("[^\\[]+\\[status=SUCCESS, result=\\[true\\]\\]");
285+
assertThat(testFuture.toString())
286+
.matches("[^\\[]+\\[status=SUCCESS, result=\\[java.lang.Boolean@\\w+\\]\\]");
247287
}
248288

249289
/**
@@ -308,7 +348,7 @@ public String pendingToString() {
308348
+ " info=\\[cause=\\[Someday...]]]]]");
309349
testFuture2.set("result string");
310350
assertThat(testFuture3.toString())
311-
.matches("[^\\[]+\\[status=SUCCESS, result=\\[result string\\]\\]");
351+
.matches("[^\\[]+\\[status=SUCCESS, result=\\[java.lang.String@\\w+\\]\\]");
312352
}
313353

314354
public void testToString_cancelled() throws Exception {
@@ -944,11 +984,11 @@ public String toString() {
944984
public void testSetIndirectSelf_toString() {
945985
final SettableFuture<Object> orig = SettableFuture.create();
946986
// unlike the above this indirection defeats the trivial cycle detection and causes a SOE
947-
orig.set(
948-
new Object() {
987+
orig.setFuture(
988+
new ForwardingListenableFuture<Object>() {
949989
@Override
950-
public String toString() {
951-
return orig.toString();
990+
protected ListenableFuture<Object> delegate() {
991+
return orig;
952992
}
953993
});
954994
assertThat(orig.toString())

‎android/guava-tests/test/com/google/common/util/concurrent/FuturesTest.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2627,8 +2627,8 @@ public ListenableFuture<String> call() throws Exception {
26272627
assertThat(futureResult.toString())
26282628
.matches(
26292629
"CombinedFuture@\\w+\\[status=PENDING,"
2630-
+ " info=\\[futures=\\[SettableFuture@\\w+\\[status=SUCCESS, result=\\[1]],"
2631-
+ " SettableFuture@\\w+\\[status=PENDING]]]]");
2630+
+ " info=\\[futures=\\[SettableFuture@\\w+\\[status=SUCCESS,"
2631+
+ " result=\\[java.lang.Integer@\\w+]], SettableFuture@\\w+\\[status=PENDING]]]]");
26322632

26332633
// Backing futures complete
26342634
Boolean booleanPartial = true;
@@ -2649,7 +2649,7 @@ public ListenableFuture<String> call() throws Exception {
26492649
String expectedResult = createCombinedResult(integerPartial, booleanPartial);
26502650
assertEquals(expectedResult, futureResult.get());
26512651
assertThat(futureResult.toString())
2652-
.matches("CombinedFuture@\\w+\\[status=SUCCESS, result=\\[" + expectedResult + "]]");
2652+
.matches("CombinedFuture@\\w+\\[status=SUCCESS, result=\\[java.lang.String@\\w+]]");
26532653
}
26542654

26552655
public void testWhenAllComplete_asyncError() throws Exception {

‎android/guava/src/com/google/common/util/concurrent/AbstractFuture.java

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1158,7 +1158,7 @@ private void addDoneString(StringBuilder builder) {
11581158
try {
11591159
V value = getUninterruptibly(this);
11601160
builder.append("SUCCESS, result=[");
1161-
appendUserObject(builder, value);
1161+
appendResultObject(builder, value);
11621162
builder.append("]");
11631163
} catch (ExecutionException e) {
11641164
builder.append("FAILURE, cause=[").append(e.getCause()).append("]");
@@ -1169,6 +1169,24 @@ private void addDoneString(StringBuilder builder) {
11691169
}
11701170
}
11711171

1172+
/**
1173+
* Any object can be the result of a Future, and not every object has a reasonable toString()
1174+
* implementation. Using a reconstruction of the default Object.toString() prevents OOMs and stack
1175+
* overflows, and helps avoid sensitive data inadvertently ending up in exception messages.
1176+
*/
1177+
private void appendResultObject(StringBuilder builder, Object o) {
1178+
if (o == null) {
1179+
builder.append("null");
1180+
} else if (o == this) {
1181+
builder.append("this future");
1182+
} else {
1183+
builder
1184+
.append(o.getClass().getName())
1185+
.append("@")
1186+
.append(Integer.toHexString(System.identityHashCode(o)));
1187+
}
1188+
}
1189+
11721190
/** Helper for printing user supplied objects into our toString method. */
11731191
private void appendUserObject(StringBuilder builder, Object o) {
11741192
// This is some basic recursion detection for when people create cycles via set/setFuture or

‎guava-tests/test/com/google/common/util/concurrent/AbstractFutureTest.java

Lines changed: 46 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import com.google.common.collect.Sets;
2626
import com.google.common.util.concurrent.internal.InternalFutureFailureAccess;
2727
import java.util.ArrayList;
28+
import java.util.Arrays;
2829
import java.util.Collections;
2930
import java.util.List;
3031
import java.util.Set;
@@ -213,6 +214,44 @@ public void testToString_allUnique() throws Exception {
213214
assertThat(SettableFuture.create().toString()).isNotEqualTo(SettableFuture.create().toString());
214215
}
215216

217+
public void testToString_oom() throws Exception {
218+
SettableFuture<Object> future = SettableFuture.create();
219+
future.set(
220+
new Object() {
221+
@Override
222+
public String toString() {
223+
throw new OutOfMemoryError();
224+
}
225+
226+
@Override
227+
public int hashCode() {
228+
throw new OutOfMemoryError();
229+
}
230+
});
231+
232+
String unused = future.toString();
233+
234+
SettableFuture<Object> future2 = SettableFuture.create();
235+
236+
// A more organic OOM from a toString implementation
237+
Object object =
238+
new Object() {
239+
@Override
240+
public String toString() {
241+
return new String(new char[50_000]);
242+
}
243+
};
244+
List<Object> list = Collections.singletonList(object);
245+
for (int i = 0; i < 10; i++) {
246+
Object[] array = new Object[500];
247+
Arrays.fill(array, list);
248+
list = Arrays.asList(array);
249+
}
250+
future2.set(list);
251+
252+
unused = future.toString();
253+
}
254+
216255
public void testToString_notDone() throws Exception {
217256
AbstractFuture<Object> testFuture =
218257
new AbstractFuture<Object>() {
@@ -243,7 +282,8 @@ public String pendingToString() {
243282
return "cause=[Because this test isn't done]";
244283
}
245284
};
246-
assertThat(testFuture.toString()).matches("[^\\[]+\\[status=SUCCESS, result=\\[true\\]\\]");
285+
assertThat(testFuture.toString())
286+
.matches("[^\\[]+\\[status=SUCCESS, result=\\[java.lang.Boolean@\\w+\\]\\]");
247287
}
248288

249289
/**
@@ -308,7 +348,7 @@ public String pendingToString() {
308348
+ " info=\\[cause=\\[Someday...]]]]]");
309349
testFuture2.set("result string");
310350
assertThat(testFuture3.toString())
311-
.matches("[^\\[]+\\[status=SUCCESS, result=\\[result string\\]\\]");
351+
.matches("[^\\[]+\\[status=SUCCESS, result=\\[java.lang.String@\\w+\\]\\]");
312352
}
313353

314354
public void testToString_cancelled() throws Exception {
@@ -944,11 +984,11 @@ public String toString() {
944984
public void testSetIndirectSelf_toString() {
945985
final SettableFuture<Object> orig = SettableFuture.create();
946986
// unlike the above this indirection defeats the trivial cycle detection and causes a SOE
947-
orig.set(
948-
new Object() {
987+
orig.setFuture(
988+
new ForwardingListenableFuture<Object>() {
949989
@Override
950-
public String toString() {
951-
return orig.toString();
990+
protected ListenableFuture<Object> delegate() {
991+
return orig;
952992
}
953993
});
954994
assertThat(orig.toString())

‎guava-tests/test/com/google/common/util/concurrent/FuturesTest.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2627,8 +2627,8 @@ public ListenableFuture<String> call() throws Exception {
26272627
assertThat(futureResult.toString())
26282628
.matches(
26292629
"CombinedFuture@\\w+\\[status=PENDING,"
2630-
+ " info=\\[futures=\\[SettableFuture@\\w+\\[status=SUCCESS, result=\\[1]],"
2631-
+ " SettableFuture@\\w+\\[status=PENDING]]]]");
2630+
+ " info=\\[futures=\\[SettableFuture@\\w+\\[status=SUCCESS,"
2631+
+ " result=\\[java.lang.Integer@\\w+]], SettableFuture@\\w+\\[status=PENDING]]]]");
26322632

26332633
// Backing futures complete
26342634
Boolean booleanPartial = true;
@@ -2649,7 +2649,7 @@ public ListenableFuture<String> call() throws Exception {
26492649
String expectedResult = createCombinedResult(integerPartial, booleanPartial);
26502650
assertEquals(expectedResult, futureResult.get());
26512651
assertThat(futureResult.toString())
2652-
.matches("CombinedFuture@\\w+\\[status=SUCCESS, result=\\[" + expectedResult + "]]");
2652+
.matches("CombinedFuture@\\w+\\[status=SUCCESS, result=\\[java.lang.String@\\w+]]");
26532653
}
26542654

26552655
public void testWhenAllComplete_asyncError() throws Exception {

‎guava/src/com/google/common/util/concurrent/AbstractFuture.java

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1156,7 +1156,7 @@ private void addDoneString(StringBuilder builder) {
11561156
try {
11571157
V value = getUninterruptibly(this);
11581158
builder.append("SUCCESS, result=[");
1159-
appendUserObject(builder, value);
1159+
appendResultObject(builder, value);
11601160
builder.append("]");
11611161
} catch (ExecutionException e) {
11621162
builder.append("FAILURE, cause=[").append(e.getCause()).append("]");
@@ -1167,6 +1167,24 @@ private void addDoneString(StringBuilder builder) {
11671167
}
11681168
}
11691169

1170+
/**
1171+
* Any object can be the result of a Future, and not every object has a reasonable toString()
1172+
* implementation. Using a reconstruction of the default Object.toString() prevents OOMs and stack
1173+
* overflows, and helps avoid sensitive data inadvertently ending up in exception messages.
1174+
*/
1175+
private void appendResultObject(StringBuilder builder, Object o) {
1176+
if (o == null) {
1177+
builder.append("null");
1178+
} else if (o == this) {
1179+
builder.append("this future");
1180+
} else {
1181+
builder
1182+
.append(o.getClass().getName())
1183+
.append("@")
1184+
.append(Integer.toHexString(System.identityHashCode(o)));
1185+
}
1186+
}
1187+
11701188
/** Helper for printing user supplied objects into our toString method. */
11711189
private void appendUserObject(StringBuilder builder, Object o) {
11721190
// This is some basic recursion detection for when people create cycles via set/setFuture or

0 commit comments

Comments
 (0)