DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT <http://nagoya.apache.org/bugzilla/show_bug.cgi?id=16204>. ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND INSERTED IN THE BUG DATABASE.
http://nagoya.apache.org/bugzilla/show_bug.cgi?id=16204 Infinite loop in StringUtils.replace(text, repl, with) + FIX Summary: Infinite loop in StringUtils.replace(text, repl, with) + FIX Product: Commons Version: 1.0 Alpha Platform: PC OS/Version: Linux Status: NEW Severity: Normal Priority: Other Component: Lang AssignedTo: [EMAIL PROTECTED] ReportedBy: [EMAIL PROTECTED] Hopefully I can give back a little after having had so much benefit of your work. I will refer to the sources from commons-lang-1.0.1-src.zip, downloaded the 17 January 2003. Detail: ======= In org.apache.commons.lang.StringUtils: If you invoke public static String replace(String text, String repl, String with) with arguments: text != null repl.equals("") with anything you get an infinite loop as "FOO".indexOf("") == 0. Demo: ===== To demonstrate the bug, please add the following lines in org.apache.commons.lang.StringUtilsTest in the body of testReplaceFunctions(), line 194: public void testReplaceFunctions() { //... existing code //-- bug demonstration, added by HoKr assertEquals("replace(String, String, String) failed", "FOO", StringUtils.replace("FOO", "", "any")); } I got an OutOfMemoryException then. Fix: ==== My suggestion to fix this in StringUtils.replace(String, String, String), line 593: public static String replace(String text, String repl, String with, int max) { if (text == null) { return null; } //-- FIX SUGGESTION START >>> //-- added by HoKr for infinite loop avoidance //-- keeps on throwing NullPointerException if repl == null //-- -->> this is faster than "".equals(repl); NPE allowed. if (repl.length() == 0) { return text; } //-- <<< FIX SUGGESTION END StringBuffer buf = new StringBuffer(text.length()); int start = 0, end = 0; while ((end = text.indexOf(repl, start)) != -1) { buf.append(text.substring(start, end)).append(with); start = end + repl.length(); if (--max == 0) { break; } } buf.append(text.substring(start)); return buf.toString(); } Further: ======== Further I suggest instead of throwing NullPointerExceptions if (repl == null || with == null) to return the parameter text then. It would meet closer the expectation of what the method should perform from my point of view in these cases. This behaviour would be payed with 2 extra comparisons to null (before the while-loop) in 'normal' operation mode though. The Code would be: public static String replace(String text, String repl, String with, int max) { if (text == null) { return null; } //-- START >>> //-- suggestion by HoKr, BUT would CHANGE outside behaviour: //-- not throwing NPE any more! if (repl == null || with == null) { return text; } //-- added by HoKr for infinite loop avoidance //-- keeps on throwing NullPointerException if repl == null if (repl.length() == 0) { return text; } //-- <<< END StringBuffer buf = new StringBuffer(text.length()); int start = 0, end = 0; while ((end = text.indexOf(repl, start)) != -1) { buf.append(text.substring(start, end)).append(with); start = end + repl.length(); if (--max == 0) { break; } } buf.append(text.substring(start)); return buf.toString(); } Regards, Holger Krauth -- To unsubscribe, e-mail: <mailto:[EMAIL PROTECTED]> For additional commands, e-mail: <mailto:[EMAIL PROTECTED]>