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

Bug 311338

Summary: [repo] RepositoryManager.contains(URI) throws IAE and breaks install when repo has bad reference
Product: [Eclipse Project] Equinox Reporter: Emanuel Graf <emanuel>
Component: p2Assignee: Susan McCourt <susan>
Status: RESOLVED FIXED QA Contact:
Severity: normal    
Priority: P3 CC: david_williams, pascal, pnehrer, thomas
Version: 3.6Flags: pascal: review+
Target Milestone: 3.6 RC1   
Hardware: PC   
OS: Linux   
Whiteboard:
Attachments:
Description Flags
patch to ProvisioningContext none

Description Emanuel Graf CLA 2010-05-03 04:07:33 EDT
1. Click Help -> Install New Software
2. Choose a Update Site
3. Check Feature
4. Click Next

The following exception is thrown:
java.lang.IllegalArgumentException: Location must be absolute: %25update.label
at org.eclipse.equinox.internal.p2.repository.helpers.AbstractRepositoryManager.checkValidLocation(AbstractRepositoryManager.java:703)
at org.eclipse.equinox.internal.p2.repository.helpers.AbstractRepositoryManager.contains(AbstractRepositoryManager.java:250)
at org.eclipse.equinox.p2.engine.ProvisioningContext.isEnabled(ProvisioningContext.java:231)
at org.eclipse.equinox.p2.engine.ProvisioningContext.loadMetadataRepository(ProvisioningContext.java:213)
at org.eclipse.equinox.p2.engine.ProvisioningContext.getLoadedMetadataRepositories(ProvisioningContext.java:179)
at org.eclipse.equinox.p2.engine.ProvisioningContext.getMetadata(ProvisioningContext.java:255)
at org.eclipse.equinox.internal.p2.director.SimplePlanner.gatherAvailableInstallableUnits(SimplePlanner.java:256)
at org.eclipse.equinox.internal.p2.director.SimplePlanner.getSolutionFor(SimplePlanner.java:316)
at org.eclipse.equinox.internal.p2.director.SimplePlanner.getProvisioningPlan(SimplePlanner.java:372)
at org.eclipse.equinox.internal.p2.operations.PlannerResolutionJob.runModal(PlannerResolutionJob.java:76)
at org.eclipse.equinox.p2.operations.ProfileChangeOperation.resolveModal(ProfileChangeOperation.java:115)
at org.eclipse.equinox.internal.p2.ui.dialogs.ProvisioningOperationWizard$1.run(ProvisioningOperationWizard.java:206)
at org.eclipse.jface.operation.ModalContext$ModalContextThread.run(ModalContext.java:121)


eclipse.buildId=I20100429-1549
java.version=1.5.0_22
java.vendor=Sun Microsystems Inc.
BootLoader constants: OS=linux, ARCH=x86_64, WS=gtk, NL=en_US
Command-line arguments:  -os linux -ws gtk -arch x86_64
Comment 1 Pascal Rapicault CLA 2010-05-03 07:28:29 EDT
What are you installing, from which update site?
Comment 2 Emanuel Graf CLA 2010-05-03 07:32:25 EDT
I tried Mylyn Task List and Eclipse Communication Framework SDK for both http://download.eclipse.org/releases/helios and http://download.eclipse.org/releases/staging
Comment 3 Susan McCourt CLA 2010-05-03 11:39:04 EDT
similar symptom in bug 311367.  I'm not marking a duplicate because the UI could be slightly more graceful in dealing with a null plan.
Comment 4 Susan McCourt CLA 2010-05-03 19:53:09 EDT
Funny that we just start hearing about this.
I wonder if the repo reference following is surfacing this.
Is it possible we've had malformed repo references all along and the errors have been swallowed before, but now are showing up during resolution since ProvisioningContext is now using the reference API?
Comment 5 Peter Nehrer CLA 2010-05-04 09:34:26 EDT
I'm seeing this with a bunch of components, like Mylyn, Linux Tools, RSE, Subversive, and Buckminster (http://download.eclipse.org/tools/buckminster/updates-3.6). Doing the same exact thing on M6 does not cause problems.

My M7 installation is now essentially un-updateable.
Comment 6 Susan McCourt CLA 2010-05-04 10:59:07 EDT
(In reply to comment #2)
> I tried Mylyn Task List and Eclipse Communication Framework SDK for both
> http://download.eclipse.org/releases/helios and
> http://download.eclipse.org/releases/staging

I am able to select and install
  Eclipse Communication Framework SDK	3.2.0.v20100202-1607	
  Mylyn Task List (Required)	3.4.0.I20100204-0200-e3x	

without issue.

We need to be able to reproduce this.  From the stack trace, I can only see that there was a reference from some repository to this bogus location.  But there seem to be some other strange things going on there.

1.  For those having the problem. 
- do you have "Contact all update sites to find required software" checked in the install wizard?  Do you have failure with or without this box checked?
- try disabling all sites besides Helios and see if the install succeeds.  It would appear some other site is involved.

2.  I'm a bit concerned by the stack because it appears to be in the repo reference following code on the first attempt to resolve.  Normally the reference following should only engage if the first attempt came up with a missing requirement.  Could be a bug, but need to reproduce

3.  I think throwing the IllegalArgumentException is really not appropriate.  The client code seems quite innocent.  It is trying to determine if the location in a repository reference is both known by the manager, and enabled by the manager.  The failure indicates that the manager confirmed it contained the location and then IAE'ed on checking the enablement.  This seems wrong.

private boolean isEnabled(IRepositoryManager manager, IRepositoryReference reference) {
  return (manager.contains(reference.getLocation()) &&  manager.isEnabled(reference.getLocation())) || ((!manager.contains(reference.getLocation())) && ((reference.getOptions() | IRepository.ENABLED) == IRepository.ENABLED));
	}
Comment 7 Peter Nehrer CLA 2010-05-04 11:40:25 EDT
You're right, the culprit is http://download.eclipse.org/releases/staging/. After disabling it in preferences I can now install Buckminster and I no longer get the error when updating.

Having only http://download.eclipse.org/releases/staging/ enabled (and nothing else) and then running Help->Check for Updates reproduces the error.
Comment 8 Susan McCourt CLA 2010-05-04 13:12:00 EDT
I can easily reproduce this now.  Taking bug to fix the problem in the repo manager.
Comment 9 Susan McCourt CLA 2010-05-04 13:30:32 EDT
(In reply to comment #6)
> 1.  For those having the problem. 
<snip>  

OK, so I've verified that there is an enabled artifact repository reference with a location of "%25update.label" in the staging repo.  I did not track down who it's coming from.  cc'ing David Williams. (where do I open a bug against the staging repo?)

> 
> 2.  I'm a bit concerned by the stack because it appears to be in the repo
> reference following code on the first attempt to resolve.  Normally the
> reference following should only engage if the first attempt came up with a
> missing requirement.  Could be a bug, but need to reproduce

I misread the stack.  The reference being followed was an artifact repository, which is expected.  Good.

> 3.  I think throwing the IllegalArgumentException is really not appropriate. 
> The client code seems quite innocent.  It is trying to determine if the
> location in a repository reference is both known by the manager, and enabled by
> the manager.  The failure indicates that the manager confirmed it contained the
> location and then IAE'ed on checking the enablement.  This seems wrong.

Actually, it IAE'd when asking contains(URI) which is even worse in my opinion.  This means anyone who hands a bogus URI to this API will IAE.  I recall some discussion very late in 3.5 about whether repo managers should be so unforgiving, but it was so late in the game, we tried to ensure the UI did all the validation up front.  In this case, though, the URI is coming from a repo reference, so no one gets a chance to validate it before the manager sees it.

I think the proper fix (post 3.6) is to fix the repo manager.  If we are going to be so strict, we should at least throw a checked exception so that clients know where they will get hit by this.  Note that this can happen in any number of places:  calling contains(URI), loading the repo, etc...

For 3.6, I can catch IAE in the ProvisioningContext code and ignore these bad references.
Comment 10 Susan McCourt CLA 2010-05-04 13:54:09 EDT
Created attachment 166994 [details]
patch to ProvisioningContext

This patch catches IllegalArgumentException in the loop where we are working with referenced repo locations.  Any metadata or artifact repo reference that is malformed will be ignored by ProvisioningContext.
Comment 11 Susan McCourt CLA 2010-05-04 13:56:37 EDT
Note that this fix prevents the error in the install wizard.  I was able to successfully install Mylyn Task List and ECF SDK with this fix.

However, the problem in the staging repository must be fixed, or else it's possible that some other install will later fail with artifacts that were not found since this reference is ignored.
Comment 12 David Williams CLA 2010-05-04 13:59:53 EDT
> OK, so I've verified that there is an enabled artifact repository reference
> with a location of "%25update.label" in the staging repo.  I did not track down
> who it's coming from.  cc'ing David Williams. (where do I open a bug against
> the staging repo?)
> 

Cross-project bugzilla component (in "Foundation" --> Community: 

https://bugs.eclipse.org/bugs/enter_bug.cgi?product=Community
Comment 13 Susan McCourt CLA 2010-05-04 14:03:45 EDT
(In reply to comment #12)
> > OK, so I've verified that there is an enabled artifact repository reference
> > with a location of "%25update.label" in the staging repo.  I did not track down
> > who it's coming from.  cc'ing David Williams. (where do I open a bug against
> > the staging repo?)
> > 
> 
> Cross-project bugzilla component (in "Foundation" --> Community: 
> 
> https://bugs.eclipse.org/bugs/enter_bug.cgi?product=Community

thx.
opened bug 311596
Comment 14 Susan McCourt CLA 2010-05-06 17:21:59 EDT
fixed in HEAD >20100506.