Bug 347468

Comment 1 Michael Gorovoy 2011-05-27 11:20:14 EDT

When an URL with # sign in file name is used to create a URI, it produces an IllegalArgumentExcepion with message 'URI has an authority component'

Comment 2 Greg Wilkins 2011-06-22 03:12:00 EDT

I'm unconvinced this is a problem.  # characters should be encoded in URLs

Can you give a more concrete example of how this problem occurs?  Also why does ResourceTest.testEncoding pass?

Comment 3 Michael Gorovoy 2011-06-22 13:45:45 EDT

Indeed, the problem occurs only when a path or a file URL contains a # character that is not encoded. FileResource works correctly when it is properly encoded.

However, since Jetty has a lot of places where a path, rather than explicit file URL, could be specified in various configuration files, it might be desirable to be able to process both encoded and not encoded paths correctly. The primary reason for this is that in the event user specifies a path that is valid for the platform, they would not have any way of knowing that it has to be URL-encoded.

I have been able to come up with a solution that works in both cases. I am going to update it and post a patch in this ticket so that we could discuss whether we want it or not.

Comment 4 Michael Gorovoy 2011-06-22 16:41:13 EDT

Created attachment 198434 [details]
Proposed Patch (Tested on Windows Only)

Comment 5 Michael Gorovoy 2011-06-22 18:58:31 EDT

Comment on attachment 198434 [details]
Proposed Patch (Tested on Windows Only)

I just applied the patch locally and ran tests on Linux workstation. Everything looks good.

Comment 6 Greg Wilkins 2011-06-22 19:20:03 EDT

I'm still unconvinced.

Jetty allows a filename or a URL in many locations, but that does not mean that a string can be both a filename or a URL.

The string must either be a legal URL (with # encoded) or a filename (without # encoded).

I'm very cautious about code like:   

    resource=resource.replace('/',File.separatorChar);


but let me ponder this a bit longer.

Comment 7 Michael Gorovoy 2011-06-23 10:46:25 EDT

(In reply to comment #6)
> Jetty allows a filename or a URL in many locations, but that does not mean that
> a string can be both a filename or a URL.
> 
> The string must either be a legal URL (with # encoded) or a filename (without #
> encoded).

Another way to tackle this issue would be to change FileResource constructor to explicitly reject any URL that does not have the 'file:' prefix, or contains any characters that are supposed to be URL-encoded as opposed to attempting to process an invalid URL as it is done right now. Also, code in the Resource.newResource(String, boolean) method should be changed to treat any resource identifier that doesn't have the URL prefix as a filename so it will process the special characters properly, as opposed to attempting to convert the filename to a URL. 

> I'm very cautious about code like:   
> 
>     resource=resource.replace('/',File.separatorChar);

The solution suggested above will eliminate the need to replace forward slashes in incomplete URLs with File.separatorChar in order to attempt to convert them to filename, or trying to decode a filename or URL that is not encoded.

Comment 8 Greg Wilkins 2011-06-27 20:59:46 EDT

Created attachment 198696 [details]
FileResource(File) constructor

this is a patch that cleans up a little bit the creation of FileResources.  It does not change anything too significantly but does expose a FileResource(File) constructor that could be used to resolve ambiguities and avoid JVM encoding deficiencies

Comment 9 Michael Gorovoy 2011-06-27 22:55:10 EDT

I've been asked to track down the original problem that this issue was started by. The error occurred in o.e.j.deploy.binding.GlobalWebappConfigBindingTest.testServerAndSystemClassesOverride() method. The exception has been masked from JUnit, but recorded in the log. The directory that it was looking for had "#n" at the end where n was a sequential number of the test run in order to create a (unique) testing directory.

java.io.FileNotFoundException: C:\code\eclipse\jetty7-trunk\jetty-deploy\target\tests\oejdb.GlobalWebappConfigBindingTest\tes...AndSystemClassesOverride (Access is denied)
	at java.io.FileInputStream.open(Native Method)
	at java.io.FileInputStream.<init>(FileInputStream.java:106)
	at org.eclipse.jetty.util.resource.FileResource.getInputStream(FileResource.java:274)
	at org.eclipse.jetty.deploy.bindings.GlobalWebappConfigBinding.processBinding(GlobalWebappConfigBinding.java:90)
	at org.eclipse.jetty.deploy.AppLifeCycle.runBindings(AppLifeCycle.java:180)
	at org.eclipse.jetty.deploy.DeploymentManager.requestAppGoal(DeploymentManager.java:482)
	at org.eclipse.jetty.deploy.DeploymentManager.addApp(DeploymentManager.java:135)
	at org.eclipse.jetty.deploy.providers.ScanningAppProvider.fileAdded(ScanningAppProvider.java:137)
	at org.eclipse.jetty.deploy.providers.ScanningAppProvider$1.fileAdded(ScanningAppProvider.java:50)
	at org.eclipse.jetty.util.Scanner.reportAddition(Scanner.java:601)
	at org.eclipse.jetty.util.Scanner.reportDifferences(Scanner.java:531)
	at org.eclipse.jetty.util.Scanner.scan(Scanner.java:394)
	at org.eclipse.jetty.util.Scanner.doStart(Scanner.java:329)
	at org.eclipse.jetty.util.component.AbstractLifeCycle.start(AbstractLifeCycle.java:58)
	at org.eclipse.jetty.deploy.providers.ScanningAppProvider.doStart(ScanningAppProvider.java:114)
	at org.eclipse.jetty.util.component.AbstractLifeCycle.start(AbstractLifeCycle.java:58)
	at org.eclipse.jetty.deploy.DeploymentManager.startAppProvider(DeploymentManager.java:543)
	at org.eclipse.jetty.deploy.DeploymentManager.doStart(DeploymentManager.java:218)
	at org.eclipse.jetty.util.component.AbstractLifeCycle.start(AbstractLifeCycle.java:58)
	at org.eclipse.jetty.util.component.AggregateLifeCycle.doStart(AggregateLifeCycle.java:41)
	at org.eclipse.jetty.server.handler.AbstractHandler.doStart(AbstractHandler.java:50)
	at org.eclipse.jetty.server.handler.HandlerWrapper.doStart(HandlerWrapper.java:90)
	at org.eclipse.jetty.server.Server.doStart(Server.java:258)
	at org.eclipse.jetty.util.component.AbstractLifeCycle.start(AbstractLifeCycle.java:58)
	at org.eclipse.jetty.deploy.test.XmlConfiguredJetty.start(XmlConfiguredJetty.java:407)
	at org.eclipse.jetty.deploy.bindings.GlobalWebappConfigBindingTest.testServerAndSystemClassesOverride(GlobalWebappConfigBindingTest.java:75)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
	at java.lang.reflect.Method.invoke(Method.java:597)
	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:44)
	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:15)
	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:41)
	at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:20)
	at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:28)
	at org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:31)
	at org.eclipse.jetty.toolchain.test.TestingDir$1.evaluate(TestingDir.java:33)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:76)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:50)
	at org.junit.runners.ParentRunner$3.run(ParentRunner.java:193)
	at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:52)
	at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:191)
	at org.junit.runners.ParentRunner.access$000(ParentRunner.java:42)
	at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:184)
	at org.junit.runners.ParentRunner.run(ParentRunner.java:236)
	at org.eclipse.jdt.internal.junit4.runner.JUnit4TestReference.run(JUnit4TestReference.java:46)
	at org.eclipse.jdt.internal.junit.runner.TestExecution.run(TestExecution.java:38)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:467)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:683)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.run(RemoteTestRunner.java:390)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.main(RemoteTestRunner.java:197)

Comment 10 Michael Gorovoy 2011-06-27 23:28:34 EDT

I found the root of the problem. In binding-test-contexts-1.xml, the configuration was incorrectly tacking 'file://' onto a path. This is caused it to have '#' character that was not encoded.

Furthermore, the code in o.e.j.deploy.binding.GlobalWebappConfigBuinding.processBinding(Node, App) uses construct 'new FileResource(new URL(String))' on the un-encoded URL string to create the resource, causing the above exception to occur.

Therefore this issue is only limited to use of the FileResource in this specific part of the code, and is not a FileResource problem in general. Sorry for the false alarm.

I will fix the configuration file and the binding to use correct method.

Comment 11 Michael Gorovoy 2011-06-28 14:00:48 EDT

Committed r3432.