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.

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

8258117: jar tool sets the time stamp of module-info.class entries to the current time #5486

Closed
wants to merge 16 commits into from
Closed
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
@@ -120,7 +120,7 @@ public int hashCode() {
Set<Entry> entries = new LinkedHashSet<>();

// module-info.class entries need to be added/updated.
Map<String,byte[]> moduleInfos = new HashMap<>();
Map<String, ModuleInfoEntry> moduleInfos = new HashMap<>();

// A paths Set for each version, where each Set contains directories
// specified by the "-C" operation.
@@ -801,7 +801,9 @@ private void expand(File dir, String[] files, Set<String> cpaths, int version)
if (f.isFile()) {
Entry e = new Entry(f, name, false);
if (isModuleInfoEntry(name)) {
moduleInfos.putIfAbsent(name, Files.readAllBytes(f.toPath()));
Long lastModified = f.lastModified() == 0 ? null : f.lastModified();
moduleInfos.putIfAbsent(name,
new StreamedModuleInfoEntry(name, Files.readAllBytes(f.toPath()), lastModified));
if (uflag)
entryMap.put(name, e);
} else if (entries.add(e)) {
@@ -895,7 +897,7 @@ private boolean equalsIgnoreCase(String s, String upper) {
*/
boolean update(InputStream in, OutputStream out,
InputStream newManifest,
Map<String,byte[]> moduleInfos,
Map<String, ModuleInfoEntry> moduleInfos,
JarIndex jarIndex) throws IOException
{
ZipInputStream zis = new ZipInputStream(in);
@@ -944,7 +946,8 @@ boolean update(InputStream in, OutputStream out,
return false;
}
} else if (moduleInfos != null && isModuleInfoEntry) {
moduleInfos.putIfAbsent(name, zis.readAllBytes());
Long lastModified = e.getTime() == -1 ? null : e.getTime();
moduleInfos.putIfAbsent(name, new StreamedModuleInfoEntry(name, zis.readAllBytes(), lastModified));
} else {
boolean isDir = e.isDirectory();
if (!entryMap.containsKey(name)) { // copy the old stuff
@@ -1028,15 +1031,16 @@ private void addIndex(JarIndex index, ZipOutputStream zos)
zos.closeEntry();
}

private void updateModuleInfo(Map<String,byte[]> moduleInfos, ZipOutputStream zos)
private void updateModuleInfo(Map<String, ModuleInfoEntry> moduleInfos, ZipOutputStream zos)
throws IOException
{
String fmt = uflag ? "out.update.module-info": "out.added.module-info";
for (Map.Entry<String,byte[]> mi : moduleInfos.entrySet()) {
for (Map.Entry<String, ModuleInfoEntry> mi : moduleInfos.entrySet()) {
String name = mi.getKey();
byte[] bytes = mi.getValue();
byte[] bytes = mi.getValue().readAllBytes();
ZipEntry e = new ZipEntry(name);
e.setTime(System.currentTimeMillis());
Long lastModified = mi.getValue().getLastModifiedTime();
e.setTime(lastModified == null ? System.currentTimeMillis() : lastModified);
if (flag0) {
crc32ModuleInfo(e, bytes);
}
@@ -1731,12 +1735,23 @@ private File createTemporaryFile(String tmpbase, String suffix) {

/**
* Associates a module descriptor's zip entry name along with its
* bytes and an optional URI. Used when describing modules.
* bytes and an optional URI.
*/
interface ModuleInfoEntry {
String name();
Optional<String> uriString();
InputStream bytes() throws IOException;
/**
* @return Returns the last modified time of the module descriptor.
* Returns null if the last modified time is unknown or cannot be
* determined.
*/
Long getLastModifiedTime();
default byte[] readAllBytes() throws IOException {
try (InputStream is = bytes()) {
return is.readAllBytes();
}
}
Copy link
Contributor

@AlanBateman AlanBateman Nov 24, 2021

Choose a reason for hiding this comment

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

I think the comment would be clear if it says "the last modified time of the module-info.class".
Also would you mind changing ModuleInfoEntry to use 4-space rather than 2-space indentation?

}

static class ZipFileModuleInfoEntry implements ModuleInfoEntry {
@@ -1750,6 +1765,12 @@ static class ZipFileModuleInfoEntry implements ModuleInfoEntry {
@Override public InputStream bytes() throws IOException {
return zipFile.getInputStream(entry);
}

@Override
public Long getLastModifiedTime() {
return entry.getTime() == -1 ? null : entry.getTime();
}

/** Returns an optional containing the effective URI. */
@Override public Optional<String> uriString() {
String uri = (Paths.get(zipFile.getName())).toUri().toString();
@@ -1761,14 +1782,28 @@ static class ZipFileModuleInfoEntry implements ModuleInfoEntry {
static class StreamedModuleInfoEntry implements ModuleInfoEntry {
private final String name;
private final byte[] bytes;
StreamedModuleInfoEntry(String name, byte[] bytes) {
private final Long lastModifiedTime;
Copy link
Contributor

@AlanBateman AlanBateman Nov 22, 2021

Choose a reason for hiding this comment

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

It might be better to use a FileTime here, otherwise we risk a NPE when unboxing.

Copy link
Member Author

@jaikiran jaikiran Nov 22, 2021

Choose a reason for hiding this comment

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

Sounds good. I've updated the PR to replace this to use FileTime.


StreamedModuleInfoEntry(String name, byte[] bytes, Long lastModifiedTime) {
this.name = name;
this.bytes = bytes;
this.lastModifiedTime = lastModifiedTime;
}
@Override public String name() { return name; }
@Override public InputStream bytes() throws IOException {
return new ByteArrayInputStream(bytes);
}

@Override
public byte[] readAllBytes() throws IOException {
return bytes;
}

@Override
public Long getLastModifiedTime() {
return lastModifiedTime;
}

/** Returns an empty optional. */
@Override public Optional<String> uriString() {
return Optional.empty(); // no URI can be derived
@@ -1820,7 +1855,8 @@ private boolean describeModuleFromStream(FileInputStream fis)
while ((e = zis.getNextEntry()) != null) {
String ename = e.getName();
if (isModuleInfoEntry(ename)) {
infos.add(new StreamedModuleInfoEntry(ename, zis.readAllBytes()));
Long lastModified = e.getTime() == -1 ? null : e.getTime();
infos.add(new StreamedModuleInfoEntry(ename, zis.readAllBytes(), lastModified));
}
}
}
@@ -2033,14 +2069,14 @@ static String toBinaryName(String classname) {
return (classname.replace('.', '/')) + ".class";
}

private boolean checkModuleInfo(byte[] moduleInfoBytes, Set<String> entries)
private boolean checkModuleInfo(ModuleInfoEntry moduleInfoEntry, Set<String> entries)
throws IOException
{
boolean ok = true;
if (moduleInfoBytes != null) { // no root module-info.class if null
if (moduleInfoEntry != null) { // no root module-info.class if null
try {
// ModuleDescriptor.read() checks open/exported pkgs vs packages
ModuleDescriptor md = ModuleDescriptor.read(ByteBuffer.wrap(moduleInfoBytes));
ModuleDescriptor md = ModuleDescriptor.read(moduleInfoEntry.bytes());
// A module must have the implementation class of the services it 'provides'.
if (md.provides().stream().map(Provides::providers).flatMap(List::stream)
.filter(p -> !entries.contains(toBinaryName(p)))
@@ -2058,15 +2094,18 @@ private boolean checkModuleInfo(byte[] moduleInfoBytes, Set<String> entries)

/**
* Adds extended modules attributes to the given module-info's. The given
* Map values are updated in-place. Returns false if an error occurs.
* Map values are updated in-place.
*/
private void addExtendedModuleAttributes(Map<String,byte[]> moduleInfos,
private void addExtendedModuleAttributes(Map<String, ModuleInfoEntry> moduleInfos,
Set<String> packages)
throws IOException
{
for (Map.Entry<String,byte[]> e: moduleInfos.entrySet()) {
ModuleDescriptor md = ModuleDescriptor.read(ByteBuffer.wrap(e.getValue()));
e.setValue(extendedInfoBytes(md, e.getValue(), packages));
for (Map.Entry<String, ModuleInfoEntry> e: moduleInfos.entrySet()) {
byte[] bytes = e.getValue().readAllBytes();
ModuleDescriptor md = ModuleDescriptor.read(ByteBuffer.wrap(bytes));
byte[] extended = extendedInfoBytes(md, bytes, packages);
// replace the entry value with the extended bytes
e.setValue(new StreamedModuleInfoEntry(e.getValue().name(), extended, e.getValue().getLastModifiedTime()));
Copy link
Contributor

@AlanBateman AlanBateman Nov 22, 2021

Choose a reason for hiding this comment

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

There are 3 calls to getValue and each one I need to remember that e.getValue is a ModuleInfoEntry. If you add ModuleInfo entry = e.getValue() then it might help the readability.

Copy link
Member Author

@jaikiran jaikiran Nov 22, 2021

Choose a reason for hiding this comment

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

That makes sense. The updated PR now has this change.

}
}