Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

String.replace() errornous compares and replaces one byte instead of two bytes #19903

Closed
bmarwell opened this issue Jul 23, 2024 · 5 comments
Closed
Assignees
Labels

Comments

@bmarwell
Copy link

Hello,

given this sample program:

    import java.util.regex.Pattern;
    
    final class Scratch {
      private static final String LEFT_BRACKET = "\u1161";
      private static final Pattern LEFT_BRACKET_PATTERN = Pattern.compile(LEFT_BRACKET, Pattern.LITERAL);
    
       private static final String TEST_STRING = "Max Mustermann / normal Customer / not relevant";
    
      public static void main(final String[] args) {
        final String replaced = TEST_STRING.replaceAll(LEFT_BRACKET, "[");
        // JDK 22 prints: Max Mustermann / normal Customer / not relevant
        // 11.0.17-tem prints: Max Mustermann / normal Customer / not relevant
        // 11.0.18-sem prints: Max Mustermann / normal Customer / not relevant
        // 11.0.22-sem prints: M[x Musterm[nn / norm[l Customer / not relev[nt
        // 11.0.23-sem prints: M[x Musterm[nn / norm[l Customer / not relev[nt
        System.out.println(replaced);
    
        final String replacedWithPattern = LEFT_BRACKET_PATTERN.matcher(replaced).replaceAll("[");
        // JDK 22 prints: Max Mustermann / normal Customer / not relevant
        // 11.0.17-tem prints: Max Mustermann / normal Customer / not relevant
        // 11.0.18-sem prints: Max Mustermann / normal Customer / not relevant
        // 11.0.22-sem prints: M[x Musterm[nn / norm[l Customer / not relev[nt
        // 11.0.23-sem prints: M[x Musterm[nn / norm[l Customer / not relev[nt
        System.out.println(replacedWithPattern);
      }
    }

Our developer expects this program to output a copy of the string, but in fact, all "a" characters get replaced. This is highly unexpected and dangerous.

Also confirmed with JDK11 build 20240723-095802 on Mac from OpenJ9 jenkins.

Case number: TS016798420

@bmarwell
Copy link
Author

Workaround: -XX:-CompactStrings

pshipton added a commit to pshipton/openj9 that referenced this issue Jul 23, 2024
@bmarwell
Copy link
Author

Does not occur with this input (added brackets):

TEST_STRING.replaceAll( "[\u1161]", "[" );

@pshipton pshipton self-assigned this Jul 23, 2024
pshipton added a commit to pshipton/openj9 that referenced this issue Jul 23, 2024
Also, return the original String in the fast paths when nothing is
replaced.

Issue eclipse-openj9#19903

Signed-off-by: Peter Shipton <[email protected]>
pshipton added a commit to pshipton/openj9 that referenced this issue Jul 23, 2024
Also, return the original String in the fast paths when nothing is
replaced.

Issue eclipse-openj9#19903

Signed-off-by: Peter Shipton <[email protected]>
pshipton added a commit to pshipton/openj9 that referenced this issue Jul 23, 2024
Also, return the original String in the fast paths when nothing is
replaced.

Issue eclipse-openj9#19903

Signed-off-by: Peter Shipton <[email protected]>
pshipton added a commit to pshipton/openj9 that referenced this issue Jul 23, 2024
Also, return the original String in the fast paths when nothing is
replaced.

Issue eclipse-openj9#19903

Signed-off-by: Peter Shipton <[email protected]>
pshipton added a commit to pshipton/openj9 that referenced this issue Jul 23, 2024
Also, return the original String in the fast paths when nothing is
replaced.

Issue eclipse-openj9#19903

Signed-off-by: Peter Shipton <[email protected]>
@keithc-ca
Copy link
Contributor

I expect the test intended to start with the same string for both paths:

        final String replacedWithPattern = LEFT_BRACKET_PATTERN.matcher(TEST_STRING).replaceAll("[");

The difference is that it shows that Pattern.Matcher.replaceAll() behaves as expected.

pshipton added a commit to pshipton/openj9 that referenced this issue Jul 23, 2024
Also, return the original String in the fast paths when nothing is
replaced.

Issue eclipse-openj9#19903

Signed-off-by: Peter Shipton <[email protected]>
pshipton added a commit to pshipton/openj9 that referenced this issue Jul 23, 2024
Also, return the original String in the fast paths when nothing is
replaced.

Issue eclipse-openj9#19903

Signed-off-by: Peter Shipton <[email protected]>
@pshipton
Copy link
Member

The fixes to head, 0.46, 0.47 are merged.

@bmarwell
Copy link
Author

Thank you so much for this quick fix!!! 👍🏻

tajila pushed a commit to tajila/openj9 that referenced this issue Jul 30, 2024
Also, return the original String in the fast paths when nothing is
replaced.

Issue eclipse-openj9#19903

Signed-off-by: Peter Shipton <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants