You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by "ParanoidUser (via GitHub)" <gi...@apache.org> on 2024/10/15 02:16:43 UTC
[PR] [LANG-1753] StringUtils.replaceEachRepeatedly regression in 3.11+ [commons-lang]
ParanoidUser opened a new pull request, #1297:
URL: https://github.com/apache/commons-lang/pull/1297
https://issues.apache.org/jira/browse/LANG-1753
The method `replaceEachRepeatedly` calls `replaceEach` repeatedly until no more replacements are possible or `timeToLive < 0`. The `timeToLive` starts with the value of `searchList.length`. The current implementation can't distinguish between infinite loops and deeply nested string patterns. In both cases, it continues replacements until it hits the time-to-live limit and throws `IllegalStateException` if replacements are still possible.
The problem with [LANG-1528](https://issues.apache.org/jira/browse/LANG-1528) fix is that in some cases, it allows partially replaced strings after TTL is exceeded. This fix reverts `replaceEach` logic back to version 3.10 (see PR #505), but allows deeper dives due to an increased initial TTL value. The initial TTL value is now set to the greater of `searchList.length` or up to 5 levels deep. This default value was chosen based on rational considerations.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [LANG-1753] StringUtils.replaceEachRepeatedly regression in 3.11+ [commons-lang]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory merged PR #1297:
URL: https://github.com/apache/commons-lang/pull/1297
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [LANG-1753] StringUtils.replaceEachRepeatedly regression in 3.11+ [commons-lang]
Posted by "palacsint (via GitHub)" <gi...@apache.org>.
palacsint commented on PR #1297:
URL: https://github.com/apache/commons-lang/pull/1297#issuecomment-2413491851
Thanks for the fix! Seems perfect for us.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [LANG-1753] StringUtils.replaceEachRepeatedly regression in 3.11+ [commons-lang]
Posted by "palacsint (via GitHub)" <gi...@apache.org>.
palacsint commented on code in PR #1297:
URL: https://github.com/apache/commons-lang/pull/1297#discussion_r1800869219
##########
src/test/java/org/apache/commons/lang3/StringUtilsTest.java:
##########
@@ -1990,7 +1990,53 @@ public void testReplace_StringStringArrayStringArrayBoolean() {
assertEquals("aba", StringUtils.replaceEachRepeatedly("aba", new String[]{null}, new String[]{"a"}));
assertEquals("wcte", StringUtils.replaceEachRepeatedly("abcde", new String[]{"ab", "d"}, new String[]{"w", "t"}));
assertEquals("tcte", StringUtils.replaceEachRepeatedly("abcde", new String[]{"ab", "d"}, new String[]{"d", "t"}));
- assertEquals("blaan", StringUtils.replaceEachRepeatedly("blllaan", new String[]{"llaan"}, new String[]{"laan"}) );
+
+ // Test recursive replacement - LANG-1528 & LANG-1753
+ assertEquals("blaan", StringUtils.replaceEachRepeatedly("blllaan", new String[]{"llaan"}, new String[]{"laan"}));
+ assertEquals("blaan", StringUtils.replaceEachRepeatedly("bllllaan", new String[]{"llaan"}, new String[]{"laan"}));
+
+ // Test default TTL for smaller search lists. 32 characters reduced to 16, then 8, 4, 2, 1.
+ assertEquals("a", StringUtils.replaceEachRepeatedly("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
+ new String[]{"aa"}, new String[]{"a"}));
Review Comment:
I would suggest adding an example of `IllegalStateException` with a shorter input, like:
assertThrows(
IllegalStateException.class,
() -> StringUtils.replaceEachRepeatedly("aaaaaab", new String[] {"ab"}, new String[] {"b"}),
"Should be a circular reference");
It might surprise newcomers that even far fewer than 32 repetitions can trigger an `IllegalStateException`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [LANG-1753] StringUtils.replaceEachRepeatedly regression in 3.11+ [commons-lang]
Posted by "palacsint (via GitHub)" <gi...@apache.org>.
palacsint commented on code in PR #1297:
URL: https://github.com/apache/commons-lang/pull/1297#discussion_r1800860645
##########
src/main/java/org/apache/commons/lang3/StringUtils.java:
##########
@@ -6611,7 +6608,8 @@ private static String replaceEach(
* @since 2.4
*/
public static String replaceEachRepeatedly(final String text, final String[] searchList, final String[] replacementList) {
- return replaceEach(text, searchList, replacementList, true, ArrayUtils.getLength(searchList));
+ int timeToLive = Math.max(ArrayUtils.getLength(searchList), DEFAULT_TTL);
Review Comment:
The Javadoc only mentions endless loops ("if the search is repeating and there is an endless loop"). It would be clearer if it also included a mention of the TTL to avoid confusion.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org