Fix various Windows-issues

File handling was broken on Windows. This commit contains a revision of the path handling with separation of file paths and urls where needed.

There may be remaining issues and there may be better ways to do this, but it is easier to start that refactoring job with a set of passing tests.

Fixes #687
Fixes #660

5 files changed, 76 insertions, 40 deletions

diff --git a/helpers/general.go b/helpers/general.go
index bfac5beae..cf3c0ac54 100644
--- a/helpers/general.go
+++ b/helpers/general.go
@@ -20,9 +20,12 @@ import (
 	"fmt"
 	"io"
 	"net"
+	"path/filepath"
 	"strings"
 )
 
+const FilePathSeparator = string(filepath.Separator)
+
 func FindAvailablePort() (*net.TCPAddr, error) {
 	l, err := net.Listen("tcp", ":0")
 	if err == nil {
diff --git a/helpers/path.go b/helpers/path.go
index e5f07450d..b65533d8a 100644
--- a/helpers/path.go
+++ b/helpers/path.go
@@ -16,15 +16,14 @@ package helpers
 import (
 	"errors"
 	"fmt"
+	"github.com/spf13/afero"
+	"github.com/spf13/viper"
 	"io"
 	"os"
 	"path/filepath"
 	"regexp"
 	"strings"
 	"unicode"
-
-	"github.com/spf13/afero"
-	"github.com/spf13/viper"
 )
 
 var sanitizeRegexp = regexp.MustCompile("[^a-zA-Z0-9./_-]")
@@ -173,13 +172,18 @@ func FileAndExt(in string) (name string, ext string) {
 	ext = filepath.Ext(in)
 	base := filepath.Base(in) // path.Base strips any trailing slash!
 
+	return FileAndExtSep(in, ext, base, FilePathSeparator), ext
+}
+
+func FileAndExtSep(in, ext, base, pathSeparator string) (name string) {
+
 	// No file name cases. These are defined as:
-	// 1. any "in" path that ends in a os.PathSeparator i.e. "/" on linux
-	// 2. any "base" consisting of just an os.PathSeparator
+	// 1. any "in" path that ends in a pathSeparator
+	// 2. any "base" consisting of just an pathSeparator
 	// 3. any "base" consisting of just an empty string
 	// 4. any "base" consisting of just the current directory i.e. "."
 	// 5. any "base" consisting of just the parent directory i.e. ".."
-	if (strings.LastIndex(in, string(os.PathSeparator)) == len(in)-1) || base == "" || base == "." || base == ".." || base == string(os.PathListSeparator) {
+	if (strings.LastIndex(in, pathSeparator) == len(in)-1) || base == "" || base == "." || base == ".." || base == pathSeparator {
 		name = "" // there is NO filename
 	} else if ext != "" { // there was an Extension
 		// return the filename minus the extension (and the ".")
@@ -190,6 +194,7 @@ func FileAndExt(in string) (name string, ext string) {
 		name = base
 	}
 	return
+
 }
 
 func GetRelativePath(path, base string) (final string, err error) {
@@ -203,14 +208,13 @@ func GetRelativePath(path, base string) (final string, err error) {
 	if err != nil {
 		return "", err
 	}
-	name = filepath.ToSlash(name)
 	return name, nil
 }
 
 // Given a source path, determine the section
 // A section is the part between the root slash and the second slash or before the first slash
 func GuessSection(in string) string {
-	parts := strings.Split(in, "/")
+	parts := strings.Split(in, FilePathSeparator)
 	// This will include an empty entry before and after paths with leading and trailing slashes
 	// eg... /sect/one/ -> ["", "sect", "one", ""]
 
@@ -256,7 +260,7 @@ func PrettifyPath(in string) string {
 	if filepath.Ext(in) == "" {
 		// /section/name/  -> /section/name/index.html
 		if len(in) < 2 {
-			return "/"
+			return FilePathSeparator
 		}
 		return filepath.Join(filepath.Clean(in), "index.html")
 	} else {
diff --git a/helpers/path_test.go b/helpers/path_test.go
index a7cca35bb..603723c27 100644
--- a/helpers/path_test.go
+++ b/helpers/path_test.go
@@ -4,7 +4,7 @@ import (
 	"fmt"
 	"io/ioutil"
 	"os"
-	"path"
+	"path/filepath"
 	"strconv"
 	"strings"
 	"testing"
@@ -119,7 +119,7 @@ func TestReplaceExtension(t *testing.T) {
 	}
 
 	for i, d := range data {
-		output := ReplaceExtension(d.input, d.newext)
+		output := ReplaceExtension(filepath.FromSlash(d.input), d.newext)
 		if d.expected != output {
 			t.Errorf("Test %d failed. Expected %q got %q", i, d.expected, output)
 		}
@@ -139,8 +139,8 @@ func TestDirExists(t *testing.T) {
 		{"../", true},
 		{"./..", true},
 		{"./../", true},
-		{"/tmp", true},
-		{"/tmp/", true},
+		{os.TempDir(), true},
+		{os.TempDir() + FilePathSeparator, true},
 		{"/", true},
 		{"/some-really-random-directory-name", false},
 		{"/some/really/random/directory/name", false},
@@ -149,7 +149,7 @@ func TestDirExists(t *testing.T) {
 	}
 
 	for i, d := range data {
-		exists, _ := DirExists(d.input, new(afero.OsFs))
+		exists, _ := DirExists(filepath.FromSlash(d.input), new(afero.OsFs))
 		if d.expected != exists {
 			t.Errorf("Test %d failed. Expected %t got %t", i, d.expected, exists)
 		}
@@ -366,8 +366,8 @@ func TestAbsPathify(t *testing.T) {
 		input, expected string
 	}
 	data := []test{
-		{os.TempDir(), path.Clean(os.TempDir())}, // TempDir has trailing slash
-		{"/banana/../dir/", "/dir"},
+		{os.TempDir(), filepath.Clean(os.TempDir())}, // TempDir has trailing slash
+		{filepath.FromSlash("/banana/../dir/"), filepath.FromSlash("/dir")},
 	}
 
 	for i, d := range data {
@@ -400,7 +400,7 @@ func TestFilename(t *testing.T) {
 	}
 
 	for i, d := range data {
-		output := Filename(d.input)
+		output := Filename(filepath.FromSlash(d.input))
 		if d.expected != output {
 			t.Errorf("Test %d failed. Expected %q got %q", i, d.expected, output)
 		}
@@ -429,7 +429,7 @@ func TestFileAndExt(t *testing.T) {
 	}
 
 	for i, d := range data {
-		file, ext := FileAndExt(d.input)
+		file, ext := FileAndExt(filepath.FromSlash(d.input))
 		if d.expectedFile != file {
 			t.Errorf("Test %d failed. Expected filename %q got %q.", i, d.expectedFile, file)
 		}
@@ -467,7 +467,7 @@ func TestGuessSection(t *testing.T) {
 	}
 
 	for i, d := range data {
-		expected := GuessSection(d.input)
+		expected := GuessSection(filepath.FromSlash(d.input))
 		if d.expected != expected {
 			t.Errorf("Test %d failed. Expected %q got %q", i, d.expected, expected)
 		}
diff --git a/helpers/url.go b/helpers/url.go
index dd8d7508b..a2dc0ac69 100644
--- a/helpers/url.go
+++ b/helpers/url.go
@@ -15,11 +15,10 @@ package helpers
 
 import (
 	"fmt"
+	"github.com/PuerkitoBio/purell"
 	"net/url"
-	"path/filepath"
+	"path"
 	"strings"
-
-	"github.com/PuerkitoBio/purell"
 )
 
 func SanitizeUrl(in string) string {
@@ -68,7 +67,7 @@ func MakePermalink(host, plink string) *url.URL {
 		panic(fmt.Errorf("Can't make permalink from absolute link %q", plink))
 	}
 
-	base.Path = filepath.Join(base.Path, p.Path)
+	base.Path = path.Join(base.Path, p.Path)
 
 	// path.Join will strip off the last /, so put it back if it was there.
 	if strings.HasSuffix(p.Path, "/") && !strings.HasSuffix(base.Path, "/") {
@@ -84,7 +83,7 @@ func UrlPrep(ugly bool, in string) string {
 		return x
 	} else {
 		x := PrettifyUrl(SanitizeUrl(in))
-		if filepath.Ext(x) == ".xml" {
+		if path.Ext(x) == ".xml" {
 			return x
 		}
 		url, err := purell.NormalizeURLString(x, purell.FlagAddTrailingSlash)
@@ -98,10 +97,10 @@ func UrlPrep(ugly bool, in string) string {
 
 // Don't Return /index.html portion.
 func PrettifyUrl(in string) string {
-	x := PrettifyPath(in)
+	x := PrettifyUrlPath(in)
 
-	if filepath.Base(x) == "index.html" {
-		return filepath.Dir(x)
+	if path.Base(x) == "index.html" {
+		return path.Dir(x)
 	}
 
 	if in == "" {
@@ -111,21 +110,43 @@ func PrettifyUrl(in string) string {
 	return x
 }
 
+// /section/name.html -> /section/name/index.html
+// /section/name/  -> /section/name/index.html
+// /section/name/index.html -> /section/name/index.html
+func PrettifyUrlPath(in string) string {
+	if path.Ext(in) == "" {
+		// /section/name/  -> /section/name/index.html
+		if len(in) < 2 {
+			return "/"
+		}
+		return path.Join(path.Clean(in), "index.html")
+	} else {
+		name, ext := ResourceAndExt(in)
+		if name == "index" {
+			// /section/name/index.html -> /section/name/index.html
+			return path.Clean(in)
+		} else {
+			// /section/name.html -> /section/name/index.html
+			return path.Join(path.Dir(in), name, "index"+ext)
+		}
+	}
+}
+
 // /section/name/index.html -> /section/name.html
 // /section/name/  -> /section/name.html
 // /section/name.html -> /section/name.html
 func Uglify(in string) string {
-	if filepath.Ext(in) == "" {
+	if path.Ext(in) == "" {
 		if len(in) < 2 {
 			return "/"
 		}
 		// /section/name/  -> /section/name.html
-		return filepath.Clean(in) + ".html"
+		return path.Clean(in) + ".html"
 	} else {
-		name, ext := FileAndExt(in)
+		name, ext := ResourceAndExt(in)
 		if name == "index" {
 			// /section/name/index.html -> /section/name.html
-			d := filepath.Dir(in)
+			d := path.Dir(in)
 			if len(d) > 1 {
 				return d + ext
 			} else {
@@ -133,7 +154,15 @@ func Uglify(in string) string {
 			}
 		} else {
 			// /section/name.html -> /section/name.html
-			return filepath.Clean(in)
+			return path.Clean(in)
 		}
 	}
 }
+
+// Same as FileAndExt, but for Urls
+func ResourceAndExt(in string) (name string, ext string) {
+	ext = path.Ext(in)
+	base := path.Base(in)
+
+	return FileAndExtSep(in, ext, base, "/"), ext
+}
diff --git a/helpers/url_test.go b/helpers/url_test.go
index 4014ee343..1d5c770ea 100644
--- a/helpers/url_test.go
+++ b/helpers/url_test.go
@@ -69,14 +69,14 @@ func TestUrlPrep(t *testing.T) {
 }
 
 func TestPretty(t *testing.T) {
-	assert.Equal(t, PrettifyPath("/section/name.html"), "/section/name/index.html")
-	assert.Equal(t, PrettifyPath("/section/sub/name.html"), "/section/sub/name/index.html")
-	assert.Equal(t, PrettifyPath("/section/name/"), "/section/name/index.html")
-	assert.Equal(t, PrettifyPath("/section/name/index.html"), "/section/name/index.html")
-	assert.Equal(t, PrettifyPath("/index.html"), "/index.html")
-	assert.Equal(t, PrettifyPath("/name.xml"), "/name/index.xml")
-	assert.Equal(t, PrettifyPath("/"), "/")
-	assert.Equal(t, PrettifyPath(""), "/")
+	assert.Equal(t, PrettifyUrlPath("/section/name.html"), "/section/name/index.html")
+	assert.Equal(t, PrettifyUrlPath("/section/sub/name.html"), "/section/sub/name/index.html")
+	assert.Equal(t, PrettifyUrlPath("/section/name/"), "/section/name/index.html")
+	assert.Equal(t, PrettifyUrlPath("/section/name/index.html"), "/section/name/index.html")
+	assert.Equal(t, PrettifyUrlPath("/index.html"), "/index.html")
+	assert.Equal(t, PrettifyUrlPath("/name.xml"), "/name/index.xml")
+	assert.Equal(t, PrettifyUrlPath("/"), "/")
+	assert.Equal(t, PrettifyUrlPath(""), "/")
 	assert.Equal(t, PrettifyUrl("/section/name.html"), "/section/name")
 	assert.Equal(t, PrettifyUrl("/section/sub/name.html"), "/section/sub/name")
 	assert.Equal(t, PrettifyUrl("/section/name/"), "/section/name")