Bug 342853

Comment 1 Andrew Niefer 2011-04-14 13:01:35 EDT

Tom pointed out that this can happen if the entry starts with "\n " so that cont == 0 when we enter the while loop.

However, none of the bundles I'm processing here have .SF files like that.  Re-running this portion of the build worked fine without problems, I'm not able to reproduce this again.

Comment 2 Thomas Watson 2011-04-14 14:26:38 EDT

(In reply to comment #0)
> java.lang.StringIndexOutOfBoundsException: String index out of range: 29556

Something else must be going on here.  If cont == 0 I would expect the message to indicate "String index out of range: -1" since we would have passed -1 in for the end index to substring.  Instead it is saying the index 29556 is out of bounds which indicates we have gone off the end of the string.

Comment 3 Thomas Watson 2011-04-14 14:30:15 EDT

Something that concerns me about this code is that it seems to depend on each line continuation being in the form of "\r\n " but I do believe the manifest can have newline specified as \r\n | \n | \r

Perhaps the standard jar command always uses \r\n

Comment 4 Pascal Rapicault 2011-05-18 09:56:53 EDT

*** Bug 342957 has been marked as a duplicate of this bug. ***

Comment 5 Andrew Niefer 2011-05-18 14:09:43 EDT

Tom, this happened just now in a test build.
The repository is available here:
http://build.eclipse.org/eclipse/e4/build/e4/downloads/drops/4.0.0/40builds/I20110518-1327/I20110518-1327/repository/

Comment 6 Andrew Niefer 2011-05-18 14:30:14 EDT

Rerunning the director to install from this repository worked without problems, so I'm not sure that you will find anything running the verifier over things.

When the build fails, there are 3 parallel calls to the director installing for 3 different platforms.  Perhaps there is a threading problem?

Comment 7 Andrew Niefer 2011-05-18 16:00:28 EDT

(In reply to comment #5)
> Tom, this happened just now in a test build.
> The repository is available here:
> http://build.eclipse.org/eclipse/e4/build/e4/downloads/drops/4.0.0/40builds/I20110518-1327/I20110518-1327/repository/

I have moved this to 
http://build.eclipse.org/eclipse/e4/build/e4/downloads/drops/4.0.0/40builds/fI20110518-1327/I20110518-1327/repository/
to avoid it being published when the regular build runs tonight.

Comment 8 Thomas Watson 2011-05-18 16:03:05 EDT

Created attachment 196045 [details]
patch

Here is a patch that re-implements the stripContinuations method.  This should prevent any possible out of bounds errors.  But it is still unclear to me how this could be happening.  Even with parallel calls.  Each call gets a new SignedBundleFile with a separate SignatureBlockProcessor.  There shouldn't be any threading issues.

Comment 9 Andrew Niefer 2011-07-13 13:19:37 EDT

(In reply to comment #8)
> Created attachment 196045 [details]
> patch
> 
> Here is a patch that re-implements the stripContinuations method.  This should
> prevent any possible out of bounds errors.  But it is still unclear to me how
> this could be happening.  Even with parallel calls.  Each call gets a new
> SignedBundleFile with a separate SignatureBlockProcessor.  There shouldn't be
> any threading issues.

This problem has occurred again in a 4.2 build.

I found the problem: if the entry being processed contains two continuations immediately following one another: 
"foo\n \n "
 0123 45 6

After processing the first continuation and continuing the loop, start = cont = 5 because the next continuation is empty.  We then attempt
entry.substring(start, cont - 1)
so that end < start which is a StringIndexOutOfBoundsException.

Comment 10 Andrew Niefer 2011-07-13 17:22:00 EDT

I had similar problems happen to me running with Tom's attached patch.
I finally managed to get a debugger on this.  The best we can tell is that there must be a vm/jit bug here.

With the new patch, the debugger sitting on 
buffer.replace(cont - 1, cont + 2, "");
with a thrown StringIndexOutOfBoundsException, we have

cont = 39896 (assigned on the previous iteration of the loop from indexOf)
buffer.length() = 39821
buffer.indexOf("\n ") = -1


Which is not possible.  It is as if the replace did not finish before getting the indexOf, or the buffer was modified after the indexOf.

Comment 11 Thomas Watson 2011-07-14 09:34:36 EDT

Created attachment 199663 [details]
Updated patch

Here is an alternative patch that handles all three variants of line continuations:

"\r\n "
"\n "
"\r "

But this patch is not likely to help Andrew's issue since it uses the same code pattern as the previous patch:

int index = buffer.indexOf(toRemove);
int length = toRemove.length();
while (index > 0) {
	buffer.replace(index, index + length, ""); //$NON-NLS-1$
	index = buffer.indexOf(toRemove, index);
}

It appears the buffer.indexOf is not telling the truth and gives us an index that is > buffer.length which seems impossible.

Comment 12 Jason Jones 2011-08-22 14:54:37 EDT

I ended up creating a similar patch in order to implement signed bundles in a project I am working on.  Any idea if / when this may be applied, and what version of Eclipse / Equinox will end up seeing it?

(In reply to comment #11)
> Created attachment 199663 [details]
> Updated patch
> 
> Here is an alternative patch that handles all three variants of line
> continuations:
> 
> "\r\n "
> "\n "
> "\r "
> 
> But this patch is not likely to help Andrew's issue since it uses the same code
> pattern as the previous patch:
> 
> int index = buffer.indexOf(toRemove);
> int length = toRemove.length();
> while (index > 0) {
>     buffer.replace(index, index + length, ""); //$NON-NLS-1$
>     index = buffer.indexOf(toRemove, index);
> }
> 
> It appears the buffer.indexOf is not telling the truth and gives us an index
> that is > buffer.length which seems impossible.

(In reply to comment #11)
> Created attachment 199663 [details]
> Updated patch
> 
> Here is an alternative patch that handles all three variants of line
> continuations:
> 
> "\r\n "
> "\n "
> "\r "
> 
> But this patch is not likely to help Andrew's issue since it uses the same code
> pattern as the previous patch:
> 
> int index = buffer.indexOf(toRemove);
> int length = toRemove.length();
> while (index > 0) {
>     buffer.replace(index, index + length, ""); //$NON-NLS-1$
>     index = buffer.indexOf(toRemove, index);
> }
> 
> It appears the buffer.indexOf is not telling the truth and gives us an index
> that is > buffer.length which seems impossible.

Comment 13 Thomas Watson 2011-08-23 09:36:45 EDT

(In reply to comment #12)
> I ended up creating a similar patch in order to implement signed bundles in a
> project I am working on.  Any idea if / when this may be applied, and what
> version of Eclipse / Equinox will end up seeing it?
> 

Were you running into issues where signed bundles were using different line continuations:

"\r\n "
"\n "
"\r "

I opened a separate bug355498 to track releasing this patch since this addresses a different issue.  We are not convinced this patch would fix the StringIndexOutOfBoundsException reported in this bug.  That issue appears to be threading and VM related.

Please comment in bug355498 about what you are doing with signed bundles that does not work with the current Equinox release.  Thanks.

Comment 14 Carlos O\'Donell 2012-02-07 10:36:19 EST

We continue to run into this issue. In the face of VM/thread issues is there anything we can do to code defensively here e.g. double check indexOf against length? I know that sounds lame, but it's better than having this fail all the time.

Comment 15 Eclipse Genie 2018-12-06 12:48:51 EST

This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet.

If you have further information on the current state of the bug, please add it. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant.

--
The automated Eclipse Genie.

Comment 16 Lars Vogel 2019-09-04 01:52:19 EDT

This bug was marked as stalebug a while ago. Marking as worksforme.

If this report is still relevant for the current release, please reopen and remove the stalebug whiteboard tag.