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

ArrayIndexOutOfBoundsException when using @Tested (was: "Method parameter names counting counts double for reference types") #243

Closed
alienisty opened this Issue Dec 4, 2015 · 6 comments

Comments

2 participants
@alienisty

alienisty commented Dec 4, 2015

The new ParameterNames.getSumOfArgumentSizes method counts double when a parameter is a non array reference type, because, when the code finds the ';' character, it does not increase 'i' again after the loop, causing the main loop to read it again and treat it as another parameter.

The following call, will reproduce the bug:
ParameterNames.registerName("A", PUBLIC, "m1", "(Ljava/lang/String;)", "J", "local", 2);

I also believe that the case for the array would not properly handle arrays of reference types.

I suggest using the following implementation:

private static int getSumOfArgumentSizes(@Nonnull String memberDesc)
   {
      int sum = 0;
      int i = 1;

      while (true) {
         char c = memberDesc.charAt(i);

         if (c == ')') {
            return sum;
         }

         if (c == 'L') {
            while (memberDesc.charAt(i) != ';') i++;
            sum++;
         }
         else if (c == '[') {
            while ((c = memberDesc.charAt(i)) == '[') i++;
            continue;
         }
         else if (isDoubleSizeType(c)) {
            sum += 2;
         }
         else {
            sum++;
         }
         i++;
      }
}
@rliesenfeld

This comment has been minimized.

Show comment
Hide comment
@rliesenfeld

rliesenfeld Dec 4, 2015

Member

Without a description of the problem (from the user's perspective), a failing example test, or sufficient information for me to write a failing test, there isn't much else to go on.

Member

rliesenfeld commented Dec 4, 2015

Without a description of the problem (from the user's perspective), a failing example test, or sufficient information for me to write a failing test, there isn't much else to go on.

@rliesenfeld rliesenfeld closed this Dec 4, 2015

@alienisty

This comment has been minimized.

Show comment
Hide comment
@alienisty

alienisty Dec 5, 2015

In the description I state:

The following call, will reproduce the bug:
ParameterNames.registerName("A", PUBLIC, "m1", "(Ljava/lang/String;)", "J", "local", 2);

If that is not enough I can craft a class and a unit test using jmockit that will break, but is easy enough to inspect the implementation of ParameterNames.getSumOfArgumentSizes() and realize that it doesn't properly parse a method descriptor, as I explained as well in the description.

alienisty commented Dec 5, 2015

In the description I state:

The following call, will reproduce the bug:
ParameterNames.registerName("A", PUBLIC, "m1", "(Ljava/lang/String;)", "J", "local", 2);

If that is not enough I can craft a class and a unit test using jmockit that will break, but is easy enough to inspect the implementation of ParameterNames.getSumOfArgumentSizes() and realize that it doesn't properly parse a method descriptor, as I explained as well in the description.

@alienisty

This comment has been minimized.

Show comment
Hide comment
@alienisty

alienisty Dec 5, 2015

Here it is a test that causes an ArrayIndexOutOfBoundException:

public class MethodParametersMiscountingTest {

    @Tested
    private CharSequence mock;


    @Test
    public void testIssue243(@Mocked File file) {
        long variableMistakenForParameter=32L;
    }
}

And the following is a class that test for the expected parameter slots count for reference types:

public class ParameterNamesArgumentParsingTest {
    @Test
    public void testParseRefernceParameter() {
        assertEquals(1, getSumOfArgumentSizes("(Ljava/lang/String;)"));
    }

    @Test
    public void testParseArraysOfRefernceParameter() {
        assertEquals(1, getSumOfArgumentSizes("([Ljava/lang/String;)"));
    }

    private int getSumOfArgumentSizes(String memberDesc) {
        return Deencapsulation.invoke(ParameterNames.class, "getSumOfArgumentSizes", memberDesc);
    }
}

alienisty commented Dec 5, 2015

Here it is a test that causes an ArrayIndexOutOfBoundException:

public class MethodParametersMiscountingTest {

    @Tested
    private CharSequence mock;


    @Test
    public void testIssue243(@Mocked File file) {
        long variableMistakenForParameter=32L;
    }
}

And the following is a class that test for the expected parameter slots count for reference types:

public class ParameterNamesArgumentParsingTest {
    @Test
    public void testParseRefernceParameter() {
        assertEquals(1, getSumOfArgumentSizes("(Ljava/lang/String;)"));
    }

    @Test
    public void testParseArraysOfRefernceParameter() {
        assertEquals(1, getSumOfArgumentSizes("([Ljava/lang/String;)"));
    }

    private int getSumOfArgumentSizes(String memberDesc) {
        return Deencapsulation.invoke(ParameterNames.class, "getSumOfArgumentSizes", memberDesc);
    }
}
@alienisty

This comment has been minimized.

Show comment
Hide comment
@alienisty

alienisty Dec 5, 2015

Hello Rogerio,
I've added comments and test cases to issue #243, which I noticed you've
already closed, so I'm not sure you would receive notifications about it.
Maybe next time give a chance to people that is trying to contribute and
help, to add more information, or have discussion, before closing issues so
swiftly.

Regards,
Alex Nistico

On Sat, 5 Dec 2015 at 02:21 Rogério Liesenfeld notifications@github.com
wrote:

Without a description of the problem (from the user's perspective), a
failing example test, or sufficient information for me to write a failing
test, there isn't much else to go on.


Reply to this email directly or view it on GitHub
#243 (comment).

Alessandro (Alex) Nistico
alienisty@gmail.com

"Any system like Eternity, which allows men to choose their own future,
will end
by choosing safety and mediocrity, and in such a Reality the stars are out
of reach."

The End of Eternity - Isaac Asimov

alienisty commented Dec 5, 2015

Hello Rogerio,
I've added comments and test cases to issue #243, which I noticed you've
already closed, so I'm not sure you would receive notifications about it.
Maybe next time give a chance to people that is trying to contribute and
help, to add more information, or have discussion, before closing issues so
swiftly.

Regards,
Alex Nistico

On Sat, 5 Dec 2015 at 02:21 Rogério Liesenfeld notifications@github.com
wrote:

Without a description of the problem (from the user's perspective), a
failing example test, or sufficient information for me to write a failing
test, there isn't much else to go on.


Reply to this email directly or view it on GitHub
#243 (comment).

Alessandro (Alex) Nistico
alienisty@gmail.com

"Any system like Eternity, which allows men to choose their own future,
will end
by choosing safety and mediocrity, and in such a Reality the stars are out
of reach."

The End of Eternity - Isaac Asimov

@rliesenfeld rliesenfeld reopened this Dec 7, 2015

@rliesenfeld rliesenfeld added bug and removed could not reproduce labels Dec 7, 2015

@rliesenfeld

This comment has been minimized.

Show comment
Hide comment
@rliesenfeld

rliesenfeld Dec 7, 2015

Member

Thanks.

Member

rliesenfeld commented Dec 7, 2015

Thanks.

@rliesenfeld rliesenfeld self-assigned this Dec 7, 2015

@rliesenfeld

This comment has been minimized.

Show comment
Hide comment
@rliesenfeld

rliesenfeld Dec 7, 2015

Member

The failure occurs whenever the parameter names of a method or constructor are extracted, provided there's at least one parameter of a reference type and the first local variable is of type long or double.

For example, if the test class has a @tested class like the following, any test will fail with an ArrayIndexOutOfBoundsException:

static class ClassWithConstructorHavingReferenceTypeParameterAndDoubleSizedLocalVar
{
    ClassWithConstructorHavingReferenceTypeParameterAndDoubleSizedLocalVar(String s)
    {
        long var = 1;
    }
}

@Tested ClassWithConstructorHavingReferenceTypeParameterAndDoubleSizedLocalVar sut;
Member

rliesenfeld commented Dec 7, 2015

The failure occurs whenever the parameter names of a method or constructor are extracted, provided there's at least one parameter of a reference type and the first local variable is of type long or double.

For example, if the test class has a @tested class like the following, any test will fail with an ArrayIndexOutOfBoundsException:

static class ClassWithConstructorHavingReferenceTypeParameterAndDoubleSizedLocalVar
{
    ClassWithConstructorHavingReferenceTypeParameterAndDoubleSizedLocalVar(String s)
    {
        long var = 1;
    }
}

@Tested ClassWithConstructorHavingReferenceTypeParameterAndDoubleSizedLocalVar sut;

@rliesenfeld rliesenfeld changed the title from Method parameter names counting counts double for reference types to ArrayIndexOutOfBoundsException when using @Tested (was: "Method parameter names counting counts double for reference types") Dec 7, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment