diff --git a/otdfctl/cmd/tdf/decrypt.go b/otdfctl/cmd/tdf/decrypt.go index 05096d7ec0..5da5328cdf 100644 --- a/otdfctl/cmd/tdf/decrypt.go +++ b/otdfctl/cmd/tdf/decrypt.go @@ -61,7 +61,9 @@ func decryptRun(cmd *cobra.Command, args []string) { defer closeIn() // cli.ExitWithError calls os.Exit, which skips deferred functions, so both - // the spooled input and the partial output have to be discarded first. + // the input and the partial output have to be discarded first. closeIn is + // what removes the spool, when there is one -- a piped TDF is spooled to get + // a seekable view of it, while a file argument or a redirect is not. // Declared before the destination exists so every exit below can use it. var outFile *streamio.OutputFile fail := func(msg string, err error) { diff --git a/otdfctl/cmd/tdf/encrypt.go b/otdfctl/cmd/tdf/encrypt.go index d4c09aa805..49d02b566c 100644 --- a/otdfctl/cmd/tdf/encrypt.go +++ b/otdfctl/cmd/tdf/encrypt.go @@ -1,6 +1,7 @@ package tdf import ( + "bytes" "errors" "io" "log/slog" @@ -29,26 +30,34 @@ var ( EncryptCmd = &encryptDoc.Command ) -// detectMimeType sniffs the payload's type from its head and rewinds, so the -// whole payload still reaches the encoder. +// detectMimeType sniffs the payload's type from its head and returns a reader +// that still yields the whole payload. // // Detection needs only the first megabyte, which is what mimetype is limited to -// anyway, so this reads a bounded prefix rather than the whole payload. -func detectMimeType(in io.ReadSeeker, fileExt string) (string, error) { +// anyway, so this reads a bounded prefix rather than the whole payload. An input +// that can be rewound is handed back unchanged, keeping it measurable; anything +// else gets the sniffed prefix pushed back in front of it, a megabyte in memory +// at most. +// +// On error the returned reader is nil, and the payload has been partly consumed. +func detectMimeType(in io.Reader, fileExt string) (string, io.Reader, error) { mimetype.SetLimit(Size1MB) // limit to 1MB + // The rewind below restores where the payload started rather than seeking to + // zero: stdin can arrive part-consumed — `{ read -r header; otdfctl encrypt; } + // < payload.txt` leaves it mid-file — and the payload is what remains. + seeker, start, _ := streamio.Seekable(in) + head := make([]byte, Size1MB) // A payload shorter than the sniff window is the common case, not an error. n, err := io.ReadFull(in, head) if err != nil && !errors.Is(err, io.EOF) && !errors.Is(err, io.ErrUnexpectedEOF) { - return "", err - } - if _, err := in.Seek(0, io.SeekStart); err != nil { - return "", err + return "", nil, err } + head = head[:n] // defaults to application/octet-stream if nothing is recognized - detected := mimetype.Detect(head[:n]).String() + detected := mimetype.Detect(head).String() if detected == "application/octet-stream" && fileExt != "" { // mime.TypeByExtension is the extension lookup. mimetype.Lookup takes a // MIME type string, so passing it a bare extension always returned nil @@ -59,7 +68,18 @@ func detectMimeType(in io.ReadSeeker, fileExt string) (string, error) { detected = byExt } } - return detected, nil + + // Rewinding keeps the payload measurable; replaying the prefix is always + // correct, since a failed lseek(2) leaves the offset untouched. So a refused + // seek is a fallback, not an error — the SDK draws the same line when it + // sizes the payload. + if seeker != nil { + if _, err := seeker.Seek(start, io.SeekStart); err == nil { + return detected, in, nil + } + slog.Debug("payload rewind failed after its position probe succeeded; replaying the sniffed prefix instead") + } + return detected, io.MultiReader(bytes.NewReader(head), in), nil } func encryptRun(cmd *cobra.Command, args []string) { @@ -95,54 +115,43 @@ func encryptRun(cmd *cobra.Command, args []string) { wrappingKeyAlgorithm = ocrypto.RSA2048Key } - piped, hasPiped, err := streamio.PipeReader(os.Stdin) - if err != nil { - cli.ExitWithError("failed to scan bytes from stdin", err) - } - - inputCount := 0 + inputName := "stdin" if filePath != "" { - inputCount++ - } - if hasPiped { - inputCount++ + inputName = filePath } - cliExit := func(s string) { - cli.ExitWithError("Must provide "+s+" of the following to encrypt: [file argument, stdin input]", nil) - } - if inputCount == 0 { - cliExit("ONE") - } else if inputCount > 1 { - cliExit("ONLY ONE") + // Whichever source it is, it goes to the SDK as-is rather than through a + // temporary file, since CreateTDF takes a plain io.Reader. + in, cleanup, err := streamio.OpenExclusive(filePath) + switch { + case errors.Is(err, streamio.ErrNoInput): + cli.ExitWithError("Must provide ONE of the following to encrypt: [file argument, stdin input]", err) + case errors.Is(err, streamio.ErrTwoInputs): + cli.ExitWithError("Must provide ONLY ONE of the following to encrypt: [file argument, stdin input]", err) + case err != nil: + cli.ExitWithError("Failed to read "+inputName+":", err) } + // cli.ExitWithError calls os.Exit, which skips deferred functions, so every + // exit below goes through fail() instead of relying on this. + defer cleanup() - // The SDK seeks to the end of the payload to size it, so the input has to be - // seekable. A file already is; a pipe is spooled to disk, which trades the - // temporary file for the memory a whole-payload read used to cost. - var in io.ReadSeeker - var cleanup func() - if filePath != "" { - f, err := os.Open(filePath) - if err != nil { - cli.ExitWithError("Failed to read file:", err) - } - in, cleanup = f, func() { f.Close() } - } else { - f, spoolCleanup, err := streamio.Spool(piped) - if err != nil { - cli.ExitWithError("Failed to read stdin:", err) + // fail does not return -- cli.ExitWithError ends in os.Exit -- but call sites + // return immediately afterwards so encryptRun reads top to bottom. With -o it + // discards the partial TDF, the no-partial-output guarantee streamio.OutputFile + // exists to provide; a stdout destination cannot be given one, since the bytes + // are already gone. Declared before tdfFile exists, which is why the nil guard. + var tdfFile *streamio.OutputFile + fail := func(msg string, err error) { + if tdfFile != nil { + tdfFile.Cleanup() } - in, cleanup = f, spoolCleanup + cleanup() + cli.ExitWithError(msg, err) } - // cli.ExitWithError calls os.Exit, which skips deferred functions, so every - // exit below goes through fail() to discard the spool and any partial output. - defer cleanup() // Resolve the destination before encrypting, so the payload streams straight // to it rather than accumulating in memory first. var dest io.Writer - var tdfFile *streamio.OutputFile if out != "" { // make sure output ends in .tdf extension if !strings.HasSuffix(out, ".tdf") { @@ -150,8 +159,8 @@ func encryptRun(cmd *cobra.Command, args []string) { } tdfFile, err = streamio.NewOutputFile(out, encryptedOutputFileMode) if err != nil { - cleanup() - cli.ExitWithError("Failed to write encrypted file "+out, err) + fail("Failed to write encrypted file "+out, err) + return } defer tdfFile.Cleanup() dest = tdfFile @@ -159,23 +168,26 @@ func encryptRun(cmd *cobra.Command, args []string) { dest = os.Stdout } - fail := func(msg string, err error) { - if tdfFile != nil { - tdfFile.Cleanup() - } - cleanup() - cli.ExitWithError(msg, err) - } - // auto-detect mime type if not provided if fileMimeType == "" { slog.Debug("detecting mime type of file") - fileMimeType, err = detectMimeType(in, fileExt) + // Assigned through temporaries rather than straight into fileMimeType and + // in: on error detectMimeType returns a nil reader, and writing that into + // in would arm a nil dereference for anyone who later drops the return. + mimeType, rest, err := detectMimeType(in, fileExt) if err != nil { - fail("Failed to read file:", err) + fail("Failed to read "+inputName+" to detect its type (pass --mime-type to skip detection):", err) + return } + fileMimeType, in = mimeType, rest } slog.Debug("encrypting file", slog.String("mime_type", fileMimeType)) + // Worth a breadcrumb: the same bytes encrypt to a slightly larger TDF this + // way, and nothing else tells the user why. Asked here rather than inside + // detectMimeType so --mime-type, which skips detection entirely, still logs. + if !streamio.Measurable(in) { + slog.Debug("payload length is not knowable up front; writing a ZIP64 archive") + } // Do the encryption err = h.Encrypt(c.Context(), dest, in, handlers.EncryptOptions{ @@ -188,12 +200,17 @@ func encryptRun(cmd *cobra.Command, args []string) { TargetMode: targetMode, }) if err != nil { + // The return matters: the Commit below would otherwise rename a + // partially-written TDF into place, which is the exact outcome + // streamio.OutputFile exists to prevent. fail("Failed to encrypt", err) + return } if tdfFile != nil { if err := tdfFile.Commit(); err != nil { fail("Failed to write encrypted file "+out, err) + return } } } diff --git a/otdfctl/cmd/tdf/encrypt_test.go b/otdfctl/cmd/tdf/encrypt_test.go index 8c22bea7ed..8e398f7d36 100644 --- a/otdfctl/cmd/tdf/encrypt_test.go +++ b/otdfctl/cmd/tdf/encrypt_test.go @@ -2,15 +2,50 @@ package tdf import ( "bytes" + "crypto/sha256" + "errors" "io" + "os" "strings" + "syscall" "testing" + "github.com/opentdf/platform/otdfctl/pkg/streamio" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) -func TestDetectMimeTypeRewindsForTheEncoder(t *testing.T) { +// pipeReader hides the Seek method of the reader it wraps, standing in for +// stdin on the end of a pipe. +type pipeReader struct{ inner io.Reader } + +func (r pipeReader) Read(p []byte) (int, error) { return r.inner.Read(p) } + +// espipeReader implements io.Seeker and refuses every seek, the way an *os.File +// on a FIFO, a process substitution, or /dev/stdin on the end of a pipe does. +// Neither of the other two shapes covers that: strings.Reader always seeks, +// pipeReader hides Seek entirely. +type espipeReader struct{ inner io.Reader } + +func (r espipeReader) Read(p []byte) (int, error) { return r.inner.Read(p) } + +func (r espipeReader) Seek(int64, int) (int64, error) { + return 0, &os.PathError{Op: "seek", Path: "/dev/fd/63", Err: syscall.ESPIPE} +} + +// readerKinds are the three input shapes detectMimeType has to cope with. Only +// the first can be rewound, so only the first may come back measurable. +var readerKinds = []struct { + name string + wrap func(io.Reader) io.Reader + rewindable bool +}{ + {name: "seekable", wrap: func(r io.Reader) io.Reader { return r }, rewindable: true}, + {name: "pipe", wrap: func(r io.Reader) io.Reader { return pipeReader{r} }}, + {name: "refuses to seek", wrap: func(r io.Reader) io.Reader { return espipeReader{r} }}, +} + +func TestDetectMimeTypePreservesThePayload(t *testing.T) { for _, tc := range []struct { name string content string @@ -18,31 +53,125 @@ func TestDetectMimeTypeRewindsForTheEncoder(t *testing.T) { }{ {name: "text", content: "hello, world\n", want: "text/plain; charset=utf-8"}, {name: "json", content: `{"a":1}`, want: "application/json"}, + // Exactly the sniff window: the shortest input that fills the buffer, so + // the shortest for which io.ReadFull returns a nil error rather than + // io.ErrUnexpectedEOF, and the boundary at which replaying the prefix + // could duplicate or drop a byte. + {name: "exactly the sniff window", content: strings.Repeat("a", Size1MB), want: "text/plain; charset=utf-8"}, // Larger than the sniff window, so a detector that consumed the reader - // instead of rewinding would truncate the payload. + // without handing the prefix back would truncate the payload. {name: "larger than sniff window", content: strings.Repeat("a", Size1MB+7), want: "text/plain; charset=utf-8"}, } { t.Run(tc.name, func(t *testing.T) { - in := strings.NewReader(tc.content) + for _, kind := range readerKinds { + t.Run(kind.name, func(t *testing.T) { + in := kind.wrap(strings.NewReader(tc.content)) + + got, rest, err := detectMimeType(in, "") + require.NoError(t, err) + assert.Equal(t, tc.want, got) - got, err := detectMimeType(in, "") - require.NoError(t, err) - assert.Equal(t, tc.want, got) + // A stream that came back looking seekable would claim a + // length the SDK then could not read. + assert.Equal(t, kind.rewindable, streamio.Measurable(rest), + "measurability must match rewindability") - // The whole payload must still reach the encoder. - all, err := io.ReadAll(in) - require.NoError(t, err) - assert.Len(t, all, len(tc.content)) + all, err := io.ReadAll(rest) + require.NoError(t, err) + assertSamePayload(t, tc.content, string(all)) + }) + } }) } } +// assertSamePayload compares payloads without rendering a megabyte-wide diff for +// the cases deliberately larger than the sniff window. +func assertSamePayload(t *testing.T, want, got string) { + t.Helper() + + const diffLimit = 4096 + if len(want) <= diffLimit && len(got) <= diffLimit { + assert.Equal(t, want, got) + return + } + // Length first, since truncation is what these cases exist to catch, then + // content by digest -- assert.Equal on a megabyte renders a useless diff. + assert.Len(t, got, len(want), "payload length") + assert.Equal(t, sha256.Sum256([]byte(want)), sha256.Sum256([]byte(got)), "payload digest") +} + +// A sniffed input that came back wrapped in io.MultiReader would silently cost +// every file encrypt its declared length, and nothing else would fail. +func TestDetectMimeTypeKeepsSeekability(t *testing.T) { + in := strings.NewReader("hello, world\n") + + _, rest, err := detectMimeType(in, "") + require.NoError(t, err) + + // Same, not merely "is an io.Seeker": the documented contract is that a + // rewindable input is handed back unchanged. + assert.Same(t, in, rest, "a rewindable input must be handed back unchanged") +} + +// A reader handed over mid-payload must be rewound to where it was, not to zero: +// `{ read -r header; otdfctl encrypt; } < payload.txt` leaves stdin part-consumed, +// and seeking to the start would encrypt the header the wrapper already took. +func TestDetectMimeTypeRestoresANonZeroOffset(t *testing.T) { + const header, payload = "header line\n", "the actual payload\n" + + in := strings.NewReader(header + payload) + _, err := in.Seek(int64(len(header)), io.SeekStart) + require.NoError(t, err) + + _, rest, err := detectMimeType(in, "") + require.NoError(t, err) + + got, err := io.ReadAll(rest) + require.NoError(t, err) + assert.Equal(t, payload, string(got), "the payload is what remains, not the whole file") +} + +// errAfterNReader yields n bytes and then fails with something that is neither +// EOF nor ErrUnexpectedEOF: a failing device, or a socket reset mid-read. Not a +// dying shell producer -- `failing-cmd | otdfctl encrypt` closes the write end, +// which reads as a clean EOF that nothing here can tell from a whole payload. +type errAfterNReader struct { + remaining int + err error +} + +func (r *errAfterNReader) Read(p []byte) (int, error) { + if r.remaining == 0 { + return 0, r.err + } + n := min(len(p), r.remaining) + for i := range n { + p[i] = 'a' + } + r.remaining -= n + return n, nil +} + +// Detection is the first thing that touches the payload, so swallowing a read +// error here would encrypt a truncated payload into a TDF that looks complete. +func TestDetectMimeTypeReportsAReadFailure(t *testing.T) { + want := errors.New("the device gave up") + + _, rest, err := detectMimeType(&errAfterNReader{remaining: 16, err: want}, "") + require.ErrorIs(t, err, want) + + // The nil is contractual: encryptRun assigns the returned reader through a + // temporary so a failed detection cannot install it as the payload. + assert.Nil(t, rest, "a failed detection must not hand back a reader") +} + func TestDetectMimeTypeFallsBackToExtension(t *testing.T) { // Bytes mimetype cannot classify, so the extension decides. ".pdf" is in // Go's builtin table, so this does not depend on the host's mime.types. unrecognized := bytes.Repeat([]byte{0x01, 0x02, 0x03, 0x04}, 8) - got, err := detectMimeType(bytes.NewReader(unrecognized), "pdf") + got, _, err := detectMimeType(bytes.NewReader(unrecognized), "pdf") require.NoError(t, err) assert.Equal(t, "application/pdf", got) } @@ -53,7 +182,7 @@ func TestDetectMimeTypeUnknownExtensionStaysOctetStream(t *testing.T) { // The previous implementation called mimetype.Lookup(fileExt).String(). // Lookup takes a MIME type string, not an extension, so it returned nil for // every extension and this path panicked. - got, err := detectMimeType(bytes.NewReader(unrecognized), "zzzznotathing") + got, _, err := detectMimeType(bytes.NewReader(unrecognized), "zzzznotathing") require.NoError(t, err) assert.Equal(t, "application/octet-stream", got) } @@ -61,7 +190,7 @@ func TestDetectMimeTypeUnknownExtensionStaysOctetStream(t *testing.T) { func TestDetectMimeTypeEmptyPayload(t *testing.T) { in := strings.NewReader("") - got, err := detectMimeType(in, "") + got, _, err := detectMimeType(in, "") require.NoError(t, err) assert.Equal(t, "text/plain", got) } diff --git a/otdfctl/cmd/tdf/inspect.go b/otdfctl/cmd/tdf/inspect.go index e28e7980e8..f2ded24f80 100644 --- a/otdfctl/cmd/tdf/inspect.go +++ b/otdfctl/cmd/tdf/inspect.go @@ -57,7 +57,8 @@ func inspectRun(cmd *cobra.Command, args []string) { } // cli.ExitWithError calls os.Exit, which does not run deferred functions, so // cleanup is also invoked explicitly before every exit below — including the - // successful one, since piped input is spooled to a temporary file. + // successful one, since a piped TDF is spooled to a temporary file. A file + // argument or a redirect from one is seekable already and is not spooled. defer cleanup() result, errs := h.InspectTDF(in) diff --git a/otdfctl/e2e/streaming.bats b/otdfctl/e2e/streaming.bats index 9bf1c69a28..c4077dad78 100755 --- a/otdfctl/e2e/streaming.bats +++ b/otdfctl/e2e/streaming.bats @@ -78,12 +78,26 @@ assert_no_leftovers() { assert_output --partial "$SECRET_TEXT" } -# A TDF's manifest lives at the end of the archive, so decrypt spools a pipe to -# disk to get a seekable view. Verify it round-trips and removes the spool. -@test "roundtrip TDF3, decrypt reading the TDF from stdin" { +# A TDF's manifest lives at the end of the archive, so decrypt needs a seekable +# view of it. A pipe cannot give one, so it is spooled to disk. Verify it +# round-trips and removes the spool. +@test "roundtrip TDF3, decrypt reading the TDF from a pipe" { ./otdfctl encrypt -o "$TDF_OUT" $COMMON "$PLAIN" # Scope TMPDIR to this test so the leftover check cannot see another test's # spool, and cannot be fooled by one either. + run bash -c "TMPDIR='$BATS_TEST_TMPDIR' cat '$TDF_OUT' | ./otdfctl decrypt $COMMON >'$RESULT'" + assert_success + diff "$PLAIN" "$RESULT" + + assert_no_leftovers "$BATS_TEST_TMPDIR/otdfctl-spool-*" +} + +# A redirect from a regular file seeks already, so there is nothing to spool. +# What this pins is the round-trip; that no spool is created at all is not +# observable from out here, and TestOpenSeekableDoesNotSpoolARegularFileStdin +# covers it. +@test "roundtrip TDF3, decrypt reading the TDF from a redirect" { + ./otdfctl encrypt -o "$TDF_OUT" $COMMON "$PLAIN" TMPDIR="$BATS_TEST_TMPDIR" ./otdfctl decrypt $COMMON <"$TDF_OUT" >"$RESULT" diff "$PLAIN" "$RESULT" @@ -163,6 +177,121 @@ assert_no_leftovers() { [ -c /dev/null ] } +# assert_payload_first checks the premise extra_field_len rests on: the archive +# opens with the payload's local file header. If the SDK ever reorders entries, +# this fails here rather than letting extra_field_len read two arbitrary bytes. +assert_payload_first() { + [ -s "$1" ] || fail "$1 is missing or empty" + # 'PK\3\4' as decimal bytes; xargs normalizes od's column padding, which BSD + # and GNU space differently. + run bash -c "od -An -tu1 -N4 '$1' | xargs" + assert_output "80 75 3 4" + # The filename follows the 30-byte header. Always '0.payload', so 9 bytes. + run bash -c "dd if='$1' bs=1 skip=30 count=9 status=none" + assert_output "0.payload" +} + +# extra_field_len reads the extra field length at offset 28 of the payload's +# local file header: non-zero for ZIP64, which carries the extended information +# extra field there, zero for ZIP32. od -tu1 rather than -tu2 because only GNU od +# can be told the endianness. +# +# It fails rather than returning a number when it cannot read a full header -- +# empty output would evaluate to 0, which is what the ZIP32 assertions pass on. +extra_field_len() { + local lo hi + [ -s "$1" ] || { echo "extra_field_len: $1 is missing or empty" >&2; return 1; } + read -r lo hi <<<"$(od -An -tu1 -j28 -N2 "$1")" + if [ -z "$lo" ] || [ -z "$hi" ]; then + echo "extra_field_len: $1 is too short to hold a local file header" >&2 + return 1 + fi + echo $((lo + hi * 256)) +} + +# Encrypting from a pipe no longer spools, so the payload cannot be measured and +# the archive has to be ZIP64; anything measurable stays ZIP32. The layout is what +# to assert on, since both forms round-trip and a quiet return to spooling would +# show up nowhere else. +@test "encrypt measures a file and streams a pipe" { + ./otdfctl encrypt -o "$TDF_OUT" $COMMON "$PLAIN" + assert_payload_first "$TDF_OUT" + [ "$(extra_field_len "$TDF_OUT")" -eq 0 ] + + # --mime-type skips detection entirely, which is the one thing that could have + # wrapped the file and cost it the ZIP32 layout. It must not change the layout. + local typed_tdf="$BATS_TEST_TMPDIR/typed.tdf" + ./otdfctl encrypt -o "$typed_tdf" $COMMON --mime-type text/plain "$PLAIN" + assert_payload_first "$typed_tdf" + [ "$(extra_field_len "$typed_tdf")" -eq 0 ] + + # A redirect arrives on stdin but seeks, so it stays ZIP32. Dropping the spool + # must not turn the whole of stdin into a stream. + local redirected_tdf="$BATS_TEST_TMPDIR/redirected.tdf" + run bash -c "./otdfctl encrypt $COMMON <'$PLAIN' >'$redirected_tdf'" + assert_success + assert_payload_first "$redirected_tdf" + [ "$(extra_field_len "$redirected_tdf")" -eq 0 ] + + local piped_tdf="$BATS_TEST_TMPDIR/piped.tdf" + run bash -c "echo '$SECRET_TEXT' | ./otdfctl encrypt $COMMON >'$piped_tdf'" + assert_success + assert_payload_first "$piped_tdf" + [ "$(extra_field_len "$piped_tdf")" -gt 0 ] + + ./otdfctl decrypt -o "$RESULT" $COMMON "$piped_tdf" + run cat "$RESULT" + assert_output "$SECRET_TEXT" +} + +# Process substitution is the shape a type assertion gets wrong: bash hands us +# /dev/fd/63, which opens as an *os.File and so satisfies io.Seeker, and then +# every seek returns ESPIPE. Only unit tests with a hand-written fake model that, +# so it is worth an end-to-end case. +@test "encrypt streams a process substitution" { + local subst_tdf="$BATS_TEST_TMPDIR/subst.tdf" + run bash -c "./otdfctl encrypt $COMMON <(cat '$PLAIN') >'$subst_tdf'" + assert_success + assert_payload_first "$subst_tdf" + # Unmeasurable, so ZIP64, exactly like a pipe. + [ "$(extra_field_len "$subst_tdf")" -gt 0 ] + + ./otdfctl decrypt -o "$RESULT" $COMMON "$subst_tdf" + diff "$PLAIN" "$RESULT" + + # --mime-type skips detection, so the ESPIPE is hit later, by the SDK's own + # sizing attempt rather than by detectMimeType's. + local typed_subst_tdf="$BATS_TEST_TMPDIR/subst-typed.tdf" + run bash -c "./otdfctl encrypt $COMMON --mime-type text/plain <(cat '$PLAIN') >'$typed_subst_tdf'" + assert_success + assert_payload_first "$typed_subst_tdf" + [ "$(extra_field_len "$typed_subst_tdf")" -gt 0 ] + + ./otdfctl decrypt -o "$RESULT" $COMMON "$typed_subst_tdf" + diff "$PLAIN" "$RESULT" +} + +# 3 MiB clears the SDK's 2 MiB default segment size, so the archive spans several +# segments whose hashes are combined on the way out and verified on the way back +# in. A pipe also feeds the encoder short reads at segment boundaries; a file +# does not. +@test "encrypt streams a multi-segment pipe" { + local big="$BATS_TEST_TMPDIR/multi.bin" + local bigtdf="$BATS_TEST_TMPDIR/multi.tdf" + local bigout="$BATS_TEST_TMPDIR/multi.out" + dd if=/dev/urandom of="$big" bs=1048576 count=3 status=none + + # A real pipe, not a redirect: a redirect from a regular file seeks, and would + # be measured and written as ZIP32. + run bash -c "cat '$big' | ./otdfctl encrypt $COMMON >'$bigtdf'" + assert_success + assert_payload_first "$bigtdf" + [ "$(extra_field_len "$bigtdf")" -gt 0 ] + + ./otdfctl decrypt -o "$bigout" $COMMON "$bigtdf" + cmp "$big" "$bigout" +} + # The point of DSPX-4499: peak RSS is bounded by segment size, not payload size. # Needs GNU time for 'Maximum resident set size'; BSD/shell time cannot report it. @test "encrypt and decrypt peak memory stay bounded on a large payload" { diff --git a/otdfctl/pkg/cli/pipe.go b/otdfctl/pkg/cli/pipe.go index 8d0472960d..73697801fa 100644 --- a/otdfctl/pkg/cli/pipe.go +++ b/otdfctl/pkg/cli/pipe.go @@ -30,10 +30,10 @@ func ReadFromArgsOrPipe(args []string, pipe *os.File) []byte { } // Deprecated: reads the entire pipe into memory and terminates the process on -// failure. Use streamio.PipeReader, which reports whether input is present -// without consuming it, paired with io.ReadAll for the equivalent []byte. +// failure. Use streamio.Piped, which reports whether input is present without +// consuming it, paired with io.ReadAll for the equivalent []byte. func ReadFromPipe(in *os.File) []byte { - r, ok, err := streamio.PipeReader(in) + r, ok, err := streamio.Piped(in) if err != nil { ExitWithError("failed to read stat from stdin", err) } diff --git a/otdfctl/pkg/handlers/tdf.go b/otdfctl/pkg/handlers/tdf.go index 0a2bb794c5..bd91fbd81b 100644 --- a/otdfctl/pkg/handlers/tdf.go +++ b/otdfctl/pkg/handlers/tdf.go @@ -52,10 +52,17 @@ type EncryptOptions struct { // bounded by the SDK's segment size rather than by the payload length, so the // payload may be larger than RAM. // -// in must be seekable: the SDK measures the payload by seeking to its end -// before encrypting, and knowing the length up front is what lets it avoid -// defaulting to ZIP64. A caller holding a pipe should spool it first. -func (h Handler) Encrypt(ctx context.Context, out io.Writer, in io.ReadSeeker, o EncryptOptions) error { +// in need not be seekable, but a seekable one is measurable, and the SDK gives a +// measured payload two things an unmeasurable one cannot have. It gets the +// compact ZIP32 layout below ~2 GiB, where an unmeasurable payload pays a few +// dozen bytes for ZIP64 — a choice fixed before the first segment goes out. And +// it gets a length check, so a reader that runs dry early fails instead of +// yielding a complete-looking TDF holding a truncated payload. +// +// Neither is worth spooling a stream to disk for: needing a writable temp +// directory to encrypt a pipe costs more than it buys. This is what the rest of +// otdfctl means by "keeping the payload measurable"; see streamio.Measurable. +func (h Handler) Encrypt(ctx context.Context, out io.Writer, in io.Reader, o EncryptOptions) error { switch o.TDFType { // Encrypt the data as a ZTDF case "", tdf.TypeTDF3, tdf.TypeZTDF: diff --git a/otdfctl/pkg/streamio/input.go b/otdfctl/pkg/streamio/input.go index dc7c29a874..59af55e1cd 100644 --- a/otdfctl/pkg/streamio/input.go +++ b/otdfctl/pkg/streamio/input.go @@ -14,25 +14,87 @@ import ( "os" ) -// PipeBufferSize is the window PipeReader buffers over a pipe — generous -// enough that a typical CLI payload is served from a single read. +// PipeBufferSize is the window Piped buffers over a pipe — generous enough that +// a typical CLI payload is served from a single read. const PipeBufferSize = 1024 * 1024 -// ErrNoInput reports that a command was given neither a file argument nor a -// non-empty pipe. It is distinct from a failure to open a named file, which -// callers report differently. -var ErrNoInput = errors.New("no input provided") +var ( + // ErrNoInput reports that a command was given neither a file argument nor a + // non-empty pipe. It is distinct from a failure to open a named file, which + // callers report differently. + ErrNoInput = errors.New("no input provided") -// PipeReader reports whether in is a pipe or redirect carrying at least one -// byte, and returns a reader over it. + // ErrTwoInputs reports that a command which takes one payload was given two. + // Only OpenExclusive returns it; Open prefers the file argument instead. + ErrTwoInputs = errors.New("file argument and stdin input both provided") +) + +// Seekable reports r's current position, and whether it could be asked for one. +// +// It probes rather than asserting io.Seeker, because an *os.File on a FIFO, a +// process substitution, or /dev/stdin on the end of a pipe all satisfy the +// interface and then return ESPIPE. Seeking to the current position never moves +// it, so this is safe on a payload about to be read. +func Seekable(r io.Reader) (io.Seeker, int64, bool) { + seeker, ok := r.(io.Seeker) + if !ok { + return nil, 0, false + } + off, err := seeker.Seek(0, io.SeekCurrent) + if err != nil { + return nil, 0, false + } + return seeker, off, true +} + +// Measurable reports whether the SDK can size r without reading it, and so +// whether the archive gets the compact layout. See handlers.Handler's Encrypt. +func Measurable(r io.Reader) bool { + _, _, ok := Seekable(r) + return ok +} + +// unmeasurable hides a reader's Seek method, so anything probing for one sizes +// the payload by reading it instead. +type unmeasurable struct{ io.Reader } + +// openFile opens path for reading, ready to hand to the SDK as-is. +// +// The file stays measurable, except when its stat cannot be trusted: a procfs or +// sysfs file is a regular file that reports zero bytes and then reads out +// content. Measuring one declares an empty payload, and the SDK limits its reads +// to the length it was given, so the encrypt would succeed with nothing in it. +// Those come back with the Seeker hidden — an archive a few dozen bytes larger +// beats an archive missing the payload. +// +// cleanup is always non-nil, including on the error return. +func openFile(path string) (io.Reader, func(), error) { + f, err := os.Open(path) + if err != nil { + return nil, func() {}, err + } + cleanup := func() { f.Close() } + stat, err := f.Stat() + if err != nil { + cleanup() + return nil, func() {}, err + } + if stat.Mode().IsRegular() && stat.Size() == 0 { + return unmeasurable{f}, cleanup, nil + } + return f, cleanup, nil +} + +// Piped reports whether in carries at least one byte of payload, and returns a +// reader over it. A terminal, or an empty redirect such as +// `otdfctl encrypt < /dev/null`, reports false. It is named for the usual case +// but takes redirects too, which is the harder half. // -// Presence is established with a one-byte Peek rather than a read, so the -// payload still reaches the caller intact and nothing is buffered beyond the -// reader's window. A terminal, or an empty redirect such as -// `otdfctl encrypt < /dev/null`, reports false — matching the behavior of a -// buffered implementation, which decides the same question by checking whether -// a full read came back empty. -func PipeReader(in *os.File) (*bufio.Reader, bool, error) { +// A redirect from a regular file — `otdfctl encrypt < payload.txt` — is handed +// back as the *os.File itself, so the payload stays measurable. Wrapping it +// would throw that away for nothing: it is the same fd either way. Everything +// else is a stream, and gets a buffered reader. +func Piped(in *os.File) (io.Reader, bool, error) { stat, err := in.Stat() if err != nil { return nil, false, err @@ -41,6 +103,21 @@ func PipeReader(in *os.File) (*bufio.Reader, bool, error) { return nil, false, nil } + // Presence comes from the stat rather than a Peek, which would advance the + // fd the caller is about to be handed. Measured from the current offset, not + // zero: `{ read -r header; otdfctl encrypt; } < payload.txt` leaves stdin + // mid-file, and what remains is the payload. + // + // Only a stat that counts bytes ahead of the offset gets to decide, since a + // zero-byte procfs file reads out content anyway (see openFile). Everything + // else falls through to the Peek, which asks the file instead of the stat and + // still answers absent for a genuinely empty or already-consumed one. + if stat.Mode().IsRegular() { + if _, off, ok := Seekable(in); ok && stat.Size() > off { + return in, true, nil + } + } + r := bufio.NewReaderSize(in, PipeBufferSize) if _, err := r.Peek(1); err != nil { if errors.Is(err, io.EOF) { @@ -51,6 +128,49 @@ func PipeReader(in *os.File) (*bufio.Reader, bool, error) { return r, true, nil } +// Open resolves a command's payload: the named file when one is given, +// otherwise stdin. It returns ErrNoInput when there is neither. +// +// Nothing is copied, so a measurable source reaches the caller measurable; see +// Measurable. A file argument wins outright and stdin is not even looked at, +// since a command run inside `while read f; do ... done < list` inherits a stdin +// that is the loop's, not a payload — and looking costs a peek that would eat it. +// +// cleanup is always non-nil, including on the error return. For a piped payload +// it does nothing: closing stdin is not ours to do. +func Open(path string) (io.Reader, func(), error) { + if path != "" { + return openFile(path) + } + piped, ok, err := Piped(os.Stdin) + switch { + case err != nil: + return nil, func() {}, err + case !ok: + return nil, func() {}, ErrNoInput + } + return piped, func() {}, nil +} + +// OpenExclusive is Open for a command that refuses to choose: being handed both +// a file argument and a payload on stdin is ErrTwoInputs rather than a silent +// preference. Telling them apart means inspecting stdin even when the file +// argument would have decided it, which for a pipe consumes what it peeked. +func OpenExclusive(path string) (io.Reader, func(), error) { + piped, hasPiped, err := Piped(os.Stdin) + switch { + case err != nil: + return nil, func() {}, err + case path != "" && hasPiped: + return nil, func() {}, ErrTwoInputs + case path != "": + return openFile(path) + case hasPiped: + return piped, func() {}, nil + } + return nil, func() {}, ErrNoInput +} + // Spool copies r into a temporary file and rewinds it, giving a seekable view // of a stream that has none. // @@ -81,34 +201,25 @@ func Spool(r io.Reader) (*os.File, func(), error) { return f, cleanup, nil } -// OpenSeekable resolves a command's input to something seekable: the named file -// when one is given, otherwise piped stdin spooled to disk. It returns -// ErrNoInput for the same "nothing to read" condition whether the file argument -// was absent or the pipe was empty. +// OpenSeekable is Open for a command that has to seek its input — reading a +// TDF, whose manifest sits at the end of the archive. +// +// Only what cannot seek is spooled to disk: a named file or a redirect from one +// seeks already, and copying it would need a TMPDIR with room for the whole TDF +// to hand back a view no better than the one it had. // // The returned cleanup must run on every path, per Spool. func OpenSeekable(path string) (*os.File, func(), error) { - if path != "" { - f, err := os.Open(path) - if err != nil { - return nil, func() {}, err - } - if _, err := f.Seek(0, io.SeekCurrent); err == nil { - return f, func() { f.Close() }, nil - } - // Not seekable (a FIFO or /dev/fd/N): spool it like piped stdin so - // callers still get something they can seek. - spooled, cleanup, err := Spool(f) - f.Close() - return spooled, cleanup, err - } - - piped, ok, err := PipeReader(os.Stdin) + in, cleanup, err := Open(path) if err != nil { return nil, func() {}, err } - if !ok { - return nil, func() {}, ErrNoInput + if f, ok := in.(*os.File); ok && Measurable(f) { + return f, cleanup, nil } - return Spool(piped) + // A pipe, a FIFO, /dev/fd/N, or a file whose stat openFile would not vouch + // for. Spooling closes over the original, so let go of it either way. + spooled, spoolCleanup, err := Spool(in) + cleanup() + return spooled, spoolCleanup, err } diff --git a/otdfctl/pkg/streamio/input_test.go b/otdfctl/pkg/streamio/input_test.go index 2c8f8ad3d5..363920ce8e 100644 --- a/otdfctl/pkg/streamio/input_test.go +++ b/otdfctl/pkg/streamio/input_test.go @@ -13,7 +13,7 @@ import ( "github.com/stretchr/testify/require" ) -func TestPipeReader(t *testing.T) { +func TestPiped(t *testing.T) { for _, tc := range []struct { name string content string @@ -32,7 +32,7 @@ func TestPipeReader(t *testing.T) { _, _ = io.WriteString(w, tc.content) }() - got, ok, err := PipeReader(r) + got, ok, err := Piped(r) require.NoError(t, err) require.Equal(t, tc.wantOK, ok) if !tc.wantOK { @@ -47,7 +47,7 @@ func TestPipeReader(t *testing.T) { } } -func TestPipeReaderPreservesPayloadLargerThanBuffer(t *testing.T) { +func TestPipedPreservesPayloadLargerThanBuffer(t *testing.T) { content := strings.Repeat("a", PipeBufferSize*2+7) r, w, err := os.Pipe() @@ -59,7 +59,7 @@ func TestPipeReaderPreservesPayloadLargerThanBuffer(t *testing.T) { _, _ = io.WriteString(w, content) }() - got, ok, err := PipeReader(r) + got, ok, err := Piped(r) require.NoError(t, err) require.True(t, ok) @@ -68,7 +68,7 @@ func TestPipeReaderPreservesPayloadLargerThanBuffer(t *testing.T) { assert.Len(t, all, len(content)) } -func TestPipeReaderOnTerminalReportsAbsent(t *testing.T) { +func TestPipedOnTerminalReportsAbsent(t *testing.T) { // A regular file is not a char device, so use os.Stdin's actual mode only // when it is one; otherwise this assertion is vacuous and we skip. stat, err := os.Stdin.Stat() @@ -76,11 +76,290 @@ func TestPipeReaderOnTerminalReportsAbsent(t *testing.T) { if (stat.Mode() & os.ModeCharDevice) == 0 { t.Skip("stdin is not a terminal under this test runner") } - _, ok, err := PipeReader(os.Stdin) + _, ok, err := Piped(os.Stdin) require.NoError(t, err) assert.False(t, ok) } +// tempFile writes content to a file that lasts as long as the test. +func tempFile(t *testing.T, content string) string { + t.Helper() + + path := filepath.Join(t.TempDir(), "payload.txt") + require.NoError(t, os.WriteFile(path, []byte(content), 0o600)) + return path +} + +// openRegularStdin stands in for `otdfctl encrypt < payload.txt`: a redirect +// from a regular file is not a pipe, and Piped has to notice. +func openRegularStdin(t *testing.T, content string) *os.File { + t.Helper() + + f, err := os.Open(tempFile(t, content)) + require.NoError(t, err) + t.Cleanup(func() { f.Close() }) + return f +} + +// useStdin makes f stdin for the duration of the test. +func useStdin(t *testing.T, f *os.File) { + t.Helper() + + orig := os.Stdin + os.Stdin = f + t.Cleanup(func() { os.Stdin = orig }) +} + +// stdinPipe makes a pipe carrying content stdin. An empty content is a producer +// that writes nothing and hangs up, the same "nothing to read" a caller gets +// from `otdfctl encrypt < /dev/null`. +func stdinPipe(t *testing.T, content string) { + t.Helper() + + r, w, err := os.Pipe() + require.NoError(t, err) + t.Cleanup(func() { r.Close() }) + go func() { + defer w.Close() + _, _ = io.WriteString(w, content) + }() + useStdin(t, r) +} + +// A regular-file redirect must come back as the file itself. Wrapping it would +// cost the payload its length for nothing -- it is the same fd either way. +func TestPipedRegularFileStaysSeekable(t *testing.T) { + const content = "hello from a redirect\n" + f := openRegularStdin(t, content) + + got, ok, err := Piped(f) + require.NoError(t, err) + require.True(t, ok) + + assert.Same(t, f, got, "a regular file must be handed back unwrapped") + + // Presence came from the stat, so nothing was read on the way through. + off, err := f.Seek(0, io.SeekCurrent) + require.NoError(t, err) + assert.Zero(t, off, "detecting presence must not consume the payload") + + all, err := io.ReadAll(got) + require.NoError(t, err) + assert.Equal(t, content, string(all)) +} + +// An empty redirect is "no input", not "a zero-byte payload" -- the same answer +// the Peek gives for an empty pipe. +func TestPipedEmptyRegularFileReportsAbsent(t *testing.T) { + got, ok, err := Piped(openRegularStdin(t, "")) + require.NoError(t, err) + assert.False(t, ok) + assert.Nil(t, got) +} + +// Presence is measured from the current offset, not from zero: +// `{ read -r hdr; otdfctl encrypt; } < f` leaves stdin mid-file, and what +// remains is the payload. +func TestPipedRegularFileAtNonZeroOffset(t *testing.T) { + const header, payload = "header line\n", "the actual payload\n" + f := openRegularStdin(t, header+payload) + _, err := f.Seek(int64(len(header)), io.SeekStart) + require.NoError(t, err) + + got, ok, err := Piped(f) + require.NoError(t, err) + require.True(t, ok) + + all, err := io.ReadAll(got) + require.NoError(t, err) + assert.Equal(t, payload, string(all), "the payload is what remains, not the whole file") +} + +// A file consumed to its end has nothing left to encrypt, so it reports absent +// even though the file itself is not empty. +func TestPipedRegularFileAtEOFReportsAbsent(t *testing.T) { + const content = "already read\n" + f := openRegularStdin(t, content) + _, err := f.Seek(int64(len(content)), io.SeekStart) + require.NoError(t, err) + + _, ok, err := Piped(f) + require.NoError(t, err) + assert.False(t, ok) +} + +// A procfs file is a readable regular file that stats as zero bytes, so the +// stat cannot be the one to say whether a payload is there. Deciding on it alone +// turned `otdfctl encrypt < /proc/cpuinfo` into "no input". +func TestPipedZeroSizedRegularFileWithContent(t *testing.T) { + f, err := os.Open(zeroSizedFileWithContent(t)) + require.NoError(t, err) + defer f.Close() + + got, ok, err := Piped(f) + require.NoError(t, err) + require.True(t, ok, "a file that stats as empty may still have content") + + all, err := io.ReadAll(got) + require.NoError(t, err) + assert.NotEmpty(t, all, "the payload has to survive the presence check") +} + +// zeroSizedFileWithContent names a file that stats as zero bytes and reads out +// content anyway. Only procfs and its kin do that, and nothing in a TempDir can +// be made to, so this skips everywhere else. +func zeroSizedFileWithContent(t *testing.T) string { + t.Helper() + + if runtime.GOOS != "linux" { + t.Skip("a readable regular file that stats as zero bytes needs procfs") + } + const path = "/proc/self/status" + stat, err := os.Stat(path) + require.NoError(t, err) + require.True(t, stat.Mode().IsRegular(), "the premise of this test") + require.Zero(t, stat.Size(), "the premise of this test") + return path +} + +// Nothing between here and sdk.CreateTDF would fail if a wrapper hid the Seeker; +// every file encrypt would just quietly stop declaring its length. +func TestOpenFileStaysMeasurable(t *testing.T) { + const content = "hello, world\n" + path := filepath.Join(t.TempDir(), "payload.txt") + require.NoError(t, os.WriteFile(path, []byte(content), 0o600)) + + in, cleanup, err := openFile(path) + require.NoError(t, err) + defer cleanup() + + assert.True(t, Measurable(in), "a file argument must reach the SDK measurable") + + got, err := io.ReadAll(in) + require.NoError(t, err) + assert.Equal(t, content, string(got)) +} + +// A stat of zero is not a measurement. The SDK limits its reads to the length it +// was given, so undercounting is worse than not counting: the encrypt succeeds +// and the TDF holds nothing. +func TestOpenFileDoesNotMeasureAZeroSizedFile(t *testing.T) { + path := filepath.Join(t.TempDir(), "empty.txt") + require.NoError(t, os.WriteFile(path, nil, 0o600)) + + in, cleanup, err := openFile(path) + require.NoError(t, err) + defer cleanup() + + assert.False(t, Measurable(in)) +} + +// The case that branch exists for: procfs reports zero and then reads out +// content. Both halves matter -- io.ReadAll drains the file either way, and it is +// the SDK's io.LimitReader, sized from the measurement, that would stop at zero. +func TestOpenFileReadsAZeroSizedFileWhole(t *testing.T) { + in, cleanup, err := openFile(zeroSizedFileWithContent(t)) + require.NoError(t, err) + defer cleanup() + + assert.False(t, Measurable(in), "the stat undercounts, so it must not be trusted") + + got, err := io.ReadAll(in) + require.NoError(t, err) + assert.NotEmpty(t, got, "the payload has to reach the SDK") +} + +func TestOpenFileReportsAMissingFile(t *testing.T) { + _, cleanup, err := openFile(filepath.Join(t.TempDir(), "absent.txt")) + require.ErrorIs(t, err, os.ErrNotExist) + + // cleanup is non-nil even here, so a caller may defer it without a nil guard. + require.NotNil(t, cleanup) + cleanup() +} + +// Open and OpenExclusive part company only over being handed both sources at +// once, so every other case is asserted of both. +func TestResolveInput(t *testing.T) { + for _, resolver := range []struct { + name string + open func(string) (io.Reader, func(), error) + }{ + {name: "Open", open: Open}, + {name: "OpenExclusive", open: OpenExclusive}, + } { + t.Run(resolver.name, func(t *testing.T) { + for _, tc := range []struct { + name, inFile, onStdin string + wantMeasurable bool + wantErr error + }{ + // A file argument reaches the SDK measurable. A pipe must not: + // encrypting one would then need a TMPDIR to spool it. + {name: "file argument", inFile: "the payload\n", wantMeasurable: true}, + {name: "stdin", onStdin: "piped payload\n"}, + {name: "neither", wantErr: ErrNoInput}, + } { + t.Run(tc.name, func(t *testing.T) { + stdinPipe(t, tc.onStdin) + path := "" + if tc.inFile != "" { + path = tempFile(t, tc.inFile) + } + + in, cleanup, err := resolver.open(path) + // cleanup is non-nil even on the error return, so a caller + // may defer it without a nil guard. + require.NotNil(t, cleanup) + defer cleanup() + + if tc.wantErr != nil { + require.ErrorIs(t, err, tc.wantErr) + return + } + require.NoError(t, err) + assert.Equal(t, tc.wantMeasurable, Measurable(in)) + + got, err := io.ReadAll(in) + require.NoError(t, err) + assert.Equal(t, tc.inFile+tc.onStdin, string(got)) // exactly one is set + }) + } + }) + } +} + +// Given both, Open takes the file argument and leaves stdin not just unused but +// unread: `while read f; do otdfctl … "$f"; done < list` loses a line for every +// byte peeked out from under the loop. +func TestOpenPrefersTheFileArgument(t *testing.T) { + const onStdin, inFile = "the loop's list\n", "the payload\n" + stdinPipe(t, onStdin) + + in, cleanup, err := Open(tempFile(t, inFile)) + require.NoError(t, err) + defer cleanup() + + got, err := io.ReadAll(in) + require.NoError(t, err) + assert.Equal(t, inFile, string(got)) + + rest, err := io.ReadAll(os.Stdin) + require.NoError(t, err) + assert.Equal(t, onStdin, string(rest), "stdin must survive a call that never wanted it") +} + +// OpenExclusive refuses to pick, rather than silently dropping one of the two +// payloads it was handed. +func TestOpenExclusiveRejectsTwoInputs(t *testing.T) { + stdinPipe(t, "a piped payload\n") + + _, cleanup, err := OpenExclusive(tempFile(t, "a file payload\n")) + require.ErrorIs(t, err, ErrTwoInputs) + require.NotNil(t, cleanup) + cleanup() +} + func TestSpoolIsSeekableAndComplete(t *testing.T) { // Larger than any plausible internal buffer, so a truncating copy shows up. content := strings.Repeat("xyz", PipeBufferSize) @@ -131,8 +410,7 @@ func TestOpenSeekableReadsNamedFile(t *testing.T) { require.NoError(t, err) assert.Equal(t, "payload", string(got)) - // A named file is opened directly, not copied through a spool. - assert.Equal(t, path, in.Name()) + assert.Equal(t, path, in.Name(), "a named file must not be copied through a spool") } func TestOpenSeekableReportsMissingFile(t *testing.T) { @@ -169,45 +447,52 @@ func TestOpenSeekableSpoolsNonSeekableNamedFile(t *testing.T) { require.NoError(t, err) assert.Equal(t, content, string(got)) - // A FIFO's own bytes were consumed into the spool; the caller's handle is a - // distinct, seekable temp file. - assert.NotEqual(t, path, in.Name()) + assert.NotEqual(t, path, in.Name(), "the FIFO's bytes went into a temp file") _, err = in.Seek(0, io.SeekStart) require.NoError(t, err, "the whole point of spooling is that the result seeks") } func TestOpenSeekableReadsFromStdinPipe(t *testing.T) { - r, w, err := os.Pipe() + const content = "piped stdin, spooled to disk" + stdinPipe(t, content) + + in, cleanup, err := OpenSeekable("") require.NoError(t, err) + defer cleanup() - origStdin := os.Stdin - os.Stdin = r - defer func() { os.Stdin = origStdin }() + got, err := io.ReadAll(in) + require.NoError(t, err) + assert.Equal(t, content, string(got)) - content := "piped stdin, spooled to disk" - go func() { - defer w.Close() - _, _ = io.WriteString(w, content) - }() + assert.NotSame(t, os.Stdin, in, "a real stream has to be spooled to seek") +} + +// A redirect from a regular file seeks already. Spooling it would copy the whole +// TDF into TMPDIR to produce a handle no better than the one we had. +func TestOpenSeekableDoesNotSpoolARegularFileStdin(t *testing.T) { + const content = "redirected stdin, read in place\n" + + useStdin(t, openRegularStdin(t, content)) in, cleanup, err := OpenSeekable("") require.NoError(t, err) defer cleanup() + assert.Same(t, os.Stdin, in, "a regular-file redirect must not be spooled") + got, err := io.ReadAll(in) require.NoError(t, err) assert.Equal(t, content, string(got)) + + // Closing stdin is not ours to do, so cleanup has to leave it usable. + cleanup() + _, err = in.Seek(0, io.SeekStart) + require.NoError(t, err, "cleanup must not close stdin") } func TestOpenSeekableReturnsErrNoInputForEmptyStdinPipe(t *testing.T) { - r, w, err := os.Pipe() - require.NoError(t, err) - require.NoError(t, w.Close()) - - origStdin := os.Stdin - os.Stdin = r - defer func() { os.Stdin = origStdin }() + stdinPipe(t, "") - _, _, err = OpenSeekable("") + _, _, err := OpenSeekable("") require.ErrorIs(t, err, ErrNoInput) }