Skip to content

Commit

Permalink
Fixing StringIndexOutOfBoundsException
Browse files Browse the repository at this point in the history 
I'm not really sure what this code is doing or what ID is supposed to look like. So this is just a minimum-intrusion change to avoid an exception.

java.lang.StringIndexOutOfBoundsException: String index out of range: -1
        at java.lang.String.substring(String.java:1875)
        at hudson.tasks.junit.PackageResult.findCorrespondingResult(PackageResult.java:86)
        at hudson.tasks.junit.TestResult.findCorrespondingResult(TestResult.java:295)
        at hudson.tasks.test.AbstractTestResultAction.findCorrespondingResult(AbstractTestResultAction.java:183)
        at hudson.tasks.test.TestResult.getResultInBuild(TestResult.java:162)
        at hudson.tasks.junit.History.getList(History.java:82)
        at hudson.tasks.junit.History$1.createDataSet(History.java:105)
        at hudson.tasks.junit.History$GraphImpl.createGraph(History.java:169)
        at hudson.util.Graph.render(Graph.java:87)
        at hudson.util.Graph.doPng(Graph.java:98)
  • Loading branch information
@kohsuke
kohsuke committed Apr 16, 2013
1 parent 5ee9ab5 commit ff7f272
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 9 deletions.
15 changes: 8 additions & 7 deletions core/src/main/java/hudson/tasks/junit/PackageResult.java
View file
Expand Up @@ -82,15 +82,16 @@ public synchronized String getSafeName() {
@Override
public TestResult findCorrespondingResult(String id) {
String myID = safe(getName());

int base = id.indexOf(myID);
String className;
String subId = null;
String className = id; // fall back value
if (base > 0) {
int classNameStart = base + myID.length() + 1;
className = id.substring(classNameStart);
} else {
className = id;
}
if (classNameStart<id.length())

This comment has been minimized.

Sign in to view

Copy link
@jglick

jglick Apr 16, 2013

Member

This will cause different behavior if classNameStart == id.length() which previously would have set className to "" (though I do not know if that would ever actually happen).

This comment has been minimized.

Sign in to view

Copy link
@kohsuke

kohsuke Apr 16, 2013

Author Member

The whole logic looks like a bunch of heuristics, so I think this change is OK. I did think about the case classNameStart==id.length() and this is based on my understanding of the original intention of the code.
className = id.substring(classNameStart);
}

This comment has been minimized.

Sign in to view

Copy link
@jglick

jglick Apr 16, 2013

Member

This change would not appear to correct the reported exception String index out of range: -1 which means that classNameStart == -1, though how that could be I do not know since we just checked that base > 0 and presumably myID.length() > 0 as well. Without a reproducible test case you cannot claim to have fixed the exception.

This comment has been minimized.

Sign in to view

Copy link
@kohsuke

kohsuke Apr 16, 2013

Author Member

My version of String.substring() looks like this, and line number 1875 is the 2nd throw statement:

public String substring(int beginIndex) {
    if (beginIndex < 0) {
        throw new StringIndexOutOfBoundsException(beginIndex);
    }
    int subLen = value.length - beginIndex;
    if (subLen < 0) {
        throw new StringIndexOutOfBoundsException(subLen);
    }
    return (beginIndex == 0) ? this : new String(value, beginIndex, subLen);
}

String subId = null;
int classNameEnd = className.indexOf('/');
if (classNameEnd > 0) {
subId = className.substring(classNameEnd + 1);
Expand All @@ -106,7 +107,7 @@ public TestResult findCorrespondingResult(String id) {
return child.findCorrespondingResult(subId);
} else {
return child;
}
}
}

return null;
Expand Down
4 changes: 2 additions & 2 deletions core/src/main/java/hudson/tasks/test/TestObject.java
View file
Expand Up @@ -138,7 +138,7 @@ public String getRelativePathFrom(TestObject it) {
StringBuilder buf = new StringBuilder();
TestObject next = this;
TestObject cur = this;
// Walk up my ancesotors from leaf to root, looking for "it"
// Walk up my ancestors from leaf to root, looking for "it"
// and accumulating a relative url as I go
while (next!=null && it!=next) {
cur = next;
Expand Down Expand Up @@ -260,7 +260,7 @@ public <T> T getTestAction(Class<T> klazz) {

/**
* Find the test result corresponding to the one identified by <code>id></code>
* withint this test result.
* within this test result.
*
* @param id The path to the original test result
* @return A corresponding test result, or null if there is no corresponding
Expand Down

1 comment on commit ff7f272

@jglick
Copy link
Member

@jglick jglick commented on ff7f272 Apr 23, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Retroactively filed JENKINS-17721.

Please sign in to comment.