From fff132537b4094221f4f099e2251f3cda613060f Mon Sep 17 00:00:00 2001 From: Cameron Moore Date: Wed, 22 Aug 2018 00:18:37 -0500 Subject: [PATCH] Fix handling of taxonomy terms containing slashes Fixes #4090 --- helpers/path.go | 29 +++++++++++++++ helpers/path_test.go | 32 ++++++++++++++++ hugolib/hugo_sites.go | 2 +- hugolib/page_paths.go | 8 +++- hugolib/page_paths_test.go | 7 ++++ hugolib/page_taxonomy_test.go | 10 +++-- hugolib/site.go | 1 - hugolib/taxonomy_test.go | 70 ++++++++++++++++++++--------------- 8 files changed, 122 insertions(+), 37 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,33 +169,43 @@ 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) } } - 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()) + fixTerm := func(s string) string { + if preserveTaxonomyNames { + return s + } + return strings.ToLower(s) } - pl1 := s.getPage(KindTaxonomy, "permalinkeds", "pl1") - permalinkeds := s.getPage(KindTaxonomyTerm, "permalinkeds") - require.NotNil(t, pl1) - 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()) + fixURL := func(s string) string { + if uglyURLs { + return strings.TrimRight(s, "/") + ".html" + } + return s } + cat1 := s.getPage(KindTaxonomy, "categories", "cat1") + require.NotNil(t, cat1) + 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") + require.NotNil(t, pl1) + require.Equal(t, fixURL("/blog/perma/pl1/"), pl1.RelPermalink()) + + permalinkeds := s.getPage(KindTaxonomyTerm, "permalinkeds") + require.NotNil(t, permalinkeds) + require.Equal(t, fixURL("/blog/permalinkeds/"), permalinkeds.RelPermalink()) + // Issue #3070 preserveTaxonomyNames if preserveTaxonomyNames { helloWorld := s.getPage(KindTaxonomy, "others", "Hello Hugo world")