[GitHub] [commons-text] nickbabcock commented on issue #115: Fix length change omission for StringSubstitutor cyclic detection

[GitBox <gi...@apache.org>]

nickbabcock commented on issue #115:  Fix length change omission for StringSubstitutor cyclic detection 
URL: https://github.com/apache/commons-text/pull/115#issuecomment-501524100
 
 
   My apologies!
   
   1. Checkout `master` branch
   2. Apply diff
   
   ```diff
   diff --git a/src/main/java/org/apache/commons/text/StringSubstitutor.java b/src/main/java/org/apache/commons/text/StringSubstitutor.java
   index f4b53ee..4680edc 100644
   --- a/src/main/java/org/apache/commons/text/StringSubstitutor.java
   +++ b/src/main/java/org/apache/commons/text/StringSubstitutor.java
   @@ -1330,7 +1330,7 @@ public class StringSubstitutor {
            final boolean top = priorVariables == null;
            boolean altered = false;
            int lengthChange = 0;
   -        char[] chars = buf.buffer;
   +        char[] chars = buf.toCharArray();
            int bufEnd = offset + length;
            int pos = offset;
            while (pos < bufEnd) {
   @@ -1346,7 +1346,7 @@ public class StringSubstitutor {
                            continue;
                        }
                        buf.deleteCharAt(pos - 1);
   -                    chars = buf.buffer; // in case buffer was altered
   +                    chars = buf.toCharArray(); // in case buffer was altered
                        lengthChange--;
                        altered = true;
                        bufEnd--;
   @@ -1432,7 +1432,7 @@ public class StringSubstitutor {
                                        pos += change;
                                        bufEnd += change;
                                        lengthChange += change;
   -                                    chars = buf.buffer; // in case buffer was altered
   +                                    chars = buf.toCharArray(); // in case buffer was altered
                                    } else if (undefinedVariableException) {
                                        throw new IllegalArgumentException(String.format(
                                                "Cannot resolve variable '%s' (enableSubstitutionInVariables=%s).", varName,
   ```
   
   3. Run tests
   4. Notice `StringIndexOutOfBoundsException` is thrown at
   
   ```java
   if (priorVariables == null) {
       priorVariables = new ArrayList<>();
       priorVariables.add(new String(chars, offset, length));
   }
   ```
   
   5. Understand "length" points passed the character array but since `chars` is implicitly relying on `TextStringBuilder` allocating behavior so the current implementation does not throw exception.
   6. The real length is `length - lengthChange`

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services