The constructor of com.bazaarvoice.jolt.common.pathelement.StarDoublePathElement class could throw an unexpected ArrayIndexOutOfBoundsException

Bug description

    public StarDoublePathElement(String key) {
        super(key);

        if ( StringTools.countMatches(key, "*") != 2 ) {
            throw new IllegalArgumentException( "StarDoublePathElement should have two '*' in its key. Was: " + key );
        }

        String[] split = key.split("\\*");
        boolean startsWithStar = key.startsWith( "*" );
        boolean endsWithStar = key.endsWith("*");
        if (  startsWithStar && endsWithStar) {
            prefix = "";
            mid = split[1];
            suffix = "";
        }
        else if ( endsWithStar ) {
            prefix = split[0];
            mid = split[1];
            suffix = "";
        }
        else if ( startsWithStar ) {
            prefix = "";
            mid = split[1];
            suffix = split[2];
        }
        else{
            prefix=split[0];
            mid=split[1];
            suffix=split[2];
        }
    }

It is found that the constructor of com.bazaarvoice.jolt.common.pathelement.StarDoublePathElement class could throw an unexpected ArrayIndexOutOfBoundsException if an invalid key string is provided. This happened because of a wrong assumption of the String::split(String) method used in the constructor. According to JDK Javadoc in https://docs.oracle.com/javase/8/docs/api/java/lang/String.html#split-java.lang.String-, the calling to the split method without providing the limit is equal to setting the limit to 0. When the limit is set to 0, all trailing empty strings are not included in the resulting array. For example, calling "**".split("*"); results in an empty String array and calling "F*".split(""); results in an array with a single element "F".

This wrong assumption of the string splitting makes the following access to the split array throw an unexpected ArrayIndexOutOfBoundsException.

For example, passing ** to the constructor passes the checking of double stars and both startsWithStar and endsWithStar will be true, but the String[] split will be an empty array and thus the call to split[1] later will throw an unexpected ArrayIndexOutOfBoundsException. Alternatively, passing F** will have a similar effect but it is triggered in different sections of the conditional branches since startsWithStar is false and endsWithStar is true for this string.

Proof of concept

Just compile any of the following Java code and run it could trigger the bug.

import com.bazaarvoice.jolt.common.pathelement.StarDoublePathElement;

public class ProofOfConcept {
  public static void main(String...args) {
    new StarDoublePathElement("**");
  }
}

import com.bazaarvoice.jolt.common.pathelement.StarDoublePathElement;

public class ProofOfConcept {
  public static void main(String...args) {
    new StarDoublePathElement("F**");
  }
}

Suggested fix

Instead of using String::split(String), use String::split(String, int) and provide a needed limit to ensure enough items exist in the resulting array. Alternatively, an additional check could be added to ensure a malformed string is denied.

bazaarvoice / jolt