Skip to content

Commit 34a5d03

Browse files
davidhsingyuchendavidhsingyuchen
committed
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 <[email protected]> (cherry picked from commit 687a5f5) Signed-off-by: Hsing-Yu (David) Chen <[email protected]>
1 parent 98e8e05 commit 34a5d03

2 files changed

Lines changed: 111 additions & 44 deletions

File tree

‎cio/io.go‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,15 @@ func NewAttach(opts ...Opt) Attach {
166166
if fifos == nil {
167167
return nil, fmt.Errorf("cannot attach, missing fifos")
168168
}
169+
if streams.Stdin == nil {
170+
fifos.Stdin = ""
171+
}
172+
if streams.Stdout == nil {
173+
fifos.Stdout = ""
174+
}
175+
if streams.Stderr == nil {
176+
fifos.Stderr = ""
177+
}
169178
return copyIO(fifos, streams)
170179
}
171180
}

‎cio/io_unix_test.go‎

Lines changed: 102 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -138,40 +138,88 @@ func TestNewFIFOSetInDir(t *testing.T) {
138138
}
139139

140140
func TestNewAttach(t *testing.T) {
141-
var (
142-
expectedStdin = "this is the stdin"
143-
expectedStdout = "this is the stdout"
144-
expectedStderr = "this is the stderr"
145-
stdin = bytes.NewBufferString(expectedStdin)
146-
stdout = new(bytes.Buffer)
147-
stderr = new(bytes.Buffer)
148-
)
149-
150-
withBytesBuffers := func(streams *Streams) {
151-
*streams = Streams{Stdin: stdin, Stdout: stdout, Stderr: stderr}
141+
testCases := []struct {
142+
name string
143+
expectedStdin, expectedStdout, expectedStderr string
144+
}{
145+
{
146+
name: "attach to all streams (stdin, stdout, and stderr)",
147+
expectedStdin: "this is the stdin",
148+
expectedStdout: "this is the stdout",
149+
expectedStderr: "this is the stderr",
150+
},
151+
{
152+
name: "don't attach to stdin",
153+
expectedStdout: "this is the stdout",
154+
expectedStderr: "this is the stderr",
155+
},
156+
{
157+
name: "don't attach to stdout",
158+
expectedStdin: "this is the stdin",
159+
expectedStderr: "this is the stderr",
160+
},
161+
{
162+
name: "don't attach to stderr",
163+
expectedStdin: "this is the stdin",
164+
expectedStdout: "this is the stdout",
165+
},
152166
}
153-
attacher := NewAttach(withBytesBuffers)
154-
155-
fifos, err := NewFIFOSetInDir("", "theid", false)
156-
assert.NoError(t, err)
157-
158-
attachedFifos, err := attacher(fifos)
159-
assert.NoError(t, err)
160-
defer attachedFifos.Close()
161167

162-
producers := setupFIFOProducers(t, attachedFifos.Config())
163-
initProducers(t, producers, expectedStdout, expectedStderr)
164-
165-
actualStdin, err := io.ReadAll(producers.Stdin)
166-
assert.NoError(t, err)
167-
168-
attachedFifos.Wait()
169-
attachedFifos.Cancel()
170-
assert.Nil(t, attachedFifos.Close())
171-
172-
assert.Equal(t, expectedStdout, stdout.String())
173-
assert.Equal(t, expectedStderr, stderr.String())
174-
assert.Equal(t, expectedStdin, string(actualStdin))
168+
for _, tc := range testCases {
169+
t.Run(tc.name, func(t *testing.T) {
170+
var (
171+
stdin = bytes.NewBufferString(tc.expectedStdin)
172+
stdout = new(bytes.Buffer)
173+
stderr = new(bytes.Buffer)
174+
175+
// The variables below have to be of the interface type (i.e., io.Reader/io.Writer)
176+
// instead of the concrete type (i.e., *bytes.Buffer) *before* being passed to NewAttach.
177+
// Otherwise, in NewAttach, the interface value won't be nil
178+
// (it's just that the concrete value inside the interface itself is nil. [1]),
179+
// which means that the corresponding FIFO path won't be set to be an empty string,
180+
// and that's not what we want.
181+
//
182+
// [1] https://go.dev/tour/methods/12
183+
stdinArg io.Reader
184+
stdoutArg, stderrArg io.Writer
185+
)
186+
if tc.expectedStdin != "" {
187+
stdinArg = stdin
188+
}
189+
if tc.expectedStdout != "" {
190+
stdoutArg = stdout
191+
}
192+
if tc.expectedStderr != "" {
193+
stderrArg = stderr
194+
}
195+
196+
attacher := NewAttach(WithStreams(stdinArg, stdoutArg, stderrArg))
197+
198+
fifos, err := NewFIFOSetInDir("", "theid", false)
199+
assert.NoError(t, err)
200+
201+
attachedFifos, err := attacher(fifos)
202+
assert.NoError(t, err)
203+
defer attachedFifos.Close()
204+
205+
producers := setupFIFOProducers(t, attachedFifos.Config())
206+
initProducers(t, producers, tc.expectedStdout, tc.expectedStderr)
207+
208+
var actualStdin []byte
209+
if producers.Stdin != nil {
210+
actualStdin, err = io.ReadAll(producers.Stdin)
211+
assert.NoError(t, err)
212+
}
213+
214+
attachedFifos.Wait()
215+
attachedFifos.Cancel()
216+
assert.Nil(t, attachedFifos.Close())
217+
218+
assert.Equal(t, tc.expectedStdout, stdout.String())
219+
assert.Equal(t, tc.expectedStderr, stderr.String())
220+
assert.Equal(t, tc.expectedStdin, string(actualStdin))
221+
})
222+
}
175223
}
176224

177225
type producers struct {
@@ -187,26 +235,36 @@ func setupFIFOProducers(t *testing.T, fifos Config) producers {
187235
ctx = context.Background()
188236
)
189237

190-
pipes.Stdin, err = fifo.OpenFifo(ctx, fifos.Stdin, syscall.O_RDONLY, 0)
191-
assert.NoError(t, err)
238+
if fifos.Stdin != "" {
239+
pipes.Stdin, err = fifo.OpenFifo(ctx, fifos.Stdin, syscall.O_RDONLY, 0)
240+
assert.NoError(t, err)
241+
}
192242

193-
pipes.Stdout, err = fifo.OpenFifo(ctx, fifos.Stdout, syscall.O_WRONLY, 0)
194-
assert.NoError(t, err)
243+
if fifos.Stdout != "" {
244+
pipes.Stdout, err = fifo.OpenFifo(ctx, fifos.Stdout, syscall.O_WRONLY, 0)
245+
assert.NoError(t, err)
246+
}
195247

196-
pipes.Stderr, err = fifo.OpenFifo(ctx, fifos.Stderr, syscall.O_WRONLY, 0)
197-
assert.NoError(t, err)
248+
if fifos.Stderr != "" {
249+
pipes.Stderr, err = fifo.OpenFifo(ctx, fifos.Stderr, syscall.O_WRONLY, 0)
250+
assert.NoError(t, err)
251+
}
198252

199253
return pipes
200254
}
201255

202256
func initProducers(t *testing.T, producers producers, stdout, stderr string) {
203-
_, err := producers.Stdout.Write([]byte(stdout))
204-
assert.NoError(t, err)
205-
assert.Nil(t, producers.Stdout.Close())
257+
if producers.Stdout != nil {
258+
_, err := producers.Stdout.Write([]byte(stdout))
259+
assert.NoError(t, err)
260+
assert.Nil(t, producers.Stdout.Close())
261+
}
206262

207-
_, err = producers.Stderr.Write([]byte(stderr))
208-
assert.NoError(t, err)
209-
assert.Nil(t, producers.Stderr.Close())
263+
if producers.Stderr != nil {
264+
_, err := producers.Stderr.Write([]byte(stderr))
265+
assert.NoError(t, err)
266+
assert.Nil(t, producers.Stderr.Close())
267+
}
210268
}
211269

212270
func TestLogURIGenerator(t *testing.T) {

0 commit comments

Comments
 (0)