resource: Fix path duplication/flattening in processed images

Fixes #4502
Closes #4501
This commit is contained in:
Bjørn Erik Pedersen 2018-03-14 09:33:32 +01:00
parent e427ba4268
commit 3fbc75534d
No known key found for this signature in database
GPG key ID: 330E6E2BD4859D8F
7 changed files with 90 additions and 25 deletions

View file

@ -209,7 +209,7 @@ type imageConfig struct {
} }
func (i *Image) isJPEG() bool { func (i *Image) isJPEG() bool {
name := strings.ToLower(i.relTargetPath) name := strings.ToLower(i.relTargetPath.file)
return strings.HasSuffix(name, ".jpg") || strings.HasSuffix(name, ".jpeg") return strings.HasSuffix(name, ".jpg") || strings.HasSuffix(name, ".jpeg")
} }
@ -237,9 +237,7 @@ func (i *Image) doWithImageConfig(action, spec string, f func(src image.Image, c
} }
} }
key := i.relTargetPathForRel(i.filenameFromConfig(conf), false) return i.spec.imageCache.getOrCreate(i, conf, func(resourceCacheFilename string) (*Image, error) {
return i.spec.imageCache.getOrCreate(i, key, func(resourceCacheFilename string) (*Image, error) {
ci := i.clone() ci := i.clone()
errOp := action errOp := action
@ -449,7 +447,6 @@ func (i *Image) decodeSource() (image.Image, error) {
func (i *Image) copyToDestination(src string) error { func (i *Image) copyToDestination(src string) error {
var res error var res error
i.copyToDestinationInit.Do(func() { i.copyToDestinationInit.Do(func() {
target := filepath.Join(i.absPublishDir, i.target()) target := filepath.Join(i.absPublishDir, i.target())
@ -498,7 +495,6 @@ func (i *Image) copyToDestination(src string) error {
} }
func (i *Image) encodeToDestinations(img image.Image, conf imageConfig, resourceCacheFilename, filename string) error { func (i *Image) encodeToDestinations(img image.Image, conf imageConfig, resourceCacheFilename, filename string) error {
target := filepath.Join(i.absPublishDir, filename) target := filepath.Join(i.absPublishDir, filename)
file1, err := i.spec.Fs.Destination.Create(target) file1, err := i.spec.Fs.Destination.Create(target)
@ -574,11 +570,12 @@ func (i *Image) clone() *Image {
} }
func (i *Image) setBasePath(conf imageConfig) { func (i *Image) setBasePath(conf imageConfig) {
i.relTargetPath = i.filenameFromConfig(conf) i.relTargetPath = i.relTargetPathFromConfig(conf)
} }
func (i *Image) filenameFromConfig(conf imageConfig) string { func (i *Image) relTargetPathFromConfig(conf imageConfig) dirFile {
p1, p2 := helpers.FileAndExt(i.relTargetPath) p1, p2 := helpers.FileAndExt(i.relTargetPath.file)
idStr := fmt.Sprintf("_hu%s_%d", i.hash, i.osFileInfo.Size()) idStr := fmt.Sprintf("_hu%s_%d", i.hash, i.osFileInfo.Size())
// Do not change for no good reason. // Do not change for no good reason.
@ -605,7 +602,11 @@ func (i *Image) filenameFromConfig(conf imageConfig) string {
idStr = "" idStr = ""
} }
return fmt.Sprintf("%s%s_%s%s", p1, idStr, key, p2) return dirFile{
dir: i.relTargetPath.dir,
file: fmt.Sprintf("%s%s_%s%s", p1, idStr, key, p2),
}
} }
func decodeImaging(m map[string]interface{}) (Imaging, error) { func decodeImaging(m map[string]interface{}) (Imaging, error) {

View file

@ -49,10 +49,17 @@ func (c *imageCache) deleteByPrefix(prefix string) {
} }
} }
func (c *imageCache) getOrCreate( func (c *imageCache) clear() {
parent *Image, key string, create func(resourceCacheFilename string) (*Image, error)) (*Image, error) { c.mu.Lock()
defer c.mu.Unlock()
c.store = make(map[string]*Image)
}
relTargetFilename := key func (c *imageCache) getOrCreate(
parent *Image, conf imageConfig, create func(resourceCacheFilename string) (*Image, error)) (*Image, error) {
relTarget := parent.relTargetPathFromConfig(conf)
key := parent.relTargetPathForRel(relTarget.path(), false)
if c.pathSpec.Language != nil { if c.pathSpec.Language != nil {
// Avoid do and store more work than needed. The language versions will in // Avoid do and store more work than needed. The language versions will in
@ -89,7 +96,7 @@ func (c *imageCache) getOrCreate(
if exists { if exists {
img = parent.clone() img = parent.clone()
img.relTargetPath = relTargetFilename img.relTargetPath.file = relTarget.file
img.absSourceFilename = cacheFilename img.absSourceFilename = cacheFilename
} else { } else {
img, err = create(cacheFilename) img, err = create(cacheFilename)

View file

@ -16,6 +16,7 @@ package resource
import ( import (
"fmt" "fmt"
"math/rand" "math/rand"
"path/filepath"
"strconv" "strconv"
"testing" "testing"
@ -278,6 +279,39 @@ func TestImageResize8BitPNG(t *testing.T) {
} }
func TestImageResizeInSubPath(t *testing.T) {
assert := require.New(t)
image := fetchImage(assert, "sub/gohugoio2.png")
assert.Equal(imaging.PNG, image.format)
assert.Equal("/a/sub/gohugoio2.png", image.RelPermalink())
assert.Equal("image", image.ResourceType())
resized, err := image.Resize("101x101")
assert.NoError(err)
assert.Equal(imaging.PNG, resized.format)
assert.Equal("/a/sub/gohugoio2_hu0e1b9e4a4be4d6f86c7b37b9ccce3fbc_73886_101x101_resize_linear_2.png", resized.RelPermalink())
assert.Equal(101, resized.Width())
assertFileCache(assert, image.spec.Fs, resized.RelPermalink(), 101, 101)
publishedImageFilename := filepath.Join("/public", resized.RelPermalink())
assertImageFile(assert, image.spec.Fs, publishedImageFilename, 101, 101)
assert.NoError(image.spec.Fs.Destination.Remove(publishedImageFilename))
// Cleare mem cache to simulate reading from the file cache.
resized.spec.imageCache.clear()
resizedAgain, err := image.Resize("101x101")
assert.NoError(err)
assert.Equal("/a/sub/gohugoio2_hu0e1b9e4a4be4d6f86c7b37b9ccce3fbc_73886_101x101_resize_linear_2.png", resizedAgain.RelPermalink())
assert.Equal(101, resizedAgain.Width())
assertFileCache(assert, image.spec.Fs, resizedAgain.RelPermalink(), 101, 101)
assertImageFile(assert, image.spec.Fs, publishedImageFilename, 101, 101)
}
func TestSVGImage(t *testing.T) { func TestSVGImage(t *testing.T) {
assert := require.New(t) assert := require.New(t)
spec := newTestResourceSpec(assert) spec := newTestResourceSpec(assert)

View file

@ -283,7 +283,7 @@ func (r *Spec) newResource(
} }
} }
gr := r.newGenericResource(targetPathBuilder, fi, absPublishDir, absSourceFilename, filepath.ToSlash(relTargetFilename), mimeType) gr := r.newGenericResource(targetPathBuilder, fi, absPublishDir, absSourceFilename, relTargetFilename, mimeType)
if mimeType == "image" { if mimeType == "image" {
ext := strings.ToLower(helpers.Ext(absSourceFilename)) ext := strings.ToLower(helpers.Ext(absSourceFilename))
@ -343,10 +343,23 @@ func (r *Spec) CacheStats() string {
return s return s
} }
type dirFile struct {
// This is the directory component with Unix-style slashes.
dir string
// This is the file component.
file string
}
func (d dirFile) path() string {
return path.Join(d.dir, d.file)
}
// genericResource represents a generic linkable resource. // genericResource represents a generic linkable resource.
type genericResource struct { type genericResource struct {
// The relative path to this resource. // The relative path to this resource.
relTargetPath string relTargetPath dirFile
file string
// Base is set when the output format's path has a offset, e.g. for AMP. // Base is set when the output format's path has a offset, e.g. for AMP.
base string base string
@ -366,11 +379,11 @@ type genericResource struct {
} }
func (l *genericResource) Permalink() string { func (l *genericResource) Permalink() string {
return l.spec.PermalinkForBaseURL(l.relPermalinkForRel(l.relTargetPath, false), l.spec.BaseURL.String()) return l.spec.PermalinkForBaseURL(l.relPermalinkForRel(l.relTargetPath.path(), false), l.spec.BaseURL.String())
} }
func (l *genericResource) RelPermalink() string { func (l *genericResource) RelPermalink() string {
return l.relPermalinkForRel(l.relTargetPath, true) return l.relPermalinkForRel(l.relTargetPath.path(), true)
} }
func (l *genericResource) Name() string { func (l *genericResource) Name() string {
@ -551,11 +564,11 @@ func replaceResourcePlaceholders(in string, counter int) string {
} }
func (l *genericResource) target() string { func (l *genericResource) target() string {
target := l.relTargetPathForRel(l.relTargetPath, false) target := l.relTargetPathForRel(l.relTargetPath.path(), false)
if l.spec.PathSpec.Languages.IsMultihost() { if l.spec.PathSpec.Languages.IsMultihost() {
target = path.Join(l.spec.PathSpec.Language.Lang, target) target = path.Join(l.spec.PathSpec.Language.Lang, target)
} }
return target return filepath.Clean(target)
} }
func (r *Spec) newGenericResource( func (r *Spec) newGenericResource(
@ -566,12 +579,17 @@ func (r *Spec) newGenericResource(
baseFilename, baseFilename,
resourceType string) *genericResource { resourceType string) *genericResource {
// This value is used both to construct URLs and file paths, but start
// with a Unix-styled path.
baseFilename = filepath.ToSlash(baseFilename)
fpath, fname := path.Split(baseFilename)
return &genericResource{ return &genericResource{
targetPathBuilder: targetPathBuilder, targetPathBuilder: targetPathBuilder,
osFileInfo: osFileInfo, osFileInfo: osFileInfo,
absPublishDir: absPublishDir, absPublishDir: absPublishDir,
absSourceFilename: absSourceFilename, absSourceFilename: absSourceFilename,
relTargetPath: baseFilename, relTargetPath: dirFile{dir: fpath, file: fname},
resourceType: resourceType, resourceType: resourceType,
spec: r, spec: r,
params: make(map[string]interface{}), params: make(map[string]interface{}),

View file

@ -93,7 +93,7 @@ func TestNewResourceFromFilenameSubPathInBaseURL(t *testing.T) {
assert.Equal("/docs/a/b/logo.png", r.RelPermalink()) assert.Equal("/docs/a/b/logo.png", r.RelPermalink())
assert.Equal("https://example.com/docs/a/b/logo.png", r.Permalink()) assert.Equal("https://example.com/docs/a/b/logo.png", r.Permalink())
img := r.(*Image) img := r.(*Image)
assert.Equal("/a/b/logo.png", img.target()) assert.Equal(filepath.FromSlash("/a/b/logo.png"), img.target())
} }

BIN
resource/testdata/sub/gohugoio2.png vendored Normal file

Binary file not shown.

After

Width:  |  Height:  |  Size: 72 KiB

View file

@ -94,7 +94,7 @@ func fetchImageForSpec(spec *Spec, assert *require.Assertions, name string) *Ima
} }
func fetchResourceForSpec(spec *Spec, assert *require.Assertions, name string) Resource { func fetchResourceForSpec(spec *Spec, assert *require.Assertions, name string) Resource {
src, err := os.Open("testdata/" + name) src, err := os.Open(filepath.FromSlash("testdata/" + name))
assert.NoError(err) assert.NoError(err)
workingDir := spec.Cfg.GetString("workingDir") workingDir := spec.Cfg.GetString("workingDir")
@ -116,8 +116,9 @@ func fetchResourceForSpec(spec *Spec, assert *require.Assertions, name string) R
return r return r
} }
func assertFileCache(assert *require.Assertions, fs *hugofs.Fs, filename string, width, height int) {
f, err := fs.Source.Open(filepath.Join("/res/_gen/images", filename)) func assertImageFile(assert *require.Assertions, fs *hugofs.Fs, filename string, width, height int) {
f, err := fs.Source.Open(filename)
assert.NoError(err) assert.NoError(err)
defer f.Close() defer f.Close()
@ -128,6 +129,10 @@ func assertFileCache(assert *require.Assertions, fs *hugofs.Fs, filename string,
assert.Equal(height, config.Height) assert.Equal(height, config.Height)
} }
func assertFileCache(assert *require.Assertions, fs *hugofs.Fs, filename string, width, height int) {
assertImageFile(assert, fs, filepath.Join("/res/_gen/images", filename), width, height)
}
func writeSource(t testing.TB, fs *hugofs.Fs, filename, content string) { func writeSource(t testing.TB, fs *hugofs.Fs, filename, content string) {
writeToFs(t, fs.Source, filename, content) writeToFs(t, fs.Source, filename, content)
} }