TREQ-21: Throw FileNotFoundException when resource is not found

[SourcePond]

• Check on construction time whether the URL points into a bundle
• Always throw a FileNotFoundException when the URL points into a bundle
  and url.openConnection throws an IOException
• Added OSGi tests

[nlebas]

Hi SourcePond,

Thanks for catching the bug, and for the quality fix. I especially appreciate that it comes with unit testing. We encountered a similar problem before with JavaEE classloaders, but most of them end up returning either a "file:" or a "jar:" URL. It seems that Felix keep things under its own control.

I wonder though if the method URLApplicationResource.checkBundle may be overengineered. It actually opens the file to determine if it's a OSGi bundle. In case the URL is not a bundle, but a remote resource, it could be slow. Would it work if we simply say use "bundle".equals(url.getProtocol()) instead?

What do you think?

PS: please see nlebas@b584329

[SourcePond]

Hi nlebas,

This was my first attempt to fix it but then I realised that the protocol is not standardised in the OSGi Core Spec. This means that the protocol name depends on the OSGi framework you actually use. For instance, on Felix the protocol is "bundle" and on Equinox "bundleentry" or "bundleresource". Because this I tried to implement the bundle check A) in a framework independent way and B) without introducing an OSGi dependency.

I'm currently investigating the Felix source to find out, if there is a better way to do this.

[SourcePond]

There seams to be no proper way to find out what the name of the protocol is. In Felix, the default URL handler is specified as a field and cannot be accessed in any way from outside especially in a standard fashion via OSGi API. I guess we have several options:

1. We could check whether the URL passed has a known network protocol (http, https) to insure that no slow JAR reading over the network happens.
2. We enhance checkBundle so that it caches the result per protocol. This would mean that the (possibly slow) check is done only once for each protocol.
3. We could configure the protocol name via system properties.

I would prefer option 2 because it is simple to realise, does not need any configuration and should be fairly performant.

[nlebas]

I see. It looks like we'll have to make some assumption here.

Well, I think OSGi support is really important and we should not compromise on that.

I also think that many people do not care about OSGi and they should not be negatively impacted by it.

Finally I think there may be custom URL protocols out there that do not fit within OSGi scope (jboss' vfs: for instance) and these might not always resolve into a jar: URL. We currently get help from Spring in those cases, but I'd like to have a simpler solution if at all possible.

What if we reverse the logic: assume that any protocol we dont know about is a local file access (which would be true for both JavaEe containers and OSGi) and only explicitly check for the remote protocols included in the JVM (the list of which is here http://hg.openjdk.java.net/jdk8/jdk8/jdk/file/687fd7c7986d/src/share/classes/sun/net/www/protocol ) ?

[nlebas]

PS: I guess this would be more or less the same as the option 1 you suggested...

[nlebas]

Or perhaps we could just always convert IOException to FileNotFoundException. There may be no impact in tiles after all...

[SourcePond]

I'm not sure if it would be enough to simply convert an IOException to a FileNotFoundException because it does not necessarily mean that the resource does not exist. For instance, if a network connection fails or the resource exists but could not be loaded for other reasons, this should not be silently ignored. I agree, it would be practicable to check for network protocols and exclude them from the check.

Did I get it right: we would check if the protocol is remote, if yes throw the IOException. If not, assume a local resource and throw a FileNotFoundException..? In this case I would provide a system property for adding additional remote-protocols, just in case somebody has its own network protocol implemented.

[nlebas]

Concerning caching, Tiles will be reading the file only once and caching the result anyway. So I do not expect caching here to make a big difference, at least with Tiles.
Otherwise we're on the same page.

[SourcePond]

I'll open a new pull-request with are more common implementation approach.