Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

MT-hardening builder.ClasspathJar.knownPackageNames #3250 #3255

Merged
merged 3 commits into from
Nov 12, 2024
Merged

MT-hardening builder.ClasspathJar.knownPackageNames #3250 #3255

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e5f93b9
MT-hardening builder.ClasspathJar.knownPackageNames #3250
EcljpseB0T Nov 5, 2024
32a1e66
removed unused JrtPackageVisitor
EcljpseB0T Nov 6, 2024
598d52c
renamed ClasspathJar.scanContent -> readKnownPackageNames
EcljpseB0T Nov 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 6 additions & 10 deletions org.eclipse.jdt.core/model/org/eclipse/jdt/internal/core/builder/ClasspathJMod.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@

import java.io.IOException;
import java.util.Enumeration;
import java.util.HashSet;
import java.util.Set;
import java.util.function.Predicate;
import java.util.zip.ZipEntry;
import java.util.zip.ZipFile;
Expand All @@ -26,14 +28,12 @@
import org.eclipse.jdt.internal.compiler.env.IBinaryType;
import org.eclipse.jdt.internal.compiler.env.IModule;
import org.eclipse.jdt.internal.compiler.env.NameEnvironmentAnswer;
import org.eclipse.jdt.internal.compiler.util.SimpleSet;
import org.eclipse.jdt.internal.compiler.util.SuffixConstants;

public class ClasspathJMod extends ClasspathJar {

public static char[] CLASSES = "classes".toCharArray(); //$NON-NLS-1$
public static final String CLASSES_FOLDER = "classes/"; //$NON-NLS-1$
private static int MODULE_DESCRIPTOR_NAME_LENGTH = IModule.MODULE_INFO_CLASS.length();

ClasspathJMod(String zipFilename, long lastModified, AccessRuleSet accessRuleSet, IPath externalAnnotationPath) {
super(zipFilename, lastModified, accessRuleSet, externalAnnotationPath, true);
Expand Down Expand Up @@ -80,8 +80,9 @@ public NameEnvironmentAnswer findClass(String binaryFileName, String qualifiedPa
return null;
}
@Override
protected String readJarContent(final SimpleSet packageSet) {
String modInfo = null;
protected Set<String> readPackageNames() {
final Set<String> packageSet = new HashSet<>();
packageSet.add(""); //$NON-NLS-1$
for (Enumeration<? extends ZipEntry> e = this.zipFile.entries(); e.hasMoreElements(); ) {
ZipEntry entry = e.nextElement();
char[] entryName = entry.getName().toCharArray();
Expand All @@ -90,15 +91,10 @@ protected String readJarContent(final SimpleSet packageSet) {
char[] folder = CharOperation.subarray(entryName, 0, index);
if (CharOperation.equals(CLASSES, folder)) {
char[] fileName = CharOperation.subarray(entryName, index + 1, entryName.length);
if (modInfo == null && fileName.length == MODULE_DESCRIPTOR_NAME_LENGTH) {
if (CharOperation.equals(fileName, IModule.MODULE_INFO_CLASS.toCharArray())) {
modInfo = new String(entryName);
}
}
addToPackageSet(packageSet, new String(fileName), false);
}
}
}
return modInfo;
return packageSet;
}
}
163 changes: 67 additions & 96 deletions org.eclipse.jdt.core/model/org/eclipse/jdt/internal/core/builder/ClasspathJar.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,9 @@
import java.util.Arrays;
import java.util.Date;
import java.util.Enumeration;
import java.util.HashSet;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import java.util.function.Predicate;
import java.util.jar.Manifest;
Expand All @@ -49,80 +51,60 @@
import org.eclipse.jdt.internal.compiler.env.IModule;
import org.eclipse.jdt.internal.compiler.env.NameEnvironmentAnswer;
import org.eclipse.jdt.internal.compiler.lookup.TypeConstants;
import org.eclipse.jdt.internal.compiler.util.SimpleSet;
import org.eclipse.jdt.internal.compiler.util.SuffixConstants;
import org.eclipse.jdt.internal.core.JavaModelManager;
import org.eclipse.jdt.internal.core.util.Util;

public class ClasspathJar extends ClasspathLocation {
final boolean isOnModulePath;

static class PackageCacheEntry {
WeakReference<ZipFile> zipFile;
long lastModified;
long fileSize;
SimpleSet packageSet;

PackageCacheEntry(ZipFile zipFile, long lastModified, long fileSize, SimpleSet packageSet) {
this.zipFile = new WeakReference<>(zipFile);
this.lastModified = lastModified;
this.fileSize = fileSize;
this.packageSet = packageSet;
}
private static record PackageCacheEntry(
WeakReference<ZipFile> zipFile,
long lastModified,
long fileSize,
Set<String> packageSet) {
}

protected static Map<String, PackageCacheEntry> PackageCache = new ConcurrentHashMap<>();
protected static final Map<String, PackageCacheEntry> packageCache = new ConcurrentHashMap<>();

protected static void addToPackageSet(SimpleSet packageSet, String fileName, boolean endsWithSep) {
protected static void addToPackageSet(Set<String> packageSet, String fileName, boolean endsWithSep) {
int last = endsWithSep ? fileName.length() : fileName.lastIndexOf('/');
while (last > 0) {
// extract the package name
String packageName = fileName.substring(0, last);
if (packageSet.addIfNotIncluded(packageName) == null)
if (!packageSet.add(packageName)) {
return; // already existed
}
last = packageName.lastIndexOf('/');
}
}

/**
* Calculate and cache the package list available in the zipFile.
* @return A SimpleSet with the all the package names in the zipFile.
*/
protected SimpleSet findPackageSet() {
PackageCacheEntry entry = PackageCache.compute(this.zipFilename, (zipFileName, cacheEntry) -> {
private Set<String> getCachedPackageNames() {
PackageCacheEntry entry = packageCache.compute(this.zipFilename, (zipFileName, cacheEntry) -> {
if(cacheEntry != null && cacheEntry.zipFile.get() == this.zipFile) {
return cacheEntry;
}
long timestamp = this.lastModified();
if (cacheEntry != null && cacheEntry.lastModified == timestamp && cacheEntry.fileSize == this.fileSize) {
cacheEntry.zipFile = new WeakReference<>(this.zipFile);
return cacheEntry;
// cacheEntry.zipFile.get() != this.zipFile => update zipFile
return new PackageCacheEntry(new WeakReference<>(this.zipFile), cacheEntry.lastModified , cacheEntry.fileSize, cacheEntry.packageSet);
}
final SimpleSet packageSet = new SimpleSet(41);
packageSet.add(""); //$NON-NLS-1$
readJarContent(packageSet);
return new PackageCacheEntry(this.zipFile, timestamp, this.fileSize, packageSet);
return new PackageCacheEntry(new WeakReference<>(this.zipFile), timestamp, this.fileSize, Set.copyOf(readPackageNames()));
});

return entry.packageSet;
}
protected String readJarContent(final SimpleSet packageSet) {
String modInfo = null;
/** overloaded */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jukzi this comment looks plain wrong.

Additionally a reference to the issue / PR is missing from the commit comment.

protected Set<String> readPackageNames() {
final Set<String> packageSet = new HashSet<>();
packageSet.add(""); //$NON-NLS-1$
for (Enumeration<? extends ZipEntry> e = this.zipFile.entries(); e.hasMoreElements(); ) {
String fileName = e.nextElement().getName();
if (fileName.startsWith("META-INF/")) //$NON-NLS-1$
continue;
if (modInfo == null) {
int folderEnd = fileName.lastIndexOf('/');
folderEnd += 1;
String className = fileName.substring(folderEnd, fileName.length());
if (className.equalsIgnoreCase(IModule.MODULE_INFO_CLASS)) {
modInfo = fileName;
}
}
addToPackageSet(packageSet, fileName, false);
}
return modInfo;
return packageSet;
}
IModule initializeModule() {
IModule mod = null;
Expand All @@ -149,30 +131,33 @@ IModule initializeModule() {
return mod;
}

String zipFilename; // keep for equals
IFile resource;
ZipFile zipFile;
long lastModified;
long fileSize;
boolean closeZipFileAtEnd;
private SimpleSet knownPackageNames;
final String zipFilename; // keep for equals
final IFile resource;
/** lazy initialized, closed and reset to null in {@link #cleanup()} **/
volatile protected ZipFile zipFile;
volatile long lastModified;
volatile long fileSize;
/** lazy initialized **/
private volatile Set<String> knownPackageNames;
// Meant for ClasspathMultiReleaseJar, not used in here
String compliance;

ClasspathJar(IFile resource, AccessRuleSet accessRuleSet, IPath externalAnnotationPath, boolean isOnModulePath) {
this.resource = resource;
String filename;
try {
java.net.URI location = resource.getLocationURI();
if (location == null) {
this.zipFilename = ""; //$NON-NLS-1$
filename = ""; //$NON-NLS-1$
} else {
File localFile = Util.toLocalFile(location, null);
this.zipFilename = localFile.getPath();
filename = localFile.getPath();
}
} catch (CoreException e) {
// ignore
this.zipFilename = ""; //$NON-NLS-1$
filename = ""; //$NON-NLS-1$
}
this.zipFilename = filename;
this.zipFile = null;
this.knownPackageNames = null;
this.accessRuleSet = accessRuleSet;
Expand All @@ -182,6 +167,7 @@ IModule initializeModule() {
}

ClasspathJar(String zipFilename, long lastModified, AccessRuleSet accessRuleSet, IPath externalAnnotationPath, boolean isOnModulePath) {
this.resource = null;
this.zipFilename = zipFilename;
this.lastModified = lastModified;
this.zipFile = null;
Expand All @@ -195,43 +181,31 @@ IModule initializeModule() {
public ClasspathJar(ZipFile zipFile, AccessRuleSet accessRuleSet, boolean isOnModulePath) {
this(zipFile.getName(), 0, accessRuleSet, null, isOnModulePath);
this.zipFile = zipFile;
this.closeZipFileAtEnd = true;
}

@Override
public void cleanup() {
if (this.closeZipFileAtEnd) {
if (this.zipFile != null) {
try {
this.zipFile.close();
if (JavaModelManager.ZIP_ACCESS_VERBOSE) {
trace("(" + Thread.currentThread() + ") [ClasspathJar.cleanup()] Closed ZipFile on " + this.zipFilename); //$NON-NLS-1$ //$NON-NLS-2$
}
} catch(IOException e) { // ignore it
JavaCore.getPlugin().getLog().log(new Status(IStatus.ERROR, JavaCore.PLUGIN_ID, "Error closing " + this.zipFile.getName(), e)); //$NON-NLS-1$
}
this.zipFile = null;
}
if (this.annotationZipFile != null) {
try {
this.annotationZipFile.close();
if (JavaModelManager.ZIP_ACCESS_VERBOSE) {
trace("(" + Thread.currentThread() + ") [ClasspathJar.cleanup()] Closed Annotation ZipFile on " + this.zipFilename); //$NON-NLS-1$ //$NON-NLS-2$
}
} catch(IOException e) { // ignore it
JavaCore.getPlugin().getLog().log(new Status(IStatus.ERROR, JavaCore.PLUGIN_ID, "Error closing " + this.annotationZipFile.getName(), e)); //$NON-NLS-1$
if (this.zipFile != null) {
try {
this.zipFile.close();
if (JavaModelManager.ZIP_ACCESS_VERBOSE) {
trace("(" + Thread.currentThread() + ") [ClasspathJar.cleanup()] Closed ZipFile on " + this.zipFilename); //$NON-NLS-1$ //$NON-NLS-2$
}
this.annotationZipFile = null;
} catch(IOException e) { // ignore it
JavaCore.getPlugin().getLog().log(new Status(IStatus.ERROR, JavaCore.PLUGIN_ID, "Error closing " + this.zipFile.getName(), e)); //$NON-NLS-1$
}
} else {
if (this.zipFile != null && JavaModelManager.ZIP_ACCESS_VERBOSE) {
try {
this.zipFile.size();
trace("(" + Thread.currentThread() + ") [ClasspathJar.cleanup()] ZipFile NOT closed on " + this.zipFilename); //$NON-NLS-1$ //$NON-NLS-2$
} catch (IllegalStateException e) {
// OK: the file was already closed
this.zipFile = null;
}
if (this.annotationZipFile != null) {
try {
this.annotationZipFile.close();
if (JavaModelManager.ZIP_ACCESS_VERBOSE) {
trace("(" + Thread.currentThread() + ") [ClasspathJar.cleanup()] Closed Annotation ZipFile on " + this.zipFilename); //$NON-NLS-1$ //$NON-NLS-2$
}
} catch(IOException e) { // ignore it
JavaCore.getPlugin().getLog().log(new Status(IStatus.ERROR, JavaCore.PLUGIN_ID, "Error closing " + this.annotationZipFile.getName(), e)); //$NON-NLS-1$
}
this.annotationZipFile = null;
}
this.module = null; // TODO(SHMOD): is this safe?
this.knownPackageNames = null;
Expand Down Expand Up @@ -295,13 +269,13 @@ public boolean isPackage(String qualifiedPackageName, String moduleName) {
return false;
}
if (this.knownPackageNames == null)
scanContent();
return this.knownPackageNames.includes(qualifiedPackageName);
readKnownPackageNames();
return this.knownPackageNames.contains(qualifiedPackageName);
}
@Override
public boolean hasCompilationUnit(String pkgName, String moduleName) {
if (scanContent()) {
if (!this.knownPackageNames.includes(pkgName)) {
if (readKnownPackageNames()) {
if (!this.knownPackageNames.contains(pkgName)) {
// Don't waste time walking through the zip if we know that it doesn't
// contain a directory that matches pkgName
return false;
Expand All @@ -322,22 +296,19 @@ public boolean hasCompilationUnit(String pkgName, String moduleName) {
return false;
}

/** Scan the contained packages and try to locate the module descriptor. */
private boolean scanContent() {
/** Scan the contained packages. */
private boolean readKnownPackageNames() {
try {
if (this.zipFile == null) {
if (JavaModelManager.ZIP_ACCESS_VERBOSE) {
trace("(" + Thread.currentThread() + ") [ClasspathJar.isPackage(String)] Creating ZipFile on " + this.zipFilename); //$NON-NLS-1$ //$NON-NLS-2$
}
this.zipFile = new ZipFile(this.zipFilename);
this.closeZipFileAtEnd = true;
this.knownPackageNames = findPackageSet();
} else {
this.knownPackageNames = findPackageSet();
}
this.knownPackageNames = getCachedPackageNames();
return true;
} catch(Exception e) {
this.knownPackageNames = new SimpleSet(); // assume for this build the zipFile is empty
this.knownPackageNames = Set.of(); // assume for this build the zipFile is empty
return false;
}
}
Expand Down Expand Up @@ -378,7 +349,7 @@ public String debugPathString() {
@Override
public IModule getModule() {
if (this.knownPackageNames == null)
scanContent();
readKnownPackageNames();
return this.module;
}

Expand All @@ -388,7 +359,7 @@ public NameEnvironmentAnswer findClass(String typeName, String qualifiedPackageN
return findClass(typeName, qualifiedPackageName, moduleName, qualifiedBinaryFileName, false, null);
}
public Manifest getManifest() {
if (!scanContent()) // ensure zipFile is initialized
if (!readKnownPackageNames()) // ensure zipFile is initialized
return null;
ZipEntry entry = this.zipFile.getEntry(TypeConstants.META_INF_MANIFEST_MF);
if (entry == null) {
Expand All @@ -403,12 +374,12 @@ public Manifest getManifest() {
}
@Override
public char[][] listPackages() {
if (!scanContent()) // ensure zipFile is initialized
if (!readKnownPackageNames()) // ensure zipFile is initialized
return null;
char[][] result = new char[this.knownPackageNames.elementSize][];
// -1 because it always contains empty string:
char[][] result = new char[this.knownPackageNames.size() - 1][];
int count = 0;
for (Object value : this.knownPackageNames.values) {
String string = (String) value;
for (String string : this.knownPackageNames) {
if (string != null &&!string.isEmpty()) {
result[count++] = string.replace('/', '.').toCharArray();
}
Expand All @@ -420,7 +391,7 @@ public char[][] listPackages() {

@Override
protected IBinaryType decorateWithExternalAnnotations(IBinaryType reader, String fileNameWithoutExtension) {
if (scanContent()) { // ensure zipFile is initialized
if (readKnownPackageNames()) { // ensure zipFile is initialized
String qualifiedBinaryFileName = fileNameWithoutExtension + ExternalAnnotationProvider.ANNOTATION_FILE_SUFFIX;
ZipEntry entry = this.zipFile.getEntry(qualifiedBinaryFileName);
if (entry != null) {
Expand Down
Loading
Loading