diff --git a/sdk/internal/zipstream/tdf3_reader.go b/sdk/internal/zipstream/tdf3_reader.go index e1b9d67657..973eca1531 100644 --- a/sdk/internal/zipstream/tdf3_reader.go +++ b/sdk/internal/zipstream/tdf3_reader.go @@ -3,6 +3,8 @@ package zipstream import ( + "errors" + "fmt" "io" ) @@ -40,8 +42,20 @@ func NewTDFReader(readSeeker io.ReadSeeker, opt ...TDFReaderOptions) (TDFReader, } // Manifest Return the manifest of the tdf. +// +// The spec name wins over the name this SDK writes when an archive carries +// both. Only a missing entry triggers the fallback: a manifest that is present +// but too large is a size failure, and retrying under the other name would +// both report the wrong reason and, in an archive holding both, hand back the +// superseded manifest. func (tdfReader TDFReader) Manifest() (string, error) { - fileContent, err := tdfReader.archiveReader.ReadAllFileData(TDFManifestFileName, tdfReader.manifestMaxSize) + fileContent, err := tdfReader.archiveReader.ReadAllFileData(TDFManifestFileNameSpec, tdfReader.manifestMaxSize) + if errors.Is(err, errZipFileNotFound) { + fileContent, err = tdfReader.archiveReader.ReadAllFileData(TDFManifestFileName, tdfReader.manifestMaxSize) + if errors.Is(err, errZipFileNotFound) { + return "", fmt.Errorf("no %s or %s entry: %w", TDFManifestFileNameSpec, TDFManifestFileName, err) + } + } if err != nil { return "", err } diff --git a/sdk/internal/zipstream/tdf3_reader_test.go b/sdk/internal/zipstream/tdf3_reader_test.go new file mode 100644 index 0000000000..cf23e2f18b --- /dev/null +++ b/sdk/internal/zipstream/tdf3_reader_test.go @@ -0,0 +1,114 @@ +// Experimental: This package is EXPERIMENTAL and may change or be removed at any time + +package zipstream + +import ( + "bytes" + "testing" + + "github.com/stretchr/testify/require" +) + +// The OpenTDF spec names the manifest entry manifest.json; this SDK writes +// 0.manifest.json. Readers accept either. +// See https://github.com/opentdf/platform/issues/3513. + +// manifestJSON builds representative JSON with the top-level manifest fields. +// It is deliberately incomplete: these tests exercise ZIP entry selection, +// and this package treats the manifest as opaque bytes. A fully valid manifest +// would add fields unrelated to the behavior under test. The literal is spelled +// out here because importing the sdk Manifest type would create an import cycle. +func manifestJSON(mimeType string) string { + return `{"payload":{"type":"reference","url":"` + TDFPayloadFileName + + `","protocol":"zip","isEncrypted":true,"mimeType":"` + mimeType + + `"},"encryptionInformation":{"type":"split"}}` +} + +var ( + // testManifest is 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. + testManifest = manifestJSON("application/octet-stream") + + // otherManifest exists only for the two tests that must tell the entries + // apart -- with identical content, neither could say which one the reader + // returned. It is deliberately shorter than testManifest, which is what + // lets the size-limit test admit one entry and not the other. + otherManifest = manifestJSON("text/plain") +) + +func manifestOf(t *testing.T, entries []rawZipEntry) (string, error) { + t.Helper() + + reader, err := NewTDFReader(bytes.NewReader(buildRawZip(t, entries, false))) + require.NoError(t, err) + + return reader.Manifest() +} + +func TestManifest_ReadsSpecName(t *testing.T) { + manifest, err := manifestOf(t, []rawZipEntry{ + {name: TDFPayloadFileName, data: []byte("payload bytes")}, + {name: "manifest.json", data: []byte(testManifest)}, + }) + + require.NoError(t, err) + require.JSONEq(t, testManifest, manifest) +} + +func TestManifest_ReadsOffspecName(t *testing.T) { + manifest, err := manifestOf(t, []rawZipEntry{ + {name: TDFPayloadFileName, data: []byte("payload bytes")}, + {name: "0.manifest.json", data: []byte(testManifest)}, + }) + + require.NoError(t, err) + require.JSONEq(t, testManifest, manifest) +} + +// The entries hold different manifests, so the assertion cannot pass by +// reading whichever one the reader happened to pick. +func TestManifest_PrefersSpecNameOverOffspec(t *testing.T) { + manifest, err := manifestOf(t, []rawZipEntry{ + {name: TDFPayloadFileName, data: []byte("payload bytes")}, + {name: "0.manifest.json", data: []byte(otherManifest)}, + {name: "manifest.json", data: []byte(testManifest)}, + }) + + require.NoError(t, err) + require.JSONEq(t, testManifest, manifest) +} + +func TestManifest_MissingUnderEitherName(t *testing.T) { + _, err := manifestOf(t, []rawZipEntry{ + {name: TDFPayloadFileName, data: []byte("payload bytes")}, + }) + + require.ErrorIs(t, err, errZipFileNotFound) + // The error has to name both candidates: a bare "file not found" gives no + // hint that a second name was tried. + require.ErrorContains(t, err, TDFManifestFileNameSpec) + require.ErrorContains(t, err, TDFManifestFileName) +} + +// An oversized manifest under the spec name is a size failure, not a missing +// entry. A reader that fell back on any error would quietly hand back the +// superseded non-aligned manifest here, so the size limit is set between the two +// entries: only the spec one exceeds it. +func TestManifest_OversizedSpecNameDoesNotFallBack(t *testing.T) { + require.Greater(t, len(testManifest), len(otherManifest)) + + data := buildRawZip(t, []rawZipEntry{ + {name: TDFPayloadFileName, data: []byte("payload bytes")}, + {name: "0.manifest.json", data: []byte(otherManifest)}, + {name: "manifest.json", data: []byte(testManifest)}, + }, false) + + reader, err := NewTDFReader(bytes.NewReader(data), WithTDFManifestMaxSize(int64(len(otherManifest)))) + require.NoError(t, err) + + manifest, err := reader.Manifest() + require.Error(t, err) + require.NotErrorIs(t, err, errZipFileNotFound) + require.Empty(t, manifest) +} diff --git a/sdk/internal/zipstream/zip_headers.go b/sdk/internal/zipstream/zip_headers.go index 881164700e..ad7b036c8a 100644 --- a/sdk/internal/zipstream/zip_headers.go +++ b/sdk/internal/zipstream/zip_headers.go @@ -17,8 +17,18 @@ const ( ) const ( + // TDFManifestFileName is the manifest entry name this SDK writes. The 0. + // prefix is a holdover from an early design that anticipated several + // payload/manifest pairs per archive and never shipped. TDFManifestFileName = "0.manifest.json" - TDFPayloadFileName = "0.payload" + // TDFManifestFileNameSpec is the manifest entry name given by the OpenTDF + // spec: https://opentdf.io/spec#tdf-structure. Readers accept it so that + // archives from implementations written against the spec can be opened. + // The writer is unchanged and still emits TDFManifestFileName. + // See https://github.com/opentdf/platform/issues/3513. + TDFManifestFileNameSpec = "manifest.json" + + TDFPayloadFileName = "0.payload" ) const ( diff --git a/sdk/sdk_test.go b/sdk/sdk_test.go index 810dede16c..3cd107332d 100644 --- a/sdk/sdk_test.go +++ b/sdk/sdk_test.go @@ -135,6 +135,31 @@ func TestNew_ShouldValidateStandardTdf(t *testing.T) { assert.True(t, isValid) } +// The OpenTDF spec names the manifest entry manifest.json. Validation is the +// first thing a third-party TDF meets, so it has to accept the spec name. +// See https://github.com/opentdf/platform/issues/3513. +func TestIsValidTdf_AcceptsSpecManifestName(t *testing.T) { + // The same TDF as TestNew_ShouldValidateStandardTdf, re-zipped with its + // manifest stored under the spec entry name instead of 0.manifest.json. + specNamedManifestTdf := "UEsDBBQACAAAAAAAAAAAAAAAAAAAAAAAAAAJAAAAMC5wYXlsb2Fktu4m+vdwl0mtjhY3U5e7TG2o1s8ifK+RAhFNjRjGTLJ7V3w5UEsHCGiY7skkAAAAJAAAAFBLAwQUAAgAAAAAAAAAAAAAAAAAAAAAAAAADQAAAG1hbmlmZXN0Lmpzb257ImVuY3J5cHRpb25JbmZvcm1hdGlvbiI6eyJ0eXBlIjoic3BsaXQiLCJwb2xpY3kiOiJleUoxZFdsa0lqb2lNR00xTkRrMlpUWXRORGM0WmkweE1XVm1MV0l5TmpFdFlqSTFZelJoTkRNeE4yRTNJaXdpWW05a2VTSTZleUprWVhSaFFYUjBjbWxpZFhSbGN5STZXM3NpWVhSMGNtbGlkWFJsSWpvaWFIUjBjSE02THk5bGVHRnRjR3hsTG1OdmJTOWhkSFJ5TDJGMGRISXhMM1poYkhWbEwzWmhiSFZsTVNJc0ltUnBjM0JzWVhsT1lXMWxJam9pSWl3aWFYTkVaV1poZFd4MElqcG1ZV3h6WlN3aWNIVmlTMlY1SWpvaUlpd2lhMkZ6VlZKTUlqb2lJbjFkTENKa2FYTnpaVzBpT2x0ZGZYMD0iLCJrZXlBY2Nlc3MiOlt7InR5cGUiOiJ3cmFwcGVkIiwidXJsIjoiaHR0cDovL2xvY2FsaG9zdDo4MDgwIiwicHJvdG9jb2wiOiJrYXMiLCJ3cmFwcGVkS2V5IjoidFVTL1BPU2lQbThlejhoci9nTFRjemNZTk9La3FDRHJWYkEwVnR2Z2tvUGxwdDNQQ2VaU3QzZ3Z5UDVWSmVwTTJjanVQYVFiSVBpcjI5VnVSdk9UV2ZkM0VIdSs4MlQrRTRFWWxKQTNuVWw3RkE0TFBmYVBLV1pNc0xMR1JCSVVMWU9FYTFiZmtTL1JvV29BMCtvN1pRRVZDYWJnSTdiRUQySldkNmhtcmptYlJzNndJcDlRVzVLOEN3SVo2VWY5RjFxMEQ1Yk5pa2xsR2grYmlSbFdTbnBMMWxwT2hXb2tYMXVCbFNFUUgzbzNibVVxUzVVWlI0ZmFMbk1ucThkdG0vMGJyY041MGhTYitMU05WZHdnWkxLM000R05sRHhnc3A5MWNFbmI2aGZLS3pnUmNFQktLTEExdW9wVzR3QkRvQWphbllqZUJWVU92QWRCOXpOOU93PT0iLCJwb2xpY3lCaW5kaW5nIjp7ImFsZyI6IkhTMjU2IiwiaGFzaCI6IlpqQXpNR1l5WXpJeFpUQm1Nek5tTWpoaE1qRmpaakkyWkROaFpUazJOREUzWkRCaFpUazNOREkyTURBMU56VTFNVFUxTVdGaU0yUmpPRFExWm1NMllnPT0ifSwia2lkIjoicjEifV0sIm1ldGhvZCI6eyJhbGdvcml0aG0iOiJBRVMtMjU2LUdDTSIsIml2IjoiIiwiaXNTdHJlYW1hYmxlIjp0cnVlfSwiaW50ZWdyaXR5SW5mb3JtYXRpb24iOnsicm9vdFNpZ25hdHVyZSI6eyJhbGciOiJIUzI1NiIsInNpZyI6IlpHVmhZbUZrTURobE1EQm1NVFZtWXpCbU1XRTBNMkpoTmpoa05qQTFaVGsxTVRkbVpqaGtaRE5rTXpJNE5XWmtNV1F4TlRWbFl6YzBNRFV4T0RNd05nPT0ifSwic2VnbWVudEhhc2hBbGciOiJHTUFDIiwic2VnbWVudFNpemVEZWZhdWx0IjoyMDk3MTUyLCJlbmNyeXB0ZWRTZWdtZW50U2l6ZURlZmF1bHQiOjIwOTcxODAsInNlZ21lbnRzIjpbeyJoYXNoIjoiTWpJM1kyRm1PVEV3TWpFeE5HUTRaREU0WXpZMFkySXlOMkkxTnpkak16az0iLCJzZWdtZW50U2l6ZSI6OCwiZW5jcnlwdGVkU2VnbWVudFNpemUiOjM2fV19fSwicGF5bG9hZCI6eyJ0eXBlIjoicmVmZXJlbmNlIiwidXJsIjoiMC5wYXlsb2FkIiwicHJvdG9jb2wiOiJ6aXAiLCJtaW1lVHlwZSI6ImFwcGxpY2F0aW9uL29jdGV0LXN0cmVhbSIsImlzRW5jcnlwdGVkIjp0cnVlfX1QSwcIMKRTpa8FAACvBQAAUEsBAhQAFAAIAAAAAAAAAGiY7skkAAAAJAAAAAkAAAAAAAAAAAAAAAAAAAAAADAucGF5bG9hZFBLAQIUABQACAAAAAAAAAAwpFOlrwUAAK8FAAANAAAAAAAAAAAAAAAAAFsAAABtYW5pZmVzdC5qc29uUEsFBgAAAAACAAIAcgAAAEUGAAAAAA==" + + decoded, err := base64.StdEncoding.DecodeString(specNamedManifestTdf) + require.NoError(t, err) + + in := bytes.NewReader(decoded) + isValid, err := sdk.IsValidTdf(in) + require.NoError(t, err) + assert.True(t, isValid) + + // Try again to see if the reader has been reset. This matters more on the + // spec name than on the non-aligned one: Manifest() may issue two reads + // against the shared seeker, so anything that leaves the stream parked + // after the first lookup breaks only here. + isValid, err = sdk.IsValidTdf(in) + require.NoError(t, err) + assert.True(t, isValid) +} + func TestNew_ShouldNotValidateBadStandardTdf(t *testing.T) { // This zip file is invalid, with a bad central directory header. badStandardTdf := "UEsDBC0ACAAAAJ2TFTEAAAAAAAAAAAAAAAAJAAAAMC5wYXlsb2Fktu4m+vdwl0mtjhY3U5e7TG2o1s8ifK+RAhFNjRjGTLJ7V3w5UEsHCGiY7skkAAAAJAAAAFBLAwQtAAgAAACdkxUxAAAAAAAAAAAAAAAADwAAADAubWFuaWZlc3QuanNvbnsiZW5jcnlwdGlvbkluZm9ybWF0aW9uIjp7InR5cGUiOiJzcGxpdCIsInBvbGljeSI6ImV5SjFkV2xrSWpvaU1HTTFORGsyWlRZdE5EYzRaaTB4TVdWbUxXSXlOakV0WWpJMVl6UmhORE14TjJFM0lpd2lZbTlrZVNJNmV5SmtZWFJoUVhSMGNtbGlkWFJsY3lJNlczc2lZWFIwY21saWRYUmxJam9pYUhSMGNITTZMeTlsZUdGdGNHeGxMbU52YlM5aGRIUnlMMkYwZEhJeEwzWmhiSFZsTDNaaGJIVmxNU0lzSW1ScGMzQnNZWGxPWVcxbElqb2lJaXdpYVhORVpXWmhkV3gwSWpwbVlXeHpaU3dpY0hWaVMyVjVJam9pSWl3aWEyRnpWVkpNSWpvaUluMWRMQ0prYVhOelpXMGlPbHRkZlgwPSIsImtleUFjY2VzcyI6W3sidXJsIjoiaHR0cDovL2xvY2FsaG9zdDo4MDgwIiwicHJvdG9jb2wiOiJrYXMiLCJ3cmFwcGVkS2V5IjoidFVTL1BPU2lQbThlejhoci9nTFRjemNZTk9La3FDRHJWYkEwVnR2Z2tvUGxwdDNQQ2VaU3QzZ3Z5UDVWSmVwTTJjanVQYVFiSVBpcjI5VnVSdk9UV2ZkM0VIdSs4MlQrRTRFWWxKQTNuVWw3RkE0TFBmYVBLV1pNc0xMR1JCSVVMWU9FYTFiZmtTL1JvV29BMCtvN1pRRVZDYWJnSTdiRUQySldkNmhtcmptYlJzNndJcDlRVzVLOEN3SVo2VWY5RjFxMEQ1Yk5pa2xsR2grYmlSbFdTbnBMMWxwT2hXb2tYMXVCbFNFUUgzbzNibVVxUzVVWlI0ZmFMbk1ucThkdG0vMGJyY041MGhTYitMU05WZHdnWkxLM000R05sRHhnc3A5MWNFbmI2aGZLS3pnUmNFQktLTEExdW9wVzR3QkRvQWphbllqZUJWVU92QWRCOXpOOU93PT0iLCJwb2xpY3lCaW5kaW5nIjp7ImFsZyI6IkhTMjU2IiwiaGFzaCI6IlpqQXpNR1l5WXpJeFpUQm1Nek5tTWpoaE1qRmpaakkyWkROaFpUazJOREUzWkRCaFpUazNOREkyTURBMU56VTFNVFUxTVdGaU0yUmpPRFExWm1NMllnPT0ifSwia2lkIjoicjEifV0sIm1ldGhvZCI6eyJhbGdvcml0aG0iOiJBRVMtMjU2LUdDTSIsIml2IjoiIiwiaXNTdHJlYW1hYmxlIjp0cnVlfSwiaW50ZWdyaXR5SW5mb3JtYXRpb24iOnsicm9vdFNpZ25hdHVyZSI6eyJhbGciOiJIUzI1NiIsInNpZyI6IlpHVmhZbUZrTURobE1EQm1NVFZtWXpCbU1XRTBNMkpoTmpoa05qQTFaVGsxTVRkbVpqaGtaRE5rTXpJNE5XWmtNV1F4TlRWbFl6YzBNRFV4T0RNd05nPT0ifSwic2VnbWVudEhhc2hBbGciOiJHTUFDIiwic2VnbWVudFNpemVEZWZhdWx0IjoyMDk3MTUyLCJlbmNyeXB0ZWRTZWdtZW50U2l6ZURlZmF1bHQiOjIwOTcxODAsInNlZ21lbnRzIjpbeyJoYXNoIjoiTWpJM1kyRm1PVEV3TWpFeE5HUTRaREU0WXpZMFkySXlOMkkxTnpkak16az0iLCJzZWdtZW50U2l6ZSI6OCwiZW5jcnlwdGVkU2VnbWVudFNpemUiOjM2fV19fSwicGF5bG9hZCI6eyJ0eXBlIjoicmVmZXJlbmNlIiwidXJsIjoiMC5wYXlsb2FkIiwicHJvdG9jb2wiOiJ6aXAiLCJtaW1lVHlwZSI6ImFwcGxpY2F0aW9uL29jdGV0LXN0cmVhbSIsImlzRW5jcnlwdGVkIjp0cnVlfX1QSwcIMKRTpa8FAACvBQAAUEsBAi0ALQAIAAAAnZMVMWiY7skkAAAAJAAAAAkAAAAAAAAAAAAAAAAAAAAAADAucGF5bG9hZFBLAQItAC0ACAAAAJ2TFTEwpFOlrwUAAK8FAAAPAAAAAAAAAAAAAAAAAFsAAAAwLm1hbmlmZXN0Lmpzb25QSwUGAAAAAAIAAgB0AAAARwYAAAAA"