Skip to content

Commit

Permalink
8303972: (zipfs) Make test/jdk/jdk/nio/zipfs/TestLocOffsetFromZip64EF…
Browse files Browse the repository at this point in the history 
….java independent of the zip command line

8301183: (zipfs) jdk/jdk/nio/zipfs/TestLocOffsetFromZip64EF.java failing with ZipException:R0 on OL9

Backport-of: 7004c2724d9b150112c66febb7f24b781ff379dd
  • Loading branch information
@shipilev
Sonia Zaldana Calles authored and shipilev committed Mar 25, 2024
1 parent 39f7178 commit 5694ad2
Show file tree
Hide file tree
Showing 2 changed files with 165 additions and 119 deletions.
2 changes: 0 additions & 2 deletions test/jdk/ProblemList.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -562,8 +562,6 @@ java/nio/channels/DatagramChannel/AfterDisconnect.java 8308807 aix-ppc6
java/nio/channels/DatagramChannel/ManySourcesAndTargets.java 8264385 macosx-aarch64
java/nio/channels/DatagramChannel/Unref.java 8233437 generic-all

jdk/nio/zipfs/TestLocOffsetFromZip64EF.java 8301183 linux-all

java/nio/channels/DatagramChannel/AfterDisconnect.java 8308807 aix-ppc64

############################################################################
Expand Down
282 changes: 165 additions & 117 deletions test/jdk/jdk/nio/zipfs/TestLocOffsetFromZip64EF.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2020, 2022, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2020, 2024, 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
Expand All @@ -21,113 +21,124 @@
* questions.
*/

import org.testng.Assert;
import org.testng.annotations.AfterClass;
import org.testng.annotations.BeforeClass;
import org.testng.annotations.DataProvider;
import org.testng.annotations.Test;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.MethodSource;

import java.io.*;
import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.nio.ByteBuffer;
import java.nio.ByteOrder;
import java.nio.charset.StandardCharsets;
import java.nio.file.FileSystem;
import java.nio.file.*;
import java.nio.file.attribute.BasicFileAttributes;
import java.util.List;
import java.nio.file.attribute.FileTime;
import java.time.Instant;
import java.util.HashSet;
import java.util.Map;
import java.util.Set;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import java.util.zip.ZipEntry;
import java.util.zip.ZipFile;
import java.util.zip.ZipOutputStream;

import static org.junit.jupiter.api.Assertions.assertEquals;

import static java.lang.String.format;

/**
* @test
* @bug 8255380 8257445
* @summary Test that Zip FS can access the LOC offset from the Zip64 extra field
* @modules jdk.zipfs
* @requires (os.family == "linux") | (os.family == "mac")
* @run testng/manual TestLocOffsetFromZip64EF
* @run junit TestLocOffsetFromZip64EF
*/
public class TestLocOffsetFromZip64EF {

private static final String ZIP_FILE_NAME = "LargeZipTest.zip";
// File that will be created with a size greater than 0xFFFFFFFF
private static final String LARGE_FILE_NAME = "LargeZipEntry.txt";
// File that will be created with a size less than 0xFFFFFFFF
private static final String SMALL_FILE_NAME = "SmallZipEntry.txt";
// The size (4GB) of the large file to be created
private static final long LARGE_FILE_SIZE = 4L * 1024L * 1024L * 1024L;
private static final String ZIP_FILE_NAME = "LocOffsetFromZip64.zip";

// Size of the data block of a Zip64 extended information field with long
// fields for 'uncompressed size', 'compressed size' and 'local header offset'
private static short ZIP64_DATA_SIZE = (short) Long.BYTES // Uncompressed size
+ Long.BYTES // Compressed size
+ Long.BYTES; // Loc offset

// Size of the extra field header
private static short EXTRA_HEADER_SIZE = Short.BYTES // tag
+ Short.BYTES; // data size

// Size of a Zip64 extended information field including the header
private static final int ZIP64_SIZE = EXTRA_HEADER_SIZE + ZIP64_DATA_SIZE;

// The Zip64 Magic value for 32-bit fields
private static final int ZIP64_MAGIC_VALUE = 0XFFFFFFFF;
// The 'unknown' tag, see APPNOTE.txt
private static final short UNKNOWN_TAG = (short) 0x9902;
// The 'Zip64 extended information' tag, see APPNOTE.txt
private static final short ZIP64_TAG = (short) 0x1;

/**
* Create the files used by this test
*
* @throws IOException if an error occurs
*/
@BeforeClass
@BeforeEach
public void setUp() throws IOException {
System.out.println("In setup");
cleanup();
createFiles();
createZipWithZip64Ext();
}

/**
* Delete files used by this test
* @throws IOException if an error occurs
*/
@AfterClass
@AfterEach
public void cleanup() throws IOException {
System.out.println("In cleanup");
Files.deleteIfExists(Path.of(ZIP_FILE_NAME));
Files.deleteIfExists(Path.of(LARGE_FILE_NAME));
Files.deleteIfExists(Path.of(SMALL_FILE_NAME));
}

/**
* Create a Zip file that will result in a Zip64 Extra (EXT) header
* being added to the CEN entry in order to find the LOC offset for
* SMALL_FILE_NAME.
*/
public static void createZipWithZip64Ext() {
System.out.println("Executing zip...");
List<String> commands = List.of("zip", "-0", ZIP_FILE_NAME,
LARGE_FILE_NAME, SMALL_FILE_NAME);
Result rc = run(new ProcessBuilder(commands));
rc.assertSuccess();
}

/*
* DataProvider used to verify that a Zip file that contains a Zip64 Extra
* MethodSource used to verify that a Zip file that contains a Zip64 Extra
* (EXT) header can be traversed
*/
@DataProvider(name = "zipInfoTimeMap")
protected Object[][] zipInfoTimeMap() {
return new Object[][]{
{Map.of()},
{Map.of("zipinfo-time", "False")},
{Map.of("zipinfo-time", "true")},
{Map.of("zipinfo-time", "false")}
};
static Stream<Map<String, String>> zipInfoTimeMap() {
return Stream.of(
Map.of(),
Map.of("zipinfo-time", "False"),
Map.of("zipinfo-time", "true"),
Map.of("zipinfo-time", "false")
);
}

/**
* Navigate through the Zip file entries using Zip FS
* @param env Zip FS properties to use when accessing the Zip file
* @throws IOException if an error occurs
*/
@Test(dataProvider = "zipInfoTimeMap")
@ParameterizedTest
@MethodSource("zipInfoTimeMap")
public void walkZipFSTest(final Map<String, String> env) throws IOException {
Set<String> entries = new HashSet<>();

try (FileSystem fs =
FileSystems.newFileSystem(Paths.get(ZIP_FILE_NAME), env)) {
for (Path root : fs.getRootDirectories()) {
Files.walkFileTree(root, new SimpleFileVisitor<>() {
@Override
public FileVisitResult visitFile(Path file, BasicFileAttributes
attrs) throws IOException {
entries.add(file.getFileName().toString());
System.out.println(Files.readAttributes(file,
BasicFileAttributes.class).toString());
return FileVisitResult.CONTINUE;
}
});
}
}
// Sanity check that ZIP file had the expected entries
assertEquals(Set.of("entry", "entry2", "entry3"), entries);
}

/**
Expand All @@ -139,92 +150,129 @@ public void walkZipFileTest() throws IOException {
try (ZipFile zip = new ZipFile(ZIP_FILE_NAME)) {
zip.stream().forEach(z -> System.out.printf("%s, %s, %s%n",
z.getName(), z.getMethod(), z.getLastModifiedTime()));
}
}

/**
* Create the files that will be added to the ZIP file
* @throws IOException if there is a problem creating the files
*/
private static void createFiles() throws IOException {
try (RandomAccessFile file = new RandomAccessFile(LARGE_FILE_NAME, "rw")
) {
System.out.printf("Creating %s%n", LARGE_FILE_NAME);
file.setLength(LARGE_FILE_SIZE);
System.out.printf("Creating %s%n", SMALL_FILE_NAME);
Files.writeString(Path.of(SMALL_FILE_NAME), "Hello");
// Sanity check that ZIP file had the expected entries
assertEquals(zip.stream().map(ZipEntry::getName).collect(Collectors.toSet()),
Set.of("entry", "entry2", "entry3"));
}
}

/**
* Utility method to execute a ProcessBuilder command
* @param pb ProcessBuilder to execute
* @return The Result(s) from the ProcessBuilder execution
* This produces a ZIP with similar features as the one created by 'Info-ZIP' which
* caused 'Extended timestamp' parsing to fail before JDK-8255380.
*
* The issue was sensitive to the ordering of 'Info-ZIP extended timestamp' fields and
* 'Zip64 extended information' fields. ZipOutputStream and 'Info-ZIP' order these differently.
*
* ZipFileSystem tried to read the Local file header while parsing the extended timestamp,
* but if the Zip64 extra field was not read yet, ZipFileSystem would incorrecly try to read
* the Local File header from offset 0xFFFFFFFF.
*
* This method creates a ZIP file which includes a CEN with the following features:
*
* - Its extra field has a 'Info-ZIP extended timestamp' field followed by a
* 'Zip64 extended information' field.
* - The sizes and offset fields values of the CEN are set to 0xFFFFFFFF (Zip64 magic values)
*
*/
private static Result run(ProcessBuilder pb) {
Process p;
System.out.printf("Running: %s%n", pb.command());
try {
p = pb.start();
} catch (IOException e) {
throw new RuntimeException(
format("Couldn't start process '%s'", pb.command()), e);
}
public void createZipWithZip64Ext() throws IOException {
ByteArrayOutputStream out = new ByteArrayOutputStream();
try (ZipOutputStream zo = new ZipOutputStream(out)) {

String output;
try {
output = toString(p.getInputStream(), p.getErrorStream());
} catch (IOException e) {
throw new RuntimeException(
format("Couldn't read process output '%s'", pb.command()), e);
}
ZipEntry e = new ZipEntry("entry");
// Add an entry, make it STORED and empty to simplify parsing
e.setMethod(ZipEntry.STORED);
e.setSize(0);
e.setCrc(0);
zo.putNextEntry(e);

// Add an additional entry as a sanity check that we can navigate past the first
ZipEntry e2 = new ZipEntry("entry2");
e2.setMethod(ZipEntry.STORED);
e2.setSize(0);
e2.setCrc(0);
zo.putNextEntry(e2);

// For good measure, add a third, DEFLATED entry with some content
ZipEntry e3 = new ZipEntry("entry3");
e3.setMethod(ZipEntry.DEFLATED);
zo.putNextEntry(e3);
zo.write("Hello".getBytes(StandardCharsets.UTF_8));

try {
p.waitFor();
} catch (InterruptedException e) {
throw new RuntimeException(
format("Process hasn't finished '%s'", pb.command()), e);
zo.closeEntry(); // At this point, all LOC headers are written.

// We want the first CEN entry to have two extra fields:
// 1: A 'Info-Zip extended timestamp' extra field, generated by ZipOutputStream
// when the following date fields are set:
e.setLastModifiedTime(FileTime.from(Instant.now()));
e.setLastAccessTime(FileTime.from(Instant.now()));

// 2: An opaque extra field, right-sized for a Zip64 extended field,
// to be updated below
byte[] zip64 = makeOpaqueExtraField();
e.setExtra(zip64);

zo.finish(); // Write out CEN and END records
}
return new Result(p.exitValue(), output);

byte[] zip = out.toByteArray();

// ZIP now has the right structure, but we need to update the CEN to Zip64 format
updateToZip64(zip);
// Write the ZIP to disk
Files.write(Path.of(ZIP_FILE_NAME), zip);
}

/**
* Utility Method for combining the output from a ProcessBuilder invocation
* @param in1 ProccessBuilder.getInputStream
* @param in2 ProcessBuilder.getErrorStream
* @return The ProcessBuilder output
* @throws IOException if an error occurs
* Returns an opaque extra field with the tag 'unknown', which makes ZipEntry.setExtra ignore it.
* The returned field has the expected field and data size of a Zip64 extended information field
* including the fields 'uncompressed size' (8 bytes), 'compressed size' (8 bytes) and
* 'local header offset' (8 bytes).
*/
static String toString(InputStream in1, InputStream in2) throws IOException {
try (ByteArrayOutputStream dst = new ByteArrayOutputStream();
InputStream concatenated = new SequenceInputStream(in1, in2)) {
concatenated.transferTo(dst);
return new String(dst.toByteArray(), StandardCharsets.UTF_8);
}
private static byte[] makeOpaqueExtraField() {
byte[] zip64 = new byte[ZIP64_SIZE];
ByteBuffer buffer = ByteBuffer.wrap(zip64).order(ByteOrder.LITTLE_ENDIAN);
// Using the 'unknown' tag makes ZipEntry.setExtra ignore it
buffer.putShort(UNKNOWN_TAG);
// Data size
buffer.putShort(ZIP64_DATA_SIZE);
return zip64;
}

/**
* Utility class used to hold the results from a ProcessBuilder execution
* Update the CEN record to Zip64 format
*/
static class Result {
final int ec;
final String output;
private static void updateToZip64(byte[] bytes) throws IOException {

private Result(int ec, String output) {
this.ec = ec;
this.output = output;
}
Result assertSuccess() {
assertTrue(ec == 0, "Expected ec 0, got: ", ec, " , output [", output, "]");
return this;
}
ByteBuffer buffer = ByteBuffer.wrap(bytes).order(ByteOrder.LITTLE_ENDIAN);

// Look up CEN offset from the End of central directory header
int cenOff = getCenOffet(buffer);

// Read name, extra field and comment lengths from CEN
short nlen = buffer.getShort(cenOff + ZipFile.CENNAM);
short elen = buffer.getShort(cenOff + ZipFile.CENEXT);

// Update CEN sizes and loc offset to 0xFFFFFFFF, meaning
// actual values should be read from the Zip64 field
buffer.putInt(cenOff + ZipFile.CENLEN, ZIP64_MAGIC_VALUE);
buffer.putInt(cenOff + ZipFile.CENSIZ, ZIP64_MAGIC_VALUE);
buffer.putInt(cenOff + ZipFile.CENOFF, ZIP64_MAGIC_VALUE);

// Offset of the extra fields
int extraOff = cenOff + ZipFile.CENHDR + nlen;

// Position at the start of the Zip64 extra field
int zip64ExtraOff = extraOff + elen - ZIP64_SIZE;

// Update tag / Header ID to be the actual Zip64 tag instead of the 'unknown'
buffer.putShort(zip64ExtraOff, ZIP64_TAG);
}
static void assertTrue(boolean cond, Object ... failedArgs) {
if (cond)
return;
StringBuilder sb = new StringBuilder();
for (Object o : failedArgs)
sb.append(o);
Assert.fail(sb.toString());

/**
* Look up the CEN offset field from the End of central directory header
*/
private static int getCenOffet(ByteBuffer buffer) {
return buffer.getInt(buffer.capacity() - ZipFile.ENDHDR + ZipFile.ENDOFF);
}
}

1 comment on commit 5694ad2

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

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

Review

Issues

Please sign in to comment.