org.junit.jupiter
junit-jupiter
diff --git a/cmdline/src/main/java/io/opentdf/platform/Command.java b/cmdline/src/main/java/io/opentdf/platform/Command.java
index c4953843..810355f5 100644
--- a/cmdline/src/main/java/io/opentdf/platform/Command.java
+++ b/cmdline/src/main/java/io/opentdf/platform/Command.java
@@ -19,6 +19,7 @@
import java.io.PrintWriter;
import java.net.URI;
import java.nio.channels.FileChannel;
+import java.nio.channels.SeekableByteChannel;
import java.nio.charset.StandardCharsets;
import java.nio.file.AtomicMoveNotSupportedException;
import java.nio.file.Files;
@@ -450,14 +451,11 @@ void decrypt(
var readerConfig = Config.newTDFReaderConfig(opts.toArray(new Consumer[0]));
- if (tdfFile.startsWith("http://") || tdfFile.startsWith("https://")) {
- var reader = sdk.loadTDF(URI.create(tdfFile), readerConfig);
+ // Opened last: a bad --with-assertion-verification-keys should fail before the
+ // TDF is touched, which over a URL means before any request goes out.
+ try (var in = openTdf(sdk, tdfFile)) {
+ var reader = sdk.loadTDF(in, readerConfig);
writePlaintext(out -> reader.readPayload(out, offset, length), output);
- } else {
- try (var in = FileChannel.open(Path.of(tdfFile), StandardOpenOption.READ)) {
- var reader = sdk.loadTDF(in, readerConfig);
- writePlaintext(out -> reader.readPayload(out, offset, length), output);
- }
}
}
}
@@ -495,6 +493,17 @@ static void moveIntoPlace(Path source, Path target) throws IOException {
}
}
+ /**
+ * Opens the TDF named by {@code -f}: an {@code http(s)://} URL, read in place with range
+ * requests, or anything else as a local file path.
+ */
+ private static SeekableByteChannel openTdf(SDK sdk, String tdfFile) throws IOException {
+ if (tdfFile.startsWith("http://") || tdfFile.startsWith("https://")) {
+ return HttpRangeChannel.open(sdk, URI.create(tdfFile));
+ }
+ return FileChannel.open(Path.of(tdfFile), StandardOpenOption.READ);
+ }
+
@CommandLine.Command(name = "metadata")
void readMetadata(
@Option(names = { "-f", "--file" }, required = true) Path tdfPath,
diff --git a/sdk/src/main/java/io/opentdf/platform/sdk/HttpRangeChannel.java b/cmdline/src/main/java/io/opentdf/platform/HttpRangeChannel.java
similarity index 54%
rename from sdk/src/main/java/io/opentdf/platform/sdk/HttpRangeChannel.java
rename to cmdline/src/main/java/io/opentdf/platform/HttpRangeChannel.java
index 26152259..03aeceb2 100644
--- a/sdk/src/main/java/io/opentdf/platform/sdk/HttpRangeChannel.java
+++ b/cmdline/src/main/java/io/opentdf/platform/HttpRangeChannel.java
@@ -1,39 +1,91 @@
-package io.opentdf.platform.sdk;
+package io.opentdf.platform;
+import io.opentdf.platform.sdk.SDK;
import okhttp3.OkHttpClient;
import okhttp3.Request;
+import javax.net.ssl.SSLContext;
+import javax.net.ssl.TrustManager;
+import javax.net.ssl.X509TrustManager;
import java.io.IOException;
import java.net.URI;
import java.nio.ByteBuffer;
import java.nio.channels.ClosedChannelException;
import java.nio.channels.NonWritableChannelException;
import java.nio.channels.SeekableByteChannel;
+import java.security.GeneralSecurityException;
import java.util.regex.Pattern;
/**
* A read-only {@link SeekableByteChannel} over a remote resource, where every read is one
- * HTTP range request. The size is learned by probing {@code bytes=0-0}, and every later
- * response must report the same total size, so a resource that changes underneath the
- * reader is rejected rather than spliced together.
+ * HTTP range request. Handed to {@link SDK#loadTDF(SeekableByteChannel, io.opentdf.platform.sdk.Config.TDFReaderConfig)}
+ * it lets the SDK read a TDF in place, downloading only the bytes it actually touches.
*
- * Redirects are not followed: the client handed in must have redirects disabled, and a 3xx
- * is reported as an error so the caller can use the final URL.
+ * The size is learned by probing {@code bytes=0-0}, and every later response must report the
+ * same total size, so a resource that changes underneath the reader is rejected rather than
+ * spliced together.
+ *
+ * Redirects are not followed, and a 3xx is reported as an error so the caller can use the
+ * final URL.
*/
// TODO: Add a read-ahead window if small ZIP header reads become a network bottleneck.
class HttpRangeChannel implements SeekableByteChannel {
private static final Pattern CONTENT_RANGE = Pattern.compile("bytes (\\d+)-(\\d+)/(\\d+)");
private final OkHttpClient client;
+ // Set only when this channel built the client, and so must shut it down on close.
+ private final boolean ownsClient;
private final URI url;
private final long size;
private long position;
private boolean open = true;
+ /**
+ * Opens {@code url} with a client that is unauthenticated -- platform credentials never go
+ * to the TDF host -- but that keeps the TLS settings the SDK was built with, so {@code
+ * --insecure} applies here too.
+ */
+ static HttpRangeChannel open(SDK sdk, URI url) throws IOException {
+ String scheme = url.getScheme();
+ if (!"http".equalsIgnoreCase(scheme) && !"https".equalsIgnoreCase(scheme)) {
+ throw new IllegalArgumentException("unsupported URL scheme: " + scheme);
+ }
+ return new HttpRangeChannel(rangeClient(sdk), true, url);
+ }
+
HttpRangeChannel(OkHttpClient client, URI url) throws IOException {
+ this(client, false, url);
+ }
+
+ private HttpRangeChannel(OkHttpClient client, boolean ownsClient, URI url) throws IOException {
this.client = client;
+ this.ownsClient = ownsClient;
this.url = url;
- this.size = fetch(ByteBuffer.allocate(1), 0);
+ try {
+ this.size = fetch(ByteBuffer.allocate(1), 0);
+ } catch (IOException | RuntimeException e) {
+ shutdown();
+ throw e;
+ }
+ }
+
+ private static OkHttpClient rangeClient(SDK sdk) {
+ // A plain client rather than one derived from the SDK's: the SDK's carries the auth
+ // interceptor, and under --plaintext it is pinned to h2c prior knowledge, while a TDF
+ // host is an ordinary HTTP server. Default protocols (HTTP/2, HTTP/1.1) are right here.
+ var builder = new OkHttpClient.Builder().followRedirects(false).followSslRedirects(false);
+ var trustManager = sdk.getTrustManager().orElse(null);
+ if (trustManager instanceof X509TrustManager) {
+ var x509 = (X509TrustManager) trustManager;
+ try {
+ var context = SSLContext.getInstance("TLS");
+ context.init(null, new TrustManager[] { x509 }, null);
+ builder.sslSocketFactory(context.getSocketFactory(), x509);
+ } catch (GeneralSecurityException e) {
+ throw new IllegalStateException("could not build a TLS context for range requests", e);
+ }
+ }
+ return builder.build();
}
@Override
@@ -70,7 +122,7 @@ private long fetch(ByteBuffer dst, long offset) throws IOException {
try (var response = client.newCall(request).execute()) {
int status = response.code();
if (status == 200) {
- throw new SDK.RangeNotSupportedException(
+ throw new RangeNotSupportedException(
"status 200 for bytes " + offset + "-" + end);
}
if (status >= 300 && status < 400) {
@@ -83,12 +135,12 @@ private long fetch(ByteBuffer dst, long offset) throws IOException {
String contentRange = response.header("Content-Range");
var m = contentRange == null ? null : CONTENT_RANGE.matcher(contentRange);
if (m == null || !m.matches()) {
- throw new SDK.RangeNotSupportedException("invalid Content-Range " + contentRange);
+ throw new RangeNotSupportedException("invalid Content-Range " + contentRange);
}
long gotStart = Long.parseLong(m.group(1));
long gotEnd = Long.parseLong(m.group(2));
if (gotStart != offset || gotEnd != end) {
- throw new SDK.RangeNotSupportedException("requested bytes " + offset + "-" + end
+ throw new RangeNotSupportedException("requested bytes " + offset + "-" + end
+ ", got " + gotStart + "-" + gotEnd);
}
var source = response.body().source();
@@ -141,6 +193,14 @@ public boolean isOpen() {
@Override
public void close() {
open = false;
+ shutdown();
+ }
+
+ private void shutdown() {
+ if (ownsClient) {
+ client.dispatcher().executorService().shutdown();
+ client.connectionPool().evictAll();
+ }
}
private void ensureOpen() throws ClosedChannelException {
@@ -148,4 +208,14 @@ private void ensureOpen() throws ClosedChannelException {
throw new ClosedChannelException();
}
}
+
+ /**
+ * The server holding a TDF did not honor an HTTP range request, so the TDF cannot be
+ * read in place; download it and decrypt it from a file instead.
+ */
+ static class RangeNotSupportedException extends IOException {
+ RangeNotSupportedException(String errorMessage) {
+ super("server did not honor the HTTP range request: " + errorMessage);
+ }
+ }
}
diff --git a/sdk/src/test/java/io/opentdf/platform/sdk/HttpRangeChannelTest.java b/cmdline/src/test/java/io/opentdf/platform/HttpRangeChannelTest.java
similarity index 55%
rename from sdk/src/test/java/io/opentdf/platform/sdk/HttpRangeChannelTest.java
rename to cmdline/src/test/java/io/opentdf/platform/HttpRangeChannelTest.java
index 3985088a..6fe9e10b 100644
--- a/sdk/src/test/java/io/opentdf/platform/sdk/HttpRangeChannelTest.java
+++ b/cmdline/src/test/java/io/opentdf/platform/HttpRangeChannelTest.java
@@ -1,20 +1,26 @@
-package io.opentdf.platform.sdk;
+package io.opentdf.platform;
import okhttp3.OkHttpClient;
import okhttp3.mockwebserver.Dispatcher;
import okhttp3.mockwebserver.MockResponse;
import okhttp3.mockwebserver.MockWebServer;
import okhttp3.mockwebserver.RecordedRequest;
+import io.opentdf.platform.sdk.ZipReader;
import okio.Buffer;
import org.junit.jupiter.api.Test;
+import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.net.URI;
import java.nio.ByteBuffer;
+import java.nio.channels.ClosedChannelException;
import java.nio.charset.StandardCharsets;
import java.util.concurrent.atomic.AtomicLong;
import java.util.function.Supplier;
import java.util.regex.Pattern;
+import java.util.zip.CRC32;
+import java.util.zip.ZipEntry;
+import java.util.zip.ZipOutputStream;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
@@ -71,12 +77,41 @@ void readsAndSeeks() throws Exception {
}
}
+ /** Only the bytes asked for cross the wire, which is the point of reading a TDF in place. */
+ @Test
+ void readsOnlyTheRequestedBytes() throws Exception {
+ byte[] data = new byte[100_000];
+ var served = new AtomicLong();
+ try (var server = new MockWebServer()) {
+ server.setDispatcher(rangeDispatcher(() -> data, served));
+ try (var ch = new HttpRangeChannel(CLIENT, url(server))) {
+ served.set(0); // discount the one-byte size probe
+ assertThat(ch.position(50_000).read(ByteBuffer.allocate(1_000))).isEqualTo(1_000);
+ }
+ assertThat(served).hasValue(1_000);
+ }
+ }
+
+ @Test
+ void usesAfterCloseAreRejected() throws Exception {
+ byte[] data = "0123456789".getBytes(StandardCharsets.UTF_8);
+ try (var server = new MockWebServer()) {
+ server.setDispatcher(rangeDispatcher(() -> data, new AtomicLong()));
+ var ch = new HttpRangeChannel(CLIENT, url(server));
+ ch.close();
+
+ assertThat(ch.isOpen()).isFalse();
+ assertThatThrownBy(() -> ch.read(ByteBuffer.allocate(1)))
+ .isInstanceOf(ClosedChannelException.class);
+ }
+ }
+
@Test
void rangeNotSupported() throws Exception {
try (var server = new MockWebServer()) {
server.enqueue(new MockResponse().setResponseCode(200).setBody("not a partial response"));
assertThatThrownBy(() -> new HttpRangeChannel(CLIENT, url(server)))
- .isInstanceOf(SDK.RangeNotSupportedException.class);
+ .isInstanceOf(HttpRangeChannel.RangeNotSupportedException.class);
}
}
@@ -86,7 +121,7 @@ void mismatchedContentRangeIsRangeNotSupported() throws Exception {
server.enqueue(new MockResponse().setResponseCode(206)
.setHeader("Content-Range", "bytes 0-9/10").setBody("0123456789"));
assertThatThrownBy(() -> new HttpRangeChannel(CLIENT, url(server)))
- .isInstanceOf(SDK.RangeNotSupportedException.class);
+ .isInstanceOf(HttpRangeChannel.RangeNotSupportedException.class);
}
}
@@ -97,7 +132,7 @@ void errorStatusesReportTheirStatus() throws Exception {
server.enqueue(new MockResponse().setResponseCode(status).setHeader("Location", "/elsewhere"));
assertThatThrownBy(() -> new HttpRangeChannel(CLIENT, url(server)))
.isInstanceOf(IOException.class)
- .isNotInstanceOf(SDK.RangeNotSupportedException.class)
+ .isNotInstanceOf(HttpRangeChannel.RangeNotSupportedException.class)
.hasMessageContaining(String.valueOf(status))
.satisfies(e -> {
if (status == 302) {
@@ -121,13 +156,57 @@ void sizeChanged() throws Exception {
}
}
+ /**
+ * The SDK reads a TDF as a zip over whatever channel it is handed, so the channel has to
+ * satisfy ZipReader's seek-heavy access pattern -- not just the reads a test drives by hand.
+ */
@Test
- void rejectsNonHttpSchemes() {
- var sdk = new SDK(new FakeServicesBuilder().build(), null, null, null, null, null);
- var uri = URI.create("file:///etc/passwd");
- var config = Config.newTDFReaderConfig();
+ void readsAZipArchiveThroughZipReader() throws Exception {
+ var zip = new ByteArrayOutputStream();
+ byte[] payload = new byte[64_000];
+ new java.util.Random(1).nextBytes(payload);
+ // STORED, like TDFWriter writes: ZipReader hands back entry bytes as they sit in the
+ // archive and does not inflate.
+ try (var out = new ZipOutputStream(zip)) {
+ out.setMethod(ZipOutputStream.STORED);
+ out.putNextEntry(stored("0.manifest.json", "{\"hello\":\"world\"}".getBytes(StandardCharsets.UTF_8)));
+ out.write("{\"hello\":\"world\"}".getBytes(StandardCharsets.UTF_8));
+ out.putNextEntry(stored("0.payload", payload));
+ out.write(payload);
+ }
+ byte[] archive = zip.toByteArray();
+
+ try (var server = new MockWebServer()) {
+ server.setDispatcher(rangeDispatcher(() -> archive, new AtomicLong()));
+ try (var ch = new HttpRangeChannel(CLIENT, url(server))) {
+ var entries = new ZipReader(ch).getEntries();
+
+ assertThat(entries).extracting(ZipReader.Entry::getName)
+ .containsExactlyInAnyOrder("0.manifest.json", "0.payload");
+ var found = entries.stream().filter(e -> e.getName().equals("0.payload")).findFirst().orElseThrow();
+ try (var data = found.getData()) {
+ assertThat(data.readAllBytes()).isEqualTo(payload);
+ }
+ }
+ }
+ }
- assertThatThrownBy(() -> sdk.loadTDF(uri, config))
- .isInstanceOf(IllegalArgumentException.class);
+ private static ZipEntry stored(String name, byte[] data) {
+ var crc = new CRC32();
+ crc.update(data);
+ var entry = new ZipEntry(name);
+ entry.setMethod(ZipEntry.STORED);
+ entry.setSize(data.length);
+ entry.setCompressedSize(data.length);
+ entry.setCrc(crc.getValue());
+ return entry;
+ }
+
+ /** The scheme is checked before the SDK is touched, so a null SDK is enough here. */
+ @Test
+ void rejectsNonHttpSchemes() {
+ assertThatThrownBy(() -> HttpRangeChannel.open(null, URI.create("file:///etc/passwd")))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessageContaining("file");
}
}
diff --git a/pom.xml b/pom.xml
index 485744f7..9da9bb3e 100644
--- a/pom.xml
+++ b/pom.xml
@@ -16,6 +16,8 @@
2.20.0
1.78.0
4.36.2
+
+ 4.12.0
1.85
2.1.3
diff --git a/sdk/pom.xml b/sdk/pom.xml
index b1fbf506..1850241b 100644
--- a/sdk/pom.xml
+++ b/sdk/pom.xml
@@ -15,7 +15,6 @@
https://github.com/CodeIntelligenceTesting/jazzer/releases/download/v${jazzer.version}
2.1.0
0.7.2
- 4.12.0
protocol/go/v0.39.0
diff --git a/sdk/src/main/java/io/opentdf/platform/sdk/SDK.java b/sdk/src/main/java/io/opentdf/platform/sdk/SDK.java
index b26f1f9e..633f1040 100644
--- a/sdk/src/main/java/io/opentdf/platform/sdk/SDK.java
+++ b/sdk/src/main/java/io/opentdf/platform/sdk/SDK.java
@@ -27,7 +27,6 @@
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
-import java.net.URI;
import java.nio.channels.SeekableByteChannel;
import java.util.ArrayList;
import java.util.Collections;
@@ -36,8 +35,6 @@
import java.util.Optional;
import java.util.stream.Collectors;
-import okhttp3.OkHttpClient;
-
/**
* The SDK class represents a software development kit for interacting with the
* opentdf platform. It
@@ -60,8 +57,6 @@ public class SDK implements AutoCloseable {
private final String platformUrl;
private final ProtocolClient platformServicesClient;
private final SrtSigner srtSigner;
- // Fetches TDFs by URL. Unauthenticated: platform credentials never go to the TDF host.
- private final OkHttpClient rangeClient;
/**
* Closes the SDK, including its associated services.
@@ -71,8 +66,6 @@ public class SDK implements AutoCloseable {
@Override
public void close() throws Exception {
services.close();
- rangeClient.dispatcher().executorService().shutdown();
- rangeClient.connectionPool().evictAll();
}
/**
@@ -121,12 +114,6 @@ public Optional getAuthInterceptor() {
}
SDK(Services services, TrustManager trustManager, Interceptor authInterceptor, ProtocolClient platformServicesClient, String platformUrl, SrtSigner srtSigner) {
- this(services, trustManager, authInterceptor, platformServicesClient, platformUrl, srtSigner,
- new OkHttpClient.Builder().followRedirects(false).followSslRedirects(false).build());
- }
-
- SDK(Services services, TrustManager trustManager, Interceptor authInterceptor, ProtocolClient platformServicesClient, String platformUrl, SrtSigner srtSigner, OkHttpClient rangeClient) {
- this.rangeClient = rangeClient;
this.platformUrl = platformUrl;
this.services = services;
this.trustManager = trustManager;
@@ -153,20 +140,6 @@ public TDF.Reader loadTDF(SeekableByteChannel channel, Config.TDFReaderConfig co
return tdf.loadTDF(channel, config, platformUrl);
}
- /**
- * Loads a TDF served over HTTP(S), reading it with range requests so only the zip
- * directory, the manifest, and the payload segments actually read are downloaded.
- * The server must answer range requests with 206; redirects are not followed.
- *
- * @throws RangeNotSupportedException if the server ignores range requests
- */
- public TDF.Reader loadTDF(URI url, Config.TDFReaderConfig config) throws SDKException, IOException {
- if (!"http".equalsIgnoreCase(url.getScheme()) && !"https".equalsIgnoreCase(url.getScheme())) {
- throw new IllegalArgumentException("unsupported URL scheme: " + url.getScheme());
- }
- return loadTDF(new HttpRangeChannel(rangeClient, url), config);
- }
-
public Manifest createTDF(InputStream payload, OutputStream outputStream, Config.TDFConfig config) throws SDKException, IOException {
var tdf = new TDF(services);
return tdf.createTDF(payload, outputStream, config).getManifest();
@@ -412,16 +385,6 @@ public boolean attributeValueExists(String valueFqn) {
return resp.getFqnAttributeValuesMap().containsKey(valueFqn);
}
- /**
- * The server holding a TDF did not honor an HTTP range request, so the TDF cannot be
- * read in place; download it and load it from a file instead.
- */
- public static class RangeNotSupportedException extends IOException {
- public RangeNotSupportedException(String errorMessage) {
- super("server did not honor the HTTP range request: " + errorMessage);
- }
- }
-
/**
* Indicates that the TDF is malformed in some way
*/
diff --git a/sdk/src/main/java/io/opentdf/platform/sdk/SDKBuilder.java b/sdk/src/main/java/io/opentdf/platform/sdk/SDKBuilder.java
index cac363d0..dda13258 100644
--- a/sdk/src/main/java/io/opentdf/platform/sdk/SDKBuilder.java
+++ b/sdk/src/main/java/io/opentdf/platform/sdk/SDKBuilder.java
@@ -483,14 +483,7 @@ private KASClient getKASClient(SrtSigner srtSigner, AuthInterceptor interceptor,
public SDK build() {
var services = buildServices();
- return new SDK(services.services, services.trustManager, services.interceptor, services.protocolClient, platformEndpoint, services.srtSigner,
- // getHttpClient() carries the TLS settings but no credentials. Drop the h2c prior
- // knowledge plaintext mode forces, since TDF hosts are ordinary HTTP servers.
- getHttpClient().newBuilder()
- .protocols(List.of(Protocol.HTTP_2, Protocol.HTTP_1_1))
- .followRedirects(false)
- .followSslRedirects(false)
- .build());
+ return new SDK(services.services, services.trustManager, services.interceptor, services.protocolClient, platformEndpoint, services.srtSigner);
}
private ProtocolClient getUnauthenticatedProtocolClient(String endpoint, OkHttpClient httpClient) {
diff --git a/sdk/src/main/java/io/opentdf/platform/sdk/TDF.java b/sdk/src/main/java/io/opentdf/platform/sdk/TDF.java
index f5a61423..4145a15b 100644
--- a/sdk/src/main/java/io/opentdf/platform/sdk/TDF.java
+++ b/sdk/src/main/java/io/opentdf/platform/sdk/TDF.java
@@ -402,8 +402,8 @@ public void readPayload(OutputStream outputStream) throws SDK.TamperException, I
/**
* Writes the plaintext bytes {@code [offset, offset + length)} to {@code outputStream}.
- * Only the segments covering the range are read, verified and decrypted, so over
- * {@link SDK#loadTDF(java.net.URI, Config.TDFReaderConfig)} only those are downloaded.
+ * Only the segments covering the range are read, verified and decrypted, so over a
+ * channel that fetches on demand only those bytes are pulled from the source.
*
* @param offset plaintext offset to start at; equal to the payload size writes nothing
* @param length number of bytes to write; negative means to the end of the payload, and
diff --git a/sdk/src/main/java/io/opentdf/platform/sdk/ZipReader.java b/sdk/src/main/java/io/opentdf/platform/sdk/ZipReader.java
index 634af2b5..60577e09 100644
--- a/sdk/src/main/java/io/opentdf/platform/sdk/ZipReader.java
+++ b/sdk/src/main/java/io/opentdf/platform/sdk/ZipReader.java
@@ -416,13 +416,17 @@ private long findStartOfData() throws IOException {
int extrafieldLength = readUnsignedShort();
long startOfData = zipChannel.position() + filenameLength + extrafieldLength;
- // the central directory can't see these two lengths, so this is the first place a
- // start past the end shows. an empty entry is never read, so unchecked it would come
- // back empty rather than fail as truncated
- if (startOfData > zipChannel.size()) {
+ // The central directory can't see these two lengths, so this is the first place a
+ // bad extent shows. The whole extent is checked, not just where it starts: an entry
+ // declaring more bytes than it holds would otherwise read, or skip over, whatever
+ // record follows it. Bounded by the end of entry data rather than by the archive --
+ // the archive also holds the central directory, so a small overrun still fits
+ // inside the file. Written as a subtraction so a huge declared size cannot overflow.
+ if (startOfData > endOfEntryData - fileSize) {
throw new InvalidZipException("The local file header of entry [" + fileName + "] at offset "
- + offsetToLocalHeader + " puts its data at offset " + startOfData
- + ", past the end of this " + zipChannel.size() + " byte archive");
+ + offsetToLocalHeader + " puts its " + fileSize + " bytes of data at offset "
+ + startOfData + ", running past the end of this archive's entry data at "
+ + endOfEntryData);
}
return startOfData;
}
@@ -451,8 +455,14 @@ private boolean doneReading() {
return offset >= fileSize;
}
- // Moves the cursor without reading: the default skip reads and discards, which
- // over HTTP would download everything skipped.
+ /**
+ * Moves the cursor without reading: the default skip reads and discards, which
+ * over a channel that fetches on demand would download everything skipped.
+ *
+ * Safe because {@link #findStartOfData()} has already bounded this entry's
+ * extent to the region before the central directory, so clamping to what the
+ * entry declares cannot step over bytes belonging to another record.
+ */
@Override
public long skip(long n) {
long skipped = Math.max(0, Math.min(n, fileSize - offset));
@@ -634,6 +644,7 @@ private Entry readCentralDirectoryFileHeader(long centralDirectoryEnd) throws IO
public ZipReader(SeekableByteChannel channel) throws IOException {
zipChannel = channel;
var centralDirectoryRecord = readEndOfCentralDirectory();
+ endOfEntryData = centralDirectoryRecord.offsetToStart;
seekWithinArchive("the central directory", centralDirectoryRecord.offsetToStart);
long centralDirectoryEnd = centralDirectoryRecord.offsetToStart + centralDirectoryRecord.size;
// a corrupt zip64 count can be negative, which would read as an empty archive; an
@@ -650,6 +661,12 @@ public ZipReader(SeekableByteChannel channel) throws IOException {
}
final SeekableByteChannel zipChannel;
+ /**
+ * Where entry data ends and the central directory begins. Every entry's data lies before
+ * it, which makes it the real bound on an entry's extent; {@link #zipChannel}'s size is a
+ * weaker one, since the archive also holds the directory and the records after it.
+ */
+ private long endOfEntryData;
final ArrayList entries = new ArrayList<>();
public List getEntries() {
diff --git a/sdk/src/test/java/io/opentdf/platform/sdk/TDFTest.java b/sdk/src/test/java/io/opentdf/platform/sdk/TDFTest.java
index cbc497de..b2c14341 100644
--- a/sdk/src/test/java/io/opentdf/platform/sdk/TDFTest.java
+++ b/sdk/src/test/java/io/opentdf/platform/sdk/TDFTest.java
@@ -13,7 +13,6 @@
import io.opentdf.platform.policy.kasregistry.ListKeyAccessServersRequest;
import io.opentdf.platform.policy.kasregistry.ListKeyAccessServersResponse;
import io.opentdf.platform.sdk.TDF.Reader;
-import okhttp3.mockwebserver.MockWebServer;
import org.apache.commons.compress.utils.SeekableInMemoryByteChannel;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;
@@ -806,25 +805,45 @@ void readPayloadRangeRejectsBadOffsetsBeforeWriting() throws Exception {
assertThat(out.size()).isZero();
}
+ /** Counts the bytes actually pulled from the archive, whatever the channel is backed by. */
+ private static class CountingChannel extends SeekableInMemoryByteChannel {
+ final AtomicLong read = new AtomicLong();
+
+ CountingChannel(byte[] data) {
+ super(data);
+ }
+
+ @Override
+ public int read(java.nio.ByteBuffer buf) throws IOException {
+ int n = super.read(buf);
+ if (n > 0) {
+ read.addAndGet(n);
+ }
+ return n;
+ }
+ }
+
+ /**
+ * The point of the ranged read: segments before the range are stepped over rather than
+ * pulled in. Over a channel that fetches on demand this is the difference between reading
+ * a slice and downloading the whole payload.
+ */
@Test
- void readPayloadRangeOverHttpFetchesOnlyCoveringSegments() throws Exception {
+ void readPayloadRangeReadsOnlyCoveringSegments() throws Exception {
var fixture = partialReadFixture();
- var served = new AtomicLong();
- try (var server = new MockWebServer()) {
- server.setDispatcher(HttpRangeChannelTest.rangeDispatcher(() -> fixture[1], served));
- var tdf = new TDF(new FakeServicesBuilder().setKas(kas)
- .setKeyAccessServerRegistryService(kasRegistryService).build());
- var reader = tdf.loadTDF(new HttpRangeChannel(HttpRangeChannelTest.CLIENT, server.url("/x.tdf").uri()),
- Config.newTDFReaderConfig(), platformUrl);
+ var channel = new CountingChannel(fixture[1]);
+ var tdf = new TDF(new FakeServicesBuilder().setKas(kas)
+ .setKeyAccessServerRegistryService(kasRegistryService).build());
+ var reader = tdf.loadTDF(channel, Config.newTDFReaderConfig(), platformUrl);
- served.set(0);
- var out = new ByteArrayOutputStream();
- reader.readPayload(out, 2L * SEG - 50, 100);
- assertThat(out.toByteArray()).containsExactly(Arrays.copyOfRange(fixture[0], 2 * SEG - 50, 2 * SEG + 50));
- // the second (full) and third (half) segments, plus the payload entry's local header
- long encryptedSegments = (SEG + 28) + (SEG / 2 + 28);
- assertThat(served.get()).isBetween(encryptedSegments, encryptedSegments + 1024);
- }
+ channel.read.set(0);
+ var out = new ByteArrayOutputStream();
+ reader.readPayload(out, 2L * SEG - 50, 100);
+
+ assertThat(out.toByteArray()).containsExactly(Arrays.copyOfRange(fixture[0], 2 * SEG - 50, 2 * SEG + 50));
+ // the second (full) and third (half) segments, plus the payload entry's local header
+ long encryptedSegments = (SEG + 28) + (SEG / 2 + 28);
+ assertThat(channel.read.get()).isBetween(encryptedSegments, encryptedSegments + 1024);
}
/**
@@ -1608,13 +1627,15 @@ void loadTDFReportsHostileArchivesAsMalformed() {
*/
@Test
void loadTDFReportsAManifestRunningPastTheArchiveAsMalformed() {
- // the manifest opens a JSON comment, so the parser reads on to the end of the archive
- // looking for its close, rather than failing at the first byte past the entry
+ // Rejected when the entry is opened, before a byte is read: the declared extent runs
+ // past where this archive's entry data ends. The manifest opens a JSON comment, so a
+ // reader that got as far as parsing would run to the end of the archive looking for
+ // its close rather than stop at the first byte past the entry.
var uncompressedPastTheEnd = HostileZips.tdfWith("/*", e -> e.uncompressedSize = 0x7FFFFFFFL);
assertThatThrownBy(() -> loadTDFFrom(uncompressedPastTheEnd))
.isInstanceOf(SDK.MalformedTDFException.class)
.hasCauseInstanceOf(InvalidZipException.class)
- .hasMessageContaining("Archive ended");
+ .hasMessageContaining("running past the end of this archive's entry data");
var storedPastTheEnd = HostileZips.tdfWith("{}", e -> e.compressedSize = 0x7FFFFFFFL);
assertThatThrownBy(() -> loadTDFFrom(storedPastTheEnd))
diff --git a/sdk/src/test/java/io/opentdf/platform/sdk/ZipReaderTest.java b/sdk/src/test/java/io/opentdf/platform/sdk/ZipReaderTest.java
index 7cc188b5..dcecbd2c 100644
--- a/sdk/src/test/java/io/opentdf/platform/sdk/ZipReaderTest.java
+++ b/sdk/src/test/java/io/opentdf/platform/sdk/ZipReaderTest.java
@@ -1007,4 +1007,83 @@ private static String readEntry(ZipFile zip, String name) throws IOException {
zip.getInputStream(zip.getEntry(name)).transferTo(data);
return data.toString(StandardCharsets.UTF_8);
}
+
+ private static byte[] payloadArchive(byte[] data, Long declaredSize) {
+ var entry = new HostileZips.Entry("0.payload", data);
+ entry.uncompressedSize = declaredSize;
+ return HostileZips.build(List.of(entry));
+ }
+
+ /** Where {@code data} sits in {@code archive}, so a test can talk about absolute offsets. */
+ private static int offsetOf(byte[] archive, byte[] data) {
+ for (int i = 0; i + data.length <= archive.length; i++) {
+ if (Arrays.equals(Arrays.copyOfRange(archive, i, i + data.length), data)) {
+ return i;
+ }
+ }
+ throw new AssertionError("entry data not found in the archive");
+ }
+
+ /**
+ * The bound on an entry's extent is where entry data ends, not the archive. The central
+ * directory follows the data, so an entry declaring one byte more than it holds still
+ * fits inside the file -- and an archive-sized bound would hand that directory byte back
+ * as entry data, or let skip() step over it and report a clean EOF.
+ */
+ @Test
+ public void aDeclaredSizeOverrunningIntoTheCentralDirectoryIsInvalid() throws Exception {
+ var data = "0123456789".getBytes(StandardCharsets.UTF_8);
+ var archive = payloadArchive(data, data.length + 1L);
+
+ // the excess byte is comfortably inside the file: this is the case the weaker bound misses
+ assertThat(offsetOf(archive, data) + data.length + 1).isLessThan(archive.length);
+
+ try (var channel = new SeekableInMemoryByteChannel(archive)) {
+ var entry = new ZipReader(channel).getEntries().get(0);
+
+ assertThatThrownBy(entry::getData)
+ .isInstanceOf(InvalidZipException.class)
+ .hasMessageContaining("running past the end of this archive's entry data");
+ }
+ }
+
+ /** The same check covers a size past the end of the whole archive. */
+ @Test
+ public void aDeclaredSizePastTheEndOfTheArchiveIsInvalid() throws Exception {
+ var archive = payloadArchive("0123456789".getBytes(StandardCharsets.UTF_8), 1_000_000L);
+ try (var channel = new SeekableInMemoryByteChannel(archive)) {
+ var entry = new ZipReader(channel).getEntries().get(0);
+
+ assertThatThrownBy(entry::getData).isInstanceOf(InvalidZipException.class);
+ }
+ }
+
+ /** The control: the same archive with an honest size reads its bytes back. */
+ @Test
+ public void aDeclaredSizeMatchingTheEntryDataReads() throws Exception {
+ var data = "0123456789".getBytes(StandardCharsets.UTF_8);
+ try (var channel = new SeekableInMemoryByteChannel(payloadArchive(data, null))) {
+ var entry = new ZipReader(channel).getEntries().get(0);
+
+ assertThat(entry.getData().readAllBytes()).isEqualTo(data);
+ }
+ }
+
+ /** A skip wholly inside the entry still just moves the cursor, and lands on the right bytes. */
+ @Test
+ public void skipWithinAnIntactEntryMovesTheCursor() throws Exception {
+ var payload = new byte[5000];
+ new Random(7).nextBytes(payload);
+ var archive = HostileZips.build(List.of(new HostileZips.Entry("0.payload", payload)));
+ try (var channel = new SeekableInMemoryByteChannel(archive)) {
+ var data = new ZipReader(channel).getEntries().get(0).getData();
+
+ assertThat(data.skip(4000)).isEqualTo(4000);
+ assertThat(data.readAllBytes()).isEqualTo(Arrays.copyOfRange(payload, 4000, 5000));
+ // clamped to what is left, and a negative request is a no-op
+ assertThat(data.skip(10)).isZero();
+ assertThat(data.skip(-5)).isZero();
+ }
+ }
+
}