Fix bugs in BinaryIO creator

mxpv · mxpv · commit 5e0d793801ae · 2019-06-19T11:15:17.000-07:00
Signed-off-by: Maksym Pavlenko &lt;makpav@amazon.com&gt;
diff --git a/cio/io.go b/cio/io.go
@@ -18,10 +18,13 @@ package cio
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"io"
 	"net/url"
 	"os"
+	"path/filepath"
+	"strings"
 	"sync"
 
 	"github.com/containerd/containerd/defaults"
@@ -242,13 +245,19 @@ func LogURI(uri *url.URL) Creator {
 // BinaryIO forwards container STDOUT|STDERR directly to a logging binary
 func BinaryIO(binary string, args map[string]string) Creator {
 	return func(_ string) (IO, error) {
+		binary = filepath.Clean(binary)
+		if !strings.HasPrefix(binary, "/") {
+			return nil, errors.New("absolute path needed")
+		}
 		uri := &url.URL{
 			Scheme: "binary",
-			Host:   binary,
+			Path:   binary,
 		}
+		q := uri.Query()
 		for k, v := range args {
-			uri.Query().Set(k, v)
+			q.Set(k, v)
 		}
+		uri.RawQuery = q.Encode()
 		return &logURI{
 			config: Config{
 				Stdout: uri.String(),
diff --git a/cio/io_test.go b/cio/io_test.go
@@ -23,6 +23,7 @@ import (
 	"context"
 	"io"
 	"io/ioutil"
+	"net/url"
 	"os"
 	"path/filepath"
 	"runtime"
@@ -153,3 +154,26 @@ func initProducers(t *testing.T, producers producers, stdout, stderr string) {
 	assert.NilError(t, err)
 	assert.NilError(t, producers.Stderr.Close())
 }
+
+func TestBinaryIOArgs(t *testing.T) {
+	res, err := BinaryIO("/file.bin", map[string]string{"id": "1"})("")
+	assert.NilError(t, err)
+	assert.Equal(t, "binary:///file.bin?id=1", res.Config().Stdout)
+	assert.Equal(t, "binary:///file.bin?id=1", res.Config().Stderr)
+}
+
+func TestBinaryIOAbsolutePath(t *testing.T) {
+	res, err := BinaryIO("/full/path/bin", nil)("!")
+	assert.NilError(t, err)
+
+	// Test parse back
+	parsed, err := url.Parse(res.Config().Stdout)
+	assert.NilError(t, err)
+	assert.Equal(t, "binary", parsed.Scheme)
+	assert.Equal(t, "/full/path/bin", parsed.Path)
+}
+
+func TestBinaryIOFailOnRelativePath(t *testing.T) {
+	_, err := BinaryIO("./bin", nil)("!")
+	assert.Error(t, err, "absolute path needed")
+}
diff --git a/runtime/v1/linux/proc/io.go b/runtime/v1/linux/proc/io.go
@@ -265,7 +265,7 @@ func NewBinaryIO(ctx context.Context, id string, uri *url.URL) (runc.IO, error)
 		}
 	}
 	ctx, cancel := context.WithCancel(ctx)
-	cmd := exec.CommandContext(ctx, uri.Host, args...)
+	cmd := exec.CommandContext(ctx, uri.Path, args...)
 	cmd.Env = append(cmd.Env,
 		"CONTAINER_ID="+id,
 		"CONTAINER_NAMESPACE="+ns,

Original file line number	Diff line number	Diff line change
`@@ -265,7 +265,7 @@ func NewBinaryIO(ctx context.Context, id string, uri *url.URL) (runc.IO, error)`
`265`	`265`	`}`
`266`	`266`	`}`
`267`	`267`	`ctx, cancel := context.WithCancel(ctx)`
`268`		`- cmd := exec.CommandContext(ctx, uri.Host, args...)`
	`268`	`+ cmd := exec.CommandContext(ctx, uri.Path, args...)`
`269`	`269`	`cmd.Env = append(cmd.Env,`
`270`	`270`	`"CONTAINER_ID="+id,`
`271`	`271`	`"CONTAINER_NAMESPACE="+ns,`