Skip to content

Commit

Permalink
hugolib: Normalize permalink path segments
Browse files Browse the repository at this point in the history
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 gohugoio#5223). Tests have been added to
check for this case.

Fixes gohugoio#4926
  • Loading branch information
moorereason authored and bep committed Oct 3, 2018
1 parent 06d28a4 commit fae48d7
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 36 deletions.
28 changes: 21 additions & 7 deletions hugolib/permalinks.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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() {
Expand Down
40 changes: 15 additions & 25 deletions hugolib/permalinks_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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 {
Expand Down
19 changes: 15 additions & 4 deletions hugolib/taxonomy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,11 @@ category = "categories"
other = "others"
empty = "empties"
permalinked = "permalinkeds"
subcats = "subcats"
[permalinks]
permalinkeds = "/perma/:slug/"
subcats = "/subcats/:slug/"
`

pageTemplate := `---
Expand All @@ -94,6 +96,8 @@ others:
%s
permalinkeds:
%s
subcats:
%s
---
# Doc
`
Expand All @@ -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)
Expand Down Expand Up @@ -175,6 +180,7 @@ permalinkeds:
"others": 2,
"empties": 0,
"permalinkeds": 1,
"subcats": 1,
}

for taxonomy, count := range taxonomyTermPageCounts {
Expand Down Expand Up @@ -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")
Expand Down

0 comments on commit fae48d7

Please sign in to comment.