From bba323ba5fad3668f85b34ee543291ab3ea21100 Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Thu, 26 Oct 2023 13:17:45 -0700 Subject: [PATCH] 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 --- .../boot/loader/zip/ByteArrayDataBlock.java | 6 +- .../boot/loader/zip/VirtualZipDataBlock.java | 30 +++-- .../loader/zip/ZipDataDescriptorRecord.java | 120 ++++++++++++++++++ .../loader/zip/ZipLocalFileHeaderRecord.java | 1 + .../loader/zip/VirtualZipDataBlockTests.java | 40 ++++++ .../zip/ZipDataDescriptorRecordTests.java | 111 ++++++++++++++++ 6 files changed, 296 insertions(+), 12 deletions(-) create mode 100644 spring-boot-project/spring-boot-tools/spring-boot-loader/src/main/java/org/springframework/boot/loader/zip/ZipDataDescriptorRecord.java create mode 100644 spring-boot-project/spring-boot-tools/spring-boot-loader/src/test/java/org/springframework/boot/loader/zip/ZipDataDescriptorRecordTests.java diff --git a/spring-boot-project/spring-boot-tools/spring-boot-loader/src/main/java/org/springframework/boot/loader/zip/ByteArrayDataBlock.java b/spring-boot-project/spring-boot-tools/spring-boot-loader/src/main/java/org/springframework/boot/loader/zip/ByteArrayDataBlock.java index 577cb2dc3b2..d1a4f7fcf98 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-loader/src/main/java/org/springframework/boot/loader/zip/ByteArrayDataBlock.java +++ b/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 { + } + } diff --git a/spring-boot-project/spring-boot-tools/spring-boot-loader/src/main/java/org/springframework/boot/loader/zip/VirtualZipDataBlock.java b/spring-boot-project/spring-boot-tools/spring-boot-loader/src/main/java/org/springframework/boot/loader/zip/VirtualZipDataBlock.java index 21021da25e5..6ba095c7419 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-loader/src/main/java/org/springframework/boot/loader/zip/VirtualZipDataBlock.java +++ b/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 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 parts, ZipLocalFileHeaderRecord originalRecord, DataBlock name, + private long addToLocal(List 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 diff --git a/spring-boot-project/spring-boot-tools/spring-boot-loader/src/main/java/org/springframework/boot/loader/zip/ZipDataDescriptorRecord.java b/spring-boot-project/spring-boot-tools/spring-boot-loader/src/main/java/org/springframework/boot/loader/zip/ZipDataDescriptorRecord.java new file mode 100644 index 00000000000..af3a85027ec --- /dev/null +++ b/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 Chapter + * 4.3.9 of the Zip File Format Specification + */ +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; + } + +} diff --git a/spring-boot-project/spring-boot-tools/spring-boot-loader/src/main/java/org/springframework/boot/loader/zip/ZipLocalFileHeaderRecord.java b/spring-boot-project/spring-boot-tools/spring-boot-loader/src/main/java/org/springframework/boot/loader/zip/ZipLocalFileHeaderRecord.java index 8d77ca585ae..daed69afb9b 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-loader/src/main/java/org/springframework/boot/loader/zip/ZipLocalFileHeaderRecord.java +++ b/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()); } + } diff --git a/spring-boot-project/spring-boot-tools/spring-boot-loader/src/test/java/org/springframework/boot/loader/zip/VirtualZipDataBlockTests.java b/spring-boot-project/spring-boot-tools/spring-boot-loader/src/test/java/org/springframework/boot/loader/zip/VirtualZipDataBlockTests.java index 42ea979673c..33f93bfb0f0 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-loader/src/test/java/org/springframework/boot/loader/zip/VirtualZipDataBlockTests.java +++ b/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 centralRecords = new ArrayList<>(); + List 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); + } + } diff --git a/spring-boot-project/spring-boot-tools/spring-boot-loader/src/test/java/org/springframework/boot/loader/zip/ZipDataDescriptorRecordTests.java b/spring-boot-project/spring-boot-tools/spring-boot-loader/src/test/java/org/springframework/boot/loader/zip/ZipDataDescriptorRecordTests.java new file mode 100644 index 00000000000..2af772eaccf --- /dev/null +++ b/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); + } + +}