Dependency conflicts on commons-codec:commons-codec, leading to inconsistent program behaviors

[HelloCoCooo]

Issue description

Hi, in metrics-cloudwatch-master, there are mulptiple versions of library commons-codec:commons-codec. However, according to Maven's dependency management strategy: "first declaration wins", only commons-codec:commons-codec:1.3 can be loaded, and commons-codec:commons-codec:1.4 will be shadowed.

As shown in the following figure, your project expects to invoke method <org.apache.commons.codec.binary.Base64: decode([B)[B> in library commons-codec:commons-codec:1.4 (along the original dependency path). As it has been shadowed, this method defined in commons-codec:commons-codec:1.3 is actually forced to be referenced via the following invocation path (along the actual dependency path):

<com.plausiblelabs.metrics.reporting.InstanceIdAdder: generateJVMDimensions()Ljava/util/Collection;> /home/wwww/wangSensor/unzip/metrics-cloudwatch-master/target/classes
<com.plausiblelabs.metrics.reporting.InstanceIdAdder: fetchInstanceId()V> /home/wwww/wangSensor/unzip/metrics-cloudwatch-master/target/classes
<org.apache.http.impl.client.AbstractHttpClient: execute(Lorg/apache/http/client/methods/HttpUriRequest;)Lorg/apache/http/HttpResponse;> /home/wwww/.m2/repository/org/apache/httpcomponents/httpclient/4.1.3/httpclient-4.1.3.jar
<org.apache.http.impl.client.AbstractHttpClient: execute(Lorg/apache/http/client/methods/HttpUriRequest;Lorg/apache/http/protocol/HttpContext;)Lorg/apache/http/HttpResponse;> /home/wwww/.m2/repository/org/apache/httpcomponents/httpclient/4.1.3/httpclient-4.1.3.jar
<org.apache.http.impl.client.AbstractHttpClient: execute(Lorg/apache/http/HttpHost;Lorg/apache/http/HttpRequest;Lorg/apache/http/protocol/HttpContext;)Lorg/apache/http/HttpResponse;> /home/wwww/.m2/repository/org/apache/httpcomponents/httpclient/4.1.3/httpclient-4.1.3.jar
<org.apache.http.impl.client.DefaultRequestDirector: execute(Lorg/apache/http/HttpHost;Lorg/apache/http/HttpRequest;Lorg/apache/http/protocol/HttpContext;)Lorg/apache/http/HttpResponse;> /home/wwww/.m2/repository/org/apache/httpcomponents/httpclient/4.1.3/httpclient-4.1.3.jar
<org.apache.http.impl.client.DefaultRequestDirector: handleResponse(Lorg/apache/http/impl/client/RoutedRequest;Lorg/apache/http/HttpResponse;Lorg/apache/http/protocol/HttpContext;)Lorg/apache/http/impl/client/RoutedRequest;> /home/wwww/.m2/repository/org/apache/httpcomponents/httpclient/4.1.3/httpclient-4.1.3.jar
<org.apache.http.impl.client.DefaultRequestDirector: processChallenges(Ljava/util/Map;Lorg/apache/http/auth/AuthState;Lorg/apache/http/client/AuthenticationHandler;Lorg/apache/http/HttpResponse;Lorg/apache/http/protocol/HttpContext;)V> /home/wwww/.m2/repository/org/apache/httpcomponents/httpclient/4.1.3/httpclient-4.1.3.jar
<org.apache.http.impl.auth.AuthSchemeBase: processChallenge(Lorg/apache/http/Header;)V> /home/wwww/.m2/repository/org/apache/httpcomponents/httpclient/4.1.3/httpclient-4.1.3.jar
<org.apache.http.impl.auth.NegotiateScheme: parseChallenge(Lorg/apache/http/util/CharArrayBuffer;II)V> /home/wwww/.m2/repository/org/apache/httpcomponents/httpclient/4.1.3/httpclient-4.1.3.jar
<org.apache.commons.codec.binary.Base64: decode([B)[B>

Although both of these conflicting libraries contain the referenced methods (with the same signature), they have different implementations. This issue will not cause runtime crashes, but it can introduce inconsistent semantic program hehaviors----

Code snippet of <org.apache.commons.codec.binary.Base64: decode([B)[B> in commons-codec:commons-codec:1.4 (shadowed but expected to invoke method):

detailed method body

public byte[] decode(byte[] pArray) {
        reset();
        if (pArray == null || pArray.length == 0) {
            return pArray;
        }
        long len = (pArray.length * 3) / 4;
        byte[] buf = new byte[(int) len];
        setInitialBuffer(buf, 0, buf.length);
        decode(pArray, 0, pArray.length);
        decode(pArray, 0, -1);
        byte[] result = new byte[pos];
        readResults(result, 0, result.length);
        return result;
    }

Code snippet of <org.apache.commons.codec.binary.Base64: decode([B)[B> in commons-codec:commons-codec:1.3 (loaded version):

detailed method body

public byte[] decode(byte[] pArray) {
        return decodeBase64(pArray);
    }
public static byte[] encodeBase64(byte[] binaryData, boolean isChunked) {
        int lengthDataBits = binaryData.length * EIGHTBIT;
        int fewerThan24bits = lengthDataBits % TWENTYFOURBITGROUP;
        int numberTriplets = lengthDataBits / TWENTYFOURBITGROUP;
        byte encodedData[] = null;
        int encodedDataLength = 0;
        int nbrChunks = 0;

        if (fewerThan24bits != 0) {
            //data not divisible by 24 bit
            encodedDataLength = (numberTriplets + 1) * 4;
        } else {
            // 16 or 8 bit
            encodedDataLength = numberTriplets * 4;
        }

        // If the output is to be "chunked" into 76 character sections, 
        // for compliance with RFC 2045 MIME, then it is important to 
        // allow for extra length to account for the separator(s)
        if (isChunked) {

            nbrChunks =
                (CHUNK_SEPARATOR.length == 0 ? 0 : (int) Math.ceil((float) encodedDataLength / CHUNK_SIZE));
            encodedDataLength += nbrChunks * CHUNK_SEPARATOR.length;
        }

        encodedData = new byte[encodedDataLength];

        byte k = 0, l = 0, b1 = 0, b2 = 0, b3 = 0;

        int encodedIndex = 0;
        int dataIndex = 0;
        int i = 0;
        int nextSeparatorIndex = CHUNK_SIZE;
        int chunksSoFar = 0;

        //log.debug("number of triplets = " + numberTriplets);
        for (i = 0; i < numberTriplets; i++) {
            dataIndex = i * 3;
            b1 = binaryData[dataIndex];
            b2 = binaryData[dataIndex + 1];
            b3 = binaryData[dataIndex + 2];

            //log.debug("b1= " + b1 +", b2= " + b2 + ", b3= " + b3);

            l = (byte) (b2 & 0x0f);
            k = (byte) (b1 & 0x03);

            byte val1 =
                ((b1 & SIGN) == 0) ? (byte) (b1 >> 2) : (byte) ((b1) >> 2 ^ 0xc0);
            byte val2 =
                ((b2 & SIGN) == 0) ? (byte) (b2 >> 4) : (byte) ((b2) >> 4 ^ 0xf0);
            byte val3 =
                ((b3 & SIGN) == 0) ? (byte) (b3 >> 6) : (byte) ((b3) >> 6 ^ 0xfc);

            encodedData[encodedIndex] = lookUpBase64Alphabet[val1];
            //log.debug( "val2 = " + val2 );
            //log.debug( "k4   = " + (k<<4) );
            //log.debug(  "vak  = " + (val2 | (k<<4)) );
            encodedData[encodedIndex + 1] =
                lookUpBase64Alphabet[val2 | (k << 4)];
            encodedData[encodedIndex + 2] =
                lookUpBase64Alphabet[(l << 2) | val3];
            encodedData[encodedIndex + 3] = lookUpBase64Alphabet[b3 & 0x3f];

            encodedIndex += 4;

            // If we are chunking, let's put a chunk separator down.
            if (isChunked) {
                // this assumes that CHUNK_SIZE % 4 == 0
                if (encodedIndex == nextSeparatorIndex) {
                    System.arraycopy(
                        CHUNK_SEPARATOR,
                        0,
                        encodedData,
                        encodedIndex,
                        CHUNK_SEPARATOR.length);
                    chunksSoFar++;
                    nextSeparatorIndex =
                        (CHUNK_SIZE * (chunksSoFar + 1)) + 
                        (chunksSoFar * CHUNK_SEPARATOR.length);
                    encodedIndex += CHUNK_SEPARATOR.length;
                }
            }
        }

        // form integral number of 6-bit groups
        dataIndex = i * 3;

        if (fewerThan24bits == EIGHTBIT) {
            b1 = binaryData[dataIndex];
            k = (byte) (b1 & 0x03);
            //log.debug("b1=" + b1);
            //log.debug("b1<<2 = " + (b1>>2) );
            byte val1 =
                ((b1 & SIGN) == 0) ? (byte) (b1 >> 2) : (byte) ((b1) >> 2 ^ 0xc0);
            encodedData[encodedIndex] = lookUpBase64Alphabet[val1];
            encodedData[encodedIndex + 1] = lookUpBase64Alphabet[k << 4];
            encodedData[encodedIndex + 2] = PAD;
            encodedData[encodedIndex + 3] = PAD;
        } else if (fewerThan24bits == SIXTEENBIT) {

            b1 = binaryData[dataIndex];
            b2 = binaryData[dataIndex + 1];
            l = (byte) (b2 & 0x0f);
            k = (byte) (b1 & 0x03);

            byte val1 =
                ((b1 & SIGN) == 0) ? (byte) (b1 >> 2) : (byte) ((b1) >> 2 ^ 0xc0);
            byte val2 =
                ((b2 & SIGN) == 0) ? (byte) (b2 >> 4) : (byte) ((b2) >> 4 ^ 0xf0);

            encodedData[encodedIndex] = lookUpBase64Alphabet[val1];
            encodedData[encodedIndex + 1] =
                lookUpBase64Alphabet[val2 | (k << 4)];
            encodedData[encodedIndex + 2] = lookUpBase64Alphabet[l << 2];
            encodedData[encodedIndex + 3] = PAD;
        }

        if (isChunked) {
            // we also add a separator to the end of the final chunk.
            if (chunksSoFar < nbrChunks) {
                System.arraycopy(
                    CHUNK_SEPARATOR,
                    0,
                    encodedData,
                    encodedDataLength - CHUNK_SEPARATOR.length,
                    CHUNK_SEPARATOR.length);
            }
        }

        return encodedData;
    }

Dependency tree--

[INFO] com.plausiblelabs.metrics:metrics-cloudwatch:jar:2.1.2.3-SNAPSHOT
[INFO] +- com.yammer.metrics:metrics-core:jar:2.1.2:compile
[INFO] | - (org.slf4j:slf4j-api:jar:1.6.4:compile - omitted for duplicate)
[INFO] +- com.amazonaws:aws-java-sdk:jar:1.3.14:compile
[INFO] | +- commons-logging:commons-logging:jar:1.1.1:compile
[INFO] | +- (org.apache.httpcomponents:httpclient:jar:4.1:compile - omitted for conflict with 4.1.3)
[INFO] | +- commons-codec:commons-codec:jar:1.3:compile
[INFO] | +- org.codehaus.jackson:jackson-core-asl:jar:1.8.9:compile
[INFO] | - org.codehaus.jackson:jackson-mapper-asl:jar:1.8.9:compile
[INFO] | - (org.codehaus.jackson:jackson-core-asl:jar:1.8.9:compile - omitted for duplicate)
[INFO] +- org.slf4j:slf4j-api:jar:1.6.4:compile
[INFO] +- org.apache.httpcomponents:httpclient:jar:4.1.3:compile
[INFO] | +- org.apache.httpcomponents:httpcore:jar:4.1.4:compile
[INFO] | +- (commons-logging:commons-logging:jar:1.1.1:compile - omitted for duplicate)
[INFO] | - (commons-codec:commons-codec:jar:1.4:compile - omitted for conflict with 1.3)
[INFO] +- org.slf4j:slf4j-jdk14:jar:1.6.4:test
[INFO] | - (org.slf4j:slf4j-api:jar:1.6.4:test - omitted for duplicate)
[INFO] +- com.yammer.metrics:metrics-core:test-jar:tests:2.1.2:test
[INFO] | - (org.slf4j:slf4j-api:jar:1.6.4:test - omitted for duplicate)
[INFO] +- junit:junit:jar:4.4:test
[INFO] - com.google.guava:guava:jar:11.0.2:test
[INFO] - com.google.code.findbugs:jsr305:jar:1.3.9:test

Suggested solutions:

Solution1: Remove the conflicting Jars.

Solution2: Declare version commons-codec:commons-codec:1.4 as a direct dependency, to override the version 1.3 (based on Maven's nearest wins loading strategy).

Solution3: reversing the declaration order of these two libraries in pom file.

Thanks.
Best regards,
Coco

[HelloCoCooo]

Executing the following test case on commons-codec:commons-codec:1.3 and 1.4 separately, the risky method <org.apache.commons.codec.binary.Base64: decode([B)[B> will get different return values:

@Test(timeout = 4000)
    public void test01()  throws Throwable  {
        Base64 base64 = new Base64();
        byte[] byteArray0 = new byte[]{(byte) 53, (byte) 67, (byte) 83};
        byte[] byteArray1 = base64.decode(byteArray0);
        assertEquals(3, byteArray1.length);
    }

Output results:

byteArray1.length == 3   //On **commons-codec:commons-codec:1.3**

byteArray1.length == 2    //On **commons-codec:commons-codec:1.4**

Variable token (defined in class org.apache.http.impl.auth.NegotiateScheme of library org.apache.httpcomponents:httpclient:jar:4.1.3) is assigned by the return value of method <org.apache.commons.codec.binary.Base64: decode([B)[B>.

As such, token's value would be changed when the client project references commons-codec:commons-codec:1.3 (compared with the shadowed but expected version 1.4), which could affect program semantic behaviors. 

/** code snippet of class org.apache.http.impl.auth.NegotiateScheme of library org.apache.httpcomponents:httpclient:jar:4.1.3 **/
    private byte[] token;
		@Override
    protected void parseChallenge(
            final CharArrayBuffer buffer,
            int beginIndex, int endIndex) throws MalformedChallengeException {
        String challenge = buffer.substringTrimmed(beginIndex, endIndex);
        if (log.isDebugEnabled()) {
            log.debug("Received challenge '" + challenge + "' from the auth server");
        }
        if (state == State.UNINITIATED) {
            token = new Base64().decode(challenge.getBytes());
            state = State.CHALLENGE_RECEIVED;
        } else {
            log.debug("Authentication already attempted");
            state = State.FAILED;
        }
    }

[HelloCoCooo]

@groves Could please help me check this issue?
May I pull a request to fix it?
Thanks again.