[GitHub] [geronimo-xbean] powerbroker commented on a diff in pull request #34: XBEAN-337 jar path with exclamation signs(!) should be resolved correctly
powerbroker commented on code in PR #34:
URL: https://github.com/apache/geronimo-xbean/pull/34#discussion_r1150773605
##
xbean-finder/src/test/java/org/apache/xbean/finder/archive/JarArchiveTest.java:
##
@@ -113,35 +114,75 @@ public void testIterator() throws Exception {
assertEquals(classes.length, actual.size());
}
-private List list(final JarArchive archive) {
-final List actual = new ArrayList<>();
-for (final Archive.Entry entry : archive) {
-actual.add(entry.getName());
+@Test
+public void testXBEAN337() throws Exception {
+
+
+// Virtual path
+
+String path = "/this!/!file!/does!/!not/exist.jar";
+URL[] urls = {new URL("jar:file:" + path +
"!/some!/!inner!/.jar!/file.jar")};
+
+try(JarArchive jar = new JarArchive(new URLClassLoader(urls),
urls[0])){
+
+}catch(Exception ex){
+Assert.assertTrue(String.format(
+"Muzz never fail on '/this', but try full path with
exclamations('%s') instead",
+path),
+ex.getCause().getMessage().contains("exist.jar"));
+}
+
+
+// Real file
+
+File tmpDir = testTmpDir.newFolder("!" +
JarArchiveTest.class.getSimpleName() + "!-temp!");
+
+File exclamated = Files.copy(JarArchiveTest.classpath.toPath(),
+tmpDir.toPath().resolve(
+JarArchiveTest.classpath.getName()))
+.toFile();
+
+urls[0] = new URL("jar:" + exclamated.toURI().toURL() + "!/");
+
+try(JarArchive jar = new JarArchive(new URLClassLoader(urls),
urls[0])){
+
+Assert.assertEquals(String.format("Muzz successfully open '%s'",
exclamated.getAbsolutePath()),
+this.archive.iterator().hasNext(),
+jar.iterator().hasNext());
+}
+
+
+// Unsupported protocols stack
+
+urls[0] = new URL("http:ftp:jar:" + exclamated.toURI().toURL() + "!/");
+
+try(JarArchive jar = new JarArchive(new URLClassLoader(urls),
urls[0])){
+Assert.fail(String.format("Muzz eat only local file URLs:"
++ " 'file:/...' or 'jar:file:/...!/' but not '%s'",
+urls[0]));
+}catch(UnsupportedOperationException ex){
Review Comment:
i just have no idea how i could commit a failing test 3 months ago... and
how @rmannibucau could accept it...
but now in my(obsolete & broken) context the test fails - there is 'throw
IllegalArgument..' in constructor AND 'catch UnsupportedOp...' in test
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@geronimo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [geronimo-xbean] powerbroker commented on a diff in pull request #34: XBEAN-337 jar path with exclamation signs(!) should be resolved correctly
powerbroker commented on code in PR #34:
URL: https://github.com/apache/geronimo-xbean/pull/34#discussion_r1150759772
##
xbean-finder/src/test/java/org/apache/xbean/finder/archive/JarArchiveTest.java:
##
@@ -113,35 +114,75 @@ public void testIterator() throws Exception {
assertEquals(classes.length, actual.size());
}
-private List list(final JarArchive archive) {
-final List actual = new ArrayList<>();
-for (final Archive.Entry entry : archive) {
-actual.add(entry.getName());
+@Test
+public void testXBEAN337() throws Exception {
+
+
+// Virtual path
+
+String path = "/this!/!file!/does!/!not/exist.jar";
+URL[] urls = {new URL("jar:file:" + path +
"!/some!/!inner!/.jar!/file.jar")};
+
+try(JarArchive jar = new JarArchive(new URLClassLoader(urls),
urls[0])){
+
+}catch(Exception ex){
+Assert.assertTrue(String.format(
+"Muzz never fail on '/this', but try full path with
exclamations('%s') instead",
+path),
+ex.getCause().getMessage().contains("exist.jar"));
+}
+
+
+// Real file
+
+File tmpDir = testTmpDir.newFolder("!" +
JarArchiveTest.class.getSimpleName() + "!-temp!");
+
+File exclamated = Files.copy(JarArchiveTest.classpath.toPath(),
+tmpDir.toPath().resolve(
+JarArchiveTest.classpath.getName()))
+.toFile();
+
+urls[0] = new URL("jar:" + exclamated.toURI().toURL() + "!/");
+
+try(JarArchive jar = new JarArchive(new URLClassLoader(urls),
urls[0])){
+
+Assert.assertEquals(String.format("Muzz successfully open '%s'",
exclamated.getAbsolutePath()),
+this.archive.iterator().hasNext(),
+jar.iterator().hasNext());
+}
+
+
+// Unsupported protocols stack
+
+urls[0] = new URL("http:ftp:jar:" + exclamated.toURI().toURL() + "!/");
+
+try(JarArchive jar = new JarArchive(new URLClassLoader(urls),
urls[0])){
+Assert.fail(String.format("Muzz eat only local file URLs:"
++ " 'file:/...' or 'jar:file:/...!/' but not '%s'",
+urls[0]));
+}catch(UnsupportedOperationException ex){
Review Comment:
it's my mistake, definitely IllegalArgumentException should be instead of
UnsupportedOperationException.
the test fails with UnsupportedOperationException, i wonder how i could miss
it.
guys, my project configuration is now broken. can i kindly ask you to
include this little fix into your commits?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@geronimo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [geronimo-xbean] powerbroker commented on a diff in pull request #34: XBEAN-337 jar path with exclamation signs(!) should be resolved correctly
powerbroker commented on code in PR #34: URL: https://github.com/apache/geronimo-xbean/pull/34#discussion_r1056038202 ## xbean-finder/src/test/java/org/apache/xbean/finder/archive/JarArchiveTest.java: ## @@ -16,15 +16,10 @@ */ package org.apache.xbean.finder.archive; -import org.acme.foo.Blue; Review Comment: in general, everything is correct, impossible to disagree. but in particular we are dealing with extremely entertaining moments. such as lack of a clear style definition in the project and need to fall back to manual labor in already well automated tasks. would like to hope the imports section is the only obstacle preventing the fix from being accomplished -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@geronimo.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geronimo-xbean] powerbroker commented on a diff in pull request #34: XBEAN-337 jar path with exclamation signs(!) should be resolved correctly
powerbroker commented on code in PR #34: URL: https://github.com/apache/geronimo-xbean/pull/34#discussion_r1056038202 ## xbean-finder/src/test/java/org/apache/xbean/finder/archive/JarArchiveTest.java: ## @@ -16,15 +16,10 @@ */ package org.apache.xbean.finder.archive; -import org.acme.foo.Blue; Review Comment: in general, everything is correct, impossible to disagree. but in particular we are dealing with extremely entertaining moments. such as the lack of a clear style definition in the project and the need to fall back to manual labor in already well automated tasks. would like to hope the imports section is the only obstacle preventing the fix from being accomplished -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@geronimo.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geronimo-xbean] powerbroker commented on a diff in pull request #34: XBEAN-337 jar path with exclamation signs(!) should be resolved correctly
powerbroker commented on code in PR #34: URL: https://github.com/apache/geronimo-xbean/pull/34#discussion_r1056038202 ## xbean-finder/src/test/java/org/apache/xbean/finder/archive/JarArchiveTest.java: ## @@ -16,15 +16,10 @@ */ package org.apache.xbean.finder.archive; -import org.acme.foo.Blue; Review Comment: in general, everything is correct, impossible to disagree. but in particular we are dealing with extremely entertaining moments. such as the lack of a clear style definition in the project and the need to fall back to manual labor in already well automated tasks. would like to hope the imports section is the only obstacle preventing the fix is accomplished -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@geronimo.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geronimo-xbean] powerbroker commented on a diff in pull request #34: XBEAN-337 jar path with exclamation signs(!) should be resolved correctly
powerbroker commented on code in PR #34: URL: https://github.com/apache/geronimo-xbean/pull/34#discussion_r1056038202 ## xbean-finder/src/test/java/org/apache/xbean/finder/archive/JarArchiveTest.java: ## @@ -16,15 +16,10 @@ */ package org.apache.xbean.finder.archive; -import org.acme.foo.Blue; Review Comment: in general, everything is correct, impossible to disagree. but in particular we are dealing with extremely entertaining moments. such as the lack of a clear style definition in the project and the need to fall back to manual labor in already well automated tasks. would like to hope the importы section is the only obstacle preventing the fix is accomplished -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@geronimo.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geronimo-xbean] powerbroker commented on a diff in pull request #34: XBEAN-337 jar path with exclamation signs(!) should be resolved correctly
powerbroker commented on code in PR #34: URL: https://github.com/apache/geronimo-xbean/pull/34#discussion_r1056038202 ## xbean-finder/src/test/java/org/apache/xbean/finder/archive/JarArchiveTest.java: ## @@ -16,15 +16,10 @@ */ package org.apache.xbean.finder.archive; -import org.acme.foo.Blue; Review Comment: in general, everything is correct, impossible to disagree. but in particular we are dealing with extremely entertaining moments. such as the lack of a clear style definition in the project and the need to roll back to manual labor in already well automated tasks. would like to hope the importы section is the only obstacle preventing the fix is accomplished -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@geronimo.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geronimo-xbean] powerbroker commented on a diff in pull request #34: XBEAN-337 jar path with exclamation signs(!) should be resolved correctly
powerbroker commented on code in PR #34: URL: https://github.com/apache/geronimo-xbean/pull/34#discussion_r1055292190 ## xbean-finder/src/test/java/org/apache/xbean/finder/archive/JarArchiveTest.java: ## @@ -16,15 +16,10 @@ */ package org.apache.xbean.finder.archive; -import org.acme.foo.Blue; Review Comment: but what are we fighting for in this place, for what values? in the main code it is clear: the shorter changes are, the faster they will be reviewed and comprehended. and here - do we save space in repository on the size of patches? in addition, the IDE sorts imports alphabetically. from my side, even if it happens from time to time it's more good than bad. p.s. can we discuss project structure by e-mail? spamcollector_@t_yandex.ru, if you don't mind -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@geronimo.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geronimo-xbean] powerbroker commented on a diff in pull request #34: XBEAN-337 jar path with exclamation signs(!) should be resolved correctly
powerbroker commented on code in PR #34: URL: https://github.com/apache/geronimo-xbean/pull/34#discussion_r1053962119 ## xbean-finder/src/test/java/org/apache/xbean/finder/archive/JarArchiveTest.java: ## @@ -16,15 +16,10 @@ */ package org.apache.xbean.finder.archive; -import org.acme.foo.Blue; Review Comment: and here I would try to understand and forgive - all removed imports were 'unused' -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@geronimo.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geronimo-xbean] powerbroker commented on a diff in pull request #34: XBEAN-337 jar path with exclamation signs(!) should be resolved correctly
powerbroker commented on code in PR #34:
URL: https://github.com/apache/geronimo-xbean/pull/34#discussion_r1053961070
##
xbean-finder/src/main/java/org/apache/xbean/finder/archive/JarArchive.java:
##
@@ -41,22 +44,65 @@ public class JarArchive implements Archive, AutoCloseable {
private final JarFile jar;
private final MJarSupport mjar = new MJarSupport();
-public JarArchive(ClassLoader loader, URL url) {
+/*
+ * Supports only 'file:/...' or 'jar:file:/...!/' URLs
+ */
+public JarArchive(ClassLoader loader, URL url){
//if (!"jar".equals(url.getProtocol())) throw new
IllegalArgumentException("not a jar url: " + url);
-try {
-this.loader = loader;
-this.url = url;
-URL u = url;
+this.loader = loader;
+this.url = url;
+File jarFile = null;
+String jarPath;
+int idx;
-String jarPath = url.getFile();
-if (jarPath.contains("!")) {
-jarPath = jarPath.substring(0, jarPath.indexOf("!"));
-u = new URL(jarPath);
+// Wipe out 'jar:' prefix AND '!/{...}' suffix(if any)
+if("jar".equalsIgnoreCase(url.getProtocol())){
+
+try{
+jarPath = url.getPath();
+url = new URL(jarPath.endsWith("!/") ?
+jarPath.substring(0, jarPath.lastIndexOf("!/"))
+: jarPath);
+}catch(MalformedURLException ex){
+throw new UnsupportedOperationException(
Review Comment:
ok
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@geronimo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [geronimo-xbean] powerbroker commented on a diff in pull request #34: XBEAN-337 jar path with exclamation signs(!) should be resolved correctly
powerbroker commented on code in PR #34:
URL: https://github.com/apache/geronimo-xbean/pull/34#discussion_r1053960992
##
xbean-finder/src/test/java/org/apache/xbean/finder/archive/JarArchiveTest.java:
##
@@ -80,7 +78,7 @@ public void testGetBytecode() throws Exception {
try {
archive.getBytecode("Fake");
-fail("ClassNotFoundException should have been thrown");
+Assert.fail("ClassNotFoundException should have been thrown");
Review Comment:
yep))
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@geronimo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [geronimo-xbean] powerbroker commented on a diff in pull request #34: XBEAN-337 jar path with exclamation signs(!) should be resolved correctly
powerbroker commented on code in PR #34:
URL: https://github.com/apache/geronimo-xbean/pull/34#discussion_r1052682163
##
xbean-finder/src/test/java/org/apache/xbean/finder/archive/JarArchiveTest.java:
##
@@ -95,14 +96,65 @@ public void testIterator() throws Exception {
actual.add(entry.getName());
}
-assertFalse(0 == actual.size());
+Assert.assertFalse(0 == actual.size());
for (Class clazz : classes) {
-assertTrue(clazz.getName(), actual.contains(clazz.getName()));
+Assert.assertTrue(clazz.getName(),
actual.contains(clazz.getName()));
}
-assertEquals(classes.length, actual.size());
+Assert.assertEquals(JarArchiveTest.classes.length, actual.size());
}
+@Test
+public void testXBEAN337() throws Exception {
Review Comment:
> Just pushed on trunk branch with `@Ignore("PR-34")`.
thanks. thanks a lot!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@geronimo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [geronimo-xbean] powerbroker commented on a diff in pull request #34: XBEAN-337 jar path with exclamation signs(!) should be resolved correctly
powerbroker commented on code in PR #34:
URL: https://github.com/apache/geronimo-xbean/pull/34#discussion_r1051587029
##
xbean-finder/src/test/java/org/apache/xbean/finder/archive/JarArchiveTest.java:
##
@@ -95,14 +96,65 @@ public void testIterator() throws Exception {
actual.add(entry.getName());
}
-assertFalse(0 == actual.size());
+Assert.assertFalse(0 == actual.size());
for (Class clazz : classes) {
-assertTrue(clazz.getName(), actual.contains(clazz.getName()));
+Assert.assertTrue(clazz.getName(),
actual.contains(clazz.getName()));
}
-assertEquals(classes.length, actual.size());
+Assert.assertEquals(JarArchiveTest.classes.length, actual.size());
}
+@Test
+public void testXBEAN337() throws Exception {
Review Comment:
well, what is easier for you - disabled on main branch so i sync, merge
locally, enable them and run; or to this branch, as enabled.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@geronimo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [geronimo-xbean] powerbroker commented on a diff in pull request #34: XBEAN-337 jar path with exclamation signs(!) should be resolved correctly
powerbroker commented on code in PR #34:
URL: https://github.com/apache/geronimo-xbean/pull/34#discussion_r1051486174
##
xbean-finder/src/test/java/org/apache/xbean/finder/archive/JarArchiveTest.java:
##
@@ -95,14 +96,65 @@ public void testIterator() throws Exception {
actual.add(entry.getName());
}
-assertFalse(0 == actual.size());
+Assert.assertFalse(0 == actual.size());
for (Class clazz : classes) {
-assertTrue(clazz.getName(), actual.contains(clazz.getName()));
+Assert.assertTrue(clazz.getName(),
actual.contains(clazz.getName()));
}
-assertEquals(classes.length, actual.size());
+Assert.assertEquals(JarArchiveTest.classes.length, actual.size());
}
+@Test
+public void testXBEAN337() throws Exception {
Review Comment:
thanks, they helped to find/fix 1 issue.
can i ask you to integrate them?
cannot force my git to apply patch, so have to do it manually. and afraid it
will turn in too much cosmetics to fix...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@geronimo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [geronimo-xbean] powerbroker commented on a diff in pull request #34: XBEAN-337 jar path with exclamation signs(!) should be resolved correctly
powerbroker commented on code in PR #34:
URL: https://github.com/apache/geronimo-xbean/pull/34#discussion_r1051477609
##
xbean-finder/src/main/java/org/apache/xbean/finder/archive/JarArchive.java:
##
@@ -41,22 +44,63 @@ public class JarArchive implements Archive, AutoCloseable {
private final JarFile jar;
private final MJarSupport mjar = new MJarSupport();
-public JarArchive(ClassLoader loader, URL url) {
+/*
+ * Supports only 'file:/...' or 'jar:file:/...!/' URLs
+ */
+public JarArchive(ClassLoader loader, URL url){
//if (!"jar".equals(url.getProtocol())) throw new
IllegalArgumentException("not a jar url: " + url);
-try {
-this.loader = loader;
-this.url = url;
-URL u = url;
+this.loader = loader;
+this.url = url;
+File jarFile = null;
+String jarPath;
+int idx;
-String jarPath = url.getFile();
-if (jarPath.contains("!")) {
-jarPath = jarPath.substring(0, jarPath.indexOf("!"));
-u = new URL(jarPath);
+// Wipe out 'jar:' prefix AND '!/{...}' suffix(if any)
+if("jar".equalsIgnoreCase(url.getProtocol())){
+
+try{
+jarPath = url.getPath();
+url = new URL(jarPath.substring(0, jarPath.lastIndexOf("!/")));
+}catch(MalformedURLException ex){
+// Probably CPU overheat and/or DRAM undervoltage
Review Comment:
agree in general, and specifically what kind of RuntimeException it will i
don't care. i don't like the exception zoo, leading in real life to try/multi
catch for logging one OhHuck$#itHappened error message
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@geronimo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [geronimo-xbean] powerbroker commented on a diff in pull request #34: XBEAN-337 jar path with exclamation signs(!) should be resolved correctly
powerbroker commented on code in PR #34:
URL: https://github.com/apache/geronimo-xbean/pull/34#discussion_r1051477609
##
xbean-finder/src/main/java/org/apache/xbean/finder/archive/JarArchive.java:
##
@@ -41,22 +44,63 @@ public class JarArchive implements Archive, AutoCloseable {
private final JarFile jar;
private final MJarSupport mjar = new MJarSupport();
-public JarArchive(ClassLoader loader, URL url) {
+/*
+ * Supports only 'file:/...' or 'jar:file:/...!/' URLs
+ */
+public JarArchive(ClassLoader loader, URL url){
//if (!"jar".equals(url.getProtocol())) throw new
IllegalArgumentException("not a jar url: " + url);
-try {
-this.loader = loader;
-this.url = url;
-URL u = url;
+this.loader = loader;
+this.url = url;
+File jarFile = null;
+String jarPath;
+int idx;
-String jarPath = url.getFile();
-if (jarPath.contains("!")) {
-jarPath = jarPath.substring(0, jarPath.indexOf("!"));
-u = new URL(jarPath);
+// Wipe out 'jar:' prefix AND '!/{...}' suffix(if any)
+if("jar".equalsIgnoreCase(url.getProtocol())){
+
+try{
+jarPath = url.getPath();
+url = new URL(jarPath.substring(0, jarPath.lastIndexOf("!/")));
+}catch(MalformedURLException ex){
+// Probably CPU overheat and/or DRAM undervoltage
Review Comment:
agree in general, and specifically what kind of RuntimeException it will i
don't care. i don't like the exception zoo, leading to try/multi catch leading
in real life to logging one OhHuck$#itHappened error message
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@geronimo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [geronimo-xbean] powerbroker commented on a diff in pull request #34: XBEAN-337 jar path with exclamation signs(!) should be resolved correctly
powerbroker commented on code in PR #34:
URL: https://github.com/apache/geronimo-xbean/pull/34#discussion_r1051477609
##
xbean-finder/src/main/java/org/apache/xbean/finder/archive/JarArchive.java:
##
@@ -41,22 +44,63 @@ public class JarArchive implements Archive, AutoCloseable {
private final JarFile jar;
private final MJarSupport mjar = new MJarSupport();
-public JarArchive(ClassLoader loader, URL url) {
+/*
+ * Supports only 'file:/...' or 'jar:file:/...!/' URLs
+ */
+public JarArchive(ClassLoader loader, URL url){
//if (!"jar".equals(url.getProtocol())) throw new
IllegalArgumentException("not a jar url: " + url);
-try {
-this.loader = loader;
-this.url = url;
-URL u = url;
+this.loader = loader;
+this.url = url;
+File jarFile = null;
+String jarPath;
+int idx;
-String jarPath = url.getFile();
-if (jarPath.contains("!")) {
-jarPath = jarPath.substring(0, jarPath.indexOf("!"));
-u = new URL(jarPath);
+// Wipe out 'jar:' prefix AND '!/{...}' suffix(if any)
+if("jar".equalsIgnoreCase(url.getProtocol())){
+
+try{
+jarPath = url.getPath();
+url = new URL(jarPath.substring(0, jarPath.lastIndexOf("!/")));
+}catch(MalformedURLException ex){
+// Probably CPU overheat and/or DRAM undervoltage
Review Comment:
agree in general, and specifically what kind of RuntimeException it will i
don't care
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@geronimo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [geronimo-xbean] powerbroker commented on a diff in pull request #34: XBEAN-337 jar path with exclamation signs(!) should be resolved correctly
powerbroker commented on code in PR #34:
URL: https://github.com/apache/geronimo-xbean/pull/34#discussion_r1051476696
##
xbean-finder/src/main/java/org/apache/xbean/finder/archive/JarArchive.java:
##
@@ -41,22 +44,63 @@ public class JarArchive implements Archive, AutoCloseable {
private final JarFile jar;
private final MJarSupport mjar = new MJarSupport();
-public JarArchive(ClassLoader loader, URL url) {
+/*
+ * Supports only 'file:/...' or 'jar:file:/...!/' URLs
+ */
+public JarArchive(ClassLoader loader, URL url){
//if (!"jar".equals(url.getProtocol())) throw new
IllegalArgumentException("not a jar url: " + url);
-try {
-this.loader = loader;
-this.url = url;
-URL u = url;
+this.loader = loader;
+this.url = url;
+File jarFile = null;
+String jarPath;
+int idx;
-String jarPath = url.getFile();
-if (jarPath.contains("!")) {
-jarPath = jarPath.substring(0, jarPath.indexOf("!"));
-u = new URL(jarPath);
+// Wipe out 'jar:' prefix AND '!/{...}' suffix(if any)
+if("jar".equalsIgnoreCase(url.getProtocol())){
+
+try{
+jarPath = url.getPath();
+url = new URL(jarPath.substring(0, jarPath.lastIndexOf("!/")));
Review Comment:
so be it. I almost look only at the JarArchive code and its unit test
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@geronimo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [geronimo-xbean] powerbroker commented on a diff in pull request #34: XBEAN-337 jar path with exclamation signs(!) should be resolved correctly
powerbroker commented on code in PR #34:
URL: https://github.com/apache/geronimo-xbean/pull/34#discussion_r1051448994
##
xbean-finder/src/main/java/org/apache/xbean/finder/archive/JarArchive.java:
##
@@ -41,22 +44,63 @@ public class JarArchive implements Archive, AutoCloseable {
private final JarFile jar;
private final MJarSupport mjar = new MJarSupport();
-public JarArchive(ClassLoader loader, URL url) {
+/*
+ * Supports only 'file:/...' or 'jar:file:/...!/' URLs
+ */
+public JarArchive(ClassLoader loader, URL url){
//if (!"jar".equals(url.getProtocol())) throw new
IllegalArgumentException("not a jar url: " + url);
-try {
-this.loader = loader;
-this.url = url;
-URL u = url;
+this.loader = loader;
+this.url = url;
+File jarFile = null;
+String jarPath;
+int idx;
-String jarPath = url.getFile();
-if (jarPath.contains("!")) {
-jarPath = jarPath.substring(0, jarPath.indexOf("!"));
-u = new URL(jarPath);
+// Wipe out 'jar:' prefix AND '!/{...}' suffix(if any)
+if("jar".equalsIgnoreCase(url.getProtocol())){
+
+try{
+jarPath = url.getPath();
+url = new URL(jarPath.substring(0, jarPath.lastIndexOf("!/")));
Review Comment:
absolutely have no idea why it should be indexOf. at the beginning of story
single-place change 'indexOf -> lastIndexOf' was fixing the issue. test passes
with lastIndexOf, as well as with indexOf with some modifications...
well, if you so want indexOf - here it is.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@geronimo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [geronimo-xbean] powerbroker commented on a diff in pull request #34: XBEAN-337 jar path with exclamation signs(!) should be resolved correctly
powerbroker commented on code in PR #34:
URL: https://github.com/apache/geronimo-xbean/pull/34#discussion_r1051445155
##
xbean-finder/src/test/java/org/apache/xbean/finder/archive/JarArchiveTest.java:
##
@@ -95,14 +96,65 @@ public void testIterator() throws Exception {
actual.add(entry.getName());
}
-assertFalse(0 == actual.size());
+Assert.assertFalse(0 == actual.size());
for (Class clazz : classes) {
-assertTrue(clazz.getName(), actual.contains(clazz.getName()));
+Assert.assertTrue(clazz.getName(),
actual.contains(clazz.getName()));
}
-assertEquals(classes.length, actual.size());
+Assert.assertEquals(JarArchiveTest.classes.length, actual.size());
}
+@Test
+public void testXBEAN337() throws Exception {
Review Comment:
well, i don't completely understand where i should look to... looking into
pulled tests in branch and see nothing.
could you please point?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@geronimo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [geronimo-xbean] powerbroker commented on a diff in pull request #34: XBEAN-337 jar path with exclamation signs(!) should be resolved correctly
powerbroker commented on code in PR #34:
URL: https://github.com/apache/geronimo-xbean/pull/34#discussion_r1051444831
##
xbean-finder/src/main/java/org/apache/xbean/finder/archive/JarArchive.java:
##
@@ -41,22 +44,63 @@ public class JarArchive implements Archive, AutoCloseable {
private final JarFile jar;
private final MJarSupport mjar = new MJarSupport();
-public JarArchive(ClassLoader loader, URL url) {
+/*
+ * Supports only 'file:/...' or 'jar:file:/...!/' URLs
+ */
+public JarArchive(ClassLoader loader, URL url){
//if (!"jar".equals(url.getProtocol())) throw new
IllegalArgumentException("not a jar url: " + url);
-try {
-this.loader = loader;
-this.url = url;
-URL u = url;
+this.loader = loader;
+this.url = url;
+File jarFile = null;
+String jarPath;
+int idx;
-String jarPath = url.getFile();
-if (jarPath.contains("!")) {
-jarPath = jarPath.substring(0, jarPath.indexOf("!"));
-u = new URL(jarPath);
+// Wipe out 'jar:' prefix AND '!/{...}' suffix(if any)
+if("jar".equalsIgnoreCase(url.getProtocol())){
+
+try{
+jarPath = url.getPath();
+url = new URL(jarPath.substring(0, jarPath.lastIndexOf("!/")));
+}catch(MalformedURLException ex){
+// Probably CPU overheat and/or DRAM undervoltage
Review Comment:
i think since test for XBEAN-337 passes, it is time to close it as fixed.
doing some cosmetic cleanup, if necessary
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@geronimo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [geronimo-xbean] powerbroker commented on a diff in pull request #34: XBEAN-337 jar path with exclamation signs(!) should be resolved correctly
powerbroker commented on code in PR #34:
URL: https://github.com/apache/geronimo-xbean/pull/34#discussion_r1051444664
##
xbean-finder/src/test/java/org/apache/xbean/finder/archive/JarArchiveTest.java:
##
@@ -63,12 +64,12 @@ public void setUp() throws Exception {
public void testGetBytecode() throws Exception {
for (Class clazz : classes) {
-assertNotNull(clazz.getName(),
archive.getBytecode(clazz.getName()));
+Assert.assertNotNull(clazz.getName(),
archive.getBytecode(clazz.getName()));
Review Comment:
there is a setting in my ide "Alter ONLY manually edited code", and seems it
doesn't work as i expect.
btw is there an Eclipse config file with code style settings for this
project?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@geronimo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [geronimo-xbean] powerbroker commented on a diff in pull request #34: XBEAN-337 jar path with exclamation signs(!) should be resolved correctly
powerbroker commented on code in PR #34:
URL: https://github.com/apache/geronimo-xbean/pull/34#discussion_r1051442994
##
xbean-finder/src/test/java/org/apache/xbean/finder/archive/JarArchiveTest.java:
##
@@ -63,12 +64,12 @@ public void setUp() throws Exception {
public void testGetBytecode() throws Exception {
for (Class clazz : classes) {
-assertNotNull(clazz.getName(),
archive.getBytecode(clazz.getName()));
+Assert.assertNotNull(clazz.getName(),
archive.getBytecode(clazz.getName()));
Review Comment:
this would be single line without 'Assert.' prefix in whole test, let us
leave it as is
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@geronimo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [geronimo-xbean] powerbroker commented on a diff in pull request #34: XBEAN-337 jar path with exclamation signs(!) should be resolved correctly
powerbroker commented on code in PR #34:
URL: https://github.com/apache/geronimo-xbean/pull/34#discussion_r1051440953
##
xbean-finder/src/test/java/org/apache/xbean/finder/archive/JarArchiveTest.java:
##
@@ -95,14 +96,65 @@ public void testIterator() throws Exception {
actual.add(entry.getName());
}
-assertFalse(0 == actual.size());
+Assert.assertFalse(0 == actual.size());
for (Class clazz : classes) {
-assertTrue(clazz.getName(), actual.contains(clazz.getName()));
+Assert.assertTrue(clazz.getName(),
actual.contains(clazz.getName()));
}
-assertEquals(classes.length, actual.size());
+Assert.assertEquals(JarArchiveTest.classes.length, actual.size());
}
+@Test
+public void testXBEAN337() throws Exception {
+
+
+// Virtual path
+
+JarArchive jar;
+String path = "/this!/!file!/does!/!not/exist.jar";
+URL[] urls = {new URL("jar:file:" + path +
"!/some!/!inner!/.jar!/file.jar")};
+
+try {
+jar = new JarArchive(new URLClassLoader(urls), urls[0]);
Review Comment:
thx
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@geronimo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [geronimo-xbean] powerbroker commented on a diff in pull request #34: XBEAN-337 jar path with exclamation signs(!) should be resolved correctly
powerbroker commented on code in PR #34:
URL: https://github.com/apache/geronimo-xbean/pull/34#discussion_r1051439651
##
xbean-finder/src/main/java/org/apache/xbean/finder/archive/JarArchive.java:
##
@@ -41,22 +44,63 @@ public class JarArchive implements Archive, AutoCloseable {
private final JarFile jar;
private final MJarSupport mjar = new MJarSupport();
-public JarArchive(ClassLoader loader, URL url) {
+/*
+ * Supports only 'file:/...' or 'jar:file:/...!/' URLs
+ */
+public JarArchive(ClassLoader loader, URL url){
//if (!"jar".equals(url.getProtocol())) throw new
IllegalArgumentException("not a jar url: " + url);
-try {
-this.loader = loader;
-this.url = url;
-URL u = url;
+this.loader = loader;
+this.url = url;
+File jarFile = null;
+String jarPath;
+int idx;
-String jarPath = url.getFile();
-if (jarPath.contains("!")) {
-jarPath = jarPath.substring(0, jarPath.indexOf("!"));
-u = new URL(jarPath);
+// Wipe out 'jar:' prefix AND '!/{...}' suffix(if any)
+if("jar".equalsIgnoreCase(url.getProtocol())){
+
+try{
+jarPath = url.getPath();
+url = new URL(jarPath.substring(0, jarPath.lastIndexOf("!/")));
+}catch(MalformedURLException ex){
+// Probably CPU overheat and/or DRAM undervoltage
Review Comment:
trying and trying - with no success... in THIS CASE i think just a contract
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@geronimo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [geronimo-xbean] powerbroker commented on a diff in pull request #34: XBEAN-337 jar path with exclamation signs(!) should be resolved correctly
powerbroker commented on code in PR #34:
URL: https://github.com/apache/geronimo-xbean/pull/34#discussion_r1051438054
##
xbean-finder/src/main/java/org/apache/xbean/finder/archive/JarArchive.java:
##
@@ -41,22 +44,63 @@ public class JarArchive implements Archive, AutoCloseable {
private final JarFile jar;
private final MJarSupport mjar = new MJarSupport();
-public JarArchive(ClassLoader loader, URL url) {
+/*
+ * Supports only 'file:/...' or 'jar:file:/...!/' URLs
+ */
+public JarArchive(ClassLoader loader, URL url){
//if (!"jar".equals(url.getProtocol())) throw new
IllegalArgumentException("not a jar url: " + url);
-try {
-this.loader = loader;
-this.url = url;
-URL u = url;
+this.loader = loader;
+this.url = url;
+File jarFile = null;
+String jarPath;
+int idx;
-String jarPath = url.getFile();
-if (jarPath.contains("!")) {
-jarPath = jarPath.substring(0, jarPath.indexOf("!"));
-u = new URL(jarPath);
+// Wipe out 'jar:' prefix AND '!/{...}' suffix(if any)
+if("jar".equalsIgnoreCase(url.getProtocol())){
+
+try{
+jarPath = url.getPath();
+url = new URL(jarPath.substring(0, jarPath.lastIndexOf("!/")));
+}catch(MalformedURLException ex){
+// Probably CPU overheat and/or DRAM undervoltage
+throw new UnsupportedOperationException(
+"Please provide 'file:/...' or 'jar:file:/...!/' URL"
++ " instead of '" +
FileArchive.decode(String.valueOf(url)) + "'");
}
-jar = new JarFile(FileArchive.decode(u.getFile())); // no more an
url
-} catch (IOException e) {
-throw new IllegalStateException(e);
+}
+
+try{
+// handle 'file:/...' URL
+if("file".equalsIgnoreCase(url.getProtocol())){
+
+// Testing if file DOEN't exists AND cutting !/{...}'
+// suffix-by-suffix until file discovered or run out of
suffixes
+for(jarPath = FileArchive.decode(url.getPath());
+!(jarFile = new File(jarPath)).exists()
+&& (idx = jarPath.lastIndexOf("!/")) > 0;
+jarPath = jarPath.substring(0, idx)){ }
+
+// No more suffixes to cut, but referenced file wasn't
discovered
+if(!jarFile.exists()){
+
+// To be caught later and wrapped into IllegalStateEx -
default behavior
+throw new
FileNotFoundException(FileArchive.decode(String.valueOf(url)));
+}
+
+}else{
+throw new UnsupportedOperationException(
+"Please provide 'file:/...' or 'jar:file:/...!/' URL"
++ " instead of '" +
FileArchive.decode(String.valueOf(url)) + "'");
+}
+
+jar = new JarFile(jarFile);
+
+}catch(IOException e){
+throw new IllegalStateException("Cannot open jar(zip) '"
++ jarFile != null ? // why can it be null? but since
compiler thinks so...
Review Comment:
ok, it knows better
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@geronimo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [geronimo-xbean] powerbroker commented on a diff in pull request #34: XBEAN-337 jar path with exclamation signs(!) should be resolved correctly
powerbroker commented on code in PR #34:
URL: https://github.com/apache/geronimo-xbean/pull/34#discussion_r1045314578
##
xbean-finder/src/main/java/org/apache/xbean/finder/archive/JarArchive.java:
##
@@ -41,22 +43,52 @@ public class JarArchive implements Archive {
private final JarFile jar;
private final MJarSupport mjar = new MJarSupport();
-public JarArchive(ClassLoader loader, URL url) {
+public JarArchive(ClassLoader loader, URL url){
//if (!"jar".equals(url.getProtocol())) throw new
IllegalArgumentException("not a jar url: " + url);
+this.loader = loader;
+this.url = url;
+File jf;
+int idx;
+
+/*
+ * True URL.getPath(...) wiping out protocol prefixes:
+ *
+ * 1. 'jar:file:/c:/temp/...' -> 'jar' + 'file:/c:/temp/...'
+ * 2. 'file:/c:/temp/...' -> 'file' + '/c:/temp/...'
+ *
+ * assuming we accept 'file:/...${xxx.jar}!/' URLs
+ * AND 'zip:file:/...!/' would be cool too, if
+ * allowed by new URL(...)
+ */
try {
-this.loader = loader;
-this.url = url;
-URL u = url;
-
-String jarPath = url.getFile();
-if (jarPath.contains("!")) {
-jarPath = jarPath.substring(0, jarPath.lastIndexOf("!"));
-u = new URL(jarPath);
+// Underlying JarFile(...) requires RandomAccessFile,
+// so only local file URLs here...
+while(!"file".equalsIgnoreCase(url.getProtocol())) {
+// no need here in .getQuery() tail, appended by getFile()
+url = new URL(url.getPath());
}
-jar = new JarFile(FileArchive.decode(u.getFile())); // no more an
url
-} catch (IOException e) {
-throw new IllegalStateException(e);
+}catch(MalformedURLException ex){
+// No more protocol(s) in path
+throw new UnsupportedOperationException("Please provide
'file:/...' or 'jar:file:/...!/' URL");
+}
+
+// TODO: drop FileArchive.decode?
+// URLDecoder.decode(url.getPath(), "UTF-8");
+String jarPath = FileArchive.decode(url.getPath());
+
+while(!(jf = new File(jarPath)).exists()){
+if((idx = jarPath.lastIndexOf('!')) > 0){
Review Comment:
thought and thought how to start the check from the beginning, and not from
the tail ... and finally didn’t come up with anything valuable. cannot even
guess, when trash-prefixes may appear after protocol name and before path
substring...
IMHO this little *internal* difference seems invisible from user API level.
code works, there are some tests proving - let us leave concept as is. anyway,
the if-file-exists check is fundamental here, not suffixes-cutting direction
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@geronimo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [geronimo-xbean] powerbroker commented on a diff in pull request #34: XBEAN-337 jar path with exclamation signs(!) should be resolved correctly
powerbroker commented on code in PR #34:
URL: https://github.com/apache/geronimo-xbean/pull/34#discussion_r1045307959
##
xbean-finder/src/main/java/org/apache/xbean/finder/archive/JarArchive.java:
##
@@ -41,22 +43,52 @@ public class JarArchive implements Archive {
private final JarFile jar;
private final MJarSupport mjar = new MJarSupport();
-public JarArchive(ClassLoader loader, URL url) {
+public JarArchive(ClassLoader loader, URL url){
//if (!"jar".equals(url.getProtocol())) throw new
IllegalArgumentException("not a jar url: " + url);
+this.loader = loader;
+this.url = url;
+File jf;
+int idx;
+
+/*
+ * True URL.getPath(...) wiping out protocol prefixes:
+ *
+ * 1. 'jar:file:/c:/temp/...' -> 'jar' + 'file:/c:/temp/...'
+ * 2. 'file:/c:/temp/...' -> 'file' + '/c:/temp/...'
+ *
+ * assuming we accept 'file:/...${xxx.jar}!/' URLs
+ * AND 'zip:file:/...!/' would be cool too, if
+ * allowed by new URL(...)
+ */
try {
-this.loader = loader;
-this.url = url;
-URL u = url;
-
-String jarPath = url.getFile();
-if (jarPath.contains("!")) {
-jarPath = jarPath.substring(0, jarPath.lastIndexOf("!"));
-u = new URL(jarPath);
+// Underlying JarFile(...) requires RandomAccessFile,
+// so only local file URLs here...
+while(!"file".equalsIgnoreCase(url.getProtocol())) {
Review Comment:
> > jar:foo:file is actually prohibited by new URL(), why not allow this
impossible scenario?
>
> Prohibited by who? Thing is that we actually only support `jar` and
`file`, now you allow `jar:(whatever:)*file` - this is allowed by `URL` and
`URLHandler` API. Don't think it is what we want so we should just unwrap once
or let the caller unwrap it more before passing the path to the archive IMHO.
i cannot do e.g. new URL("jar:foa:file:/c:/Temp"), but - wow - can new
URL("http:rtsp:ftp:jar:file:/c:/Temp")!
that's technically bullshit, but you're right - possible.
btw, absolhuck-o-lutely have no idea why don't URL.getProtocol() just say
once "jar:file" instead of nesting URLs. or, if nesting, not to be e.g.
"file:/usr/lib.jar:jar:/inner/file.txt"!
> Nested jar support is done through other `Archive` implementation like
https://github.com/Talend/component-runtime/blob/master/component-runtime-manager/src/main/java/org/talend/sdk/component/runtime/manager/xbean/NestedJarArchive.java
but it requires some knowledge about the enclosing structure so better to not
try to be too clever there IMHO.
yep, too clever with restrictions as well ;) can't shake the feeling that
this is what I'm doing...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@geronimo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [geronimo-xbean] powerbroker commented on a diff in pull request #34: XBEAN-337 jar path with exclamation signs(!) should be resolved correctly
powerbroker commented on code in PR #34:
URL: https://github.com/apache/geronimo-xbean/pull/34#discussion_r1045307959
##
xbean-finder/src/main/java/org/apache/xbean/finder/archive/JarArchive.java:
##
@@ -41,22 +43,52 @@ public class JarArchive implements Archive {
private final JarFile jar;
private final MJarSupport mjar = new MJarSupport();
-public JarArchive(ClassLoader loader, URL url) {
+public JarArchive(ClassLoader loader, URL url){
//if (!"jar".equals(url.getProtocol())) throw new
IllegalArgumentException("not a jar url: " + url);
+this.loader = loader;
+this.url = url;
+File jf;
+int idx;
+
+/*
+ * True URL.getPath(...) wiping out protocol prefixes:
+ *
+ * 1. 'jar:file:/c:/temp/...' -> 'jar' + 'file:/c:/temp/...'
+ * 2. 'file:/c:/temp/...' -> 'file' + '/c:/temp/...'
+ *
+ * assuming we accept 'file:/...${xxx.jar}!/' URLs
+ * AND 'zip:file:/...!/' would be cool too, if
+ * allowed by new URL(...)
+ */
try {
-this.loader = loader;
-this.url = url;
-URL u = url;
-
-String jarPath = url.getFile();
-if (jarPath.contains("!")) {
-jarPath = jarPath.substring(0, jarPath.lastIndexOf("!"));
-u = new URL(jarPath);
+// Underlying JarFile(...) requires RandomAccessFile,
+// so only local file URLs here...
+while(!"file".equalsIgnoreCase(url.getProtocol())) {
Review Comment:
> > jar:foo:file is actually prohibited by new URL(), why not allow this
impossible scenario?
>
> Prohibited by who? Thing is that we actually only support `jar` and
`file`, now you allow `jar:(whatever:)*file` - this is allowed by `URL` and
`URLHandler` API. Don't think it is what we want so we should just unwrap once
or let the caller unwrap it more before passing the path to the archive IMHO.
i cannot do e.g. new URL("jar:foa:file:/c:/Temp"), but - wow - can new
URL("http:rtsp:ftp:jar:file:/c:/Temp")!
that's technically bullshit, but you're right - possible.
btw, absolhuck-o-lutely have no idea why don't URL.getProtocol() just say
once "jar:file" instead of nesting URLs. or, if nesting, not to be e.g.
"file:/usr/lib.jar:jar:/inner/file.txt"!
> Nested jar support is done through other `Archive` implementation like
https://github.com/Talend/component-runtime/blob/master/component-runtime-manager/src/main/java/org/talend/sdk/component/runtime/manager/xbean/NestedJarArchive.java
but it requires some knowledge about the enclosing structure so better to not
try to be too clever there IMHO.
yep, too clever with restrictions as well ;) can't shake the feeling that
this is what I'm doing...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@geronimo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [geronimo-xbean] powerbroker commented on a diff in pull request #34: XBEAN-337 jar path with exclamation signs(!) should be resolved correctly
powerbroker commented on code in PR #34:
URL: https://github.com/apache/geronimo-xbean/pull/34#discussion_r1045307959
##
xbean-finder/src/main/java/org/apache/xbean/finder/archive/JarArchive.java:
##
@@ -41,22 +43,52 @@ public class JarArchive implements Archive {
private final JarFile jar;
private final MJarSupport mjar = new MJarSupport();
-public JarArchive(ClassLoader loader, URL url) {
+public JarArchive(ClassLoader loader, URL url){
//if (!"jar".equals(url.getProtocol())) throw new
IllegalArgumentException("not a jar url: " + url);
+this.loader = loader;
+this.url = url;
+File jf;
+int idx;
+
+/*
+ * True URL.getPath(...) wiping out protocol prefixes:
+ *
+ * 1. 'jar:file:/c:/temp/...' -> 'jar' + 'file:/c:/temp/...'
+ * 2. 'file:/c:/temp/...' -> 'file' + '/c:/temp/...'
+ *
+ * assuming we accept 'file:/...${xxx.jar}!/' URLs
+ * AND 'zip:file:/...!/' would be cool too, if
+ * allowed by new URL(...)
+ */
try {
-this.loader = loader;
-this.url = url;
-URL u = url;
-
-String jarPath = url.getFile();
-if (jarPath.contains("!")) {
-jarPath = jarPath.substring(0, jarPath.lastIndexOf("!"));
-u = new URL(jarPath);
+// Underlying JarFile(...) requires RandomAccessFile,
+// so only local file URLs here...
+while(!"file".equalsIgnoreCase(url.getProtocol())) {
Review Comment:
> > jar:foo:file is actually prohibited by new URL(), why not allow this
impossible scenario?
>
> Prohibited by who? Thing is that we actually only support `jar` and
`file`, now you allow `jar:(whatever:)*file` - this is allowed by `URL` and
`URLHandler` API. Don't think it is what we want so we should just unwrap once
or let the caller unwrap it more before passing the path to the archive IMHO.
i cannot do e.g. new URL("jar:foa:file:/c:/Temp"), but - wow - can new
URL("http:rtsp:ftp:jar:file:/c:/Temp")!
that's technically bullshit, but you're right - possible.
btw, absolhuck-o-lutely have no idea why don't URL.getProtocol() just say
once "jar:file" instead of nesting URLs. or, if nesting, not to be e.g.
"file:/usr/lib.jar:jar:/inner/file.txt"!
> Nested jar support is done through other `Archive` implementation like
https://github.com/Talend/component-runtime/blob/master/component-runtime-manager/src/main/java/org/talend/sdk/component/runtime/manager/xbean/NestedJarArchive.java
but it requires some knowledge about the enclosing structure so better to not
try to be too clever there IMHO.
yep, too clever with restrictions as well ;) can't shake the feeling that
this is what I'm doing...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@geronimo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [geronimo-xbean] powerbroker commented on a diff in pull request #34: XBEAN-337 jar path with exclamation signs(!) should be resolved correctly
powerbroker commented on code in PR #34:
URL: https://github.com/apache/geronimo-xbean/pull/34#discussion_r1045269988
##
xbean-finder/src/main/java/org/apache/xbean/finder/archive/JarArchive.java:
##
@@ -41,22 +43,52 @@ public class JarArchive implements Archive {
private final JarFile jar;
private final MJarSupport mjar = new MJarSupport();
-public JarArchive(ClassLoader loader, URL url) {
+public JarArchive(ClassLoader loader, URL url){
//if (!"jar".equals(url.getProtocol())) throw new
IllegalArgumentException("not a jar url: " + url);
+this.loader = loader;
+this.url = url;
+File jf;
+int idx;
+
+/*
+ * True URL.getPath(...) wiping out protocol prefixes:
+ *
+ * 1. 'jar:file:/c:/temp/...' -> 'jar' + 'file:/c:/temp/...'
+ * 2. 'file:/c:/temp/...' -> 'file' + '/c:/temp/...'
+ *
+ * assuming we accept 'file:/...${xxx.jar}!/' URLs
+ * AND 'zip:file:/...!/' would be cool too, if
+ * allowed by new URL(...)
+ */
try {
-this.loader = loader;
-this.url = url;
-URL u = url;
-
-String jarPath = url.getFile();
-if (jarPath.contains("!")) {
-jarPath = jarPath.substring(0, jarPath.lastIndexOf("!"));
-u = new URL(jarPath);
+// Underlying JarFile(...) requires RandomAccessFile,
+// so only local file URLs here...
+while(!"file".equalsIgnoreCase(url.getProtocol())) {
+// no need here in .getQuery() tail, appended by getFile()
+url = new URL(url.getPath());
}
-jar = new JarFile(FileArchive.decode(u.getFile())); // no more an
url
-} catch (IOException e) {
-throw new IllegalStateException(e);
+}catch(MalformedURLException ex){
+// No more protocol(s) in path
+throw new UnsupportedOperationException("Please provide
'file:/...' or 'jar:file:/...!/' URL");
+}
+
+// TODO: drop FileArchive.decode?
+// URLDecoder.decode(url.getPath(), "UTF-8");
+String jarPath = FileArchive.decode(url.getPath());
+
+while(!(jf = new File(jarPath)).exists()){
+if((idx = jarPath.lastIndexOf('!')) > 0){
Review Comment:
what do you mean?
start from beginning FAILS, start from end SUCCEEDS. yes, success may cost
some resources, that's the price.
here we (almost) always meet ! at the end, since new URL() requires it in
'jar:file:/...!/'. so, we try access file, cut ! off, and - if everything is
near default scenario - meet available JAR file(even if it has !s in path or
not - doesn't matter) => profit! once.
things become worse when there is a tail full of !s after 'file.jar!/'.
definitely non-standard case, and each extra '!' costs new String, new File and
*extra file system access*.
if i missed something...?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@geronimo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [geronimo-xbean] powerbroker commented on a diff in pull request #34: XBEAN-337 jar path with exclamation signs(!) should be resolved correctly
powerbroker commented on code in PR #34:
URL: https://github.com/apache/geronimo-xbean/pull/34#discussion_r1045269988
##
xbean-finder/src/main/java/org/apache/xbean/finder/archive/JarArchive.java:
##
@@ -41,22 +43,52 @@ public class JarArchive implements Archive {
private final JarFile jar;
private final MJarSupport mjar = new MJarSupport();
-public JarArchive(ClassLoader loader, URL url) {
+public JarArchive(ClassLoader loader, URL url){
//if (!"jar".equals(url.getProtocol())) throw new
IllegalArgumentException("not a jar url: " + url);
+this.loader = loader;
+this.url = url;
+File jf;
+int idx;
+
+/*
+ * True URL.getPath(...) wiping out protocol prefixes:
+ *
+ * 1. 'jar:file:/c:/temp/...' -> 'jar' + 'file:/c:/temp/...'
+ * 2. 'file:/c:/temp/...' -> 'file' + '/c:/temp/...'
+ *
+ * assuming we accept 'file:/...${xxx.jar}!/' URLs
+ * AND 'zip:file:/...!/' would be cool too, if
+ * allowed by new URL(...)
+ */
try {
-this.loader = loader;
-this.url = url;
-URL u = url;
-
-String jarPath = url.getFile();
-if (jarPath.contains("!")) {
-jarPath = jarPath.substring(0, jarPath.lastIndexOf("!"));
-u = new URL(jarPath);
+// Underlying JarFile(...) requires RandomAccessFile,
+// so only local file URLs here...
+while(!"file".equalsIgnoreCase(url.getProtocol())) {
+// no need here in .getQuery() tail, appended by getFile()
+url = new URL(url.getPath());
}
-jar = new JarFile(FileArchive.decode(u.getFile())); // no more an
url
-} catch (IOException e) {
-throw new IllegalStateException(e);
+}catch(MalformedURLException ex){
+// No more protocol(s) in path
+throw new UnsupportedOperationException("Please provide
'file:/...' or 'jar:file:/...!/' URL");
+}
+
+// TODO: drop FileArchive.decode?
+// URLDecoder.decode(url.getPath(), "UTF-8");
+String jarPath = FileArchive.decode(url.getPath());
+
+while(!(jf = new File(jarPath)).exists()){
+if((idx = jarPath.lastIndexOf('!')) > 0){
Review Comment:
what do you mean?
start from beginning FAILS, start from end SUCCEEDS. yes, success may cost
some resources, that's the price.
here we (almost) always meet ! at the end, since new URL() requires it in
'jar:file:/...!/'. so, we try access file, cut ! off, and - if everything is
near default scenario - meet available JAR file(even if it has !s in path or
not - doesn't matter) => profit! once.
things become worse when there is a tail full of !s after 'file.jar!/'.
definitely non-standard case, and each extra '!' cost new String, new File and
*extra file system access*.
if i missed something...?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@geronimo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [geronimo-xbean] powerbroker commented on a diff in pull request #34: XBEAN-337 jar path with exclamation signs(!) should be resolved correctly
powerbroker commented on code in PR #34:
URL: https://github.com/apache/geronimo-xbean/pull/34#discussion_r1045269988
##
xbean-finder/src/main/java/org/apache/xbean/finder/archive/JarArchive.java:
##
@@ -41,22 +43,52 @@ public class JarArchive implements Archive {
private final JarFile jar;
private final MJarSupport mjar = new MJarSupport();
-public JarArchive(ClassLoader loader, URL url) {
+public JarArchive(ClassLoader loader, URL url){
//if (!"jar".equals(url.getProtocol())) throw new
IllegalArgumentException("not a jar url: " + url);
+this.loader = loader;
+this.url = url;
+File jf;
+int idx;
+
+/*
+ * True URL.getPath(...) wiping out protocol prefixes:
+ *
+ * 1. 'jar:file:/c:/temp/...' -> 'jar' + 'file:/c:/temp/...'
+ * 2. 'file:/c:/temp/...' -> 'file' + '/c:/temp/...'
+ *
+ * assuming we accept 'file:/...${xxx.jar}!/' URLs
+ * AND 'zip:file:/...!/' would be cool too, if
+ * allowed by new URL(...)
+ */
try {
-this.loader = loader;
-this.url = url;
-URL u = url;
-
-String jarPath = url.getFile();
-if (jarPath.contains("!")) {
-jarPath = jarPath.substring(0, jarPath.lastIndexOf("!"));
-u = new URL(jarPath);
+// Underlying JarFile(...) requires RandomAccessFile,
+// so only local file URLs here...
+while(!"file".equalsIgnoreCase(url.getProtocol())) {
+// no need here in .getQuery() tail, appended by getFile()
+url = new URL(url.getPath());
}
-jar = new JarFile(FileArchive.decode(u.getFile())); // no more an
url
-} catch (IOException e) {
-throw new IllegalStateException(e);
+}catch(MalformedURLException ex){
+// No more protocol(s) in path
+throw new UnsupportedOperationException("Please provide
'file:/...' or 'jar:file:/...!/' URL");
+}
+
+// TODO: drop FileArchive.decode?
+// URLDecoder.decode(url.getPath(), "UTF-8");
+String jarPath = FileArchive.decode(url.getPath());
+
+while(!(jf = new File(jarPath)).exists()){
+if((idx = jarPath.lastIndexOf('!')) > 0){
Review Comment:
what do you mean?
start from beginning FAILS, start from end SUCCEEDS. yes, success may cost
some resources, that's the price.
here we (almost) always meet ! at the end, since new URL() requires it in
'jar:file:/...!/'. so, we try access file, cut ! off, and - if everything is
near default scenario - meet available JAR file(even if it has !s in path or
not - doesn't matter) => profit! once.
things become worse when there is a tail full of !s after 'file.jar!/'.
definitely non-standard case, and each extra '!' cost new String, new File and
extra file system access.
if i missed something...?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@geronimo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [geronimo-xbean] powerbroker commented on a diff in pull request #34: XBEAN-337 jar path with exclamation signs(!) should be resolved correctly
powerbroker commented on code in PR #34:
URL: https://github.com/apache/geronimo-xbean/pull/34#discussion_r1045269988
##
xbean-finder/src/main/java/org/apache/xbean/finder/archive/JarArchive.java:
##
@@ -41,22 +43,52 @@ public class JarArchive implements Archive {
private final JarFile jar;
private final MJarSupport mjar = new MJarSupport();
-public JarArchive(ClassLoader loader, URL url) {
+public JarArchive(ClassLoader loader, URL url){
//if (!"jar".equals(url.getProtocol())) throw new
IllegalArgumentException("not a jar url: " + url);
+this.loader = loader;
+this.url = url;
+File jf;
+int idx;
+
+/*
+ * True URL.getPath(...) wiping out protocol prefixes:
+ *
+ * 1. 'jar:file:/c:/temp/...' -> 'jar' + 'file:/c:/temp/...'
+ * 2. 'file:/c:/temp/...' -> 'file' + '/c:/temp/...'
+ *
+ * assuming we accept 'file:/...${xxx.jar}!/' URLs
+ * AND 'zip:file:/...!/' would be cool too, if
+ * allowed by new URL(...)
+ */
try {
-this.loader = loader;
-this.url = url;
-URL u = url;
-
-String jarPath = url.getFile();
-if (jarPath.contains("!")) {
-jarPath = jarPath.substring(0, jarPath.lastIndexOf("!"));
-u = new URL(jarPath);
+// Underlying JarFile(...) requires RandomAccessFile,
+// so only local file URLs here...
+while(!"file".equalsIgnoreCase(url.getProtocol())) {
+// no need here in .getQuery() tail, appended by getFile()
+url = new URL(url.getPath());
}
-jar = new JarFile(FileArchive.decode(u.getFile())); // no more an
url
-} catch (IOException e) {
-throw new IllegalStateException(e);
+}catch(MalformedURLException ex){
+// No more protocol(s) in path
+throw new UnsupportedOperationException("Please provide
'file:/...' or 'jar:file:/...!/' URL");
+}
+
+// TODO: drop FileArchive.decode?
+// URLDecoder.decode(url.getPath(), "UTF-8");
+String jarPath = FileArchive.decode(url.getPath());
+
+while(!(jf = new File(jarPath)).exists()){
+if((idx = jarPath.lastIndexOf('!')) > 0){
Review Comment:
what do you mean?
start from beginning FAILS, start from end SUCCEEDS. yes, success may cost
some resources, that's the price.
here we (almost) always meet ! at the end, since new URL() requires it in
'jar:file:/...!/'. so, we try access file, cut ! off, and - if everything is
near default scenario - meet available JAR file(even if it has !s in path or
not - doesn't matter) => profit! once.
things become worst when there is a tail full of !s after 'file.jar!/'.
definitely non-standard case, and each extra '!' cost new String, new File and
extra file system access.
if i missed something...?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@geronimo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [geronimo-xbean] powerbroker commented on a diff in pull request #34: XBEAN-337 jar path with exclamation signs(!) should be resolved correctly
powerbroker commented on code in PR #34:
URL: https://github.com/apache/geronimo-xbean/pull/34#discussion_r1045269358
##
xbean-finder/src/test/java/org/apache/xbean/finder/archive/JarArchiveTest.java:
##
@@ -85,34 +87,53 @@ public void testLoadClass() throws Exception {
@Test
public void testIterator() throws Exception {
-List actual = new ArrayList<>();
-for (Archive.Entry entry : this.archive) {
+List actual = new ArrayList();
+for (Archive.Entry entry : archive) {
actual.add(entry.getName());
}
Assert.assertFalse(0 == actual.size());
-for (Class clazz : JarArchiveTest.classes) {
+for (Class clazz : classes) {
Assert.assertTrue(clazz.getName(),
actual.contains(clazz.getName()));
}
Assert.assertEquals(JarArchiveTest.classes.length, actual.size());
}
-
@Test
public void testXBEAN337() throws Exception {
+// Virtual path
+JarArchive jar;
+
String path = "/this!/!file!/does!/!not/exist.jar";
-URL[] urls = {new URL("jar:file:" + path + "!/")};
+URL[] urls = {new URL("jar:file:" + path +
"!/some!/!inner!/.jar!/file.jar")};
try {
-this.archive = new JarArchive(new URLClassLoader(urls), urls[0]);
+jar = new JarArchive(new URLClassLoader(urls), urls[0]);
}catch(Exception ex){
Assert.assertTrue(
"Muzz never fail on '/this', but try full path with
exclamations('%s') instead"
.formatted(path),
-ex.getCause().getMessage().contains("exist.jar"));
+ex.getMessage().contains("exist.jar"));
}
+
+// Real file
+
+Path tmpDir = Files.createTempDirectory("!" +
JarArchiveTest.class.getSimpleName() + "!-");
Review Comment:
doesn't matter where it is in 4.22 - everything fails. think, test should
prove it works in any position
thx, good idea - @Rule TemporaryFolder. oh, those temp files... ((
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@geronimo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [geronimo-xbean] powerbroker commented on a diff in pull request #34: XBEAN-337 jar path with exclamation signs(!) should be resolved correctly
powerbroker commented on code in PR #34:
URL: https://github.com/apache/geronimo-xbean/pull/34#discussion_r1045271076
##
xbean-finder/src/test/java/org/apache/xbean/finder/archive/JarArchiveTest.java:
##
@@ -85,34 +87,53 @@ public void testLoadClass() throws Exception {
@Test
public void testIterator() throws Exception {
-List actual = new ArrayList<>();
-for (Archive.Entry entry : this.archive) {
+List actual = new ArrayList();
+for (Archive.Entry entry : archive) {
actual.add(entry.getName());
}
Assert.assertFalse(0 == actual.size());
-for (Class clazz : JarArchiveTest.classes) {
+for (Class clazz : classes) {
Assert.assertTrue(clazz.getName(),
actual.contains(clazz.getName()));
}
Assert.assertEquals(JarArchiveTest.classes.length, actual.size());
}
-
Review Comment:
should JarArchive always throw
IllegalStateException(FileNotFoundException("...${url}..."))?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@geronimo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [geronimo-xbean] powerbroker commented on a diff in pull request #34: XBEAN-337 jar path with exclamation signs(!) should be resolved correctly
powerbroker commented on code in PR #34:
URL: https://github.com/apache/geronimo-xbean/pull/34#discussion_r1045269988
##
xbean-finder/src/main/java/org/apache/xbean/finder/archive/JarArchive.java:
##
@@ -41,22 +43,52 @@ public class JarArchive implements Archive {
private final JarFile jar;
private final MJarSupport mjar = new MJarSupport();
-public JarArchive(ClassLoader loader, URL url) {
+public JarArchive(ClassLoader loader, URL url){
//if (!"jar".equals(url.getProtocol())) throw new
IllegalArgumentException("not a jar url: " + url);
+this.loader = loader;
+this.url = url;
+File jf;
+int idx;
+
+/*
+ * True URL.getPath(...) wiping out protocol prefixes:
+ *
+ * 1. 'jar:file:/c:/temp/...' -> 'jar' + 'file:/c:/temp/...'
+ * 2. 'file:/c:/temp/...' -> 'file' + '/c:/temp/...'
+ *
+ * assuming we accept 'file:/...${xxx.jar}!/' URLs
+ * AND 'zip:file:/...!/' would be cool too, if
+ * allowed by new URL(...)
+ */
try {
-this.loader = loader;
-this.url = url;
-URL u = url;
-
-String jarPath = url.getFile();
-if (jarPath.contains("!")) {
-jarPath = jarPath.substring(0, jarPath.lastIndexOf("!"));
-u = new URL(jarPath);
+// Underlying JarFile(...) requires RandomAccessFile,
+// so only local file URLs here...
+while(!"file".equalsIgnoreCase(url.getProtocol())) {
+// no need here in .getQuery() tail, appended by getFile()
+url = new URL(url.getPath());
}
-jar = new JarFile(FileArchive.decode(u.getFile())); // no more an
url
-} catch (IOException e) {
-throw new IllegalStateException(e);
+}catch(MalformedURLException ex){
+// No more protocol(s) in path
+throw new UnsupportedOperationException("Please provide
'file:/...' or 'jar:file:/...!/' URL");
+}
+
+// TODO: drop FileArchive.decode?
+// URLDecoder.decode(url.getPath(), "UTF-8");
+String jarPath = FileArchive.decode(url.getPath());
+
+while(!(jf = new File(jarPath)).exists()){
+if((idx = jarPath.lastIndexOf('!')) > 0){
Review Comment:
what do you mean?
start from beginning FAILS, start from end SUCCEEDS. yes, success may cost
some resources, that's the price.
if i missed something...?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@geronimo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [geronimo-xbean] powerbroker commented on a diff in pull request #34: XBEAN-337 jar path with exclamation signs(!) should be resolved correctly
powerbroker commented on code in PR #34:
URL: https://github.com/apache/geronimo-xbean/pull/34#discussion_r1045269358
##
xbean-finder/src/test/java/org/apache/xbean/finder/archive/JarArchiveTest.java:
##
@@ -85,34 +87,53 @@ public void testLoadClass() throws Exception {
@Test
public void testIterator() throws Exception {
-List actual = new ArrayList<>();
-for (Archive.Entry entry : this.archive) {
+List actual = new ArrayList();
+for (Archive.Entry entry : archive) {
actual.add(entry.getName());
}
Assert.assertFalse(0 == actual.size());
-for (Class clazz : JarArchiveTest.classes) {
+for (Class clazz : classes) {
Assert.assertTrue(clazz.getName(),
actual.contains(clazz.getName()));
}
Assert.assertEquals(JarArchiveTest.classes.length, actual.size());
}
-
@Test
public void testXBEAN337() throws Exception {
+// Virtual path
+JarArchive jar;
+
String path = "/this!/!file!/does!/!not/exist.jar";
-URL[] urls = {new URL("jar:file:" + path + "!/")};
+URL[] urls = {new URL("jar:file:" + path +
"!/some!/!inner!/.jar!/file.jar")};
try {
-this.archive = new JarArchive(new URLClassLoader(urls), urls[0]);
+jar = new JarArchive(new URLClassLoader(urls), urls[0]);
}catch(Exception ex){
Assert.assertTrue(
"Muzz never fail on '/this', but try full path with
exclamations('%s') instead"
.formatted(path),
-ex.getCause().getMessage().contains("exist.jar"));
+ex.getMessage().contains("exist.jar"));
}
+
+// Real file
+
+Path tmpDir = Files.createTempDirectory("!" +
JarArchiveTest.class.getSimpleName() + "!-");
Review Comment:
doesn't matter where it is in 4.22 - everything fails. think, test should
prove it works in any position
thx, good idea - @Rule TemporaryFolder. those temp files... ((
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@geronimo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [geronimo-xbean] powerbroker commented on a diff in pull request #34: XBEAN-337 jar path with exclamation signs(!) should be resolved correctly
powerbroker commented on code in PR #34:
URL: https://github.com/apache/geronimo-xbean/pull/34#discussion_r1045268414
##
xbean-finder/src/test/java/org/apache/xbean/finder/archive/JarArchiveTest.java:
##
@@ -85,34 +87,53 @@ public void testLoadClass() throws Exception {
@Test
public void testIterator() throws Exception {
-List actual = new ArrayList<>();
-for (Archive.Entry entry : this.archive) {
+List actual = new ArrayList();
+for (Archive.Entry entry : archive) {
actual.add(entry.getName());
}
Assert.assertFalse(0 == actual.size());
-for (Class clazz : JarArchiveTest.classes) {
+for (Class clazz : classes) {
Assert.assertTrue(clazz.getName(),
actual.contains(clazz.getName()));
}
Assert.assertEquals(JarArchiveTest.classes.length, actual.size());
}
-
@Test
public void testXBEAN337() throws Exception {
+// Virtual path
+JarArchive jar;
+
String path = "/this!/!file!/does!/!not/exist.jar";
-URL[] urls = {new URL("jar:file:" + path + "!/")};
+URL[] urls = {new URL("jar:file:" + path +
"!/some!/!inner!/.jar!/file.jar")};
Review Comment:
no, be sure that such tails - with !s - don't break jar file access
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@geronimo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [geronimo-xbean] powerbroker commented on a diff in pull request #34: XBEAN-337 jar path with exclamation signs(!) should be resolved correctly
powerbroker commented on code in PR #34:
URL: https://github.com/apache/geronimo-xbean/pull/34#discussion_r1045268414
##
xbean-finder/src/test/java/org/apache/xbean/finder/archive/JarArchiveTest.java:
##
@@ -85,34 +87,53 @@ public void testLoadClass() throws Exception {
@Test
public void testIterator() throws Exception {
-List actual = new ArrayList<>();
-for (Archive.Entry entry : this.archive) {
+List actual = new ArrayList();
+for (Archive.Entry entry : archive) {
actual.add(entry.getName());
}
Assert.assertFalse(0 == actual.size());
-for (Class clazz : JarArchiveTest.classes) {
+for (Class clazz : classes) {
Assert.assertTrue(clazz.getName(),
actual.contains(clazz.getName()));
}
Assert.assertEquals(JarArchiveTest.classes.length, actual.size());
}
-
@Test
public void testXBEAN337() throws Exception {
+// Virtual path
+JarArchive jar;
+
String path = "/this!/!file!/does!/!not/exist.jar";
-URL[] urls = {new URL("jar:file:" + path + "!/")};
+URL[] urls = {new URL("jar:file:" + path +
"!/some!/!inner!/.jar!/file.jar")};
Review Comment:
no, be sure that such tails don't break jar file access
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@geronimo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [geronimo-xbean] powerbroker commented on a diff in pull request #34: XBEAN-337 jar path with exclamation signs(!) should be resolved correctly
powerbroker commented on code in PR #34:
URL: https://github.com/apache/geronimo-xbean/pull/34#discussion_r1045268118
##
xbean-finder/src/test/java/org/apache/xbean/finder/archive/JarArchiveTest.java:
##
@@ -71,12 +73,12 @@ public void testGetBytecode() throws Exception {
@Test
public void testLoadClass() throws Exception {
-for (Class clazz : JarArchiveTest.classes) {
+for (Class clazz : classes) {
Assert.assertEquals(clazz.getName(), clazz,
this.archive.loadClass(clazz.getName()));
}
try {
-this.archive.loadClass("Fake");
+archive.loadClass("Fake");
Review Comment:
+4
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@geronimo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [geronimo-xbean] powerbroker commented on a diff in pull request #34: XBEAN-337 jar path with exclamation signs(!) should be resolved correctly
powerbroker commented on code in PR #34:
URL: https://github.com/apache/geronimo-xbean/pull/34#discussion_r1045268062
##
xbean-finder/src/main/java/org/apache/xbean/finder/archive/JarArchive.java:
##
@@ -41,22 +43,52 @@ public class JarArchive implements Archive {
private final JarFile jar;
private final MJarSupport mjar = new MJarSupport();
-public JarArchive(ClassLoader loader, URL url) {
+public JarArchive(ClassLoader loader, URL url){
//if (!"jar".equals(url.getProtocol())) throw new
IllegalArgumentException("not a jar url: " + url);
+this.loader = loader;
+this.url = url;
+File jf;
+int idx;
+
+/*
+ * True URL.getPath(...) wiping out protocol prefixes:
+ *
+ * 1. 'jar:file:/c:/temp/...' -> 'jar' + 'file:/c:/temp/...'
+ * 2. 'file:/c:/temp/...' -> 'file' + '/c:/temp/...'
+ *
+ * assuming we accept 'file:/...${xxx.jar}!/' URLs
+ * AND 'zip:file:/...!/' would be cool too, if
+ * allowed by new URL(...)
+ */
try {
-this.loader = loader;
-this.url = url;
-URL u = url;
-
-String jarPath = url.getFile();
-if (jarPath.contains("!")) {
-jarPath = jarPath.substring(0, jarPath.lastIndexOf("!"));
-u = new URL(jarPath);
+// Underlying JarFile(...) requires RandomAccessFile,
+// so only local file URLs here...
+while(!"file".equalsIgnoreCase(url.getProtocol())) {
+// no need here in .getQuery() tail, appended by getFile()
+url = new URL(url.getPath());
}
-jar = new JarFile(FileArchive.decode(u.getFile())); // no more an
url
-} catch (IOException e) {
-throw new IllegalStateException(e);
+}catch(MalformedURLException ex){
+// No more protocol(s) in path
+throw new UnsupportedOperationException("Please provide
'file:/...' or 'jar:file:/...!/' URL");
+}
+
+// TODO: drop FileArchive.decode?
+// URLDecoder.decode(url.getPath(), "UTF-8");
+String jarPath = FileArchive.decode(url.getPath());
+
+while(!(jf = new File(jarPath)).exists()){
+if((idx = jarPath.lastIndexOf('!')) > 0){
+jarPath = jarPath.substring(0, idx);
+}else{
+throw new IllegalStateException("Cannot find any files by '%s'
url".formatted(url));
+}
+}
+
+try{
+this.jar = new JarFile(jf);
+}catch(IOException e){
+throw new IllegalStateException("Cannot open jar
'%s'".formatted(jf.getAbsolutePath()), e);
Review Comment:
.formatted(), i guess
##
xbean-finder/src/test/java/org/apache/xbean/finder/archive/JarArchiveTest.java:
##
@@ -42,27 +44,27 @@ public class JarArchiveTest {
@BeforeClass
public static void classSetUp() throws Exception {
-JarArchiveTest.classpath = Archives.jarArchive(JarArchiveTest.classes);
+classpath = Archives.jarArchive(classes);
}
@Before
public void setUp() throws Exception {
-URL[] urls = {new URL("jar:" +
JarArchiveTest.classpath.toURI().toURL() + "!/")};
+URL[] urls = {new URL("jar:" + classpath.toURI().toURL() + "!/")};
-this.archive = new JarArchive(new URLClassLoader(urls), urls[0]);
+archive = new JarArchive(new URLClassLoader(urls), urls[0]);
}
@Test
public void testGetBytecode() throws Exception {
-for (Class clazz : JarArchiveTest.classes) {
+for (Class clazz : classes) {
Assert.assertNotNull(clazz.getName(),
this.archive.getBytecode(clazz.getName()));
}
try {
-this.archive.getBytecode("Fake");
Review Comment:
+3
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@geronimo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [geronimo-xbean] powerbroker commented on a diff in pull request #34: XBEAN-337 jar path with exclamation signs(!) should be resolved correctly
powerbroker commented on code in PR #34:
URL: https://github.com/apache/geronimo-xbean/pull/34#discussion_r1045267857
##
xbean-finder/src/test/java/org/apache/xbean/finder/archive/JarArchiveTest.java:
##
@@ -42,27 +44,27 @@ public class JarArchiveTest {
@BeforeClass
public static void classSetUp() throws Exception {
-JarArchiveTest.classpath = Archives.jarArchive(JarArchiveTest.classes);
+classpath = Archives.jarArchive(classes);
}
@Before
public void setUp() throws Exception {
-URL[] urls = {new URL("jar:" +
JarArchiveTest.classpath.toURI().toURL() + "!/")};
+URL[] urls = {new URL("jar:" + classpath.toURI().toURL() + "!/")};
-this.archive = new JarArchive(new URLClassLoader(urls), urls[0]);
+archive = new JarArchive(new URLClassLoader(urls), urls[0]);
Review Comment:
+2
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@geronimo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [geronimo-xbean] powerbroker commented on a diff in pull request #34: XBEAN-337 jar path with exclamation signs(!) should be resolved correctly
powerbroker commented on code in PR #34:
URL: https://github.com/apache/geronimo-xbean/pull/34#discussion_r1045267812
##
xbean-finder/src/test/java/org/apache/xbean/finder/archive/JarArchiveTest.java:
##
@@ -42,27 +44,27 @@ public class JarArchiveTest {
@BeforeClass
public static void classSetUp() throws Exception {
-JarArchiveTest.classpath = Archives.jarArchive(JarArchiveTest.classes);
+classpath = Archives.jarArchive(classes);
}
@Before
public void setUp() throws Exception {
-URL[] urls = {new URL("jar:" +
JarArchiveTest.classpath.toURI().toURL() + "!/")};
+URL[] urls = {new URL("jar:" + classpath.toURI().toURL() + "!/")};
Review Comment:
+1
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@geronimo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [geronimo-xbean] powerbroker commented on a diff in pull request #34: XBEAN-337 jar path with exclamation signs(!) should be resolved correctly
powerbroker commented on code in PR #34:
URL: https://github.com/apache/geronimo-xbean/pull/34#discussion_r1045267775
##
xbean-finder/src/test/java/org/apache/xbean/finder/archive/JarArchiveTest.java:
##
@@ -42,27 +44,27 @@ public class JarArchiveTest {
@BeforeClass
public static void classSetUp() throws Exception {
-JarArchiveTest.classpath = Archives.jarArchive(JarArchiveTest.classes);
+classpath = Archives.jarArchive(classes);
Review Comment:
ok
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@geronimo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [geronimo-xbean] powerbroker commented on a diff in pull request #34: XBEAN-337 jar path with exclamation signs(!) should be resolved correctly
powerbroker commented on code in PR #34:
URL: https://github.com/apache/geronimo-xbean/pull/34#discussion_r1045267541
##
xbean-finder/src/main/java/org/apache/xbean/finder/archive/JarArchive.java:
##
@@ -41,22 +43,52 @@ public class JarArchive implements Archive {
private final JarFile jar;
private final MJarSupport mjar = new MJarSupport();
-public JarArchive(ClassLoader loader, URL url) {
+public JarArchive(ClassLoader loader, URL url){
//if (!"jar".equals(url.getProtocol())) throw new
IllegalArgumentException("not a jar url: " + url);
+this.loader = loader;
+this.url = url;
+File jf;
+int idx;
+
+/*
+ * True URL.getPath(...) wiping out protocol prefixes:
+ *
+ * 1. 'jar:file:/c:/temp/...' -> 'jar' + 'file:/c:/temp/...'
+ * 2. 'file:/c:/temp/...' -> 'file' + '/c:/temp/...'
+ *
+ * assuming we accept 'file:/...${xxx.jar}!/' URLs
+ * AND 'zip:file:/...!/' would be cool too, if
+ * allowed by new URL(...)
+ */
try {
-this.loader = loader;
-this.url = url;
-URL u = url;
-
-String jarPath = url.getFile();
-if (jarPath.contains("!")) {
-jarPath = jarPath.substring(0, jarPath.lastIndexOf("!"));
-u = new URL(jarPath);
+// Underlying JarFile(...) requires RandomAccessFile,
+// so only local file URLs here...
+while(!"file".equalsIgnoreCase(url.getProtocol())) {
+// no need here in .getQuery() tail, appended by getFile()
+url = new URL(url.getPath());
}
-jar = new JarFile(FileArchive.decode(u.getFile())); // no more an
url
-} catch (IOException e) {
-throw new IllegalStateException(e);
+}catch(MalformedURLException ex){
+// No more protocol(s) in path
+throw new UnsupportedOperationException("Please provide
'file:/...' or 'jar:file:/...!/' URL");
+}
+
+// TODO: drop FileArchive.decode?
+// URLDecoder.decode(url.getPath(), "UTF-8");
+String jarPath = FileArchive.decode(url.getPath());
+
+while(!(jf = new File(jarPath)).exists()){
+if((idx = jarPath.lastIndexOf('!')) > 0){
+jarPath = jarPath.substring(0, idx);
+}else{
+throw new IllegalStateException("Cannot find any files by '%s'
url".formatted(url));
Review Comment:
ok
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@geronimo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [geronimo-xbean] powerbroker commented on a diff in pull request #34: XBEAN-337 jar path with exclamation signs(!) should be resolved correctly
powerbroker commented on code in PR #34:
URL: https://github.com/apache/geronimo-xbean/pull/34#discussion_r1045267357
##
xbean-finder/src/main/java/org/apache/xbean/finder/archive/JarArchive.java:
##
@@ -41,22 +43,52 @@ public class JarArchive implements Archive {
private final JarFile jar;
private final MJarSupport mjar = new MJarSupport();
-public JarArchive(ClassLoader loader, URL url) {
+public JarArchive(ClassLoader loader, URL url){
//if (!"jar".equals(url.getProtocol())) throw new
IllegalArgumentException("not a jar url: " + url);
+this.loader = loader;
+this.url = url;
+File jf;
+int idx;
+
+/*
+ * True URL.getPath(...) wiping out protocol prefixes:
+ *
+ * 1. 'jar:file:/c:/temp/...' -> 'jar' + 'file:/c:/temp/...'
+ * 2. 'file:/c:/temp/...' -> 'file' + '/c:/temp/...'
+ *
+ * assuming we accept 'file:/...${xxx.jar}!/' URLs
+ * AND 'zip:file:/...!/' would be cool too, if
+ * allowed by new URL(...)
+ */
try {
-this.loader = loader;
-this.url = url;
-URL u = url;
-
-String jarPath = url.getFile();
-if (jarPath.contains("!")) {
-jarPath = jarPath.substring(0, jarPath.lastIndexOf("!"));
-u = new URL(jarPath);
+// Underlying JarFile(...) requires RandomAccessFile,
+// so only local file URLs here...
+while(!"file".equalsIgnoreCase(url.getProtocol())) {
+// no need here in .getQuery() tail, appended by getFile()
+url = new URL(url.getPath());
}
-jar = new JarFile(FileArchive.decode(u.getFile())); // no more an
url
-} catch (IOException e) {
-throw new IllegalStateException(e);
+}catch(MalformedURLException ex){
+// No more protocol(s) in path
+throw new UnsupportedOperationException("Please provide
'file:/...' or 'jar:file:/...!/' URL");
+}
+
+// TODO: drop FileArchive.decode?
Review Comment:
ok
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@geronimo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [geronimo-xbean] powerbroker commented on a diff in pull request #34: XBEAN-337 jar path with exclamation signs(!) should be resolved correctly
powerbroker commented on code in PR #34:
URL: https://github.com/apache/geronimo-xbean/pull/34#discussion_r1045267258
##
xbean-finder/src/main/java/org/apache/xbean/finder/archive/JarArchive.java:
##
@@ -41,22 +43,52 @@ public class JarArchive implements Archive {
private final JarFile jar;
private final MJarSupport mjar = new MJarSupport();
-public JarArchive(ClassLoader loader, URL url) {
+public JarArchive(ClassLoader loader, URL url){
//if (!"jar".equals(url.getProtocol())) throw new
IllegalArgumentException("not a jar url: " + url);
+this.loader = loader;
+this.url = url;
+File jf;
+int idx;
+
+/*
+ * True URL.getPath(...) wiping out protocol prefixes:
+ *
+ * 1. 'jar:file:/c:/temp/...' -> 'jar' + 'file:/c:/temp/...'
+ * 2. 'file:/c:/temp/...' -> 'file' + '/c:/temp/...'
+ *
+ * assuming we accept 'file:/...${xxx.jar}!/' URLs
+ * AND 'zip:file:/...!/' would be cool too, if
+ * allowed by new URL(...)
+ */
try {
-this.loader = loader;
-this.url = url;
-URL u = url;
-
-String jarPath = url.getFile();
-if (jarPath.contains("!")) {
-jarPath = jarPath.substring(0, jarPath.lastIndexOf("!"));
-u = new URL(jarPath);
+// Underlying JarFile(...) requires RandomAccessFile,
+// so only local file URLs here...
+while(!"file".equalsIgnoreCase(url.getProtocol())) {
Review Comment:
jar:foo:file is actually prohibited by new URL(), why not allow this
impossible scenario?
from other side, if they let e.g. tar:gz:file, this will work here and, if
fail - deeper, where e.g. 'tar' format should be supported.
in general, AFAIK we are interested here in url, pointing to (available,
because of possible '!'s in path) local zip file: '...file:/.../some.jar'.
i think:
1. a File parameter, instead of URL would be appropriate
2. extra prefixes and suffixes may cause warning, but shouldn't ruin jar
access.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@geronimo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
