Add tests for DataBlockInputStream and fix implementation oddities

Fix issues with `DataBlockInputStream` including the fact that remain
bytes were not tracked correctly. Also add some tests and fix a few
other unusual details with the implementation.

Closes gh-38066

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

@@ -73,8 +73,9 @@ public interface DataBlock {
 	/**
 	 * Return this {@link DataBlock} as an {@link InputStream}.
 	 * @return an {@link InputStream} to read the data block content
+	 * @throws IOException on IO error
 	 */
-	default InputStream asInputStream() {
+	default InputStream asInputStream() throws IOException {
 		return new DataBlockInputStream(this);
 	}
 

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

@@ -20,7 +20,6 @@ import java.io.Closeable;
 import java.io.IOException;
 import java.io.InputStream;
 import java.nio.ByteBuffer;
-import java.util.zip.ZipException;
 
 /**
  * {@link InputStream} backed by a {@link DataBlock}.
@@ -35,10 +34,11 @@ class DataBlockInputStream extends InputStream {
 
 	private long remaining;
 
-	private volatile boolean closing;
+	private volatile boolean closed;
 
-	DataBlockInputStream(DataBlock dataBlock) {
+	DataBlockInputStream(DataBlock dataBlock) throws IOException {
 		this.dataBlock = dataBlock;
+		this.remaining = dataBlock.size();
 	}
 
 	@Override
@@ -49,7 +49,6 @@ class DataBlockInputStream extends InputStream {
 
 	@Override
 	public int read(byte[] b, int off, int len) throws IOException {
-		int result;
 		ensureOpen();
 		ByteBuffer dst = ByteBuffer.wrap(b, off, len);
 		int count = this.dataBlock.read(dst, this.pos);
@@ -57,23 +56,15 @@ class DataBlockInputStream extends InputStream {
 			this.pos += count;
 			this.remaining -= count;
 		}
-		result = count;
+		return count;
-		if (this.remaining == 0) {
-			close();
-		}
-		return result;
 	}
 
 	@Override
 	public long skip(long n) throws IOException {
-		long result;
+		long count = (n > 0) ? maxForwardSkip(n) : maxBackwardSkip(n);
-		result = (n > 0) ? maxForwardSkip(n) : maxBackwardSkip(n);
+		this.pos += count;
-		this.pos += result;
+		this.remaining -= count;
-		this.remaining -= result;
+		return count;
-		if (this.remaining == 0) {
-			close();
-		}
-		return result;
 	}
 
 	private long maxForwardSkip(long n) {
@@ -87,21 +78,24 @@ class DataBlockInputStream extends InputStream {
 
 	@Override
 	public int available() {
+		if (this.closed) {
+			return 0;
+		}
 		return (this.remaining < Integer.MAX_VALUE) ? (int) this.remaining : Integer.MAX_VALUE;
 	}
 
-	private void ensureOpen() throws ZipException {
+	private void ensureOpen() throws IOException {
-		if (this.closing) {
+		if (this.closed) {
-			throw new ZipException("InputStream closed");
+			throw new IOException("InputStream closed");
 		}
 	}
 
 	@Override
 	public void close() throws IOException {
-		if (this.closing) {
+		if (this.closed) {
 			return;
 		}
-		this.closing = true;
+		this.closed = true;
 		if (this.dataBlock instanceof Closeable closeable) {
 			closeable.close();
 		}

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

@@ -0,0 +1,137 @@
+/*
+ * 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.io.InputStream;
+
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatIOException;
+import static org.mockito.BDDMockito.then;
+import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.times;
+
+/**
+ * Tests for {@link DataBlockInputStream}.
+ *
+ * @author Phillip Webb
+ */
+class DataBlockInputStreamTests {
+
+	private ByteArrayDataBlock dataBlock;
+
+	private InputStream inputStream;
+
+	@BeforeEach
+	void setup() throws Exception {
+		this.dataBlock = new ByteArrayDataBlock(new byte[] { 0, 1, 2 });
+		this.inputStream = this.dataBlock.asInputStream();
+	}
+
+	@Test
+	void readSingleByteReadsByte() throws Exception {
+		assertThat(this.inputStream.read()).isEqualTo(0);
+		assertThat(this.inputStream.read()).isEqualTo(1);
+		assertThat(this.inputStream.read()).isEqualTo(2);
+		assertThat(this.inputStream.read()).isEqualTo(-1);
+	}
+
+	@Test
+	void readByteArrayWhenNotOpenThrowsException() throws Exception {
+		byte[] bytes = new byte[10];
+		this.inputStream.close();
+		assertThatIOException().isThrownBy(() -> this.inputStream.read(bytes)).withMessage("InputStream closed");
+	}
+
+	@Test
+	void readByteArrayWhenReadingMultipleTimesReadsBytes() throws Exception {
+		byte[] bytes = new byte[3];
+		assertThat(this.inputStream.read(bytes, 0, 2)).isEqualTo(2);
+		assertThat(this.inputStream.read(bytes, 2, 1)).isEqualTo(1);
+		assertThat(bytes).containsExactly(0, 1, 2);
+	}
+
+	@Test
+	void readByteArrayWhenReadingMoreThanAvailableReadsRemainingBytes() throws Exception {
+		byte[] bytes = new byte[5];
+		assertThat(this.inputStream.read(bytes, 0, 5)).isEqualTo(3);
+		assertThat(bytes).containsExactly(0, 1, 2, 0, 0);
+	}
+
+	@Test
+	void skipSkipsBytes() throws Exception {
+		assertThat(this.inputStream.skip(2)).isEqualTo(2);
+		assertThat(this.inputStream.read()).isEqualTo(2);
+		assertThat(this.inputStream.read()).isEqualTo(-1);
+	}
+
+	@Test
+	void skipWhenSkippingMoreThanRemainingSkipsBytes() throws Exception {
+		assertThat(this.inputStream.skip(100)).isEqualTo(3);
+		assertThat(this.inputStream.read()).isEqualTo(-1);
+	}
+
+	@Test
+	void skipBackwardsSkipsBytes() throws IOException {
+		assertThat(this.inputStream.skip(2)).isEqualTo(2);
+		assertThat(this.inputStream.skip(-1)).isEqualTo(-1);
+		assertThat(this.inputStream.read()).isEqualTo(1);
+	}
+
+	@Test
+	void skipBackwardsPastBeginingSkipsBytes() throws Exception {
+		assertThat(this.inputStream.skip(1)).isEqualTo(1);
+		assertThat(this.inputStream.skip(-100)).isEqualTo(-1);
+		assertThat(this.inputStream.read()).isEqualTo(0);
+	}
+
+	@Test
+	void availableReturnsRemainingBytes() throws IOException {
+		assertThat(this.inputStream.available()).isEqualTo(3);
+		this.inputStream.read();
+		assertThat(this.inputStream.available()).isEqualTo(2);
+		this.inputStream.skip(1);
+		assertThat(this.inputStream.available()).isEqualTo(1);
+	}
+
+	@Test
+	void availableWhenClosedReturnsZero() throws IOException {
+		this.inputStream.close();
+		assertThat(this.inputStream.available()).isZero();
+	}
+
+	@Test
+	void closeClosesDataBlock() throws Exception {
+		this.dataBlock = spy(new ByteArrayDataBlock(new byte[] { 0, 1, 2 }));
+		this.inputStream = this.dataBlock.asInputStream();
+		this.inputStream.close();
+		then(this.dataBlock).should().close();
+	}
+
+	@Test
+	void closeMultipleTimesClosesDataBlockOnce() throws Exception {
+		this.dataBlock = spy(new ByteArrayDataBlock(new byte[] { 0, 1, 2 }));
+		this.inputStream = this.dataBlock.asInputStream();
+		this.inputStream.close();
+		this.inputStream.close();
+		then(this.dataBlock).should(times(1)).close();
+	}
+
+}

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

@@ -68,7 +68,7 @@ class DataBlockTests {
 	}
 
 	@Test
-	void asInputStreamReturnsDataBlockInputStream() {
+	void asInputStreamReturnsDataBlockInputStream() throws Exception {
 		DataBlock dataBlock = mock(DataBlock.class, withSettings().defaultAnswer(CALLS_REAL_METHODS));
 		assertThat(dataBlock.asInputStream()).isInstanceOf(DataBlockInputStream.class);
 	}