Fix non-ASCII path handling for Page resources

Fixes #4241

Author: bep

hugolib/page.go

@@ -228,9 +228,9 @@ type Page struct {
 	permalink    string
 	relPermalink string
 
-	// relPermalink without extension and any base path element from the baseURL.
+	// relative target path without extension and any base path element from the baseURL.
 	// This is used to construct paths in the page resources.
-	relPermalinkBase string
+	relTargetPathBase string
 
 	layoutDescriptor output.LayoutDescriptor
 
@@ -989,8 +989,8 @@ func (p *Page) RelPermalink() string {
 	return p.relPermalink
 }
 
-func (p *Page) subResourceLinkFactory(base string) string {
-	return path.Join(p.relPermalinkBase, base)
+func (p *Page) subResourceTargetPathFactory(base string) string {
+	return path.Join(p.relTargetPathBase, base)
 }
 
 func (p *Page) prepareForRender(cfg *BuildCfg) error {

hugolib/page_bundler_capture_test.go

@@ -144,6 +144,7 @@ D:
 __bundle/en/work/base/_index.md/resources/en/work/base/_1.png
 __bundle/en/work/base/a/b/index.md/resources/en/work/base/a/b/ab1.md
 __bundle/en/work/base/b/index.md/resources/en/work/base/b/1.md|en/work/base/b/2.md|en/work/base/b/c/logo.png|en/work/base/b/custom-mime.bep|en/work/base/b/sunset1.jpg|en/work/base/b/sunset2.jpg
+__bundle/en/work/base/c/index.md/resources/en/work/base/c/logo-은행.png
 C:
 /work/base/assets/pic1.png
 /work/base/assets/pic2.png

hugolib/page_bundler_handlers.go

@@ -319,7 +319,7 @@ func (c *contentHandlers) createResource() contentHandler {
 		}
 
 		resource, err := c.s.resourceSpec.NewResourceFromFilename(
-			ctx.parentPage.subResourceLinkFactory,
+			ctx.parentPage.subResourceTargetPathFactory,
 			c.s.absPublishDir(),
 			ctx.source.Filename(), ctx.target)
 

hugolib/page_bundler_test.go

@@ -51,6 +51,7 @@ func TestPageBundlerSite(t *testing.T) {
 				cfg.Set("permalinks", map[string]string{
 					"a": ":sections/:filename",
 					"b": ":year/:slug/",
+					"c": ":sections/:slug",
 				})
 
 				cfg.Set("outputFormats", map[string]interface{}{
@@ -74,7 +75,7 @@ func TestPageBundlerSite(t *testing.T) {
 				th := testHelper{s.Cfg, s.Fs, t}
 
 				// Singles (2), Below home (1), Bundle (1)
-				assert.Len(s.RegularPages, 6)
+				assert.Len(s.RegularPages, 7)
 
 				singlePage := s.getPage(KindPage, "a/1.md")
 
@@ -99,6 +100,8 @@ func TestPageBundlerSite(t *testing.T) {
 				assert.NotNil(leafBundle1)
 				leafBundle2 := s.getPage(KindPage, "a/b/index.md")
 				assert.NotNil(leafBundle2)
+				unicodeBundle := s.getPage(KindPage, "c/index.md")
+				assert.NotNil(unicodeBundle)
 
 				pageResources := leafBundle1.Resources.ByType(pageResourceType)
 				assert.Len(pageResources, 2)
@@ -136,12 +139,18 @@ func TestPageBundlerSite(t *testing.T) {
 
 					assert.Equal("/a/b.html", leafBundle2.RelPermalink())
 
+					// 은행
+					assert.Equal("/c/%EC%9D%80%ED%96%89.html", unicodeBundle.RelPermalink())
+					th.assertFileContent(filepath.FromSlash("/work/public/c/은행.html"), "Content for 은행")
+					th.assertFileContent(filepath.FromSlash("/work/public/c/은행/logo-은행.png"), "은행 PNG")
+
 				} else {
 					assert.Equal("/2017/pageslug/", leafBundle1.RelPermalink())
 					th.assertFileContent(filepath.FromSlash("/work/public/2017/pageslug/index.html"), "TheContent")
 					th.assertFileContent(filepath.FromSlash("/work/public/cpath/2017/pageslug/cindex.html"), "TheContent")
 
 					assert.Equal("/a/b/", leafBundle2.RelPermalink())
+
 				}
 
 			})
@@ -261,6 +270,18 @@ Short Thumb Width: {{ $thumb.Width }}
 	writeSource(t, fs, filepath.Join(workDir, "base", "b", "custom-mime.bep"), "bepsays")
 	writeSource(t, fs, filepath.Join(workDir, "base", "b", "c", "logo.png"), "content")
 
+	// Bundle with 은행 slug
+	// See https://github.com/gohugoio/hugo/issues/4241
+	writeSource(t, fs, filepath.Join(workDir, "base", "c", "index.md"), `---
+title: "은행 은행"
+slug: 은행
+date: 2017-10-09
+---
+
+Content for 은행.
+`)
+	writeSource(t, fs, filepath.Join(workDir, "base", "c", "logo-은행.png"), "은행 PNG")
+
 	// Write a real image into one of the bundle above.
 	src, err := os.Open("testdata/sunset.jpg")
 	assert.NoError(err)

hugolib/page_paths.go

@@ -129,7 +129,8 @@ func (p *Page) initURLs() error {
 	if len(p.outputFormats) == 0 {
 		p.outputFormats = p.s.outputFormats[p.Kind]
 	}
-	rel := p.createRelativePermalink()
+	target := filepath.ToSlash(p.createRelativeTargetPath())
+	rel := p.s.PathSpec.URLizeFilename(target)
 
 	var err error
 	f := p.outputFormats[0]
@@ -138,7 +139,7 @@ func (p *Page) initURLs() error {
 		return err
 	}
 
-	p.relPermalinkBase = strings.TrimSuffix(rel, f.MediaType.FullSuffix())
+	p.relTargetPathBase = strings.TrimSuffix(target, f.MediaType.FullSuffix())
 	p.relPermalink = p.s.PathSpec.PrependBasePath(rel)
 	p.layoutDescriptor = p.createLayoutDescriptor()
 	return nil
@@ -267,7 +268,7 @@ func createTargetPath(d targetPathDescriptor) string {
 	return d.PathSpec.MakePathSanitized(pagePath)
 }
 
-func (p *Page) createRelativePermalink() string {
+func (p *Page) createRelativeTargetPath() string {
 
 	if len(p.outputFormats) == 0 {
 		if p.Kind == kindUnknown {
@@ -279,11 +280,15 @@ func (p *Page) createRelativePermalink() string {
 	// Choose the main output format. In most cases, this will be HTML.
 	f := p.outputFormats[0]
 
-	return p.createRelativePermalinkForOutputFormat(f)
+	return p.createRelativeTargetPathForOutputFormat(f)
 
 }
 
 func (p *Page) createRelativePermalinkForOutputFormat(f output.Format) string {
+	return p.s.PathSpec.URLizeFilename(p.createRelativeTargetPathForOutputFormat(f))
+}
+
+func (p *Page) createRelativeTargetPathForOutputFormat(f output.Format) string {
 	tp, err := p.createTargetPath(f, p.s.owner.IsMultihost())
 
 	if err != nil {
@@ -296,7 +301,7 @@ func (p *Page) createRelativePermalinkForOutputFormat(f output.Format) string {
 		tp = strings.TrimSuffix(tp, f.BaseFilename())
 	}
 
-	return p.s.PathSpec.URLizeFilename(tp)
+	return tp
 }
 
 func (p *Page) TargetPath() (outfile string) {

resource/image.go

@@ -185,7 +185,7 @@ type imageConfig struct {
 }
 
 func (i *Image) isJPEG() bool {
-	name := strings.ToLower(i.rel)
+	name := strings.ToLower(i.relTargetPath)
 	return strings.HasSuffix(name, ".jpg") || strings.HasSuffix(name, ".jpeg")
 }
 
@@ -206,7 +206,7 @@ func (i *Image) doWithImageConfig(action, spec string, f func(src image.Image, c
 		conf.Filter = imageFilters[conf.FilterStr]
 	}
 
-	key := i.relPermalinkForRel(i.filenameFromConfig(conf), false)
+	key := i.relTargetPathForRel(i.filenameFromConfig(conf), false)
 
 	return i.spec.imageCache.getOrCreate(i.spec, key, func(resourceCacheFilename string) (*Image, error) {
 		ci := i.clone()
@@ -521,11 +521,11 @@ func (i *Image) clone() *Image {
 }
 
 func (i *Image) setBasePath(conf imageConfig) {
-	i.rel = i.filenameFromConfig(conf)
+	i.relTargetPath = i.filenameFromConfig(conf)
 }
 
 func (i *Image) filenameFromConfig(conf imageConfig) string {
-	p1, p2 := helpers.FileAndExt(i.rel)
+	p1, p2 := helpers.FileAndExt(i.relTargetPath)
 	idStr := fmt.Sprintf("_hu%s_%d", i.hash, i.osFileInfo.Size())
 
 	// Do not change for no good reason.

resource/resource.go

@@ -112,27 +112,28 @@ func NewSpec(s *helpers.PathSpec, mimeTypes media.Types) (*Spec, error) {
 }
 
 func (r *Spec) NewResourceFromFile(
-	linker func(base string) string,
+	targetPathBuilder func(base string) string,
 	absPublishDir string,
 	file source.File, relTargetFilename string) (Resource, error) {
 
-	return r.newResource(linker, absPublishDir, file.Filename(), file.FileInfo(), relTargetFilename)
+	return r.newResource(targetPathBuilder, absPublishDir, file.Filename(), file.FileInfo(), relTargetFilename)
 }
 
+//  p.s.PathSpec.URLizeFilename
 func (r *Spec) NewResourceFromFilename(
-	linker func(base string) string,
+	targetPathBuilder func(base string) string,
 	absPublishDir,
 	absSourceFilename, relTargetFilename string) (Resource, error) {
 
 	fi, err := r.Fs.Source.Stat(absSourceFilename)
 	if err != nil {
 		return nil, err
 	}
-	return r.newResource(linker, absPublishDir, absSourceFilename, fi, relTargetFilename)
+	return r.newResource(targetPathBuilder, absPublishDir, absSourceFilename, fi, relTargetFilename)
 }
 
 func (r *Spec) newResource(
-	linker func(base string) string,
+	targetPathBuilder func(base string) string,
 	absPublishDir,
 	absSourceFilename string, fi os.FileInfo, relTargetFilename string) (Resource, error) {
 
@@ -150,7 +151,7 @@ func (r *Spec) newResource(
 		}
 	}
 
-	gr := r.newGenericResource(linker, fi, absPublishDir, absSourceFilename, filepath.ToSlash(relTargetFilename), mimeType)
+	gr := r.newGenericResource(targetPathBuilder, fi, absPublishDir, absSourceFilename, filepath.ToSlash(relTargetFilename), mimeType)
 
 	if mimeType == "image" {
 		f, err := r.Fs.Source.Open(absSourceFilename)
@@ -203,7 +204,7 @@ func (r *Spec) CacheStats() string {
 // genericResource represents a generic linkable resource.
 type genericResource struct {
 	// The relative path to this resource.
-	rel string
+	relTargetPath string
 
 	// Base is set when the output format's path has a offset, e.g. for AMP.
 	base string
@@ -214,16 +215,16 @@ type genericResource struct {
 	resourceType      string
 	osFileInfo        os.FileInfo
 
-	spec *Spec
-	link func(rel string) string
+	spec              *Spec
+	targetPathBuilder func(rel string) string
 }
 
 func (l *genericResource) Permalink() string {
-	return l.spec.PermalinkForBaseURL(l.relPermalinkForRel(l.rel, false), l.spec.BaseURL.String())
+	return l.spec.PermalinkForBaseURL(l.relPermalinkForRel(l.relTargetPath, false), l.spec.BaseURL.String())
 }
 
 func (l *genericResource) RelPermalink() string {
-	return l.relPermalinkForRel(l.rel, true)
+	return l.relPermalinkForRel(l.relTargetPath, true)
 }
 
 // Implement the Cloner interface.
@@ -233,8 +234,12 @@ func (l genericResource) WithNewBase(base string) Resource {
 }
 
 func (l *genericResource) relPermalinkForRel(rel string, addBasePath bool) string {
-	if l.link != nil {
-		rel = l.link(rel)
+	return l.spec.PathSpec.URLizeFilename(l.relTargetPathForRel(rel, addBasePath))
+}
+
+func (l *genericResource) relTargetPathForRel(rel string, addBasePath bool) string {
+	if l.targetPathBuilder != nil {
+		rel = l.targetPathBuilder(rel)
 	}
 
 	if l.base != "" {
@@ -249,7 +254,7 @@ func (l *genericResource) relPermalinkForRel(rel string, addBasePath bool) strin
 		rel = "/" + rel
 	}
 
-	return l.spec.PathSpec.URLizeFilename(rel)
+	return rel
 }
 
 func (l *genericResource) ResourceType() string {
@@ -273,27 +278,27 @@ func (l *genericResource) Publish() error {
 }
 
 func (l *genericResource) target() string {
-	target := l.relPermalinkForRel(l.rel, false)
+	target := l.relTargetPathForRel(l.relTargetPath, false)
 	if l.spec.PathSpec.Languages.IsMultihost() {
 		target = path.Join(l.spec.PathSpec.Language.Lang, target)
 	}
 	return target
 }
 
 func (r *Spec) newGenericResource(
-	linker func(base string) string,
+	targetPathBuilder func(base string) string,
 	osFileInfo os.FileInfo,
 	absPublishDir,
 	absSourceFilename,
 	baseFilename,
 	resourceType string) *genericResource {
 
 	return &genericResource{
-		link:              linker,
+		targetPathBuilder: targetPathBuilder,
 		osFileInfo:        osFileInfo,
 		absPublishDir:     absPublishDir,
 		absSourceFilename: absSourceFilename,
-		rel:               baseFilename,
+		relTargetPath:     baseFilename,
 		resourceType:      resourceType,
 		spec:              r,
 	}