Skip to content

增加更多memorycompiler对jar内容的获取 #3104

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

Open
1luik wants to merge 3 commits into
base: master
Choose a base branch
from
+189 −20
Open

增加更多memorycompiler对jar内容的获取 #3104

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
0055c55
修复#3103
Dec 16, 2025
dc9d8fe
修复#3103
Dec 16, 2025
244fea5
修复#3103
Dec 17, 2025
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
113 changes: 113 additions & 0 deletions memorycompiler/src/main/java/com/taobao/arthas/compiler/GetZipFile.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
package com.taobao.arthas.compiler;

import javax.lang.model.element.Modifier;
Copy link

Copilot AI Jan 14, 2026

Choose a reason for hiding this comment

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

The unused import javax.lang.model.element.Modifier is imported but never used in this class.

Copilot uses AI. Check for mistakes.
import javax.lang.model.element.NestingKind;
Copy link

Copilot AI Jan 14, 2026

Choose a reason for hiding this comment

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

The unused import javax.lang.model.element.NestingKind is imported but never used in this class.

Copilot uses AI. Check for mistakes.
import javax.tools.JavaFileObject;
import java.io.*;
import java.net.URI;
import java.nio.charset.Charset;
Copy link

Copilot AI Jan 14, 2026

Choose a reason for hiding this comment

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

The unused import java.nio.charset.Charset is imported but never used in this class.

Suggested change
import java.nio.charset.Charset;

Copilot uses AI. Check for mistakes.
import java.util.ArrayList;
import java.util.Enumeration;
import java.util.List;
import java.util.stream.Stream;
import java.util.zip.ZipEntry;
import java.util.zip.ZipException;
import java.util.zip.ZipFile;

public class GetZipFile extends ZipFile {
Copy link

Copilot AI Jan 14, 2026

Choose a reason for hiding this comment

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

The class name GetZipFile is poorly named as it uses a verb ("Get") for a class that represents an extended ZipFile. A better name would be something like CustomZipFile, ExtendedZipFile, or StreamableZipFile to better describe what the class represents.

Copilot uses AI. Check for mistakes.

public GetZipFile(String name) throws IOException {
super(name);
}

public GetZipFile(File file, int mode) throws IOException {
super(file, mode);
}

public GetZipFile(File file) throws ZipException, IOException {
super(file);
}

public Stream<ZipEntry> stream() {
Enumeration<? extends ZipEntry> entries = super.entries();
List<ZipEntry> entryList = new ArrayList<>();
while (entries.hasMoreElements()) {
entryList.add(entries.nextElement());
}
return entryList.stream();
Comment on lines +32 to +37
Copy link

Copilot AI Jan 14, 2026

Choose a reason for hiding this comment

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

The stream() method loads all ZipEntry elements into an ArrayList before converting to a stream, which is inefficient for large zip files. This creates unnecessary memory overhead. Consider using Collections.list(entries).stream() or implementing a custom Spliterator for better performance and memory efficiency.

Suggested change
Enumeration<? extends ZipEntry> entries = super.entries();
List<ZipEntry> entryList = new ArrayList<>();
while (entries.hasMoreElements()) {
entryList.add(entries.nextElement());
}
return entryList.stream();
return super.stream().map(e -> (ZipEntry) e);

Copilot uses AI. Check for mistakes.
}

public class ZipJavaFileObject implements JavaFileObject {
private final String className;
private final URI uri;
private final ZipEntry file;

public ZipJavaFileObject(String className, URI uri, ZipEntry file) {
this.uri = uri;
this.className = className;
this.file = file;
}

public URI toUri() {
return uri;
}

public InputStream openInputStream() throws IOException {
return GetZipFile.this.getInputStream(file);
}

public OutputStream openOutputStream() {
throw new UnsupportedOperationException();
}

public String getName() {
return this.className;
}

public Reader openReader(boolean ignoreEncodingErrors) {
throw new UnsupportedOperationException();
}

public CharSequence getCharContent(boolean ignoreEncodingErrors) {
throw new UnsupportedOperationException();
}

public Writer openWriter() throws IOException {
throw new UnsupportedOperationException();
}

public long getLastModified() {
return 0;
Copy link

Copilot AI Jan 14, 2026

Choose a reason for hiding this comment

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

The getLastModified() method always returns 0, which is not accurate. It should return the actual last modified time from the ZipEntry. Consider using file.getTime() to get the correct timestamp.

Suggested change
return 0;
return file.getTime();

Copilot uses AI. Check for mistakes.
}

public boolean delete() {
throw new UnsupportedOperationException();
}

public Kind getKind() {
return Kind.CLASS;
}

public boolean isNameCompatible(String simpleName, Kind kind) {
return Kind.CLASS.equals(getKind())
&& this.className.endsWith(simpleName);
}

public NestingKind getNestingKind() {
throw new UnsupportedOperationException();
}

public Modifier getAccessLevel() {
throw new UnsupportedOperationException();
}

public String getClassName() {
return this.className;
}


public String toString() {
return this.getClass().getName() + "[" + this.toUri() + "]";
}
}
}
Comment on lines +17 to +113
Copy link

Copilot AI Jan 14, 2026

Choose a reason for hiding this comment

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

The new GetZipFile class and its inner ZipJavaFileObject class lack any documentation. Adding class-level and method-level JavaDoc comments would help other developers understand the purpose of these classes and how they should be used, especially since this is handling a complex edge case with custom URL handlers.

Copilot uses AI. Check for mistakes.
96 changes: 76 additions & 20 deletions memorycompiler/src/main/java/com/taobao/arthas/compiler/PackageInternalsFinder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,7 @@
import javax.tools.JavaFileObject;
import java.io.File;
import java.io.IOException;
import java.net.JarURLConnection;
import java.net.URI;
import java.net.URL;
import java.net.URLDecoder;
import java.net.*;
Copy link

Copilot AI Jan 14, 2026

Choose a reason for hiding this comment

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

Using wildcard import import java.net.*; reduces code clarity. It's better to explicitly import only the classes that are actually used (URLConnection, JarURLConnection, URI, URL, URLDecoder) so readers can easily see the dependencies.

Suggested change
import java.net.*;
import java.net.JarURLConnection;
import java.net.URI;
import java.net.URL;
import java.net.URLConnection;
import java.net.URLDecoder;

Copilot uses AI. Check for mistakes.
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
Expand All @@ -36,8 +33,12 @@
import java.util.List;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
import java.util.function.Consumer;
Copy link

Copilot AI Jan 14, 2026

Choose a reason for hiding this comment

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

The unused import java.util.function.Consumer should be removed as it's not used anywhere in the code.

Suggested change
import java.util.function.Consumer;

Copilot uses AI. Check for mistakes.
import java.util.jar.JarEntry;
import java.util.jar.JarFile;
import java.util.stream.Collectors;
import java.util.zip.ZipEntry;
import java.util.zip.ZipException;

public class PackageInternalsFinder {
private final ClassLoader classLoader;
Expand Down Expand Up @@ -88,32 +89,87 @@ private List<JavaFileObject> processJar(String packageName, URL packageFolderURL
// ignore
}
// 保底
return fuse(packageFolderURL);
return fuse(packageFolderURL, packageName);
}

private List<JavaFileObject> fuse(URL packageFolderURL) {
private List<JavaFileObject> fuse(URL packageFolderURL, String packageName) {
List<JavaFileObject> result = new ArrayList<JavaFileObject>();
try {
String jarUri = packageFolderURL.toExternalForm().substring(0, packageFolderURL.toExternalForm().lastIndexOf("!/"));

JarURLConnection jarConn = (JarURLConnection) packageFolderURL.openConnection();
String rootEntryName = jarConn.getEntryName();
URLConnection urlConnection = packageFolderURL.openConnection();
Enumeration<JarEntry> entryEnum;
if (urlConnection instanceof JarURLConnection){
try {
int rootEnd = 0;
JarURLConnection jarConn = (JarURLConnection) urlConnection;
String rootEntryName = jarConn.getEntryName();
//可能为 null（自己没有类文件时）
if (rootEntryName != null) {
rootEnd = rootEntryName.length() + 1;
entryEnum = jarConn.getJarFile().entries();
} else {
return result;
}

while (entryEnum.hasMoreElements()) {
JarEntry jarEntry = entryEnum.nextElement();
String name = jarEntry.getName();
if (name.startsWith(rootEntryName) && name.indexOf('/', rootEnd) == -1 && name.endsWith(CLASS_FILE_EXTENSION)) {
URI uri = URI.create(jarUri + "!/" + name);
String binaryName = name.replaceAll("/", ".");
binaryName = binaryName.replaceAll(CLASS_FILE_EXTENSION + "$", "");

if (rootEntryName != null) {
//可能为 null（自己没有类文件时）
int rootEnd = rootEntryName.length() + 1;
result.add(new CustomJavaFileObject(binaryName, uri));
}
}
} catch (Exception e) {
throw new RuntimeException("Failed to open " + packageFolderURL + " as a jar file", e);
}
}else {
//是否可以在硬盘上找到对应的文件
try {
URI uri1 = new URI(jarUri);
// 获取 authority（host:port） + path + query + fragment
Comment on lines +130 to +133
Copy link

Copilot AI Jan 14, 2026

Choose a reason for hiding this comment

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

Comment line contains Chinese text: "获取 authority（host:port） + path + query + fragment" (translation: "Get authority (host:port) + path + query + fragment"). Consider translating to English for consistency with international development practices.

Suggested change
//是否可以在硬盘上找到对应的文件
try {
URI uri1 = new URI(jarUri);
// 获取 authority（host:port） + path + query + fragment
// Check whether the corresponding file can be found on the local file system
try {
URI uri1 = new URI(jarUri);
// Get authority (host:port) + path + query + fragment from the URI

Copilot uses AI. Check for mistakes.
StringBuilder sb = new StringBuilder();
if (uri1.getAuthority() != null) {
sb.append(uri1.getAuthority());
}
if (uri1.getPath() != null) {
sb.append(uri1.getPath());
}
if (uri1.getQuery() != null) {
sb.append('?').append(uri1.getQuery());
}
if (uri1.getFragment() != null) {
sb.append('#').append(uri1.getFragment());
Comment on lines +132 to +145
Copy link

Copilot AI Jan 14, 2026

Choose a reason for hiding this comment

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

The variable name uri1 is not descriptive. Consider using a more meaningful name like parsedJarUri or jarFileUri to clearly indicate what this URI represents.

Suggested change
URI uri1 = new URI(jarUri);
// 获取 authority（host:port） + path + query + fragment
StringBuilder sb = new StringBuilder();
if (uri1.getAuthority() != null) {
sb.append(uri1.getAuthority());
}
if (uri1.getPath() != null) {
sb.append(uri1.getPath());
}
if (uri1.getQuery() != null) {
sb.append('?').append(uri1.getQuery());
}
if (uri1.getFragment() != null) {
sb.append('#').append(uri1.getFragment());
URI parsedJarUri = new URI(jarUri);
// 获取 authority（host:port） + path + query + fragment
StringBuilder sb = new StringBuilder();
if (parsedJarUri.getAuthority() != null) {
sb.append(parsedJarUri.getAuthority());
}
if (parsedJarUri.getPath() != null) {
sb.append(parsedJarUri.getPath());
}
if (parsedJarUri.getQuery() != null) {
sb.append('?').append(parsedJarUri.getQuery());
}
if (parsedJarUri.getFragment() != null) {
sb.append('#').append(parsedJarUri.getFragment());

Copilot uses AI. Check for mistakes.
}

Enumeration<JarEntry> entryEnum = jarConn.getJarFile().entries();
while (entryEnum.hasMoreElements()) {
JarEntry jarEntry = entryEnum.nextElement();
String name = jarEntry.getName();
if (name.startsWith(rootEntryName) && name.indexOf('/', rootEnd) == -1 && name.endsWith(CLASS_FILE_EXTENSION)) {
URI uri = URI.create(jarUri + "!/" + name);
String binaryName = name.replaceAll("/", ".");
binaryName = binaryName.replaceAll(CLASS_FILE_EXTENSION + "$", "");
File file = new File(sb.toString());
if (file.exists()) {
//可能是文件或目录
if (file.isDirectory()) {
result.addAll(processDir(packageName, file));
} else {
try {
GetZipFile zipFile = new GetZipFile(file);
zipFile.stream().forEach(e -> {
String name = e.getName();
if (name.startsWith(packageName) && name.endsWith(CLASS_FILE_EXTENSION)) {
URI uri = URI.create(jarUri + "!/" + name);
String binaryName = name.replaceAll("/", ".");
binaryName = binaryName.replaceAll(CLASS_FILE_EXTENSION + "$", "");

result.add(new CustomJavaFileObject(binaryName, uri));
result.add(zipFile.new ZipJavaFileObject(binaryName, uri, e));
}
});
Comment on lines +156 to +165
Copy link

Copilot AI Jan 14, 2026

Choose a reason for hiding this comment

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

The parameter name e in the forEach lambda is not descriptive. Consider using a more meaningful name like zipEntry or entry to improve code readability.

Copilot uses AI. Check for mistakes.
Comment on lines +155 to +165
Copy link

Copilot AI Jan 14, 2026

Choose a reason for hiding this comment

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

The GetZipFile resource is not properly closed. The ZipFile is created but never closed, which can lead to resource leaks and file handle exhaustion. Consider using try-with-resources or ensuring the file is closed after processing.

Copilot uses AI. Check for mistakes.
}catch (ZipException e) {
// ignore
}
}
}
}catch (SecurityException e) {
Copy link

Copilot AI Jan 14, 2026

Choose a reason for hiding this comment

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

Missing a URISyntaxException handler. The new URI(jarUri) call on line 132 can throw URISyntaxException, but the catch block on line 171 only catches SecurityException. This means URISyntaxException will be caught by the outer catch block on line 175, which may not provide the appropriate error message for URI syntax issues.

Suggested change
}catch (SecurityException e) {
} catch (URISyntaxException e) {
throw new RuntimeException("Invalid URI syntax for jarUri: " + jarUri, e);
} catch (SecurityException e) {

Copilot uses AI. Check for mistakes.
throw new RuntimeException("Failed to open " + packageFolderURL + " as a jar file", e);
}
}
Comment on lines +129 to 174
Copy link

Copilot AI Jan 14, 2026

Choose a reason for hiding this comment

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

The new fallback logic (lines 129-174) for handling non-JarURLConnection cases lacks test coverage. Given the complexity of the URI parsing and file system access logic, and the fact that this addresses a specific issue with Minecraft Forge's custom URL handlers, adding test cases for this code path would help ensure it works correctly and prevent regressions.

Copilot uses AI. Check for mistakes.
} catch (Exception e) {
Expand Down
Loading