From bbae6dafbd1647569007dee33878ca7944046f3c Mon Sep 17 00:00:00 2001
From: Maksim An <maksiman@microsoft.com>
Date: Wed, 13 Jan 2021 16:30:11 -0800
Subject: [PATCH] Add better handling of windows-style paths for io_binary

Signed-off-by: Maksim An <maksiman@microsoft.com>
---
 internal/cmd/io_binary.go      | 30 +++++++++++++++++++++++-------
 internal/cmd/io_binary_test.go | 33 ++++++++++++++++++++++++---------
 2 files changed, 47 insertions(+), 16 deletions(-)

diff --git a/internal/cmd/io_binary.go b/internal/cmd/io_binary.go
index 1283a2a1cc..1fde77e482 100644
--- a/internal/cmd/io_binary.go
+++ b/internal/cmd/io_binary.go
@@ -7,12 +7,15 @@ import (
 	"net"
 	"net/url"
 	"os/exec"
+	"path/filepath"
+	"strings"
 	"sync"
 	"time"
 
 	"github.com/Microsoft/go-winio"
 	"github.com/containerd/containerd/namespaces"
 	"github.com/pkg/errors"
+	"github.com/sirupsen/logrus"
 
 	"github.com/Microsoft/hcsshim/internal/log"
 )
@@ -96,6 +99,13 @@ func NewBinaryIO(ctx context.Context, id string, uri *url.URL) (_ UpstreamIO, er
 		return nil, errors.New("failed to start binary logger: timeout")
 	}
 
+	log.G(ctx).WithFields(logrus.Fields{
+		"containerID":        id,
+		"containerNamespace": ns,
+		"binaryCmd":          cmd,
+		"binaryProcessID":    cmd.Process.Pid,
+	}).Debug("binary io process started")
+
 	return &binaryIO{
 		cmd:    cmd,
 		stdout: stdoutPipePath,
@@ -105,8 +115,19 @@ func NewBinaryIO(ctx context.Context, id string, uri *url.URL) (_ UpstreamIO, er
 	}, nil
 }
 
+// sanitizePath parses the URL object and returns a clean path to the logging driver
+func sanitizePath(uri *url.URL) string {
+	path := filepath.Clean(uri.Path)
+
+	if strings.Contains(path, `:\`) {
+		return strings.TrimPrefix(path, "\\")
+	}
+
+	return path
+}
+
 func newBinaryCmd(ctx context.Context, uri *url.URL, envs []string) (*exec.Cmd, error) {
-	if uri.Host == "" && uri.Path == "" {
+	if uri.Path == "" {
 		return nil, errors.New("no logging driver path provided")
 	}
 
@@ -118,12 +139,7 @@ func newBinaryCmd(ctx context.Context, uri *url.URL, envs []string) (*exec.Cmd,
 		}
 	}
 
-	execPath := uri.Path
-	// Absolute path is required, treat "binary://path/to/binary" and "binary:///path/to/binary"
-	// as the same.
-	if uri.Host != "" {
-		execPath = "/" + uri.Host + uri.Path
-	}
+	execPath := sanitizePath(uri)
 
 	cmd := exec.CommandContext(ctx, execPath, args...)
 	cmd.Env = append(cmd.Env, envs...)
diff --git a/internal/cmd/io_binary_test.go b/internal/cmd/io_binary_test.go
index 8c7d384700..7ff5b24176 100644
--- a/internal/cmd/io_binary_test.go
+++ b/internal/cmd/io_binary_test.go
@@ -18,19 +18,34 @@ func Test_newBinaryCmd_Key_Value_Pair(t *testing.T) {
 
 	tests := []*config{
 		{
-			name:      "use-path",
+			name:      "Path_With_Fwd_Slashes",
 			urlString: "binary:///executable?-key=value",
-			expected:  "/executable -key value",
+			expected:  `\executable -key value`,
 		},
 		{
-			name:      "use-host",
-			urlString: "binary://executable?-key=value",
-			expected:  "/executable -key value",
+			name:      "Path_With_Back_Slashes",
+			urlString: `binary:///\executable?-key=value`,
+			expected:  `\executable -key value`,
 		},
 		{
-			name:      "use-host-and-path",
-			urlString: "binary://path/to/executable?flag",
-			expected:  "/path/to/executable flag",
+			name:      "Clean_Path_With_Dots_And_Multiple_Fwd_Slashes",
+			urlString: "binary:///../path/to///to/../executable",
+			expected:  `\path\to\executable`,
+		},
+		{
+			name:      "Clean_Path_With_Dots_And_Multiple_Back_Slashes",
+			urlString: `binary:///..\path\to\\\from\..\executable`,
+			expected:  `\path\to\executable`,
+		},
+		{
+			name:      "Stripped_Path_With_Forward_Slashes",
+			urlString: "binary:///D:/path/to/executable",
+			expected:  `D:\path\to\executable`,
+		},
+		{
+			name:      "Stripped_Path_With_Back_Slashes",
+			urlString: `binary:///D:\path\to\executable`,
+			expected:  `D:\path\to\executable`,
 		},
 	}
 
@@ -77,7 +92,7 @@ func Test_newBinaryCmd_flags(t *testing.T) {
 	urlString := "schema:///path/to/binary?foo&bar&baz"
 	uri, _ := url.Parse(urlString)
 
-	expectedPath := "/path/to/binary"
+	expectedPath := `\path\to\binary`
 	expectedFlags := map[string]bool{"foo": true, "bar": true, "baz": true}
 
 	cmd, err := newBinaryCmd(ctx, uri, nil)