Regression on VirtualFile.toURL()

[wolfc]

Because of a regression in VFS where virtualFile.toURL() used to return 'vfszip:/tmp/ejb3_pkg_scope.ear/ejb3_pkg_scope.jar' and is now returning 'vfszip:/tmp/ejb3_pkg_scope.ear/ejb3_pkg_scope.jar/' JPA deployers doesn't get the Persistence Unit root URL anymore.

This is most harmful for jar-file entries in persistence xml which are supposed to be relative to that URL. Thus resulting in entries like 'vfszip:/tmp/ejb3_pkg_scope.ear/ejb3_pkg_scope.jar/lib/common.jar' instead of 'vfszip:/tmp/ejb3_pkg_scope.ear/lib/common.jar'.

I say this needs to be fixed soon.

[adrian.brock]

We'll never have time to fix any issues if people keep raising duplicate issues
because they can't be bothered tracking existing discussions.

Please subscribe to the jboss-development mailing list and read it
(or at least search it before posting).

[alesj]

So, the only proper fix to Carlo's JPA issue is to bring back "no '/' at the end" behavior?

[adrian.brock]

That's what I told you yesterday. The URL you are returning is not valid.
Its a packed jar not a directory.

Seems I can't avoid wasting time on repetition.

[alesj]

That's what I told you yesterday. The URL you are returning is not valid.
Its a packed jar not a directory.

I don't get it anymore.
Who says this is not valid? The spec?

I thought it was about consistency.
e.g. have all non-leaves with "/" at the end

Or what's the rule(s)?

Seems I can't avoid wasting time on repetition.

Nope, not while Carlo and me are around. :-)

[starksm64]

"alesj" wrote:

That's what I told you yesterday. The URL you are returning is not valid.
Its a packed jar not a directory.

I don't get it anymore.
Who says this is not valid? The spec?

I thought it was about consistency.
e.g. have all non-leaves with "/" at the end

The main problem is that new URL(".../x/", "y") and URL(".../x", "y") result in two different resolved URLs:

import java.net.*;

public class x
{
 public static void main(String[] args)
 throws Exception
 {
 URL dir = new URL("file:/tmp/x/");
 URL file = new URL("file:/tmp/x");
 URL subdir = new URL(dir, "y");
 URL subfile = new URL(file, "y");
 System.out.println(subdir);
 System.out.println(subfile);
 }
}

... output:

file:/tmp/x/y
file:/tmp/y

This is what Carlo is saying about how the relative url against the jar is being resolved.

[alesj]

I'm aware of Carlo's problem. ;-)

All I'm saying is what does the spec say (if it says anything at all).
Or what rules to follow to make it consistent?

Or how to make this doesn't break some 3rd party lib.
e.g. Facelets, although that might be patch's fault

[jason.greene]

The URL spec doesn't care what the path represents. It only cares whether or not it is a directory, and that is determined by the use of /. So, if VFS returns a url with a slash at the end, it's a directory. The problem is that the JPA code assumes it's getting a file and not a directory. Although I can't blame it since it is making a call to DI.getFile().

The main problem with the VFS returning a jar url as a directory, is that it introduces ambiguity with a directory having the same name.

[adrian.brock]

"alesj" wrote:

All I'm saying is what does the spec say (if it says anything at all).

http://www.ietf.org/rfc/rfc2396.txt

5.2. Resolving Relative References to Absolute Form

..blah..

6) If this step is reached, then we are resolving a relative-path
reference. The relative path needs to be merged with the base
URI's path. Although there are many ways to do this, we will
describe a simple method using a separate string buffer.

a) All but the last segment of the base URI's path component is
copied to the buffer. In other words, any characters after the
last (right-most) slash character, if any, are excluded.

So:

/a/b/c + d = /a/b/d
/a/b/c/ + d = /a/b/c/d

and hence the change in behaviour of the relative url resolution.

[alesj]

OK, I'll fix this appropriately tomorrow.

Dirs end with '/', everything else doesn't.
OK?

But wouldn't that make Carlo's impl fail on exploded jar?

[alesj]

Although I can't blame it since it is making a call to DI.getFile().

This should actually be getVirtualFile.
To be even stricter, they should call DI::getRoot.

[jason.greene]

Looking at this in more detail, the JPA code is definitely wrong and an ugly hack, no matter the decision on the trailing /:

URL url = di.getFile("").toURL();
return new URL(url, jar);

It should instead be calling getParent().getFile(jar);

[jason.greene]

"alesj" wrote:

But wouldn't that make Carlo's impl fail on exploded jar?

Yes it would most definitely break on exploded jars.

[alesj]

Looking at this in more detail, the JPA code is definitely wrong and an ugly hack, no matter the decision on the trailing /:

Exactly. ;-)
I've said this to Carlo before - relying on impl details == very bad. :-)
As this impl will fail on exploded deployments.

The DI::getParent is tricky, as it might not always work - can be null.
It depends on what the VFSContext's root is.
It's then again an impl detail to know that we actually have
a context root that's higher up the hierarchy.

[alesj]

"alesj" wrote:

The DI::getParent is tricky, as it might not always work - can be null.
It depends on what the VFSContext's root is.
It's then again an impl detail to know that we actually have
a context root that's higher up the hierarchy.

But I guess we can let this one slip
as it would be actually weird to have a sub-deployment
who wouldn't have a proper parent.