Copy zip data descriptor records when creating virtual zip data

The zip specification states that when 'bit 3' of the general purpose
flags is set then a data descriptor record must be present. Prior to
this commit, our `VirtualZipDataBlock` ignored such records and would
create invalid data.

Although the generated data would work for zip parsers that read the
central directory records, it causes problems with streaming reader
implementations such as `JarInputStream`.

This commit updates the code so that it now copies the data descriptor
records. It support both blocks that have a signature and those that
don't. It also updates the generation logic to correctly deal with
any extra data bytes present after the local file header record.

Fixes gh-38063

spring-boot-project/spring-boot-tools/spring-boot-loader/src/main/java/org/springframework/boot/loader/zip/ByteArrayDataBlock.java

@@ -24,7 +24,7 @@ import java.nio.ByteBuffer;
  *
  * @author Phillip Webb
  */
-class ByteArrayDataBlock implements DataBlock {
+class ByteArrayDataBlock implements CloseableDataBlock {
 
 	private final byte[] bytes;
 
@@ -53,4 +53,8 @@ class ByteArrayDataBlock implements DataBlock {
 		return length;
 	}
 
+	@Override
+	public void close() throws IOException {
+	}
+
 }

spring-boot-project/spring-boot-tools/spring-boot-loader/src/main/java/org/springframework/boot/loader/zip/VirtualZipDataBlock.java

@@ -30,7 +30,7 @@ import java.util.List;
  */
 class VirtualZipDataBlock extends VirtualDataBlock implements CloseableDataBlock {
 
-	private final FileChannelDataBlock data;
+	private final CloseableDataBlock data;
 
 	/**
 	 * Create a new {@link VirtualZipDataBlock} for the given entries.
@@ -40,7 +40,7 @@ class VirtualZipDataBlock extends VirtualDataBlock implements CloseableDataBlock
 	 * @param centralRecordPositions the record positions in the data block.
 	 * @throws IOException on I/O error
 	 */
-	VirtualZipDataBlock(FileChannelDataBlock data, NameOffsetLookups nameOffsetLookups,
+	VirtualZipDataBlock(CloseableDataBlock data, NameOffsetLookups nameOffsetLookups,
 			ZipCentralDirectoryFileHeaderRecord[] centralRecords, long[] centralRecordPositions) throws IOException {
 		this.data = data;
 		List<DataBlock> parts = new ArrayList<>();
@@ -54,12 +54,14 @@ class VirtualZipDataBlock extends VirtualDataBlock implements CloseableDataBlock
 			DataBlock name = new DataPart(
 					centralRecordPos + ZipCentralDirectoryFileHeaderRecord.FILE_NAME_OFFSET + nameOffset,
 					(centralRecord.fileNameLength() & 0xFFFF) - nameOffset);
-			ZipLocalFileHeaderRecord localRecord = ZipLocalFileHeaderRecord.load(this.data,
-					centralRecord.offsetToLocalHeader());
-			DataBlock content = new DataPart(centralRecord.offsetToLocalHeader() + localRecord.size(),
-					centralRecord.compressedSize());
+			long localRecordPos = centralRecord.offsetToLocalHeader() & 0xFFFFFFFF;
+			ZipLocalFileHeaderRecord localRecord = ZipLocalFileHeaderRecord.load(this.data, localRecordPos);
+			DataBlock content = new DataPart(localRecordPos + localRecord.size(), centralRecord.compressedSize());
+			boolean hasDescriptorRecord = ZipDataDescriptorRecord.isPresentBasedOnFlag(centralRecord);
+			ZipDataDescriptorRecord dataDescriptorRecord = (!hasDescriptorRecord) ? null
+					: ZipDataDescriptorRecord.load(data, localRecordPos + localRecord.size() + content.size());
 			sizeOfCentralDirectory += addToCentral(centralParts, centralRecord, centralRecordPos, name, (int) offset);
-			offset += addToLocal(parts, localRecord, name, content);
+			offset += addToLocal(parts, centralRecord, localRecord, dataDescriptorRecord, name, content);
 		}
 		parts.addAll(centralParts);
 		ZipEndOfCentralDirectoryRecord eocd = new ZipEndOfCentralDirectoryRecord((short) centralRecords.length,
@@ -83,14 +85,20 @@ class VirtualZipDataBlock extends VirtualDataBlock implements CloseableDataBlock
 		return record.size();
 	}
 
-	private long addToLocal(List<DataBlock> parts, ZipLocalFileHeaderRecord originalRecord, DataBlock name,
+	private long addToLocal(List<DataBlock> parts, ZipCentralDirectoryFileHeaderRecord centralRecord,
+			ZipLocalFileHeaderRecord originalRecord, ZipDataDescriptorRecord dataDescriptorRecord, DataBlock name,
 			DataBlock content) throws IOException {
-		ZipLocalFileHeaderRecord record = originalRecord.withExtraFieldLength((short) 0)
-			.withFileNameLength((short) (name.size() & 0xFFFF));
+		ZipLocalFileHeaderRecord record = originalRecord.withFileNameLength((short) (name.size() & 0xFFFF));
+		long originalRecordPos = centralRecord.offsetToLocalHeader() & 0xFFFFFFFF;
+		int extraFieldLength = originalRecord.extraFieldLength() & 0xFFFF;
 		parts.add(new ByteArrayDataBlock(record.asByteArray()));
 		parts.add(name);
+		parts.add(new DataPart(originalRecordPos + originalRecord.size() - extraFieldLength, extraFieldLength));
 		parts.add(content);
-		return record.size() + content.size();
+		if (dataDescriptorRecord != null) {
+			parts.add(new ByteArrayDataBlock(dataDescriptorRecord.asByteArray()));
+		}
+		return record.size() + content.size() + ((dataDescriptorRecord != null) ? dataDescriptorRecord.size() : 0);
 	}
 
 	@Override

spring-boot-project/spring-boot-tools/spring-boot-loader/src/main/java/org/springframework/boot/loader/zip/ZipDataDescriptorRecord.java

@@ -0,0 +1,120 @@
+/*
+ * Copyright 2012-2023 the original author or authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.springframework.boot.loader.zip;
+
+import java.io.IOException;
+import java.nio.ByteBuffer;
+import java.nio.ByteOrder;
+
+import org.springframework.boot.loader.log.DebugLogger;
+
+/**
+ * A ZIP File "Data Descriptor" record.
+ *
+ * @param includeSignature if the signature bytes are written or not (see note in spec)
+ * @param crc32 the CRC32 checksum
+ * @param compressedSize the size of the entry when compressed
+ * @param uncompressedSize the size of the entry when uncompressed
+ * @author Phillip Webb
+ * @see <a href="https://pkware.cachefly.net/webdocs/casestudies/APPNOTE.TXT">Chapter
+ * 4.3.9 of the Zip File Format Specification</a>
+ */
+record ZipDataDescriptorRecord(boolean includeSignature, int crc32, int compressedSize, int uncompressedSize) {
+
+	private static final DebugLogger debug = DebugLogger.get(ZipDataDescriptorRecord.class);
+
+	private static final int SIGNATURE = 0x08074b50;
+
+	private static final int DATA_SIZE = 12;
+
+	private static final int SIGNATURE_SIZE = 4;
+
+	long size() {
+		return (!includeSignature()) ? DATA_SIZE : DATA_SIZE + SIGNATURE_SIZE;
+	}
+
+	/**
+	 * Return the contents of this record as a byte array suitable for writing to a zip.
+	 * @return the record as a byte array
+	 */
+	byte[] asByteArray() {
+		ByteBuffer buffer = ByteBuffer.allocate((int) size());
+		buffer.order(ByteOrder.LITTLE_ENDIAN);
+		if (this.includeSignature) {
+			buffer.putInt(SIGNATURE);
+		}
+		buffer.putInt(this.crc32);
+		buffer.putInt(this.compressedSize);
+		buffer.putInt(this.uncompressedSize);
+		return buffer.array();
+	}
+
+	/**
+	 * Load the {@link ZipDataDescriptorRecord} from the given data block.
+	 * @param dataBlock the source data block
+	 * @param pos the position of the record
+	 * @return a new {@link ZipLocalFileHeaderRecord} instance
+	 * @throws IOException on I/O error
+	 */
+	static ZipDataDescriptorRecord load(DataBlock dataBlock, long pos) throws IOException {
+		debug.log("Loading ZipDataDescriptorRecord from position %s", pos);
+		ByteBuffer buffer = ByteBuffer.allocate(SIGNATURE_SIZE + DATA_SIZE);
+		buffer.order(ByteOrder.LITTLE_ENDIAN);
+		buffer.limit(SIGNATURE_SIZE);
+		dataBlock.readFully(buffer, pos);
+		buffer.rewind();
+		int signatureOrCrc = buffer.getInt();
+		boolean hasSignature = (signatureOrCrc == SIGNATURE);
+		buffer.rewind();
+		buffer.limit((!hasSignature) ? DATA_SIZE - SIGNATURE_SIZE : DATA_SIZE);
+		dataBlock.readFully(buffer, pos + SIGNATURE_SIZE);
+		buffer.rewind();
+		return new ZipDataDescriptorRecord(hasSignature, (!hasSignature) ? signatureOrCrc : buffer.getInt(),
+				buffer.getInt(), buffer.getInt());
+	}
+
+	/**
+	 * Return if the {@link ZipDataDescriptorRecord} is present based on the general
+	 * purpose bit flag in the given {@link ZipLocalFileHeaderRecord}.
+	 * @param localRecord the local record to check
+	 * @return if the bit flag is set
+	 */
+	static boolean isPresentBasedOnFlag(ZipLocalFileHeaderRecord localRecord) {
+		return isPresentBasedOnFlag(localRecord.generalPurposeBitFlag());
+	}
+
+	/**
+	 * Return if the {@link ZipDataDescriptorRecord} is present based on the general
+	 * purpose bit flag in the given {@link ZipCentralDirectoryFileHeaderRecord}.
+	 * @param centralRecord the central record to check
+	 * @return if the bit flag is set
+	 */
+	static boolean isPresentBasedOnFlag(ZipCentralDirectoryFileHeaderRecord centralRecord) {
+		return isPresentBasedOnFlag(centralRecord.generalPurposeBitFlag());
+	}
+
+	/**
+	 * Return if the {@link ZipDataDescriptorRecord} is present based on the given general
+	 * purpose bit flag.
+	 * @param generalPurposeBitFlag the general purpose bit flag to check
+	 * @return if the bit flag is set
+	 */
+	static boolean isPresentBasedOnFlag(int generalPurposeBitFlag) {
+		return (generalPurposeBitFlag & 0b0000_1000) != 0;
+	}
+
+}

spring-boot-project/spring-boot-tools/spring-boot-loader/src/main/java/org/springframework/boot/loader/zip/ZipLocalFileHeaderRecord.java

@@ -121,4 +121,5 @@ record ZipLocalFileHeaderRecord(short versionNeededToExtract, short generalPurpo
 				buffer.getShort(), buffer.getInt(), buffer.getInt(), buffer.getInt(), buffer.getShort(),
 				buffer.getShort());
 	}
+
 }

spring-boot-project/spring-boot-tools/spring-boot-loader/src/test/java/org/springframework/boot/loader/zip/VirtualZipDataBlockTests.java

@@ -25,6 +25,8 @@ import java.nio.file.Files;
 import java.nio.file.NoSuchFileException;
 import java.util.ArrayList;
 import java.util.List;
+import java.util.zip.ZipEntry;
+import java.util.zip.ZipOutputStream;
 
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
@@ -95,4 +97,42 @@ class VirtualZipDataBlockTests {
 		}
 	}
 
+	@Test // gh-38063
+	void createWithDescriptorRecordContainsValidZipContent() throws Exception {
+		try (ZipOutputStream zip = new ZipOutputStream(new FileOutputStream(this.file))) {
+			ZipEntry entry = new ZipEntry("META-INF/");
+			entry.setMethod(ZipEntry.DEFLATED);
+			zip.putNextEntry(entry);
+			zip.write(new byte[] { 0, 1, 2, 3, 4, 5, 6, 7, 8 });
+			zip.closeEntry();
+		}
+		byte[] bytes = Files.readAllBytes(this.file.toPath());
+		CloseableDataBlock data = new ByteArrayDataBlock(bytes);
+		List<ZipCentralDirectoryFileHeaderRecord> centralRecords = new ArrayList<>();
+		List<Long> centralRecordPositions = new ArrayList<>();
+		ZipEndOfCentralDirectoryRecord eocd = ZipEndOfCentralDirectoryRecord.load(data).endOfCentralDirectoryRecord();
+		long pos = eocd.offsetToStartOfCentralDirectory();
+		for (int i = 0; i < eocd.totalNumberOfCentralDirectoryEntries(); i++) {
+			ZipCentralDirectoryFileHeaderRecord centralRecord = ZipCentralDirectoryFileHeaderRecord.load(data, pos);
+			centralRecords.add(centralRecord);
+			centralRecordPositions.add(pos);
+			pos += centralRecord.size();
+		}
+		NameOffsetLookups nameOffsetLookups = new NameOffsetLookups(0, centralRecords.size());
+		for (int i = 0; i < centralRecords.size(); i++) {
+			nameOffsetLookups.enable(i, true);
+		}
+		nameOffsetLookups.enable(0, true);
+		File outputFile = new File(this.tempDir, "out.jar");
+		try (VirtualZipDataBlock block = new VirtualZipDataBlock(data, nameOffsetLookups,
+				centralRecords.toArray(ZipCentralDirectoryFileHeaderRecord[]::new),
+				centralRecordPositions.stream().mapToLong(Long::longValue).toArray())) {
+			try (FileOutputStream out = new FileOutputStream(outputFile)) {
+				block.asInputStream().transferTo(out);
+			}
+		}
+		byte[] virtualBytes = Files.readAllBytes(outputFile.toPath());
+		assertThat(bytes).isEqualTo(virtualBytes);
+	}
+
 }

spring-boot-project/spring-boot-tools/spring-boot-loader/src/test/java/org/springframework/boot/loader/zip/ZipDataDescriptorRecordTests.java

@@ -0,0 +1,111 @@
+/*
+ * Copyright 2012-2023 the original author or authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.springframework.boot.loader.zip;
+
+import org.junit.jupiter.api.Test;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+/**
+ * Tests for {@link ZipDataDescriptorRecord}.
+ *
+ * @author Phillip Webb
+ */
+class ZipDataDescriptorRecordTests {
+
+	private static final short S0 = 0;
+
+	@Test
+	void loadWhenHasSignatureLoadsData() throws Exception {
+		DataBlock dataBlock = new ByteArrayDataBlock(new byte[] { //
+				0x50, 0x4b, 0x07, 0x08, //
+				0x01, 0x00, 0x00, 0x00, //
+				0x02, 0x00, 0x00, 0x00, //
+				0x03, 0x00, 0x00, 0x00 }); //
+		ZipDataDescriptorRecord record = ZipDataDescriptorRecord.load(dataBlock, 0);
+		assertThat(record.includeSignature()).isTrue();
+		assertThat(record.crc32()).isEqualTo(1);
+		assertThat(record.compressedSize()).isEqualTo(2);
+		assertThat(record.uncompressedSize()).isEqualTo(3);
+	}
+
+	@Test
+	void loadWhenHasNoSignatureLoadsData() throws Exception {
+		DataBlock dataBlock = new ByteArrayDataBlock(new byte[] { //
+				0x01, 0x00, 0x00, 0x00, //
+				0x02, 0x00, 0x00, 0x00, //
+				0x03, 0x00, 0x00, 0x00 }); //
+		ZipDataDescriptorRecord record = ZipDataDescriptorRecord.load(dataBlock, 0);
+		assertThat(record.includeSignature()).isFalse();
+		assertThat(record.crc32()).isEqualTo(1);
+		assertThat(record.compressedSize()).isEqualTo(2);
+		assertThat(record.uncompressedSize()).isEqualTo(3);
+	}
+
+	@Test
+	void sizeWhenIncludeSignatureReturnsSize() {
+		ZipDataDescriptorRecord record = new ZipDataDescriptorRecord(true, 0, 0, 0);
+		assertThat(record.size()).isEqualTo(16);
+	}
+
+	@Test
+	void sizeWhenNotIncludeSignatureReturnsSize() {
+		ZipDataDescriptorRecord record = new ZipDataDescriptorRecord(false, 0, 0, 0);
+		assertThat(record.size()).isEqualTo(12);
+	}
+
+	@Test
+	void asByteArrayWhenIncludeSignatureReturnsByteArray() throws Exception {
+		byte[] bytes = new byte[] { //
+				0x50, 0x4b, 0x07, 0x08, //
+				0x01, 0x00, 0x00, 0x00, //
+				0x02, 0x00, 0x00, 0x00, //
+				0x03, 0x00, 0x00, 0x00 }; //
+		ZipDataDescriptorRecord record = ZipDataDescriptorRecord.load(new ByteArrayDataBlock(bytes), 0);
+		assertThat(record.asByteArray()).isEqualTo(bytes);
+	}
+
+	@Test
+	void asByteArrayWhenNotIncludeSignatureReturnsByteArray() throws Exception {
+		byte[] bytes = new byte[] { //
+				0x01, 0x00, 0x00, 0x00, //
+				0x02, 0x00, 0x00, 0x00, //
+				0x03, 0x00, 0x00, 0x00 }; //
+		ZipDataDescriptorRecord record = ZipDataDescriptorRecord.load(new ByteArrayDataBlock(bytes), 0);
+		assertThat(record.asByteArray()).isEqualTo(bytes);
+	}
+
+	@Test
+	void isPresentBasedOnFlagWhenPresentReturnsTrue() {
+		testIsPresentBasedOnFlag((short) 0x8, true);
+	}
+
+	@Test
+	void isPresentBasedOnFlagWhenNotPresentReturnsFalse() {
+		testIsPresentBasedOnFlag((short) 0x0, false);
+	}
+
+	private void testIsPresentBasedOnFlag(short flag, boolean expected) {
+		ZipCentralDirectoryFileHeaderRecord centralRecord = new ZipCentralDirectoryFileHeaderRecord(S0, S0, flag, S0,
+				S0, S0, S0, S0, S0, S0, S0, S0, S0, S0, S0, S0);
+		ZipLocalFileHeaderRecord localRecord = new ZipLocalFileHeaderRecord(S0, flag, S0, S0, S0, S0, S0, S0, S0, S0);
+		assertThat(ZipDataDescriptorRecord.isPresentBasedOnFlag(flag)).isEqualTo(expected);
+		assertThat(ZipDataDescriptorRecord.isPresentBasedOnFlag(centralRecord)).isEqualTo(expected);
+		assertThat(ZipDataDescriptorRecord.isPresentBasedOnFlag(localRecord)).isEqualTo(expected);
+	}
+
+}