From 8d5b2669ef6e4d545bd679a4dabf9c507cd8cb03 Mon Sep 17 00:00:00 2001 From: Naveed Khan Date: Tue, 29 Sep 2026 23:43:59 +0530 Subject: [PATCH] fix infinite loop in TextHelpAppendable.makeColumnQueue at width 1 indexOfWrap chops a line without whitespace at limit - 1, which is startPos when the width is 1, so makeColumnQueue re-reads the same position and grows its queue until the heap is exhausted. appendList hits this with default settings for an 8 character entry containing a line break because it sizes the wrap width from the entry length, and throws for shorter entries. Always consume at least one character in indexOfWrap and wrap list entries at the configured width. --- .../commons/cli/help/TextHelpAppendable.java | 11 ++++-- .../cli/help/TextHelpAppendableTest.java | 37 +++++++++++++++++++ 2 files changed, 44 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/apache/commons/cli/help/TextHelpAppendable.java b/src/main/java/org/apache/commons/cli/help/TextHelpAppendable.java index 2b053106c..0da919f29 100644 --- a/src/main/java/org/apache/commons/cli/help/TextHelpAppendable.java +++ b/src/main/java/org/apache/commons/cli/help/TextHelpAppendable.java @@ -99,8 +99,9 @@ public static int indexOfWrap(final CharSequence text, final int width, final in break; } } - // if we found it return it, otherwise just chop at limit - return pos > startPos ? pos : limit - 1; + // if we found it return it, otherwise just chop at limit, always consuming at least one character so that a width of 1 + // still advances past startPos and the wrap loop in makeColumnQueue terminates. + return pos > startPos ? pos : Math.max(limit - 1, startPos + 1); } /** @@ -218,12 +219,14 @@ public void appendHeader(final int level, final CharSequence text) throws IOExce @Override public void appendList(final boolean ordered, final Collection list) throws IOException { if (list != null && !list.isEmpty()) { - final TextStyle.Builder builder = TextStyle.builder().setLeftPad(textStyleBuilder.getLeftPad()).setIndent(DEFAULT_LIST_INDENT); + // wrap at the configured width rather than the entry length: an entry with a line break that is shorter than the + // list indent would otherwise leave no room for its continuation lines. + final TextStyle.Builder builder = TextStyle.builder().setLeftPad(textStyleBuilder.getLeftPad()).setIndent(DEFAULT_LIST_INDENT) + .setMaxWidth(textStyleBuilder.getMaxWidth()); int i = 1; for (final CharSequence line : list) { final String entry = ordered ? String.format(" %s. %s", i++, Util.defaultValue(line, BLANK_LINE)) : String.format(" * %s", Util.defaultValue(line, BLANK_LINE)); - builder.setMaxWidth(Math.min(textStyleBuilder.getMaxWidth(), entry.length())); printQueue(makeColumnQueue(entry, builder.get())); } output.append(System.lineSeparator()); diff --git a/src/test/java/org/apache/commons/cli/help/TextHelpAppendableTest.java b/src/test/java/org/apache/commons/cli/help/TextHelpAppendableTest.java index 182cf50fc..88eb9a314 100644 --- a/src/test/java/org/apache/commons/cli/help/TextHelpAppendableTest.java +++ b/src/test/java/org/apache/commons/cli/help/TextHelpAppendableTest.java @@ -157,6 +157,21 @@ void testAppendList() throws IOException { assertEquals(expected, actual, "null list failed"); } + @Test + void testAppendListWithLineBreak() throws IOException { + // an entry shorter than the list indent used to throw and an 8 character entry used to loop until the heap was exhausted + final List expected = new ArrayList<>(); + expected.add(" * a"); + expected.add(" b"); + expected.add(" * ab"); + expected.add(" cd"); + expected.add(""); + + underTest.appendList(false, Arrays.asList("a\nb", "ab\ncd")); + final List actual = IOUtils.readLines(new StringReader(sb.toString())); + assertEquals(expected, actual); + } + @Test void testAppendParagraph() throws IOException { final String[] expected = { " Hello World", "" }; @@ -281,6 +296,10 @@ void testindexOfWrapPos() { assertThrows(IllegalArgumentException.class, () -> TextHelpAppendable.indexOfWrap("", 0, 0)); assertEquals(3, TextHelpAppendable.indexOfWrap("Hello", 4, 0)); + // a width of 1 must still consume one character, otherwise makeColumnQueue never advances + assertEquals(1, TextHelpAppendable.indexOfWrap("Hello", 1, 0), "width of 1 did not advance"); + assertEquals(3, TextHelpAppendable.indexOfWrap("Hello", 1, 2), "width of 1 did not advance past startPos"); + // startPos + width must not overflow when width is TextStyle.UNSET_MAX_WIDTH assertEquals(30, TextHelpAppendable.indexOfWrap(testString, TextStyle.UNSET_MAX_WIDTH, 0), "did not find break character with unbounded width"); assertEquals(testString.length(), TextHelpAppendable.indexOfWrap(testString, TextStyle.UNSET_MAX_WIDTH, 31), "overflow produced a negative wrap index"); @@ -357,6 +376,24 @@ void testMakeColumnQueueWithMultipleTrailingLineBreaks() { assertEquals(expected, result, "left aligned failed"); } + @Test + void testMakeColumnQueueWithWidthOfOne() { + // an indent one less than the max width leaves a usable width of 1 for the continuation lines, which used to loop forever + final String text = "hello world"; + final TextStyle.Builder styleBuilder = TextStyle.builder().setMaxWidth(5).setIndent(4).setLeftPad(0); + + final Queue expected = new LinkedList<>(); + expected.add("hello"); + expected.add(" w"); + expected.add(" o"); + expected.add(" r"); + expected.add(" l"); + expected.add(" d"); + + final Queue result = underTest.makeColumnQueue(text, styleBuilder.get()); + assertEquals(expected, result); + } + @Test void testPrintWrapped() throws IOException { String text = "The quick brown fox jumps over the lazy dog";