This is an automated email from the ASF dual-hosted git repository.
garydgregory pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/commons-lang.git
The following commit(s) were added to refs/heads/master by this push:
new 552d7d07d
ToStringStyle.appendDetail(StringBuffer,String,Collection|Map) bypass (#1648)
552d7d07d is described below
commit 552d7d07d06708961b12272280cca3704901b835
Author: Gary Gregory <[email protected]>
AuthorDate: Sun May 17 10:10:10 2026 -0400
ToStringStyle.appendDetail(StringBuffer,String,Collection|Map) bypass
(#1648)
cycle registry.
Fixes Stack overflow on mutually-referential collections
---
.../commons/lang3/builder/ToStringStyle.java | 39 +++++++++++--
.../builder/ToStringBuilderAppendCycleTest.java | 68 ++++++++++++++++++++++
2 files changed, 103 insertions(+), 4 deletions(-)
diff --git a/src/main/java/org/apache/commons/lang3/builder/ToStringStyle.java
b/src/main/java/org/apache/commons/lang3/builder/ToStringStyle.java
index 1400a6f38..30e44779c 100644
--- a/src/main/java/org/apache/commons/lang3/builder/ToStringStyle.java
+++ b/src/main/java/org/apache/commons/lang3/builder/ToStringStyle.java
@@ -20,10 +20,10 @@
import java.io.Serializable;
import java.lang.reflect.Array;
import java.util.Collection;
+import java.util.IdentityHashMap;
import java.util.Map;
import java.util.Map.Entry;
import java.util.Objects;
-import java.util.WeakHashMap;
import org.apache.commons.lang3.ClassUtils;
import org.apache.commons.lang3.ObjectUtils;
@@ -577,8 +577,10 @@ private Object readResolve() {
/**
* A registry of objects used by {@code reflectionToString} methods to
detect cyclical object references and avoid infinite loops.
+ * Identity-based comparison is required so that cyclic objects (e.g. an
ArrayList whose hashCode() would recurse) can be
+ * registered and looked up without triggering infinite recursion through
equals/hashCode.
*/
- private static final ThreadLocal<WeakHashMap<Object, Object>> REGISTRY =
ThreadLocal.withInitial(WeakHashMap::new);
+ private static final ThreadLocal<IdentityHashMap<Object, Object>> REGISTRY
= ThreadLocal.withInitial(IdentityHashMap::new);
/*
* Note that objects of this class are generally shared between threads,
so an instance variable would not be suitable here.
*
@@ -1187,7 +1189,20 @@ protected void appendDetail(final StringBuffer buffer,
final String fieldName, f
* @param coll the {@link Collection} to add to the {@code toString},
not {@code null}.
*/
protected void appendDetail(final StringBuffer buffer, final String
fieldName, final Collection<?> coll) {
- buffer.append(coll);
+ buffer.append('['); // backward compatibility
+ boolean first = true;
+ for (final Object item : coll) {
+ if (!first) {
+ buffer.append(", "); // backward compatibility
+ }
+ first = false;
+ if (item == null) {
+ appendNullText(buffer, fieldName);
+ } else {
+ appendInternal(buffer, fieldName, item, true);
+ }
+ }
+ buffer.append(']'); // backward compatibility
}
/**
@@ -1334,7 +1349,23 @@ protected void appendDetail(final StringBuffer buffer,
final String fieldName, f
* @param map the {@link Map} to add to the {@code toString}, not
{@code null}.
*/
protected void appendDetail(final StringBuffer buffer, final String
fieldName, final Map<?, ?> map) {
- buffer.append(map);
+ buffer.append('{'); // backward compatibility
+ boolean first = true;
+ for (final Map.Entry<?, ?> item : map.entrySet()) {
+ if (!first) {
+ buffer.append(getArraySeparator());
+ buffer.append(' '); // backward compatibility
+ }
+ first = false;
+ if (item == null) {
+ appendNullText(buffer, fieldName);
+ } else {
+ appendInternal(buffer, fieldName, item.getKey(), true);
+ buffer.append(getFieldNameValueSeparator());
+ appendInternal(buffer, fieldName, item.getValue(), true);
+ }
+ }
+ buffer.append('}'); // backward compatibility
}
/**
diff --git
a/src/test/java/org/apache/commons/lang3/builder/ToStringBuilderAppendCycleTest.java
b/src/test/java/org/apache/commons/lang3/builder/ToStringBuilderAppendCycleTest.java
new file mode 100644
index 000000000..4dcaffffa
--- /dev/null
+++
b/src/test/java/org/apache/commons/lang3/builder/ToStringBuilderAppendCycleTest.java
@@ -0,0 +1,68 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.commons.lang3.builder;
+
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import org.junit.jupiter.api.Test;
+
+/**
+ * Tests {@code ToStringStyle} collection toString can overflow the stack.
+ *
+ * Pre-patch: appendDetail(StringBuffer, String, Collection) calls
buffer.append(coll) which invokes the collection's own toString(), bypassing
the cycle
+ * registry. Two mutually-referencing ArrayLists cause StackOverflowError.
Post-patch: the cycle is detected and a safe representation is produced.
+ */
+class ToStringBuilderAppendCycleTest {
+
+ @Test
+ void testMutuallyCyclicCollection() {
+ final List<Object> a = new ArrayList<>();
+ final List<Object> b = new ArrayList<>();
+ a.add(b);
+ b.add(a);
+ assertNotNull(new ToStringBuilder(new Object(),
ToStringStyle.DEFAULT_STYLE).append("field", a).build());
+ }
+
+ @Test
+ void testMutuallyCyclicMap() {
+ final Map<Object, Object> a = new HashMap<>();
+ final Map<Object, Object> b = new HashMap<>();
+ a.put(b, b);
+ b.put(a, a);
+ assertNotNull(new ToStringBuilder(new Object(),
ToStringStyle.DEFAULT_STYLE).append("field", a).build());
+ }
+
+ @Test
+ void testSelfReferentialCollection() {
+ final List<Object> a = new ArrayList<>();
+ a.add(a);
+ assertNotNull(new ToStringBuilder(new Object(),
ToStringStyle.DEFAULT_STYLE).append("field", a).build());
+ }
+
+ @Test
+ void testSelfReferentialObjectArray() {
+ final Object[] a = { null };
+ a[0] = a;
+ assertNotNull(new ToStringBuilder(new Object(),
ToStringStyle.DEFAULT_STYLE).append("field", a).build());
+ }
+}
