Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix relURL with leading slash when baseURL includes a subdirectory #10002

Merged
merged 1 commit into from
Jun 13, 2022
Merged

Fix relURL with leading slash when baseURL includes a subdirectory #10002

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions helpers/pathspec_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ func TestNewPathSpecFromConfig(t *testing.T) {
v.Set("uglyURLs", true)
v.Set("canonifyURLs", true)
v.Set("paginatePath", "side")
v.Set("baseURL", "http://base.com")
v.Set("baseURL", "http://base.com/foo")
v.Set("themesDir", "thethemes")
v.Set("layoutDir", "thelayouts")
v.Set("workingDir", "thework")
Expand All @@ -53,7 +53,10 @@ func TestNewPathSpecFromConfig(t *testing.T) {
c.Assert(p.Language.Lang, qt.Equals, "no")
c.Assert(p.PaginatePath, qt.Equals, "side")

c.Assert(p.BaseURL.String(), qt.Equals, "http://base.com")
c.Assert(p.BaseURL.String(), qt.Equals, "http://base.com/foo")
c.Assert(p.BaseURLString, qt.Equals, "http://base.com/foo")
c.Assert(p.BaseURLNoPathString, qt.Equals, "http://base.com")

c.Assert(p.ThemesDir, qt.Equals, "thethemes")
c.Assert(p.WorkingDir, qt.Equals, "thework")
}
25 changes: 14 additions & 11 deletions helpers/url.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,17 +103,11 @@ func (p *PathSpec) AbsURL(in string, addLanguage bool) string {
}

if url.IsAbs() || strings.HasPrefix(in, "//") {
// It is already absolute, return it as is.
return in
}

var baseURL string
if strings.HasPrefix(in, "/") {
u := p.BaseURL.URL()
u.Path = ""
baseURL = u.String()
} else {
baseURL = p.BaseURL.String()
}
baseURL := p.getBaseURLRoot(in)

if addLanguage {
prefix := p.GetLanguagePrefix()
Expand All @@ -140,13 +134,22 @@ func (p *PathSpec) AbsURL(in string, addLanguage bool) string {
}
}
}

return paths.MakePermalink(baseURL, in).String()
}

// RelURL creates a URL relative to the BaseURL root.
// Note: The result URL will not include the context root if canonifyURLs is enabled.
func (p *PathSpec) getBaseURLRoot(path string) string {
if strings.HasPrefix(path, "/") {
// Treat it as relative to the server root.
return p.BaseURLNoPathString
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like a regression to me. p.BaseURLNoPathString is not the p.BaseURL we used to return, therefore the path is stripped here. relURL in turn will output an invalid path, which doesn't contain the path (/foo in helpers/pathspec_test.go)

} else {
// Treat it as relative to the baseURL.
return p.BaseURLString
}
}

func (p *PathSpec) RelURL(in string, addLanguage bool) string {
baseURL := p.BaseURL.String()
baseURL := p.getBaseURLRoot(in)
canonifyURLs := p.CanonifyURLs
if (!strings.HasPrefix(in, baseURL) && strings.HasPrefix(in, "http")) || strings.HasPrefix(in, "//") {
return in
Expand Down
24 changes: 17 additions & 7 deletions helpers/url_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
"strings"
"testing"

qt "github.com/frankban/quicktest"
"github.com/gohugoio/hugo/hugofs"
"github.com/gohugoio/hugo/langs"
)
Expand Down Expand Up @@ -59,6 +60,7 @@ func TestAbsURL(t *testing.T) {
}

func doTestAbsURL(t *testing.T, defaultInSubDir, addLanguage, multilingual bool, lang string) {
c := qt.New(t)
v := newTestCfg()
v.Set("multilingual", multilingual)
v.Set("defaultContentLanguage", "en")
Expand All @@ -69,6 +71,10 @@ func doTestAbsURL(t *testing.T, defaultInSubDir, addLanguage, multilingual bool,
baseURL string
expected string
}{
// Issue 9994
{"foo/bar", "https://example.org/foo/", "https://example.org/foo/MULTIfoo/bar"},
{"/foo/bar", "https://example.org/foo/", "https://example.org/MULTIfoo/bar"},

{"/test/foo", "http://base/", "http://base/MULTItest/foo"},
{"/" + lang + "/test/foo", "http://base/", "http://base/" + lang + "/test/foo"},
{"", "http://base/ace/", "http://base/ace/MULTI"},
Expand Down Expand Up @@ -113,9 +119,8 @@ func doTestAbsURL(t *testing.T, defaultInSubDir, addLanguage, multilingual bool,
} else {
expected = strings.Replace(expected, "MULTI", "", 1)
}
if output != expected {
t.Fatalf("Expected %#v, got %#v\n", expected, output)
}

c.Assert(output, qt.Equals, expected)
}
}

Expand All @@ -132,6 +137,7 @@ func TestRelURL(t *testing.T) {
}

func doTestRelURL(t *testing.T, defaultInSubDir, addLanguage, multilingual bool, lang string) {
c := qt.New(t)
v := newTestCfg()
v.Set("multilingual", multilingual)
v.Set("defaultContentLanguage", "en")
Expand All @@ -143,13 +149,18 @@ func doTestRelURL(t *testing.T, defaultInSubDir, addLanguage, multilingual bool,
canonify bool
expected string
}{

// Issue 9994
{"/foo/bar", "https://example.org/foo/", false, "MULTI/foo/bar"},
{"foo/bar", "https://example.org/foo/", false, "/fooMULTI/foo/bar"},

{"/test/foo", "http://base/", false, "MULTI/test/foo"},
{"/" + lang + "/test/foo", "http://base/", false, "/" + lang + "/test/foo"},
{lang + "/test/foo", "http://base/", false, "/" + lang + "/test/foo"},
{"test.css", "http://base/sub", false, "/subMULTI/test.css"},
{"test.css", "http://base/sub", true, "MULTI/test.css"},
{"/test/", "http://base/", false, "MULTI/test/"},
{"/test/", "http://base/sub/", false, "/subMULTI/test/"},
{"test/", "http://base/sub/", false, "/subMULTI/test/"},
{"/test/", "http://base/sub/", true, "MULTI/test/"},
{"", "http://base/ace/", false, "/aceMULTI/"},
{"", "http://base/ace", false, "/aceMULTI"},
Expand Down Expand Up @@ -189,9 +200,8 @@ func doTestRelURL(t *testing.T, defaultInSubDir, addLanguage, multilingual bool,
expected = strings.Replace(expected, "MULTI", "", 1)
}

if output != expected {
t.Errorf("[%d][%t] Expected %#v, got %#v\n", i, test.canonify, expected, output)
}
c.Assert(output, qt.Equals, expected, qt.Commentf("[%d] %s", i, test.input))

}
}

Expand Down
15 changes: 12 additions & 3 deletions hugolib/paths/paths.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ type Paths struct {
Cfg config.Provider

BaseURL
BaseURLString string
BaseURLNoPathString string

// If the baseURL contains a base path, e.g. https://example.com/docs, then "/docs" will be the BasePath.
BasePath string
Expand Down Expand Up @@ -145,10 +147,17 @@ func New(fs *hugofs.Fs, cfg config.Provider) (*Paths, error) {
}
}

var baseURLString = baseURL.String()
var baseURLNoPath = baseURL.URL()
baseURLNoPath.Path = ""
var baseURLNoPathString = baseURLNoPath.String()

p := &Paths{
Fs: fs,
Cfg: cfg,
BaseURL: baseURL,
Fs: fs,
Cfg: cfg,
BaseURL: baseURL,
BaseURLString: baseURLString,
BaseURLNoPathString: baseURLNoPathString,

DisablePathToLower: cfg.GetBool("disablePathToLower"),
RemovePathAccents: cfg.GetBool("removePathAccents"),
Expand Down