fix: allow attaching to any combination of stdin/stdout/stderr

Before this PR, if a stdin/stdout/stderr stream is nil,
and the corresponding FIFO is not an empty string,
a panic will occur when Read/Write of the nil stream is invoked in io.CopyBuffer.

Signed-off-by: Hsing-Yu (David) Chen <davidhsingyuchen@gmail.com>

cio/io.go

@@ -166,6 +166,15 @@ func NewAttach(opts ...Opt) Attach {
 		if fifos == nil {
 			return nil, fmt.Errorf("cannot attach, missing fifos")
 		}
+		if streams.Stdin == nil {
+			fifos.Stdin = ""
+		}
+		if streams.Stdout == nil {
+			fifos.Stdout = ""
+		}
+		if streams.Stderr == nil {
+			fifos.Stderr = ""
+		}
 		return copyIO(fifos, streams)
 	}
 }

cio/io_unix_test.go

@@ -138,40 +138,88 @@ func TestNewFIFOSetInDir(t *testing.T) {
 }
 
 func TestNewAttach(t *testing.T) {
-	var (
+	testCases := []struct {
-		expectedStdin  = "this is the stdin"
+		name                                          string
-		expectedStdout = "this is the stdout"
+		expectedStdin, expectedStdout, expectedStderr string
-		expectedStderr = "this is the stderr"
+	}{
-		stdin          = bytes.NewBufferString(expectedStdin)
+		{
-		stdout         = new(bytes.Buffer)
+			name:           "attach to all streams (stdin, stdout, and stderr)",
-		stderr         = new(bytes.Buffer)
+			expectedStdin:  "this is the stdin",
-	)
+			expectedStdout: "this is the stdout",
-
+			expectedStderr: "this is the stderr",
-	withBytesBuffers := func(streams *Streams) {
+		},
-		*streams = Streams{Stdin: stdin, Stdout: stdout, Stderr: stderr}
+		{
+			name:           "don't attach to stdin",
+			expectedStdout: "this is the stdout",
+			expectedStderr: "this is the stderr",
+		},
+		{
+			name:           "don't attach to stdout",
+			expectedStdin:  "this is the stdin",
+			expectedStderr: "this is the stderr",
+		},
+		{
+			name:           "don't attach to stderr",
+			expectedStdin:  "this is the stdin",
+			expectedStdout: "this is the stdout",
+		},
 	}
-	attacher := NewAttach(withBytesBuffers)
 
-	fifos, err := NewFIFOSetInDir("", "theid", false)
+	for _, tc := range testCases {
-	assert.NoError(t, err)
+		t.Run(tc.name, func(t *testing.T) {
+			var (
+				stdin  = bytes.NewBufferString(tc.expectedStdin)
+				stdout = new(bytes.Buffer)
+				stderr = new(bytes.Buffer)
 
-	attachedFifos, err := attacher(fifos)
+				// The variables below have to be of the interface type (i.e., io.Reader/io.Writer)
-	assert.NoError(t, err)
+				// instead of the concrete type (i.e., *bytes.Buffer) *before* being passed to NewAttach.
-	defer attachedFifos.Close()
+				// Otherwise, in NewAttach, the interface value won't be nil
+				// (it's just that the concrete value inside the interface itself is nil. [1]),
+				// which means that the corresponding FIFO path won't be set to be an empty string,
+				// and that's not what we want.
+				//
+				// [1] https://go.dev/tour/methods/12
+				stdinArg             io.Reader
+				stdoutArg, stderrArg io.Writer
+			)
+			if tc.expectedStdin != "" {
+				stdinArg = stdin
+			}
+			if tc.expectedStdout != "" {
+				stdoutArg = stdout
+			}
+			if tc.expectedStderr != "" {
+				stderrArg = stderr
+			}
 
-	producers := setupFIFOProducers(t, attachedFifos.Config())
+			attacher := NewAttach(WithStreams(stdinArg, stdoutArg, stderrArg))
-	initProducers(t, producers, expectedStdout, expectedStderr)
 
-	actualStdin, err := io.ReadAll(producers.Stdin)
+			fifos, err := NewFIFOSetInDir("", "theid", false)
-	assert.NoError(t, err)
+			assert.NoError(t, err)
 
-	attachedFifos.Wait()
+			attachedFifos, err := attacher(fifos)
-	attachedFifos.Cancel()
+			assert.NoError(t, err)
-	assert.Nil(t, attachedFifos.Close())
+			defer attachedFifos.Close()
 
-	assert.Equal(t, expectedStdout, stdout.String())
+			producers := setupFIFOProducers(t, attachedFifos.Config())
-	assert.Equal(t, expectedStderr, stderr.String())
+			initProducers(t, producers, tc.expectedStdout, tc.expectedStderr)
-	assert.Equal(t, expectedStdin, string(actualStdin))
+
+			var actualStdin []byte
+			if producers.Stdin != nil {
+				actualStdin, err = io.ReadAll(producers.Stdin)
+				assert.NoError(t, err)
+			}
+
+			attachedFifos.Wait()
+			attachedFifos.Cancel()
+			assert.Nil(t, attachedFifos.Close())
+
+			assert.Equal(t, tc.expectedStdout, stdout.String())
+			assert.Equal(t, tc.expectedStderr, stderr.String())
+			assert.Equal(t, tc.expectedStdin, string(actualStdin))
+		})
+	}
 }
 
 type producers struct {
@@ -187,26 +235,36 @@ func setupFIFOProducers(t *testing.T, fifos Config) producers {
 		ctx   = context.Background()
 	)
 
-	pipes.Stdin, err = fifo.OpenFifo(ctx, fifos.Stdin, syscall.O_RDONLY, 0)
+	if fifos.Stdin != "" {
-	assert.NoError(t, err)
+		pipes.Stdin, err = fifo.OpenFifo(ctx, fifos.Stdin, syscall.O_RDONLY, 0)
+		assert.NoError(t, err)
+	}
 
-	pipes.Stdout, err = fifo.OpenFifo(ctx, fifos.Stdout, syscall.O_WRONLY, 0)
+	if fifos.Stdout != "" {
-	assert.NoError(t, err)
+		pipes.Stdout, err = fifo.OpenFifo(ctx, fifos.Stdout, syscall.O_WRONLY, 0)
+		assert.NoError(t, err)
+	}
 
-	pipes.Stderr, err = fifo.OpenFifo(ctx, fifos.Stderr, syscall.O_WRONLY, 0)
+	if fifos.Stderr != "" {
-	assert.NoError(t, err)
+		pipes.Stderr, err = fifo.OpenFifo(ctx, fifos.Stderr, syscall.O_WRONLY, 0)
+		assert.NoError(t, err)
+	}
 
 	return pipes
 }
 
 func initProducers(t *testing.T, producers producers, stdout, stderr string) {
-	_, err := producers.Stdout.Write([]byte(stdout))
+	if producers.Stdout != nil {
-	assert.NoError(t, err)
+		_, err := producers.Stdout.Write([]byte(stdout))
-	assert.Nil(t, producers.Stdout.Close())
+		assert.NoError(t, err)
+		assert.Nil(t, producers.Stdout.Close())
+	}
 
-	_, err = producers.Stderr.Write([]byte(stderr))
+	if producers.Stderr != nil {
-	assert.NoError(t, err)
+		_, err := producers.Stderr.Write([]byte(stderr))
-	assert.Nil(t, producers.Stderr.Close())
+		assert.NoError(t, err)
+		assert.Nil(t, producers.Stderr.Close())
+	}
 }
 
 func TestLogURIGenerator(t *testing.T) {