Update locator functions to handle Windows file names

Signed-off-by: Jan Dubois <[email protected]>

Author: jandubois

pkg/limatmpl/abs.go

@@ -4,7 +4,9 @@ import (
 	"errors"
 	"fmt"
 	"net/url"
+	"path"
 	"path/filepath"
+	"runtime"
 	"strings"
 )
 
@@ -57,33 +59,62 @@ func (tmpl *Template) useAbsLocators() error {
 	return tmpl.evalExpr()
 }
 
-// basePath returns the locator without the filename part.
+// withVolume adds the volume name of the current working directory to a path without volume name.
+// On Windows filepath.Abs() only returns a "rooted" name, but does not add the volume name.
+// withVolume also normalizes all path separators to the platform native one.
+func withVolume(path string) (string, error) {
+	if runtime.GOOS == "windows" && len(filepath.VolumeName(path)) == 0 {
+		root, err := filepath.Abs("/")
+		if err != nil {
+			return "", err
+		}
+		path = filepath.VolumeName(root) + path
+	}
+	return filepath.Clean(path), nil
+}
+
+// basePath returns the locator in absolute format, but without the filename part.
 func basePath(locator string) (string, error) {
 	u, err := url.Parse(locator)
-	if err != nil || u.Scheme == "" {
-		return filepath.Abs(filepath.Dir(locator))
+	// Single-letter schemes will be drive names on Windows, e.g. "c:/foo"
+	if err == nil && len(u.Scheme) > 1 {
+		// path.Dir("") returns ".", which must be removed for url.JoinPath() to do the right thing later
+		return u.Scheme + "://" + strings.TrimSuffix(path.Dir(path.Join(u.Host, u.Path)), "."), nil
+	}
+	base, err := filepath.Abs(filepath.Dir(locator))
+	if err != nil {
+		return "", err
 	}
-	// filepath.Dir("") returns ".", which must be removed for url.JoinPath() to do the right thing later
-	return u.Scheme + "://" + strings.TrimSuffix(filepath.Dir(filepath.Join(u.Host, u.Path)), "."), nil
+	return withVolume(base)
 }
 
 // absPath either returns the locator directly, or combines it with the basePath if the locator is a relative path.
 func absPath(locator, basePath string) (string, error) {
 	u, err := url.Parse(locator)
-	if (err == nil && u.Scheme != "") || filepath.IsAbs(locator) {
+	if err == nil && len(u.Scheme) > 1 {
 		return locator, nil
 	}
-	switch {
-	case basePath == "":
-		return "", errors.New("basePath is empty")
-	case basePath == "-":
-		return "", errors.New("can't use relative paths when reading template from STDIN")
-	case strings.Contains(locator, "../"):
-		return "", fmt.Errorf("relative locator path %q must not contain '../' segments", locator)
-	}
-	u, err = url.Parse(basePath)
-	if err != nil {
-		return "", err
+	// Check for rooted locator; filepath.IsAbs() returns false on Windows when the volume name is missing
+	volumeLen := len(filepath.VolumeName(locator))
+	if locator[volumeLen] != '/' && locator[volumeLen] != filepath.Separator {
+		switch {
+		case basePath == "":
+			return "", errors.New("basePath is empty")
+		case basePath == "-":
+			return "", errors.New("can't use relative paths when reading template from STDIN")
+		case strings.Contains(locator, "../"):
+			return "", fmt.Errorf("relative locator path %q must not contain '../' segments", locator)
+		case volumeLen != 0:
+			return "", fmt.Errorf("relative locator path %q must not include a volume name", locator)
+		}
+		u, err = url.Parse(basePath)
+		if err != nil {
+			return "", err
+		}
+		if len(u.Scheme) > 1 {
+			return u.JoinPath(locator).String(), nil
+		}
+		locator = filepath.Join(basePath, locator)
 	}
-	return u.JoinPath(locator).String(), nil
+	return withVolume(locator)
 }

pkg/limatmpl/abs_test.go

@@ -1,6 +1,8 @@
 package limatmpl
 
 import (
+	"path/filepath"
+	"runtime"
 	"strings"
 	"testing"
 
@@ -18,8 +20,8 @@ var useAbsLocatorsTestCases = []useAbsLocatorsTestCase{
 	{
 		"Template without basedOn or script file",
 		"template://foo",
-		`foo: bar`,
-		`foo: bar`,
+		`arch: aarch64`,
+		`arch: aarch64`,
 	},
 	{
 		"Single string base template",
@@ -102,98 +104,161 @@ func RunUseAbsLocatorTest(t *testing.T, tc useAbsLocatorsTestCase) {
 }
 
 func TestBasePath(t *testing.T) {
-	actual, err := basePath("/foo")
+	// On Windows the root will be something like "C:\"
+	root, err := filepath.Abs("/")
 	assert.NilError(t, err)
-	assert.Equal(t, actual, "/")
+	volume := filepath.VolumeName(root)
 
-	actual, err = basePath("/foo/bar")
-	assert.NilError(t, err)
-	assert.Equal(t, actual, "/foo")
+	t.Run("", func(t *testing.T) {
+		actual, err := basePath("/foo")
+		assert.NilError(t, err)
+		assert.Equal(t, actual, root)
+	})
 
-	actual, err = basePath("template://foo")
-	assert.NilError(t, err)
-	assert.Equal(t, actual, "template://")
+	t.Run("", func(t *testing.T) {
+		actual, err := basePath("/foo/bar")
+		assert.NilError(t, err)
+		assert.Equal(t, actual, filepath.Clean(volume+"/foo"))
+	})
 
-	actual, err = basePath("template://foo/bar")
-	assert.NilError(t, err)
-	assert.Equal(t, actual, "template://foo")
+	t.Run("", func(t *testing.T) {
+		actual, err := basePath("template://foo")
+		assert.NilError(t, err)
+		assert.Equal(t, actual, "template://")
+	})
 
-	actual, err = basePath("http://host/foo")
-	assert.NilError(t, err)
-	assert.Equal(t, actual, "http://host")
+	t.Run("", func(t *testing.T) {
+		actual, err := basePath("template://foo/bar")
+		assert.NilError(t, err)
+		assert.Equal(t, actual, "template://foo")
+	})
 
-	actual, err = basePath("http://host/foo/bar")
-	assert.NilError(t, err)
-	assert.Equal(t, actual, "http://host/foo")
+	t.Run("", func(t *testing.T) {
+		actual, err := basePath("http://host/foo")
+		assert.NilError(t, err)
+		assert.Equal(t, actual, "http://host")
+	})
 
-	actual, err = basePath("file:///foo")
-	assert.NilError(t, err)
-	assert.Equal(t, actual, "file:///")
+	t.Run("", func(t *testing.T) {
+		actual, err := basePath("http://host/foo/bar")
+		assert.NilError(t, err)
+		assert.Equal(t, actual, "http://host/foo")
+	})
 
-	actual, err = basePath("file:///foo/bar")
-	assert.NilError(t, err)
-	assert.Equal(t, actual, "file:///foo")
+	t.Run("", func(t *testing.T) {
+		actual, err := basePath("file:///foo")
+		assert.NilError(t, err)
+		assert.Equal(t, actual, "file:///")
+	})
+
+	t.Run("", func(t *testing.T) {
+		actual, err := basePath("file:///foo/bar")
+		assert.NilError(t, err)
+		assert.Equal(t, actual, "file:///foo")
+	})
 }
 
 func TestAbsPath(t *testing.T) {
-	// If the locator is already an absolute path, it is returned unchanged (no extension appended either)
-	actual, err := absPath("/foo", "/root")
+	root, err := filepath.Abs("/")
 	assert.NilError(t, err)
-	assert.Equal(t, actual, "/foo")
+	volume := filepath.VolumeName(root)
 
-	actual, err = absPath("template://foo", "/root")
-	assert.NilError(t, err)
-	assert.Equal(t, actual, "template://foo")
+	t.Run("If the locator is already an absolute path, it is returned unchanged", func(t *testing.T) {
+		actual, err := absPath(volume+"/foo", volume+"/root")
+		assert.NilError(t, err)
+		assert.Equal(t, actual, filepath.Clean(volume+"/foo"))
+	})
 
-	actual, err = absPath("http://host/foo", "/root")
-	assert.NilError(t, err)
-	assert.Equal(t, actual, "http://host/foo")
+	t.Run("If the locator is a rooted path without volume name, then the volume will be added", func(t *testing.T) {
+		actual, err := absPath("/foo", volume+"/root")
+		assert.NilError(t, err)
+		assert.Equal(t, actual, filepath.Clean(volume+"/foo"))
+	})
 
-	actual, err = absPath("file:///foo", "/root")
-	assert.NilError(t, err)
-	assert.Equal(t, actual, "file:///foo")
+	t.Run("", func(t *testing.T) {
+		actual, err := absPath("template://foo", volume+"/root")
+		assert.NilError(t, err)
+		assert.Equal(t, actual, "template://foo")
+	})
 
-	// Can't have relative path when reading from STDIN
-	_, err = absPath("foo", "-")
-	assert.ErrorContains(t, err, "STDIN")
+	t.Run("", func(t *testing.T) {
+		actual, err := absPath("http://host/foo", volume+"/root")
+		assert.NilError(t, err)
+		assert.Equal(t, actual, "http://host/foo")
+	})
 
-	// Relative paths must be underneath the basePath
-	_, err = absPath("../foo", "/root")
-	assert.ErrorContains(t, err, "'../'")
+	t.Run("", func(t *testing.T) {
+		actual, err := absPath("file:///foo", volume+"/root")
+		assert.NilError(t, err)
+		assert.Equal(t, actual, "file:///foo")
+	})
 
-	// basePath must not be empty
-	_, err = absPath("foo", "")
-	assert.ErrorContains(t, err, "empty")
+	t.Run("Can't have relative path when reading from STDIN", func(t *testing.T) {
+		_, err = absPath("foo", "-")
+		assert.ErrorContains(t, err, "STDIN")
+	})
 
-	_, err = absPath("./foo", "")
-	assert.ErrorContains(t, err, "empty")
+	t.Run("Relative paths must be underneath the basePath", func(t *testing.T) {
+		_, err = absPath("../foo", volume+"/root")
+		assert.ErrorContains(t, err, "'../'")
+	})
 
-	// Check relative paths with all the supported schemes
-	actual, err = absPath("./foo", "/root")
-	assert.NilError(t, err)
-	assert.Equal(t, actual, "/root/foo")
+	t.Run("basePath must not be empty", func(t *testing.T) {
+		_, err = absPath("foo", "")
+		assert.ErrorContains(t, err, "empty")
+	})
 
-	actual, err = absPath("foo", "template://")
-	assert.NilError(t, err)
-	assert.Equal(t, actual, "template://foo")
+	t.Run("", func(t *testing.T) {
+		_, err = absPath("./foo", "")
+		assert.ErrorContains(t, err, "empty")
+	})
 
-	actual, err = absPath("bar", "template://foo")
-	assert.NilError(t, err)
-	assert.Equal(t, actual, "template://foo/bar")
+	t.Run("", func(t *testing.T) {
+		actual, err := absPath("./foo", volume+"/root")
+		assert.NilError(t, err)
+		assert.Equal(t, actual, filepath.Clean(volume+"/root/foo"))
+	})
 
-	actual, err = absPath("foo", "http://host")
-	assert.NilError(t, err)
-	assert.Equal(t, actual, "http://host/foo")
+	if runtime.GOOS == "windows" {
+		t.Run("Relative locators must not include volume names", func(t *testing.T) {
+			_, err := absPath(volume+"foo", volume+"/root")
+			assert.ErrorContains(t, err, "volume")
+		})
+	}
+	
+	t.Run("", func(t *testing.T) {
+		actual, err := absPath("foo", "template://")
+		assert.NilError(t, err)
+		assert.Equal(t, actual, "template://foo")
+	})
 
-	actual, err = absPath("bar", "http://host/foo")
-	assert.NilError(t, err)
-	assert.Equal(t, actual, "http://host/foo/bar")
+	t.Run("", func(t *testing.T) {
+		actual, err := absPath("bar", "template://foo")
+		assert.NilError(t, err)
+		assert.Equal(t, actual, "template://foo/bar")
+	})
 
-	actual, err = absPath("foo", "file:///")
-	assert.NilError(t, err)
-	assert.Equal(t, actual, "file:///foo")
+	t.Run("", func(t *testing.T) {
+		actual, err := absPath("foo", "http://host")
+		assert.NilError(t, err)
+		assert.Equal(t, actual, "http://host/foo")
+	})
 
-	actual, err = absPath("bar", "file:///foo")
-	assert.NilError(t, err)
-	assert.Equal(t, actual, "file:///foo/bar")
+	t.Run("", func(t *testing.T) {
+		actual, err := absPath("bar", "http://host/foo")
+		assert.NilError(t, err)
+		assert.Equal(t, actual, "http://host/foo/bar")
+	})
+
+	t.Run("", func(t *testing.T) {
+		actual, err := absPath("foo", "file:///")
+		assert.NilError(t, err)
+		assert.Equal(t, actual, "file:///foo")
+	})
+
+	t.Run("", func(t *testing.T) {
+		actual, err := absPath("bar", "file:///foo")
+		assert.NilError(t, err)
+		assert.Equal(t, actual, "file:///foo/bar")
+	})
 }

pkg/limatmpl/locator.go

@@ -153,13 +153,14 @@ func SeemsYAMLPath(arg string) bool {
 	return strings.HasSuffix(lower, ".yml") || strings.HasSuffix(lower, ".yaml")
 }
 
-// SeemsFilePath returns true if arg either contains a slash or has a file extension that
+// SeemsFilePath returns true if arg either contains a path separator or has a file extension that
 // does not start with a digit. `my.yaml` is a file path, `ubuntu-20.10` is not.
 func SeemsFilePath(arg string) bool {
-	if u, err := url.Parse(arg); err == nil && u.Scheme != "" {
+	// Single-letter schemes will be drive names on Windows, e.g. "c:/foo.yaml"
+	if u, err := url.Parse(arg); err == nil && len(u.Scheme) > 1 {
 		return false
 	}
-	if strings.Contains(arg, "/") {
+	if strings.ContainsRune(arg, '/') || strings.ContainsRune(arg, filepath.Separator) {
 		return true
 	}
 	ext := filepath.Ext(arg)