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 7 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));
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.

I'd prefer to see this split into two two statements as there is just too much going on one the one line.

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.

Done. I have updated the PR to split this line into more than 1 statement.

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.

}
}

@@ -0,0 +1,219 @@
/*
* Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License version 2 only, as
* published by the Free Software Foundation.
*
* This code is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* version 2 for more details (a copy is included in the LICENSE file that
* accompanied this code).
*
* You should have received a copy of the GNU General Public License version
* 2 along with this work; if not, write to the Free Software Foundation,
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
* or visit www.oracle.com if you need additional information or have any
* questions.
*/

import org.testng.annotations.BeforeClass;
import org.testng.annotations.Test;

import java.io.IOException;
import java.io.PrintWriter;
import java.io.StringWriter;
import java.lang.module.ModuleDescriptor;
import java.nio.file.FileVisitResult;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.nio.file.SimpleFileVisitor;
import java.nio.file.attribute.BasicFileAttributes;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.jar.JarEntry;
import java.util.jar.JarInputStream;
import java.util.spi.ToolProvider;

import static org.testng.Assert.assertEquals;
import static org.testng.Assert.assertFalse;
import static org.testng.Assert.assertNotNull;

/**
* @test
* @bug 8258117
* @summary Tests that the content generated for module-info.class, using the jar command, is reproducible
* @run testng JarToolModuleDescriptorReproducibilityTest
*/
public class JarToolModuleDescriptorReproducibilityTest {

private static final String MODULE_NAME = "foo";
private static final String MODULE_VERSION = "1.2.3";
private static final String UPDATED_MODULE_VERSION = "1.2.4";
private static final String MAIN_CLASS = "jdk.test.foo.Foo";
private static final Path MODULE_CLASSES_DIR = Paths.get("8258117-module-classes", MODULE_NAME).toAbsolutePath();
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.

You can use Path.of here.

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.

Done. Replaced this and one other place in this test to use Path.of.

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.

Test continues to pass with all these new changes.


private static final ToolProvider JAR_TOOL = ToolProvider.findFirst("jar")
.orElseThrow(()
-> new RuntimeException("jar tool not found")
);
private static final ToolProvider JAVAC_TOOL = ToolProvider.findFirst("javac")
.orElseThrow(()
-> new RuntimeException("javac tool not found")
);


@BeforeClass
public static void setup() throws Exception {
compileModuleClasses();
}

/**
* Launches a "jar --create" command multiple times with a module-info.class. The module-info.class
* is internally updated by the jar tool to add additional data. Expects that each such generated
* jar has the exact same bytes.
*/
@Test
public void testJarCreate() throws Exception {
List<Path> jarFiles = new ArrayList<>();
for (int i = 0; i < 3; i++) {
Path targetJar = Files.createTempFile(Path.of("."), "8258117-jar-create", ".jar");
jarFiles.add(targetJar);
if (i > 0) {
// the timestamp that gets embedded in (Zip/Jar)Entry gets narrowed
// down to SECONDS unit. So we make sure that there's at least a second
// gap between the jar file creations, to be sure that the jar file
// was indeed generated at "different times"
Thread.sleep(1000);
}
// create a modular jar
runJarCommand("--create",
"--file=" + targetJar,
"--main-class=" + MAIN_CLASS,
"--module-version=" + MODULE_VERSION,
"--no-manifest",
"-C", MODULE_CLASSES_DIR.toString(), ".");
// verify the module descriptor in the jar
assertExpectedModuleInfo(targetJar, MODULE_VERSION);
}
assertAllFileContentsAreSame(jarFiles);
}

/**
* Launches a "jar --update" process multiple times to update the module-info.class
* descriptor with the same content and then expects that the modular jar created by
* each of these processes has the exact same bytes.
*/
@Test
public void testJarUpdate() throws Exception {
List<Path> jarFiles = new ArrayList<>();
for (int i = 0; i < 3; i++) {
Path targetJar = Files.createTempFile(Path.of("."), "8258117-jar-update", ".jar");
jarFiles.add(targetJar);
if (i > 0) {
// the timestamp that gets embedded in (Zip/Jar)Entry gets narrowed
// down to SECONDS unit. So we make sure that there's at least a second
// gap between the jar file creations, to be sure that the jar file
// was indeed generated at "different times"
Thread.sleep(1000);
}
// first create the modular jar
runJarCommand("--create",
"--file=" + targetJar,
"--module-version=" + MODULE_VERSION,
"--no-manifest",
"-C", MODULE_CLASSES_DIR.toString(), ".");
assertExpectedModuleInfo(targetJar, MODULE_VERSION);
// now update the same modular jar
runJarCommand("--update",
"--file=" + targetJar,
"--module-version=" + UPDATED_MODULE_VERSION,
"--no-manifest",
"-C", MODULE_CLASSES_DIR.toString(), "module-info.class");
// verify the module descriptor in the jar
assertExpectedModuleInfo(targetJar, UPDATED_MODULE_VERSION);
}
assertAllFileContentsAreSame(jarFiles);
}

// compiles using javac tool the classes used in the test module
private static void compileModuleClasses() throws Exception {
Path sourcePath = Paths.get(System.getProperty("test.src", "."),
"src", MODULE_NAME);
List<String> sourceFiles = new ArrayList<>();
Files.walkFileTree(sourcePath, new SimpleFileVisitor<>() {
@Override
public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IOException {
if (file.toString().endsWith(".java")) {
sourceFiles.add(file.toString());
}
return FileVisitResult.CONTINUE;
}
});
Path classesDir = Files.createDirectories(MODULE_CLASSES_DIR);
List<String> javacArgs = new ArrayList<>();
javacArgs.add("-d");
javacArgs.add(classesDir.toString());
sourceFiles.forEach((f) -> javacArgs.add(f));
System.out.println("Launching javac command with args: " + javacArgs);
StringWriter sw = new StringWriter();
try (PrintWriter pw = new PrintWriter(sw)) {
int exitCode = JAVAC_TOOL.run(pw, pw, javacArgs.toArray(new String[0]));
assertEquals(exitCode, 0, "Module compilation failed: " + sw.toString());
}
System.out.println("Module classes successfully compiled to directory " + classesDir);
}

// runs the "jar" command passing it the "jarArgs" and verifying that the command
// execution didn't fail
private static void runJarCommand(String... jarArgs) {
StringWriter sw = new StringWriter();
System.out.println("Launching jar command with args: " + Arrays.toString(jarArgs));
try (PrintWriter pw = new PrintWriter(sw)) {
int exitCode = JAR_TOOL.run(pw, pw, jarArgs);
assertEquals(exitCode, 0, "jar command execution failed: " + sw.toString());
}
}

// verifies the byte equality of the contents in each of the files
private static void assertAllFileContentsAreSame(List<Path> files) throws Exception {
Path firstFile = files.get(0);
for (int i = 1; i < files.size(); i++) {
assertEquals(Files.mismatch(firstFile, files.get(i)), -1,
"Content in file " + files.get(i) + " isn't the same as in file " + firstFile);
}
}

// verifies that a module-info.class is present in the jar and the module name and version are the expected
// ones
private static void assertExpectedModuleInfo(Path jar, String expectedModuleVersion) throws Exception {
try (JarInputStream jaris = new JarInputStream(Files.newInputStream(jar))) {
JarEntry moduleInfoEntry = null;
JarEntry entry = null;
while ((entry = jaris.getNextJarEntry()) != null) {
if (entry.getName().equals("module-info.class")) {
moduleInfoEntry = entry;
break;
}
}
assertNotNull(moduleInfoEntry, "module-info.class is missing from jar " + jar);

ModuleDescriptor md = ModuleDescriptor.read(jaris);
assertEquals(md.name(), MODULE_NAME, "Unexpected module name");
assertFalse(md.rawVersion().isEmpty(), "Module version missing from descriptor");

String actualVersion = md.rawVersion().get();
assertEquals(actualVersion, expectedModuleVersion, "Unexpected module version");

System.out.println(moduleInfoEntry.getName() + " has a timestamp of "
+ moduleInfoEntry.getTime() + " for version " + actualVersion);
}
}
}