Skip to content

Commit 4a6a984

Browse files
authored
StringUtils.abbreviate(String, String, int) contract violations (#1572)
* add unit tests showing mishandling of null abbrevMarker * fix handling of null abbrevMarker - treat null marker as empty string - ensure offset and maxWidth are applied as usual (simple 'substring' won't cut it) * add unit tests showing 'abbreviate' contract violations - Abbreviated strings should always retain the 'offset' character * remove surplus blank lines from tests * fix StringUtils.abbreviate for short strings Adjust logic so it doesn't overwrite the offset value and get confused about which character to preserve. Instead, just return immediately if the offset is so close to the end that no ending abbrevMarker is possible.
1 parent e23f953 commit 4a6a984

2 files changed

Lines changed: 8 additions & 9 deletions

File tree

src/main/java/org/apache/commons/lang3/StringUtils.java

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -369,22 +369,16 @@ public static String abbreviate(final String str, String abbrevMarker, int offse
369369
if (strLen <= maxWidth) {
370370
return str;
371371
}
372-
if (offset > strLen) {
373-
offset = strLen;
374-
}
375-
if (strLen - offset < maxWidth - abbrevMarkerLength) {
376-
offset = strLen - (maxWidth - abbrevMarkerLength);
372+
if (strLen - offset <= maxWidth - abbrevMarkerLength) {
373+
return abbrevMarker + str.substring(strLen - (maxWidth - abbrevMarkerLength));
377374
}
378375
if (offset <= abbrevMarkerLength + 1) {
379376
return str.substring(0, maxWidth - abbrevMarkerLength) + abbrevMarker;
380377
}
381378
if (maxWidth < minAbbrevWidthOffset) {
382379
throw new IllegalArgumentException(String.format("Minimum abbreviation width with offset is %d", minAbbrevWidthOffset));
383380
}
384-
if (offset + maxWidth - abbrevMarkerLength < strLen) {
385-
return abbrevMarker + abbreviate(str.substring(offset), abbrevMarker, maxWidth - abbrevMarkerLength);
386-
}
387-
return abbrevMarker + str.substring(strLen - (maxWidth - abbrevMarkerLength));
381+
return abbrevMarker + abbreviate(str.substring(offset), abbrevMarker, maxWidth - abbrevMarkerLength);
388382
}
389383

390384
/**

src/test/java/org/apache/commons/lang3/StringUtilsAbbreviateTest.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,8 @@ void testAbbreviate_StringIntInt() {
101101
assertAbbreviateWithOffset("...ijklmno", 15, 10);
102102
assertAbbreviateWithOffset("...ijklmno", 16, 10);
103103
assertAbbreviateWithOffset("...ijklmno", Integer.MAX_VALUE, 10);
104+
// abbreviating a shorter string allows maxWidth < 7
105+
assertEquals("...efg", StringUtils.abbreviate("abcdefg", 5, 6));
104106
}
105107

106108
@Test
@@ -163,6 +165,9 @@ void testAbbreviate_StringStringIntInt() {
163165
assertAbbreviateWithAbbrevMarkerAndOffset("999ijklmno", "999", 15, 10);
164166
assertAbbreviateWithAbbrevMarkerAndOffset("_ghijklmno", "_", 16, 10);
165167
assertAbbreviateWithAbbrevMarkerAndOffset("+ghijklmno", "+", Integer.MAX_VALUE, 10);
168+
// abbreviating a shorter string allows maxWidth < abbrevMarker.length * 2 + 1
169+
assertEquals("..de", StringUtils.abbreviate("abcde", "..", 4, 4));
170+
assertEquals("....fg", StringUtils.abbreviate("abcdefg", "....", 5, 6));
166171
}
167172

168173
// Fixed LANG-1463

0 commit comments

Comments
 (0)