hugolib: Normalize permalink path segments

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
This commit is contained in:
Cameron Moore 2018-09-21 14:03:17 -05:00 committed by Bjørn Erik Pedersen
parent 06d28a464d
commit fae48d7457
3 changed files with 51 additions and 36 deletions

View file

@ -152,11 +152,15 @@ func pageToPermalinkDate(p *Page, dateField string) (string, error) {
// pageToPermalinkTitle returns the URL-safe form of the title // pageToPermalinkTitle returns the URL-safe form of the title
func pageToPermalinkTitle(p *Page, _ string) (string, error) { func pageToPermalinkTitle(p *Page, _ string) (string, error) {
// Page contains Node which has Title if p.Kind == "taxonomy" {
// (also contains URLPath which has Slug, sometimes) // 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.URLize(p.title), nil
} }
return p.s.PathSpec.MakeSegment(p.title), nil
}
// pageToPermalinkFilename returns the URL-safe form of the filename // pageToPermalinkFilename returns the URL-safe form of the filename
func pageToPermalinkFilename(p *Page, _ string) (string, error) { func pageToPermalinkFilename(p *Page, _ string) (string, error) {
name := p.File.TranslationBaseName() name := p.File.TranslationBaseName()
@ -166,7 +170,7 @@ func pageToPermalinkFilename(p *Page, _ string) (string, error) {
_, name = filepath.Split(dir) _, 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 // 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, "-") { if strings.HasSuffix(p.Slug, "-") {
p.Slug = p.Slug[0 : len(p.Slug)-1] 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) return pageToPermalinkTitle(p, a)
} }
func pageToPermalinkSection(p *Page, _ string) (string, error) { func pageToPermalinkSection(p *Page, _ string) (string, error) {
// Page contains Node contains URLPath which has Section // 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) { func pageToPermalinkSections(p *Page, _ string) (string, error) {
// TODO(bep) we have some superflous URLize in this file, but let's // TODO(bep) we have some superflous URLize in this file, but let's
// deal with that later. // 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() { func init() {

View file

@ -19,36 +19,26 @@ import (
) )
// testdataPermalinks is used by a couple of tests; the expandsTo content is // 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 { var testdataPermalinks = []struct {
spec string spec string
valid bool valid bool
expandsTo string 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"},
//{":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"},
{"/:year-:month-:title", true, "/2012-04-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"}, {"/:year/:yearday/:month/:monthname/:day/:weekday/:weekdayname/", true, "/2012/97/04/April/06/5/Friday/"}, // Dates
//{"/blog/:fred", false, ""}, {"/:section/", true, "/blue/"}, // Section
//{"/:year//:title", false, ""}, {"/:title/", true, "/spf13-vim-3.0-release-and-new-website/"}, // Title
//{ {"/:slug/", true, "/spf13-vim-3-0-release-and-new-website/"}, // Slug
//"/:section/:year/:month/:day/:weekdayname/:yearday/:title", {"/:filename/", true, "/test-page/"}, // Filename
//true, // TODO(moorereason): need test scaffolding for this.
//"/blue/2012/04/06/Friday/97/spf13-vim-3.0-release-and-new-website", //{"/:sections/", false, "/blue/"}, // Sections
//},
//{ // Failures
//"/:weekday/:weekdayname/:month/:monthname", {"/blog/:fred", false, ""},
//true, {"/:year//:title", false, ""},
//"/5/Friday/04/April",
//},
//{
//"/:slug/:title",
//true,
//"/spf13-vim-3-0-release-and-new-website/spf13-vim-3.0-release-and-new-website",
//},
} }
func TestPermalinkValidation(t *testing.T) { 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") page, err := s.NewPageFrom(strings.NewReader(simplePageJSON), "blue/test-page.md")
if err != nil { 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 { for _, item := range testdataPermalinks {
if !item.valid { if !item.valid {

View file

@ -79,9 +79,11 @@ category = "categories"
other = "others" other = "others"
empty = "empties" empty = "empties"
permalinked = "permalinkeds" permalinked = "permalinkeds"
subcats = "subcats"
[permalinks] [permalinks]
permalinkeds = "/perma/:slug/" permalinkeds = "/perma/:slug/"
subcats = "/subcats/:slug/"
` `
pageTemplate := `--- pageTemplate := `---
@ -94,6 +96,8 @@ others:
%s %s
permalinkeds: permalinkeds:
%s %s
subcats:
%s
--- ---
# Doc # Doc
` `
@ -105,10 +109,11 @@ permalinkeds:
fs := th.Fs 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/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/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/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/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, "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) writeNewContentFile(t, fs.Source, "Tag1 List", "2017-01-01", "content/tags/Tag1/_index.md", 10)
@ -175,6 +180,7 @@ permalinkeds:
"others": 2, "others": 2,
"empties": 0, "empties": 0,
"permalinkeds": 1, "permalinkeds": 1,
"subcats": 1,
} }
for taxonomy, count := range taxonomyTermPageCounts { for taxonomy, count := range taxonomyTermPageCounts {
@ -217,6 +223,11 @@ permalinkeds:
require.NotNil(t, permalinkeds) require.NotNil(t, permalinkeds)
require.Equal(t, fixURL("/blog/permalinkeds/"), permalinkeds.RelPermalink()) 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 // Issue #3070 preserveTaxonomyNames
if preserveTaxonomyNames { if preserveTaxonomyNames {
helloWorld := s.getPage(KindTaxonomy, "others", "Hello Hugo world") helloWorld := s.getPage(KindTaxonomy, "others", "Hello Hugo world")