Fix handling of taxonomy terms containing slashes

Fixes #4090

8 files changed, 119 insertions, 34 deletions

diff --git a/helpers/path.go b/helpers/path.go
index 3586c5744..7af56960c 100644
--- a/helpers/path.go
+++ b/helpers/path.go
@@ -74,6 +74,35 @@ func (filepathBridge) Separator() string {
 
 var fpb filepathBridge
 
+// segmentReplacer replaces some URI-reserved characters in a path segments.
+var segmentReplacer = strings.NewReplacer("/", "-", "#", "-")
+
+// MakeSegment returns a copy of string s that is appropriate for a path
+// segment.  MakeSegment is similar to MakePath but disallows the '/' and
+// '#' characters because of their reserved meaning in URIs.
+func (p *PathSpec) MakeSegment(s string) string {
+	s = p.MakePathSanitized(strings.Trim(segmentReplacer.Replace(s), "- "))
+
+	var pos int
+	var last byte
+	b := make([]byte, len(s))
+
+	for i := 0; i < len(s); i++ {
+		// consolidate dashes
+		if s[i] == '-' && last == '-' {
+			continue
+		}
+
+		b[pos], last = s[i], s[i]
+		pos++
+	}
+
+	if p.DisablePathToLower {
+		return string(b[:pos])
+	}
+	return strings.ToLower(string(b[:pos]))
+}
+
 // MakePath takes a string with any characters and replace it
 // so the string could be used in a path.
 // It does so by creating a Unicode-sanitized string, with the spaces replaced,
diff --git a/helpers/path_test.go b/helpers/path_test.go
index c249a519d..a97e3d507 100644
--- a/helpers/path_test.go
+++ b/helpers/path_test.go
@@ -36,6 +36,38 @@ import (
 	"github.com/spf13/viper"
 )
 
+func TestMakeSegment(t *testing.T) {
+	tests := []struct {
+		input    string
+		expected string
+	}{
+		{"  FOO bar  ", "foo-bar"},
+		{"Foo.Bar/fOO_bAr-Foo", "foo.bar-foo_bar-foo"},
+		{"FOO,bar:FooBar", "foobarfoobar"},
+		{"foo/BAR.HTML", "foo-bar.html"},
+		{"трям/трям", "трям-трям"},
+		{"은행", "은행"},
+		{"Say What??", "say-what"},
+		{"Your #1 Fan", "your-1-fan"},
+		{"Red & Blue", "red-blue"},
+		{"double//slash", "double-slash"},
+		{"My // Taxonomy", "my-taxonomy"},
+	}
+
+	for _, test := range tests {
+		v := newTestCfg()
+
+		l := langs.NewDefaultLanguage(v)
+		p, err := NewPathSpec(hugofs.NewMem(v), l)
+		require.NoError(t, err)
+
+		output := p.MakeSegment(test.input)
+		if output != test.expected {
+			t.Errorf("Expected %#v, got %#v\n", test.expected, output)
+		}
+	}
+}
+
 func TestMakePath(t *testing.T) {
 	tests := []struct {
 		input         string
diff --git a/hugolib/hugo_sites.go b/hugolib/hugo_sites.go
index 11e7bfa4e..e5358ec19 100644
--- a/hugolib/hugo_sites.go
+++ b/hugolib/hugo_sites.go
@@ -494,7 +494,7 @@ func (h *HugoSites) createMissingPages() error {
 						origKey := key
 
 						if s.Info.preserveTaxonomyNames {
-							key = s.PathSpec.MakePathSanitized(key)
+							key = s.PathSpec.MakeSegment(key)
 						}
 						for _, p := range taxonomyPages {
 							// Some people may have /authors/MaxMustermann etc. as paths.
diff --git a/hugolib/page_paths.go b/hugolib/page_paths.go
index 550df130d..999537de4 100644
--- a/hugolib/page_paths.go
+++ b/hugolib/page_paths.go
@@ -197,7 +197,13 @@ func createTargetPath(d targetPathDescriptor) string {
 		if d.ExpandedPermalink != "" {
 			pagePath = filepath.Join(pagePath, d.ExpandedPermalink)
 		} else {
-			pagePath = filepath.Join(d.Sections...)
+			pagePath = ""
+			for i, section := range d.Sections {
+				if i > 0 {
+					pagePath += helpers.FilePathSeparator
+				}
+				pagePath += d.PathSpec.MakeSegment(section)
+			}
 		}
 		needsBase = false
 	}
diff --git a/hugolib/page_paths_test.go b/hugolib/page_paths_test.go
index 8f8df6ec1..9e8d4ecb7 100644
--- a/hugolib/page_paths_test.go
+++ b/hugolib/page_paths_test.go
@@ -151,6 +151,13 @@ func TestPageTargetPath(t *testing.T) {
 									BaseName: "mypage",
 									Addends:  "c/d/e",
 									Type:     output.HTMLFormat}, "/a/b/mypage/c/d/e/index.html"},
+							{
+								"Unclean Taxonomy Term", targetPathDescriptor{
+									Kind:     KindTaxonomy,
+									BaseName: "_index",
+									Sections: []string{"tags", "x/y"},
+									Type:     output.HTMLFormat,
+									Addends:  "page/3"}, "/tags/x-y/page/3/index.html"},
 						}
 
 						for i, test := range tests {
diff --git a/hugolib/page_taxonomy_test.go b/hugolib/page_taxonomy_test.go
index ed1d2565d..df6e0e857 100644
--- a/hugolib/page_taxonomy_test.go
+++ b/hugolib/page_taxonomy_test.go
@@ -20,7 +20,7 @@ import (
 )
 
 var pageYamlWithTaxonomiesA = `---
-tags: ['a', 'B', 'c']
+tags: ['a', 'B', 'c', 'x/y']
 categories: 'd'
 ---
 YAML frontmatter with tags and categories taxonomy.`
@@ -30,6 +30,7 @@ tags:
  - "a"
  - "B"
  - "c"
+ - "x/y"
 categories: 'd'
 ---
 YAML frontmatter with tags and categories taxonomy.`
@@ -45,13 +46,14 @@ var pageJSONWithTaxonomies = `{
   "tags": [
     "a",
     "b",
-    "c"
+    "c",
+    "x/y"
   ]
 }
 JSON Front Matter with tags and categories`
 
 var pageTomlWithTaxonomies = `+++
-tags = [ "a", "B", "c" ]
+tags = [ "a", "B", "c", "x/y" ]
 categories = "d"
 +++
 TOML Front Matter with tags and categories`
@@ -75,7 +77,7 @@ func TestParseTaxonomies(t *testing.T) {
 		param := p.getParamToLower("tags")
 
 		if params, ok := param.([]string); ok {
-			expected := []string{"a", "b", "c"}
+			expected := []string{"a", "b", "c", "x/y"}
 			if !reflect.DeepEqual(params, expected) {
 				t.Errorf("Expected %s: got: %s", expected, params)
 			}
diff --git a/hugolib/site.go b/hugolib/site.go
index b55b6040b..f239cf607 100644
--- a/hugolib/site.go
+++ b/hugolib/site.go
@@ -1834,7 +1834,6 @@ func (s *Site) newTaxonomyPage(plural, key string) *Page {
 		// We make the first character upper case, mostly because
 		// it is easier to reason about in the tests.
 		p.title = helpers.FirstUpper(key)
-		key = s.PathSpec.MakePathSanitized(key)
 	} else {
 		p.title = strings.Replace(s.titleFunc(key), "-", " ", -1)
 	}
diff --git a/hugolib/taxonomy_test.go b/hugolib/taxonomy_test.go
index 0445de58f..23321cb55 100644
--- a/hugolib/taxonomy_test.go
+++ b/hugolib/taxonomy_test.go
@@ -45,8 +45,9 @@ func TestByCountOrderOfTaxonomies(t *testing.T) {
 		st = append(st, t.Name)
 	}
 
-	if !reflect.DeepEqual(st, []string{"a", "b", "c"}) {
-		t.Fatalf("ordered taxonomies do not match [a, b, c].  Got: %s", st)
+	expect := []string{"a", "b", "c", "x/y"}
+	if !reflect.DeepEqual(st, expect) {
+		t.Fatalf("ordered taxonomies do not match %v.  Got: %s", expect, st)
 	}
 }
 
@@ -104,13 +105,7 @@ permalinkeds:
 
 	fs := th.Fs
 
-	if preserveTaxonomyNames {
-		writeSource(t, fs, "content/p1.md", fmt.Sprintf(pageTemplate, "t1/c1", "- tag1", "- cat1", "- o1", "- pl1"))
-	} else {
-		// Check lower-casing of tags
-		writeSource(t, fs, "content/p1.md", fmt.Sprintf(pageTemplate, "t1/c1", "- Tag1", "- cAt1", "- o1", "- pl1"))
-
-	}
+	writeSource(t, fs, "content/p1.md", fmt.Sprintf(pageTemplate, "t1/c1", "- Tag1", "- cat1\n- \"cAt/dOg\"", "- o1", "- pl1"))
 	writeSource(t, fs, "content/p2.md", fmt.Sprintf(pageTemplate, "t2/c1", "- tag2", "- cat1", "- o1", "- pl1"))
 	writeSource(t, fs, "content/p3.md", fmt.Sprintf(pageTemplate, "t2/c12", "- tag2", "- cat2", "- o1", "- pl1"))
 	writeSource(t, fs, "content/p4.md", fmt.Sprintf(pageTemplate, "Hello World", "", "", "- \"Hello Hugo world\"", "- pl1"))
@@ -118,9 +113,7 @@ permalinkeds:
 	writeNewContentFile(t, fs.Source, "Category Terms", "2017-01-01", "content/categories/_index.md", 10)
 	writeNewContentFile(t, fs.Source, "Tag1 List", "2017-01-01", "content/tags/Tag1/_index.md", 10)
 
-	err := h.Build(BuildCfg{})
-
-	require.NoError(t, err)
+	require.NoError(t, h.Build(BuildCfg{}))
 
 	// So what we have now is:
 	// 1. categories with terms content page, but no content page for the only c1 category
@@ -137,6 +130,13 @@ permalinkeds:
 
 	// 1.
 	th.assertFileContent(pathFunc("public/categories/cat1/index.html"), "List", "Cat1")
+	if preserveTaxonomyNames {
+		// As of this writing, term titles are given a first upper.
+		// See hugolib.Site.newTaxonomyPage().
+		th.assertFileContent(pathFunc("public/categories/cat-dog/index.html"), "List", "CAt/dOg")
+	} else {
+		th.assertFileContent(pathFunc("public/categories/cat-dog/index.html"), "List", "Cat/Dog")
+	}
 	th.assertFileContent(pathFunc("public/categories/index.html"), "Terms List", "Category Terms")
 
 	// 2.
@@ -160,7 +160,7 @@ permalinkeds:
 	// of KindTaxonomy pages in its Pages slice.
 	taxonomyTermPageCounts := map[string]int{
 		"tags":         2,
-		"categories":   2,
+		"categories":   3,
 		"others":       2,
 		"empties":      0,
 		"permalinkeds": 1,
@@ -169,32 +169,42 @@ permalinkeds:
 	for taxonomy, count := range taxonomyTermPageCounts {
 		term := s.getPage(KindTaxonomyTerm, taxonomy)
 		require.NotNil(t, term)
-		require.Len(t, term.Pages, count)
+		require.Len(t, term.Pages, count, taxonomy)
 
 		for _, page := range term.Pages {
 			require.Equal(t, KindTaxonomy, page.Kind)
 		}
 	}
 
+	fixTerm := func(s string) string {
+		if preserveTaxonomyNames {
+			return s
+		}
+		return strings.ToLower(s)
+	}
+
+	fixURL := func(s string) string {
+		if uglyURLs {
+			return strings.TrimRight(s, "/") + ".html"
+		}
+		return s
+	}
+
 	cat1 := s.getPage(KindTaxonomy, "categories", "cat1")
 	require.NotNil(t, cat1)
-	if uglyURLs {
-		require.Equal(t, "/blog/categories/cat1.html", cat1.RelPermalink())
-	} else {
-		require.Equal(t, "/blog/categories/cat1/", cat1.RelPermalink())
-	}
+	require.Equal(t, fixURL("/blog/categories/cat1/"), cat1.RelPermalink())
+
+	catdog := s.getPage(KindTaxonomy, "categories", fixTerm("cAt/dOg"))
+	require.NotNil(t, catdog)
+	require.Equal(t, fixURL("/blog/categories/cat-dog/"), catdog.RelPermalink())
 
 	pl1 := s.getPage(KindTaxonomy, "permalinkeds", "pl1")
-	permalinkeds := s.getPage(KindTaxonomyTerm, "permalinkeds")
 	require.NotNil(t, pl1)
+	require.Equal(t, fixURL("/blog/perma/pl1/"), pl1.RelPermalink())
+
+	permalinkeds := s.getPage(KindTaxonomyTerm, "permalinkeds")
 	require.NotNil(t, permalinkeds)
-	if uglyURLs {
-		require.Equal(t, "/blog/perma/pl1.html", pl1.RelPermalink())
-		require.Equal(t, "/blog/permalinkeds.html", permalinkeds.RelPermalink())
-	} else {
-		require.Equal(t, "/blog/perma/pl1/", pl1.RelPermalink())
-		require.Equal(t, "/blog/permalinkeds/", permalinkeds.RelPermalink())
-	}
+	require.Equal(t, fixURL("/blog/permalinkeds/"), permalinkeds.RelPermalink())
 
 	// Issue #3070 preserveTaxonomyNames
 	if preserveTaxonomyNames {