hugolib: Normalize permalink path segments
authorCameron Moore <moorereason@gmail.com>
Fri, 21 Sep 2018 19:03:17 +0000 (14:03 -0500)
committerBjørn Erik Pedersen <bjorn.erik.pedersen@gmail.com>
Wed, 3 Oct 2018 08:02:15 +0000 (11:02 +0300)
When constructing permalinks, ensure that most inputs used as path
segments are normalized with PathSpec.MakeSegment instead of
PathSpec.URLize.

The primary exception to that rule is with taxonomy titles in
pageToPermalinkTitle(). The approach taken here is to use URLize for
taxonomy pages. Everything else will use MakeSegment. The reason for
this exception is that people use taxonomies such as "s1/p1" to generate
URLs precisely they way they wish (see #5223). Tests have been added to
check for this case.

Fixes #4926

hugolib/permalinks.go
hugolib/permalinks_test.go
hugolib/taxonomy_test.go

index 7640db6c1c04e5811d1dad5773291a9fbcee2a89..c452cf55d68a365f30d1ef55d0e573bcc79e9e66 100644 (file)
@@ -152,9 +152,13 @@ func pageToPermalinkDate(p *Page, dateField string) (string, error) {
 
 // pageToPermalinkTitle returns the URL-safe form of the title
 func pageToPermalinkTitle(p *Page, _ string) (string, error) {
-       // Page contains Node which has Title
-       // (also contains URLPath which has Slug, sometimes)
-       return p.s.PathSpec.URLize(p.title), nil
+       if p.Kind == "taxonomy" {
+               // Taxonomies are allowed to have '/' characters, so don't normalize
+               // them with MakeSegment.
+               return p.s.PathSpec.URLize(p.title), nil
+       }
+
+       return p.s.PathSpec.MakeSegment(p.title), nil
 }
 
 // pageToPermalinkFilename returns the URL-safe form of the filename
@@ -166,7 +170,7 @@ func pageToPermalinkFilename(p *Page, _ string) (string, error) {
                _, name = filepath.Split(dir)
        }
 
-       return p.s.PathSpec.URLize(name), nil
+       return p.s.PathSpec.MakeSegment(name), nil
 }
 
 // if the page has a slug, return the slug, else return the title
@@ -181,20 +185,30 @@ func pageToPermalinkSlugElseTitle(p *Page, a string) (string, error) {
                if strings.HasSuffix(p.Slug, "-") {
                        p.Slug = p.Slug[0 : len(p.Slug)-1]
                }
-               return p.s.PathSpec.URLize(p.Slug), nil
+               return p.s.PathSpec.MakeSegment(p.Slug), nil
        }
        return pageToPermalinkTitle(p, a)
 }
 
 func pageToPermalinkSection(p *Page, _ string) (string, error) {
        // Page contains Node contains URLPath which has Section
-       return p.Section(), nil
+       return p.s.PathSpec.MakeSegment(p.Section()), nil
 }
 
 func pageToPermalinkSections(p *Page, _ string) (string, error) {
        // TODO(bep) we have some superflous URLize in this file, but let's
        // deal with that later.
-       return path.Join(p.CurrentSection().sections...), nil
+
+       cs := p.CurrentSection()
+       if cs == nil {
+               return "", errors.New("\":sections\" attribute requires parent page but is nil")
+       }
+
+       sections := make([]string, len(cs.sections))
+       for i := range cs.sections {
+               sections[i] = p.s.PathSpec.MakeSegment(cs.sections[i])
+       }
+       return path.Join(sections...), nil
 }
 
 func init() {
index 7a4bf78c240e18f7a87b0dba13297285bcfd189d..f9ff8e708d3dab1422e6848e3f1ea9b10ed353f4 100644 (file)
@@ -19,36 +19,26 @@ import (
 )
 
 // testdataPermalinks is used by a couple of tests; the expandsTo content is
-// subject to the data in SIMPLE_PAGE_JSON.
+// subject to the data in simplePageJSON.
 var testdataPermalinks = []struct {
        spec      string
        valid     bool
        expandsTo string
 }{
-       //{"/:year/:month/:title/", true, "/2012/04/spf13-vim-3.0-release-and-new-website/"},
-       //{"/:title", true, "/spf13-vim-3.0-release-and-new-website"},
-       //{":title", true, "spf13-vim-3.0-release-and-new-website"},
-       //{"/blog/:year/:yearday/:title", true, "/blog/2012/97/spf13-vim-3.0-release-and-new-website"},
+       {":title", true, "spf13-vim-3.0-release-and-new-website"},
        {"/:year-:month-:title", true, "/2012-04-spf13-vim-3.0-release-and-new-website"},
-       {"/blog/:year-:month-:title", true, "/blog/2012-04-spf13-vim-3.0-release-and-new-website"},
-       {"/blog-:year-:month-:title", true, "/blog-2012-04-spf13-vim-3.0-release-and-new-website"},
-       //{"/blog/:fred", false, ""},
-       //{"/:year//:title", false, ""},
-       //{
-       //"/:section/:year/:month/:day/:weekdayname/:yearday/:title",
-       //true,
-       //"/blue/2012/04/06/Friday/97/spf13-vim-3.0-release-and-new-website",
-       //},
-       //{
-       //"/:weekday/:weekdayname/:month/:monthname",
-       //true,
-       //"/5/Friday/04/April",
-       //},
-       //{
-       //"/:slug/:title",
-       //true,
-       //"/spf13-vim-3-0-release-and-new-website/spf13-vim-3.0-release-and-new-website",
-       //},
+
+       {"/:year/:yearday/:month/:monthname/:day/:weekday/:weekdayname/", true, "/2012/97/04/April/06/5/Friday/"}, // Dates
+       {"/:section/", true, "/blue/"},                                // Section
+       {"/:title/", true, "/spf13-vim-3.0-release-and-new-website/"}, // Title
+       {"/:slug/", true, "/spf13-vim-3-0-release-and-new-website/"},  // Slug
+       {"/:filename/", true, "/test-page/"},                          // Filename
+       // TODO(moorereason): need test scaffolding for this.
+       //{"/:sections/", false, "/blue/"},                              // Sections
+
+       // Failures
+       {"/blog/:fred", false, ""},
+       {"/:year//:title", false, ""},
 }
 
 func TestPermalinkValidation(t *testing.T) {
@@ -75,7 +65,7 @@ func TestPermalinkExpansion(t *testing.T) {
        page, err := s.NewPageFrom(strings.NewReader(simplePageJSON), "blue/test-page.md")
 
        if err != nil {
-               t.Fatalf("failed before we began, could not parse SIMPLE_PAGE_JSON: %s", err)
+               t.Fatalf("failed before we began, could not parse simplePageJSON: %s", err)
        }
        for _, item := range testdataPermalinks {
                if !item.valid {
index 1085f4fbadbad331865fa9bb7b89e8ce3766aa0c..ec55dc42866fe9a8f4e21f5b4a7e0702adc2ca67 100644 (file)
@@ -79,9 +79,11 @@ category = "categories"
 other = "others"
 empty = "empties"
 permalinked = "permalinkeds"
+subcats = "subcats"
 
 [permalinks]
 permalinkeds = "/perma/:slug/"
+subcats = "/subcats/:slug/"
 `
 
        pageTemplate := `---
@@ -94,6 +96,8 @@ others:
 %s
 permalinkeds:
 %s
+subcats:
+%s
 ---
 # Doc
 `
@@ -105,10 +109,11 @@ permalinkeds:
 
        fs := th.Fs
 
-       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"))
+       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", ""))
+       writeSource(t, fs, "content/p5.md", fmt.Sprintf(pageTemplate, "Sub/categories", "", "", "", "", "- \"sc0/sp1\""))
 
        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)
@@ -175,6 +180,7 @@ permalinkeds:
                "others":       2,
                "empties":      0,
                "permalinkeds": 1,
+               "subcats":      1,
        }
 
        for taxonomy, count := range taxonomyTermPageCounts {
@@ -217,6 +223,11 @@ permalinkeds:
        require.NotNil(t, permalinkeds)
        require.Equal(t, fixURL("/blog/permalinkeds/"), permalinkeds.RelPermalink())
 
+       // Issue #5223
+       sp1 := s.getPage(KindTaxonomy, "subcats", "sc0/sp1")
+       require.NotNil(t, sp1)
+       require.Equal(t, fixURL("/blog/subcats/sc0/sp1/"), sp1.RelPermalink())
+
        // Issue #3070 preserveTaxonomyNames
        if preserveTaxonomyNames {
                helloWorld := s.getPage(KindTaxonomy, "others", "Hello Hugo world")