[WIP] Allow IDE enable/disable escaping classpath entries in jars#4843
[WIP] Allow IDE enable/disable escaping classpath entries in jars#4843iloveeclipse wants to merge 1 commit into
Conversation
iloveeclipse
changed the title
Since b74a7b8 JDT Core (but not ecj!) supported Class-Path attribute for external jars and added all entries from such jar to the current project classpath. This change introduces new JavaCore preference that allows JDT ignore "escaping" classpath entries read from external jars (entries, containing ".." path segment). By default, JDT disables now old behavior and disallows such paths. See eclipse-jdt#258 See https://bugs.eclipse.org/bugs/show_bug.cgi?id=198572
iloveeclipse
force-pushed
the
escape_jar
branch
from
88868e7 to
b780b9f
Compare
DO NOT MERGE, it is a draft PR.The only reason it is not "draft" is this: #4844 - I want see test results. |
fedejeanne
left a comment
fedejeanne
left a comment
There was a problem hiding this comment.
Thank you very much for looking into this, @iloveeclipse !
I just have some minor comments.
Also, how do I test this?
| * <dt>Default:</dt> | ||
| * <dd><code>"disabled"</code></dd> |
There was a problem hiding this comment.
The JavaDoc is confusing:
This option should be disabled to be consistent with command line compiler, but it is enabled by default for backward compatibility reasons ...
I see that it is DISABLED in JavaCorePreferenceInitializer.
| if (JavaCore.DISABLED.equals(escapePref) && calledFileName.indexOf(DOT_DOT) != -1 | ||
| && hasDotDot(Path.fromPortableString(calledFileName))) { | ||
| if (JavaModelManager.CP_RESOLVE_VERBOSE_FAILURE) { | ||
| trace("Invalid (escaping jar directory) Class-Path entry " + calledFileName //$NON-NLS-1$ |
There was a problem hiding this comment.
NIT: to be able to search the log files more easily (a search for Invalid Class-Path would yield all problems)
| trace("Invalid (escaping jar directory) Class-Path entry " + calledFileName //$NON-NLS-1$ | |
| trace("Invalid Class-Path entry (escaping jar directory)" + calledFileName //$NON-NLS-1$ |
| String escapePref = JavaCore.getOptions() | ||
| .get(JavaCore.CORE_ENABLE_ESACAPING_CP_ENTRIES_IN_JAR_MANIFEST); | ||
| if (JavaCore.DISABLED.equals(escapePref) && calledFileName.indexOf(DOT_DOT) != -1 | ||
| && hasDotDot(Path.fromPortableString(calledFileName))) { |
There was a problem hiding this comment.
Question: the JavaDoc of Path.fromPortableString(String) mentions:
The path string must have been produced by a previous call to
IPath.toPortableString.
... and also
Instead of calling this method it is recommended to call
IPath.fromPortableString(String)instead
w.r.t. 1) I couldn't find any calls to toPortableString in the generation, but I assume it is safe to use this method anyway?
w.r.t. to 2) The code already calls IPath.fromPortableString(...)
so I would simply skip the indirection and call it directly:
| && hasDotDot(Path.fromPortableString(calledFileName))) { | |
| && hasDotDot(IPath.fromPortableString(calledFileName))) { |
2 participants
Since b74a7b8 JDT Core (but not ecj!) supported Class-Path attribute for external jars and added all entries from such jar to the current project classpath.
This change introduces new JavaCore preference that allows JDT ignore "escaping" classpath entries read from external jars (entries, containing ".." path segment).
By default, JDT disables now old behavior and disallows such paths.
See #258 See https://bugs.eclipse.org/bugs/show_bug.cgi?id=198572