Community
Participate
Working Groups
User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.0.14) Gecko/2009090217 Ubuntu/9.04 (jaunty) Firefox/3.0.14 Build Identifier: I20090611-1540 According to the wiki page the p2 director application expects the arguments for metadataRepository und artifactRepository to be in URL format. However, if you pass an actual URL it still gets encoded a second time which destroys the URL if there have been escaped characters. A URL like that: file:/Path%20With%20Spaces/ will for example be 'encoded' into file:/Path%2520With%2520Spaces This made me suspect that maybe the documentation is wrong and the director is not really expecting a URL, but a path (since it does encoding on its own). But plain file paths don't work either: java.lang.IllegalArgumentException: Location must be absolute: /var/lib/hudson/jobs/MailApp%20with%20Spaces/workspace/arch/x86/buckminster.output/org.eclipse.buckminster.tutorial.mailapp.product.feature_1.0.0.null-eclipse.feature/site.p2/ The spaces got encoded correctly, but since the file scheme is missing, it is assumed to be a relative location. Only this seems to work for the given example: file:/var/lib/hudson/jobs/MailApp with Spaces/workspace/arch/x86/buckminster.output/org.eclipse.buckminster.tutorial.mailapp.product.feature_1.0.0.null-eclipse.feature/site.p2/ But for me this looks like a very odd mixture between an URL/URI and a regular file path. More details see here: https://hudson.dev.java.net/issues/show_bug.cgi?id=4537 After a short look in the code, the DirectorApplication#processArguments seems to be the problem: if (OPTION_REPOSITORIES.isOption(opt)) { String arg = getRequiredArgument(args, ++i); getURIs(metadataRepositoryLocations, arg); getURIs(artifactRepositoryLocations, arg); continue; } getURI does the encoding, but why is encoding done when the user itself is expected to provide a valid URI? Reproducible: Always Steps to Reproduce: 1. try to create a product from a local repository that has spaces in its path
Which version of eclipse do you use? Could you indicate the ID of the director application (the argument after -application)?
(In reply to comment #1) > Which version of eclipse do you use? I used a headless version of Buckminster (3.5.1) with the director application installed. These are the bundle versions I tested with: org.eclipse.equinox.p2.director_1.0.100.v20090520-1905.jar org.eclipse.equinox.p2.director.app_1.0.100.v20091002-1358.jar > Could you indicate the ID of the director application (the argument after > -application)? -application org.eclipse.equinox.p2.director
I think the wiki doc is out dated. The implementation of getURIs(List, String) eventually calls URIUtil.fromString which will encode things properly.
(In reply to comment #3) > I think the wiki doc is out dated. The implementation of getURIs(List, String) > eventually calls URIUtil.fromString which will encode things properly. But that's the problem I think. Either the application expects an URI, then this URI must be assumed to be already encoded and therefore not be encoded again, or it expects a path and a path does not need to begin with an URI scheme, which the director seems to expect. URI scheme + not encoded URI seems like an odd combination to me.
The URL should not be encoded by users. We do the encoding in the application in order to maintain backwards compatibility with what we had in Eclipse 3.4.
(In reply to comment #5) > The URL should not be encoded by users. We do the encoding in the application > in order to maintain backwards compatibility with what we had in Eclipse 3.4. I understand that you want to maintain backwards compatibility, but isn't it rather unusual that invalid URIs are expected? Something like file:/path with spaces/a resource with an ü umlaut/ Is not what I'd expect after reading the documentation. Especially since this actually requires for many use cases (invoking from a setup or build process for example) to turn perfectly valid URIs into invalid ones. Ant for example is probably often used to invoke the director application. I can't even think of an easy way to do URL decoding in ant right now. Ant-Contrib has a url encoder task, but a decoder?
On the other end forcing our users to encode was even worse and more complex. In fact you are the first that I see that was ready to provide a properly encoded URI. Now as for passing decoded URI, it is something that most browsers do support for example I can open "file:/Users/Pascal/dev/p 2/" and Safari will change encode. So at this point the only thing I can see us do is update the wiki.
(In reply to comment #7) > Now as for passing decoded URI, it is something that most browsers do support > for example I can open "file:/Users/Pascal/dev/p 2/" and Safari will change > encode. Yes, Firefox does the same and it's good to have something like that because it's easier for the user. But there's a big difference in my opinion: I don't know about Safari, but Firefox resolves file:/Users/Pascal/dev/p%202/ perfectly fine instead of transforming it into file:/Users/Pascal/dev/p%25202/
Interestingly enough I recall a browser where if %20 was part of the URI then it would turn it into %2520, but Safari and Firefox are not. John, I think we should try to do something at least in the various applications to allow for completely encoded strings to be passed in. Johannes, if in the meantime you want to provide a patch that allows both syntax you would be very welcome.
Hi Pascal, thank you for considering to change this behavior. My idea for a fix would simply be to change the getURIs method. First it tries to create an URI instance from the string with the single argument constructor (since this constructor expects properly encoded URIs). If that fails, it means that the URI is not encoded yet and the URI gets encoded just like it's done at the moment. I'll attach a patch for this right away
Created attachment 149305 [details] Fixed URI handling for already encoded URIs I hereby declare that I have authored 100% of the code and that I have the rights to donate the code to Eclipse under the EPL.
Fix released.
*** Bug 286876 has been marked as a duplicate of this bug. ***
