Issue #9511 Verify location of scanned class files #10777
Conversation
|
@janbartel make sure we also test for |
|
@sbordet there are already tests for that. |
…etTestResourcePathFile Signed-off-by: Olivier Lamy <olamy@apache.org>
Signed-off-by: Olivier Lamy <olamy@apache.org>
Signed-off-by: Olivier Lamy <olamy@apache.org>
Signed-off-by: Olivier Lamy <olamy@apache.org>
Signed-off-by: Olivier Lamy <olamy@apache.org>
olamy
force-pushed
the
jetty-12.0.x-9511-verify-scanned-class-location
branch
from
dad52e5 to
de7498a
Compare
| //Check that the named class exists in the containingResource at the correct location. | ||
| //eg given the class with name "com.foo.Acme" and the containingResource "jar:file://some/place/something.jar!/" | ||
| //then the file "jar:file://some/place/something.jar!/com/foo/Acme.class" must exist. | ||
| if (_containingResource.resolve(name + ".class") == null) |
There was a problem hiding this comment.
this is causing issues for osgi bundles as classes are not resolved anymore within bundles
Accounting for WEB-INF/classes.
...etty-ee10-annotations/src/main/java/org/eclipse/jetty/ee10/annotations/AnnotationParser.java
Outdated
Show resolved
Hide resolved
Potentially we could move it into a core |
| if (relative == null) | ||
| return false; //unable to be verified | ||
|
|
||
| StringTokenizer tokenizer = new StringTokenizer(classname, "/", false); |
There was a problem hiding this comment.
Doesn't the classname have '.' separators rather than '/'?
If it has '/' separators, then maybe we should split it with Path?
There was a problem hiding this comment.
The classname when referenced via a resource has slashes.
| if (i == relative.getNameCount() - 1) | ||
| { | ||
| if ("class".equals(FileID.getExtension(s))) | ||
| s = s.substring(0, s.length() - 6); | ||
| } |
There was a problem hiding this comment.
A comment saying what this is doing would be good. something like // strip the .class suffix from the last path item.
Also, if the .class is missing then we should not match
Finally, it should be an equalsIgnoreCase
There was a problem hiding this comment.
This could be changed to ...
// strip class extensions
if (FileID.isExtension(s, "class"))
s = s.substring(0, s.length() - 6)As that will verify the extension in a case insensitive way.
|
@gregw shall we can this PR? |
|
@janbartel @gregw shall we close this PR as not useful? |
|
These changes aren't crucial. |
Closes #9511
Check that a class file that is about to be scanned is in the correct location, skipping it if not.
For example, given a class with fully qualified name
com.acme.Foothat comes fromjar:file://someplace/somewhere.jar!/ensure that class is located atjar:file://someplace/somewhere.jar!/com/acme/Foo.class.This would skip the class if it was in eg
jar:file://someplace/somewhere.jar!/this/is/the/wrong/place/com/acme/Foo.class. See the original issue #9400 for more examples.