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 5e903498..3d03d58c 100644 --- a/sdk/src/main/java/io/opentdf/platform/sdk/SDK.java +++ b/sdk/src/main/java/io/opentdf/platform/sdk/SDK.java @@ -157,7 +157,16 @@ public Optional getSrtSigner() { * Checks to see if this has the structure of a Z-TDF in that it is a zip file * containing * a `manifest.json` and a `0.payload` - * + *

+ * The off-spec `0.manifest.json` that this SDK writes is also accepted, matching + * {@link TDFReader}. Entries beyond the manifest and payload are ignored rather than + * disqualifying: an archive carrying both manifest names holds three, and the reader + * accepts it, so a count check here would reject what the reader it screens for reads. + *

+ * An archive that lists one name twice is rejected, again matching {@link TDFReader}: + * passing it here and then failing the read would hand callers an exception from the + * very check this method exists to spare them. + * * @param channel A channel containing the bytes of the potential Z-TDF * @return `true` if */ @@ -169,11 +178,13 @@ public static boolean isTDF(SeekableByteChannel channel) { return false; } var entries = zipReader.getEntries(); - if (entries.size() != 2) { + var names = entries.stream().map(ZipReader.Entry::getName).collect(Collectors.toSet()); + if (names.size() != entries.size()) { return false; } - return entries.stream().anyMatch(e -> "0.manifest.json".equals(e.getName())) - && entries.stream().anyMatch(e -> "0.payload".equals(e.getName())); + return (names.contains(TDFWriter.TDF_MANIFEST_FILE_NAME_SPEC) + || names.contains(TDFWriter.TDF_MANIFEST_FILE_NAME)) + && names.contains(TDFWriter.TDF_PAYLOAD_FILE_NAME); } /** diff --git a/sdk/src/main/java/io/opentdf/platform/sdk/TDFReader.java b/sdk/src/main/java/io/opentdf/platform/sdk/TDFReader.java index 6e9f32d2..93548d67 100644 --- a/sdk/src/main/java/io/opentdf/platform/sdk/TDFReader.java +++ b/sdk/src/main/java/io/opentdf/platform/sdk/TDFReader.java @@ -9,6 +9,7 @@ import java.util.stream.Collectors; import static io.opentdf.platform.sdk.TDFWriter.TDF_MANIFEST_FILE_NAME; +import static io.opentdf.platform.sdk.TDFWriter.TDF_MANIFEST_FILE_NAME_SPEC; import static io.opentdf.platform.sdk.TDFWriter.TDF_PAYLOAD_FILE_NAME; /** @@ -22,18 +23,29 @@ public class TDFReader { private final InputStream payload; public TDFReader(SeekableByteChannel tdf) throws SDKException, IOException { + // A zip may legally list the same name twice, and readers disagree about which copy + // wins, so reject rather than pick one. Without a merge function this collector throws + // IllegalStateException -- outside the constructor's declared error model, and outside + // what callers screening untrusted input catch. Map entries = new ZipReader(tdf).getEntries() .stream() - .collect(Collectors.toMap(ZipReader.Entry::getName, e -> e)); + .collect(Collectors.toMap(ZipReader.Entry::getName, e -> e, (first, second) -> { + throw new IllegalArgumentException("tdf contains more than one entry named " + first.getName()); + })); - if (!entries.containsKey(TDF_MANIFEST_FILE_NAME)) { + // An archive carrying both names is read, not rejected, and the spec name wins, so a + // conformant entry is never passed over for a superseded one. Two entries under the + // two names are distinct entries -- unlike the duplicate above, where one name is + // listed twice and there is no principled way to choose. + var manifest = entries.getOrDefault(TDF_MANIFEST_FILE_NAME_SPEC, entries.get(TDF_MANIFEST_FILE_NAME)); + if (manifest == null) { throw new IllegalArgumentException("tdf doesn't contain a manifest"); } if (!entries.containsKey(TDF_PAYLOAD_FILE_NAME)) { throw new IllegalArgumentException("tdf doesn't contain a payload"); } - manifestEntry = entries.get(TDF_MANIFEST_FILE_NAME); + manifestEntry = manifest; payload = entries.get(TDF_PAYLOAD_FILE_NAME).getData(); } diff --git a/sdk/src/main/java/io/opentdf/platform/sdk/TDFWriter.java b/sdk/src/main/java/io/opentdf/platform/sdk/TDFWriter.java index 7137c232..93046f83 100644 --- a/sdk/src/main/java/io/opentdf/platform/sdk/TDFWriter.java +++ b/sdk/src/main/java/io/opentdf/platform/sdk/TDFWriter.java @@ -10,7 +10,24 @@ */ public class TDFWriter { public static final String TDF_PAYLOAD_FILE_NAME = "0.payload"; + + /** + * The manifest entry name this SDK writes. The {@code 0.} prefix is a holdover from an + * early design that anticipated several payload/manifest pairs per archive and never + * shipped; the spec names the entry {@link #TDF_MANIFEST_FILE_NAME_SPEC}. Changing what + * the writer emits is a breaking file-format change and is left to a separate change. + * See platform#3513. + */ public static final String TDF_MANIFEST_FILE_NAME = "0.manifest.json"; + + /** + * The manifest entry name given by the OpenTDF spec: + * opentdf.io/spec. Read-side only -- + * {@link TDFReader} accepts it so archives from spec-conformant implementations can be + * read, and the writer does not yet emit it. + */ + public static final String TDF_MANIFEST_FILE_NAME_SPEC = "manifest.json"; + private final ZipWriter archiveWriter; public TDFWriter(OutputStream destination) { diff --git a/sdk/src/test/java/io/opentdf/platform/sdk/SDKTest.java b/sdk/src/test/java/io/opentdf/platform/sdk/SDKTest.java index 289d2f42..bd099a00 100644 --- a/sdk/src/test/java/io/opentdf/platform/sdk/SDKTest.java +++ b/sdk/src/test/java/io/opentdf/platform/sdk/SDKTest.java @@ -10,9 +10,11 @@ import org.junit.jupiter.api.Test; import org.mockito.Mockito; +import java.io.ByteArrayOutputStream; import java.io.IOException; import java.nio.ByteBuffer; import java.nio.channels.SeekableByteChannel; +import java.nio.charset.StandardCharsets; import java.util.Random; import static org.assertj.core.api.Assertions.assertThat; @@ -29,6 +31,94 @@ void testExaminingValidZTDF() throws IOException { } } + /** + * The spec names the manifest entry {@code manifest.json}; the {@code sample.txt.tdf} + * fixture that {@link #testExaminingValidZTDF} reads is an archive this SDK wrote, which + * names it {@code 0.manifest.json}. Both are recognized. + * See platform#3513. + */ + @Test + void testExaminingTDFWithSpecManifestName() throws IOException { + try (var chan = zipOf("0.payload", "manifest.json")) { + assertThat(SDK.isTDF(chan)).isTrue(); + } + } + + /** + * The reader accepts an archive carrying both manifest names, so the sniffer that + * screens for it must not turn that third entry into a rejection. + */ + @Test + void testExaminingTDFWithBothManifestNames() throws IOException { + try (var chan = zipOf("0.payload", "manifest.json", "0.manifest.json")) { + assertThat(SDK.isTDF(chan)).isTrue(); + } + } + + @Test + void testExaminingTDFWithAnExtraEntry() throws IOException { + try (var chan = zipOf("0.payload", "manifest.json", "something-else")) { + assertThat(SDK.isTDF(chan)).isTrue(); + } + } + + @Test + void testExaminingZipWithNoManifest() throws IOException { + try (var chan = zipOf("0.payload", "something-else")) { + assertThat(SDK.isTDF(chan)).isFalse(); + } + } + + @Test + void testExaminingZipWithNoPayload() throws IOException { + try (var chan = zipOf("manifest.json", "something-else")) { + assertThat(SDK.isTDF(chan)).isFalse(); + } + } + + /** + * Dropping the entry-count check widened what counts as a Z-TDF, so the negative cases + * have to cover archives larger than two entries too -- otherwise "any zip with more + * than two entries" would pass this suite. + */ + @Test + void testExaminingLargerZipWithNoManifest() throws IOException { + try (var chan = zipOf("0.payload", "something-else", "and-another")) { + assertThat(SDK.isTDF(chan)).isFalse(); + } + } + + /** + * {@link TDFReader} refuses an archive that lists one name twice rather than guess which + * copy wins, so the sniffer that screens for it has to refuse the same archive -- otherwise + * a caller clears {@link SDK#isTDF} and then takes an exception on the read it gated. + */ + @Test + void testExaminingZipThatListsThePayloadNameTwice() throws IOException { + try (var chan = zipOf("0.payload", "0.payload", "manifest.json")) { + assertThat(SDK.isTDF(chan)).isFalse(); + } + } + + /** The reader rejects any repeated name, not just a repeated manifest or payload. */ + @Test + void testExaminingZipThatListsAnUnrelatedNameTwice() throws IOException { + try (var chan = zipOf("0.payload", "manifest.json", "something-else", "something-else")) { + assertThat(SDK.isTDF(chan)).isFalse(); + } + } + + /** Builds a zip holding the named entries; contents are irrelevant to {@link SDK#isTDF}. */ + private static SeekableInMemoryByteChannel zipOf(String... names) throws IOException { + var out = new ByteArrayOutputStream(); + var writer = new ZipWriter(out); + for (var name : names) { + writer.data(name, name.getBytes(StandardCharsets.UTF_8)); + } + writer.finish(); + return new SeekableInMemoryByteChannel(out.toByteArray()); + } + @Test void testReadingProtocolClient() { var platformServicesClient = mock(ProtocolClient.class); diff --git a/sdk/src/test/java/io/opentdf/platform/sdk/TDFReaderTest.java b/sdk/src/test/java/io/opentdf/platform/sdk/TDFReaderTest.java new file mode 100644 index 00000000..119dc1a3 --- /dev/null +++ b/sdk/src/test/java/io/opentdf/platform/sdk/TDFReaderTest.java @@ -0,0 +1,167 @@ +package io.opentdf.platform.sdk; + +import org.apache.commons.compress.utils.SeekableInMemoryByteChannel; +import org.junit.jupiter.api.Test; + +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.nio.charset.StandardCharsets; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +/** + * The OpenTDF spec names the manifest entry {@code manifest.json}; this SDK writes + * {@code 0.manifest.json}, so the reader accepts either. + * See platform#3513. + */ +public class TDFReaderTest { + + /** + * Manifest-shaped, but deliberately not schema-complete: {@link TDFReader#manifest()} + * hands back the entry's bytes without parsing them, and these tests are about which + * entry it picks. {@link Manifest#readManifest} would reject this literal -- it has no + * integrityInformation -- so anything that parses belongs in a test that builds a real + * manifest instead. + */ + private static String manifestJson(String mimeType) { + return "{\"payload\":{\"type\":\"reference\",\"url\":\"" + TDFWriter.TDF_PAYLOAD_FILE_NAME + + "\",\"protocol\":\"zip\",\"isEncrypted\":true,\"mimeType\":\"" + mimeType + + "\"},\"encryptionInformation\":{\"type\":\"split\"}}"; + } + + /** + * What nearly every fixture here stores. These tests exercise the entry name, not + * manifest contents, so the same manifest serves whichever name it is filed under. + */ + private static final String MANIFEST = manifestJson("application/octet-stream"); + + /** + * Exists only for the test that must tell the two entries apart -- with identical + * content, it could not say which one the reader returned. + */ + private static final String OTHER_MANIFEST = manifestJson("text/plain"); + + private static final String PAYLOAD = "payload bytes"; + + /** + * Builds a zip holding exactly the given name/content pairs, in the order given. Writes + * each pair straight through, so a name may repeat -- that is a legal zip and one of the + * inputs under test. + */ + private static SeekableInMemoryByteChannel archiveOf(String... namesAndContents) throws IOException { + if (namesAndContents.length % 2 != 0) { + throw new IllegalArgumentException("expected name/content pairs"); + } + var out = new ByteArrayOutputStream(); + var writer = new ZipWriter(out); + for (int i = 0; i < namesAndContents.length; i += 2) { + writer.data(namesAndContents[i], namesAndContents[i + 1].getBytes(StandardCharsets.UTF_8)); + } + writer.finish(); + return new SeekableInMemoryByteChannel(out.toByteArray()); + } + + @Test + void readsManifestUnderTheSpecName() throws IOException { + try (var tdf = archiveOf( + TDFWriter.TDF_PAYLOAD_FILE_NAME, PAYLOAD, + "manifest.json", MANIFEST)) { + assertThat(new TDFReader(tdf).manifest()).isEqualTo(MANIFEST); + } + } + + @Test + void readsManifestUnderTheOffspecName() throws IOException { + try (var tdf = archiveOf( + TDFWriter.TDF_PAYLOAD_FILE_NAME, PAYLOAD, + "0.manifest.json", MANIFEST)) { + assertThat(new TDFReader(tdf).manifest()).isEqualTo(MANIFEST); + } + } + + /** + * The two entries hold different manifests, so this cannot pass by reading whichever + * one the reader happened to pick. + */ + @Test + void prefersTheSpecNameWhenAnArchiveCarriesBoth() throws IOException { + try (var tdf = archiveOf( + TDFWriter.TDF_PAYLOAD_FILE_NAME, PAYLOAD, + "0.manifest.json", OTHER_MANIFEST, + "manifest.json", MANIFEST)) { + assertThat(new TDFReader(tdf).manifest()).isEqualTo(MANIFEST); + } + } + + @Test + void rejectsAnArchiveWithNoManifestUnderEitherName() throws IOException { + try (var tdf = archiveOf(TDFWriter.TDF_PAYLOAD_FILE_NAME, PAYLOAD)) { + assertThatThrownBy(() -> new TDFReader(tdf)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("tdf doesn't contain a manifest"); + } + } + + /** + * The match is exact and rooted. The spec requires the manifest to "reside within the + * root of the OpenTDF Zip archive", so a nested or differently-cased entry is not it -- + * without this, a basename or case-insensitive match would look equally correct. + */ + @Test + void rejectsNearMissManifestEntryNames() throws IOException { + try (var tdf = archiveOf( + TDFWriter.TDF_PAYLOAD_FILE_NAME, PAYLOAD, + "Manifest.json", MANIFEST, + "sub/manifest.json", MANIFEST, + "evil-manifest.json", MANIFEST)) { + assertThatThrownBy(() -> new TDFReader(tdf)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("tdf doesn't contain a manifest"); + } + } + + /** + * A zip may list one name twice; readers disagree about which copy wins, so this one + * refuses to choose. Distinct from an archive carrying both manifest names, which is + * read: there, the spec settles the choice. + */ + @Test + void rejectsAnArchiveThatListsOneNameTwice() throws IOException { + try (var tdf = archiveOf( + TDFWriter.TDF_PAYLOAD_FILE_NAME, PAYLOAD, + TDFWriter.TDF_PAYLOAD_FILE_NAME, PAYLOAD, + "manifest.json", MANIFEST)) { + assertThatThrownBy(() -> new TDFReader(tdf)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("more than one entry named " + TDFWriter.TDF_PAYLOAD_FILE_NAME); + } + } + + @Test + void rejectsAnArchiveThatListsTheManifestNameTwice() throws IOException { + try (var tdf = archiveOf( + TDFWriter.TDF_PAYLOAD_FILE_NAME, PAYLOAD, + "manifest.json", MANIFEST, + "manifest.json", OTHER_MANIFEST)) { + assertThatThrownBy(() -> new TDFReader(tdf)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("more than one entry named manifest.json"); + } + } + + /** The buffer is one byte longer than the payload, so a short read would show up. */ + @Test + void readsThePayloadAlongsideASpecNamedManifest() throws IOException { + var expected = PAYLOAD.getBytes(StandardCharsets.UTF_8); + try (var tdf = archiveOf( + TDFWriter.TDF_PAYLOAD_FILE_NAME, PAYLOAD, + "manifest.json", MANIFEST)) { + var reader = new TDFReader(tdf); + var buf = new byte[expected.length + 1]; + + assertThat(reader.readPayloadBytes(buf)).isEqualTo(expected.length); + assertThat(buf).startsWith(expected); + } + } +}