Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 338081

Summary:

StringIndexOutOfBoundsException in BaseAdaptor.listEntryPaths

Product:

[Eclipse Project] Equinox

Reporter:

Glyn Normington <glyn.normington>

Component:

Framework

Assignee:

John Ross <jwross>

Status:

RESOLVED FIXED

QA Contact:

Severity:

blocker

Priority:

CC:

eclipse, tjwatson

Version:

3.7

Target Milestone:

3.7 M6

Hardware:

Macintosh

OS:

Mac OS X - Carbon (unsup.)

Whiteboard:

Attachments:

Description	Flags
Proposed patch	none

Description Glyn Normington

2011-02-24 08:49:49 EST

We hit this problem in Equinox build 3.7.0.v20110221 which we need to fix bug 337538.

The calling sequence is that Virgo's integration test ApplicationContextDependencyMonitorIntegrationTests initialises the Virgo kernel. This attempts to issue a "kernel starting" message using logback and this needs to find a suitable NLS resource bundle.

The problem is due to the following line in BaseAdaptor.listEntryPaths when executed with path = "":

path += path.charAt(path.length() - 1) == '/' ? filePattern : '/' + filePattern;

The context is as follows:

The Virgo medic component calls BundleHost.findEntries, which is dispatched to AbstractBundle.findEntries, with parameters:

* path = ""
* filePattern =  "EventLogMessages_en_US.properties"
* recurse = false

This calls BaseAdaptor.findEntries with parameters:

* datas = [org.eclipse.virgo.kernel.core_3.0.0.BUILD-20110224114907]
* path = ""
* filePattern = "EventLogMessages_en_US.properties"
* options = 0

This calls BaseAdaptor.listEntryPaths with parameters:

* bundleFiles = [null] (which will be the subject of another bug)
* path = ""
* filePattern = "EventLogMessages_en_US.properties"
* options = 0

Comment 1 John Ross

2011-02-24 11:28:35 EST

Created attachment 189711 [details]
Proposed patch

This patch checks the length of the path string. If zero, the path simply becomes the filePattern. If greater than zero, the filePattern is appended to the end of the path, adding '/' where necessary.

The spec appears to be silent (javadoc at least) on how implementations should handle a null or an empty path string for Bundle.findEntries. It states '/' represents the bundle root. The original code treated '' as '/' for the bundle root and threw an NPE for a null path string, so this patch follows suit.

The patch also includes a new test in org.eclipse.osgi.tests.

Comment 2 Glyn Normington

2011-02-24 11:32:03 EST

Hi John: so far I haven't managed to build Equinox, so please could you skype me a built JAR for testing?

Comment 3 Thomas Watson

2011-02-24 12:45:26 EST

The good news is that the fix in bug337370 should help performance in this case.  The bad news is that this bug was introduced.  The patch looks good.  I modified the test to not use the 1.5 method String.contains.

The tests are still able to be run on the 1.4 VM for now.  Patch has been released.

Comment 4 Glyn Normington

2011-02-24 16:52:47 EST

nfirming that the patched build passes all the Virgo kernel tests, so problem solved. Please would you confirm that the fix is scheduled for the next I build and advise when that will be available in UK time.

Comment 5 Thomas Watson

2011-02-24 17:00:50 EST

(In reply to comment #4)
> nfirming that the patched build passes all the Virgo kernel tests, so problem
> solved. Please would you confirm that the fix is scheduled for the next I build
> and advise when that will be available in UK time.

Glyn, our I builds are every Tuesday at 8:00 AM EST (I guess 2 PM UK time?).  Bottom line, you likely will not see a build UK time until Wednesday morning.  The fix will be included in the nightly build tonight if you cannot wait.

  http://www.eclipse.org/eclipse/platform-releng/buildSchedule.html

Thanks for testing.

Comment 6 Glyn Normington

2011-02-24 17:07:01 EST

We've put off M02 until next week, so we'll wait for the I build thanks. Just don't put more bugs in. ;-)

Comment 7 Thomas Watson

2011-02-24 17:25:30 EST

(In reply to comment #6)
> We've put off M02 until next week, so we'll wait for the I build thanks. Just
> don't put more bugs in. ;-)

Before I saw this note I decided to request an I-Build respin.  There should be one scheduled for tomorrow.  Just waiting to hear back from Kim, our release engineer.  Even though you are delaying M02 until next week, I would like to get you the proper fix so you can do additional testing before you release M02.

Comment 8 Thomas Watson

2011-02-24 17:45:49 EST

OK, Kim reacted faster than expected.  Looks like the I-Build started immediately.  You should have an I-Build ready for you tomorrow morning UK time.  Check http://download.eclipse.org/equinox/ under the 3.7 Integration Builds section.

Comment 9 Glyn Normington

2011-02-25 04:30:04 EST

Thanks Tom. This will help our scheduling of M02. Please note that all our testing for milestones is automated and mandatory as part of the release process. Essentially we run a script and it does all the testing and performs the release. We only have to expend manual effort if something goes wrong. So the gating factors for picking up an Equinox fix are the availability of an Equinox build and the absence of regressions in Equinox. By spinning an I build early, you have helped both these: the build is available early and the number of possible changes, and therefore regressions, relative to John's build I just tested is small.

Comment 10 Glyn Normington

2011-02-25 06:25:24 EST

(In reply to comment #8)
> OK, Kim reacted faster than expected.  Looks like the I-Build started
> immediately.  You should have an I-Build ready for you tomorrow morning UK
> time.  Check http://download.eclipse.org/equinox/ under the 3.7 Integration
> Builds section.

The latest I build is I20110222-0800 dated Tue, 22 Feb 2011 -- 08:00 (-0500). Please could you tell us the status of Kim's I build.