java - Android Bug? : String.substring(5).replace(“”, “”) // empty string

Question

Here is my code:

String str = "just_a_string";
System.out.println("]" + str + "[");
System.out.println("]" + str.replace("", "") + "[");
System.out.println("]" + str.substring(5) + "[");
System.out.println("]" + str.substring(5).replace("", "") + "[");
System.out.println("]" + str.substring(3, 8) + "[");
System.out.println("]" + str.substring(3, 8).replace("", "") + "[");
System.out.println("]" + "sdajndan".substring(5).replace("", "") + "[");

and here is the output

05-09 19:09:20.570: I/System.out(23801): ]just_a_string[
05-09 19:09:20.570: I/System.out(23801): ]just_a_string[
05-09 19:09:20.570: I/System.out(23801): ]a_string[
05-09 19:09:20.570: I/System.out(23801): ]a_s[      **
05-09 19:09:20.570: I/System.out(23801): ]t_a_s[
05-09 19:09:20.570: I/System.out(23801): ]t_[       **
05-09 19:09:20.570: I/System.out(23801): ][         **

Obviously, the lines marked with ** are unexpected.

This issue happens to my Android phone A (LG P920 Optimus 3D, Android 2.3.3). While i test on my Android phone B (LG E720 Optimus Chic, Android 2.2), it halts. i guess it runs into an infinite loop.

i have tested on both phones, with Java 1.5 and 1.6. Both result in the same behaviour respectively.

i have also tested on my same Eclipse with a Java project, for 1.5, 1.6, and 1.7. All of their outputs are correct, as expected.

i wonder this might be a device specific issue of implementing String.replace(“”, “”) against the String's backing array.

Could you please help me to test in your devices?

Could anyone please provide me the Android source code of the String.replace(CharSequence, CharSequence) method? (like what in docjar)

Thanks a lot!

---

i have modified the code a bit, so it can also display on the Android device. (It is just the same code anyway).

Tested on both my phone A and phone B. Behaviours are still the same, as mentioned above.

package com.example.testprojectnew;

import android.app.Activity;
import android.os.Bundle;
import android.widget.TextView;

public class MainActivity extends Activity {

    String output_text = "";

    @Override
    protected void onCreate(Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);
        setContentView(R.layout.activity_main);

        String str = "just_a_string";
        process("1]" + str + "[");
        process("2]" + str.replace("", "") + "[");
        process("3]" + str.substring(5) + "[");
        process("4]" + str.substring(5).replace("", "") + "[");
        process("5]" + str.substring(3, 8) + "[");
        process("6]" + str.substring(3, 8).replace("", "") + "[");
        process("7]" + "sdajndan".substring(5).replace("", "") + "[");

        output_text = output_text.concat("

Lines (1 & 2), (3 & 4), (5 & 6), should be the same.");

        ((TextView) findViewById(R.id.a_string)).setText(output_text);
    }
    private void process(String str) {
        System.out.println(str);
        output_text = output_text.concat(str).concat("
");
    }
}

Answer 1

Bingo! i have found the bug!

Thanks @izht for providing the link of the source code. i have located the bug regarding this problem.

This only happens when the String's backing array is having a different (longer) value than the actual String. In particular, when the String.offset (private variable) is larger than zero.

Here's the fix:

public String replace(CharSequence target, CharSequence replacement) {
    if (target == null) {
        throw new NullPointerException("target == null");
    }
    if (replacement == null) {
        throw new NullPointerException("replacement == null");
    }

    String targetString = target.toString();
    int matchStart = indexOf(targetString, 0);
    if (matchStart == -1) {
        // If there's nothing to replace, return the original string untouched.
        return this;
    }

    String replacementString = replacement.toString();

    // The empty target matches at the start and end and between each character.
    int targetLength = targetString.length();
    if (targetLength == 0) {
        int resultLength = (count + 2) * replacementString.length();
        StringBuilder result = new StringBuilder(resultLength);
        result.append(replacementString);
//        for (int i = offset; i < count; ++i) {             // original, bug
        for (int i = offset; i < (count + offset); ++i) {    // fix
            result.append(value[i]);
            result.append(replacementString);
        }
        return result.toString();
    }

    StringBuilder result = new StringBuilder(count);
    int searchStart = 0;
    do {
        // Copy characters before the match...
        result.append(value, offset + searchStart, matchStart - searchStart);
        // Insert the replacement...
        result.append(replacementString);
        // And skip over the match...
        searchStart = matchStart + targetLength;
    } while ((matchStart = indexOf(targetString, searchStart)) != -1);
    // Copy any trailing chars...
    result.append(value, offset + searchStart, count - searchStart);
    return result.toString();
}

i am not sure why Android has to alter (and altered wrongly) the replace() in this way. The original Java implementation doesn't have this issue.

By-the-way, what's now? What can i do with it? (other than using replace() with extra care, or throw away my Android phones :-/)

---

Btw i m quite sure my LG E720 Optimus Chic (Android 2.2) is using a different source code than that one. It keeps halting (suspect infinite looping) upon String.replace() with an empty target string. Lately i found it throws this error message:

05-10 16:41:13.155: E/AndroidRuntime(9384): FATAL EXCEPTION: main
05-10 16:41:13.155: E/AndroidRuntime(9384): java.lang.OutOfMemoryError
05-10 16:41:13.155: E/AndroidRuntime(9384):   at java.lang.AbstractStringBuilder.enlargeBuffer(AbstractStringBuilder.java:97)
05-10 16:41:13.155: E/AndroidRuntime(9384):   at java.lang.AbstractStringBuilder.append0(AbstractStringBuilder.java:157)
05-10 16:41:13.155: E/AndroidRuntime(9384):   at java.lang.StringBuilder.append(StringBuilder.java:217)
05-10 16:41:13.155: E/AndroidRuntime(9384):   at java.lang.String.replace(String.java:1497)
05-10 16:41:13.155: E/AndroidRuntime(9384):   at com.example.testprojectnew.MainActivity.onCreate(MainActivity.java:22)
05-10 16:41:13.155: E/AndroidRuntime(9384):   at android.app.Instrumentation.callActivityOnCreate(Instrumentation.java:1047)
05-10 16:41:13.155: E/AndroidRuntime(9384):   at android.app.ActivityThread.performLaunchActivity(ActivityThread.java:2627)
05-10 16:41:13.155: E/AndroidRuntime(9384):   at android.app.ActivityThread.handleLaunchActivity(ActivityThread.java:2679)
05-10 16:41:13.155: E/AndroidRuntime(9384):   at android.app.ActivityThread.access$2300(ActivityThread.java:125)
05-10 16:41:13.155: E/AndroidRuntime(9384):   at android.app.ActivityThread$H.handleMessage(ActivityThread.java:2033)
05-10 16:41:13.155: E/AndroidRuntime(9384):   at android.os.Handler.dispatchMessage(Handler.java:99)
05-10 16:41:13.155: E/AndroidRuntime(9384):   at android.os.Looper.loop(Looper.java:123)
05-10 16:41:13.155: E/AndroidRuntime(9384):   at android.app.ActivityThread.main(ActivityThread.java:4627)
05-10 16:41:13.155: E/AndroidRuntime(9384):   at java.lang.reflect.Method.invokeNative(Native Method)
05-10 16:41:13.155: E/AndroidRuntime(9384):   at java.lang.reflect.Method.invoke(Method.java:521)
05-10 16:41:13.155: E/AndroidRuntime(9384):   at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:878)
05-10 16:41:13.155: E/AndroidRuntime(9384):   at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:636)
05-10 16:41:13.155: E/AndroidRuntime(9384):   at dalvik.system.NativeStart.main(Native Method)

---

At a second thought, if that for-loop thingy is the bug. It should be a compile time issue. Why would it act differently in different phones (different versions of Android)?

---

Complete Workaround

Got an update from Google, that they have patched it, and will correct it in the future release.

Meanwhile, i have written a patched method, based on their code:

(This is necessary because (1) we still have to wait for the correct release, (2) we need to take care of devices that didnt make that fixed update)

/** Patch for the String.replace(CharSequence target, CharSequence replacement),
 *  because the original is buggy when CharSequence target is empty, i.e. "".
 *  Patched by Google Android: https://android-review.googlesource.com/58393
 */
public static String replacePatched(final String string, final CharSequence target, final CharSequence replacement) {
    if (target == null) {
        throw new NullPointerException("target == null");
    }
    if (replacement == null) {
        throw new NullPointerException("replacement == null");
    }

    final String targetString = target.toString();
    int matchStart = string.indexOf(targetString, 0);
    if (matchStart == -1) {
        // If there's nothing to replace, return the original string untouched.
        return new String(string);
    }

    final char[] value = string.toCharArray();                              // required in patch
    final int count = value.length;                                         // required in patch

    final String replacementString = replacement.toString();

    // The empty target matches at the start and end and between each character.
    if (targetString.length() == 0) {
        // The result contains the original 'count' characters, a copy of the
        // replacement string before every one of those characters, and a final
        // copy of the replacement string at the end.
        final StringBuilder result = new StringBuilder(count + (count + 1) * replacementString.length());
        result.append(replacementString);
        for (int i = 0; i < count; ++i) {
            result.append(value[i]);
            result.append(replacementString);
        }
        return new String(result);      // StringBuilder.toString() does not give exact length
    }

    final StringBuilder result = new StringBuilder(count);
    int searchStart = 0;
    do {
        // Copy characters before the match...
        result.append(value, searchStart, matchStart - searchStart);
        // Insert the replacement...
        result.append(replacementString);
        // And skip over the match...
        searchStart = matchStart + targetString.length();
    } while ((matchStart = string.indexOf(targetString, searchStart)) != -1);
    // Copy any trailing chars...
    result.append(value, searchStart, count - searchStart);
    return new String(result);          // StringBuilder.toString() does not give exact length
}

The verbose version:

/** Patch for the String.replace(CharSequence target, CharSequence replacement),
 *  because the original is buggy when CharSequence target is empty, i.e. "".
 *  Patched by Google Android: https://android-review.googlesource.com/58393
 */
public static String replacePatched(final String string, final CharSequence target, final CharSequence replacement) {
    if (target == null) {
        throw new NullPointerException("target == null");
    }
    if (replacement == null) {
        throw new NullPointerException("replacement == null");
    }

//    String targetString = target.toString();                                    // original
    final String targetString = target.toString();
//    int matchStart = indexOf(targetString, 0);                                  // original
    int matchStart = string.indexOf(targetString, 0);
    if (matchStart == -1) {
        // If there's nothing to replace, return the original string untouched.
//        return this;                                                            // original
        return new String(string);
    }

    final char[] value = string.toCharArray();                              // required in patch
    final int count = value.length;                                         // required in patch

//    String replacementString = replacement.toString();                          // original
    final String replacementString = replacement.toString();

    // The empty target matches at the start and end and between each character.
//    int targetLength = targetString.length();                                   // original
//    if (targetLength == 0) {                                                    // original
    if (targetString.length() == 0) {
//        int resultLength = (count + 2) * replacementString.length();            // original
//        // The result contains the original 'count' characters, a copy of the
//        // replacement string before every one of those characters, and a final
//        // copy of the replacement string at the end.
//        int resultLength = count + (count + 1) * replacementString.length();    // patched by Google Android
//        StringBuilder result = new StringBuilder(resultLength);                 // original
        final StringBuilder result = new StringBuilder(count + (count + 1) * replacementString.length());
        result.append(replacementString);
//        for (int i = offset; i < count; ++i) {                                  // original
//        int end = offset + count;                                               // patched by Google Android
//        for (int i = offset; i != end; ++i) {                                   // patched by Google Android
        for (int i = 0; i < count; ++i) {
            result.append(value[i]);
            result.append(replacementString);
        }
//        return result.toString();                                               // original
        return new String(result);      // StringBuilder.toString() does not give exact length
    }

//    StringBuilder result = new StringBuilder(count);