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.

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

`#` not escaped in Permalink when in title value #4926

Closed
tcg opened this Issue Jul 10, 2018 · 9 comments

Comments

Projects
None yet
6 participants
@tcg

tcg commented Jul 10, 2018

Initially posed a question in the forum: https://discourse.gohugo.io/t/hash-pound-sign-not-escaped-in-title-permalink-breaking-urls/12825

This seems really strange to me as a default behavior, and feels like a bug. Allow me to asplain?

I have a post title, like tcg's series of posts #1.

The Permalinks configuration examples in the docs contain the following example:

post = "/:year/:month/:title/"

... which lead me to believe that this would pretty much automatically work for my not-unreasonable post titles. ("work" here meaning "escape the URL for .Permalink and filename value automatically).

In the forum, maiki suggested that I might use :slug as a workaround, meaning that I would need to always manually provide :slug.

Also, the aforementioned docs says this about :slug:

the content’s slug (or title if no slug is provided in the front matter)

Meaning that it will always fall back to :title if I fail to provide a slug. Which seems to me like I always have to be sure to manually provide a URL-friendly value... as a human, writing a document. That doesn't feel right.

I think this is a bug. (Sadly, I am not yet familiar enough with Go to find a spot and try to offer a PR for the expected behavior).

I am willing to be wrong, and understand I may be Doing It Wrong.

EDIT:

Using versions 0.42.2 and 0.43, on macOS

@frenck

This comment has been minimized.

frenck commented Jul 12, 2018

Confirmed, same issue on my machine.
Causes Netlify to fail as well:

9:37:37 PM: failed during stage 'deploying site': Invalid filename 'blog/1/01/01/home-assistant-podcast-#9/index.html'. Deployed filenames cannot contain # or ? characters

I've even used :slug, which renders the same issue.

@moorereason

This comment has been minimized.

Contributor

moorereason commented Jul 12, 2018

Assuming this would be fixed by #4388.

@moorereason moorereason added the Bug label Jul 12, 2018

@tcg

This comment has been minimized.

tcg commented Aug 18, 2018

Great news! Hope to see it in soon!

@tcg

This comment has been minimized.

tcg commented Sep 5, 2018

I saw #4388 was merged, and I am assuming it made it into v0.48, is that correct?

On the site where I've run into this the most, I'm not seeing this fixed, if that's the case. Or has that merge not made it into a release? 🤷‍♂️ Just wanted to get this closed if it was fixed, but I'm not sure if that's the case.

@Jos512

This comment has been minimized.

Jos512 commented Sep 5, 2018

On the site where I've run into this the most, I'm not seeing this fixed, if that's the case. Or has that merge not made it into a release?

According to the release notes it's included in Hugo 0.48. The issue is the third bullet point in the 'Fixes' part of the 0.48 release notes.

@onedrawingperday

This comment has been minimized.

Contributor

onedrawingperday commented Sep 5, 2018

It seems that this issue was not closed when #4388 was merged.

I can reproduce in Hugo 0.48

In the meantime @tcg you can always remove the hash # from the Permalink string in your templates with the replaceRE function.

@moorereason moorereason self-assigned this Sep 5, 2018

@tcg

This comment has been minimized.

tcg commented Sep 5, 2018

@onedrawingperday Thanks for the suggestion. I think I mentioned previously how I'm working around it. Specifically on one site, I am manually specifying the url value for the posts in question, to work around .Permalink containing a bad value (when set to :title).

I'm of the opinion that the Theme code shouldn't necessarily concern itself with fixing permalink values, etc.

moorereason added a commit to moorereason/hugo that referenced this issue Sep 6, 2018

hugolib: Normalize permalink path segments
When constructing permalinks, ensure that path segments are normalized
with PathSpec.MakeSegment instead of PathSpec.URLize.

Fixes gohugoio#4926

moorereason added a commit to moorereason/hugo that referenced this issue Sep 6, 2018

hugolib: Normalize permalink path segments
When constructing permalinks, ensure that path segments are normalized
with PathSpec.MakeSegment instead of PathSpec.URLize.

Fixes gohugoio#4926

moorereason added a commit to moorereason/hugo that referenced this issue Sep 6, 2018

hugolib: Normalize permalink path segments
When constructing permalinks, ensure that path segments are normalized
with PathSpec.MakeSegment instead of PathSpec.URLize.

Fixes gohugoio#4926

@bep bep closed this in #5170 Sep 14, 2018

bep added a commit that referenced this issue Sep 14, 2018

hugolib: Normalize permalink path segments
When constructing permalinks, ensure that path segments are normalized
with PathSpec.MakeSegment instead of PathSpec.URLize.

Fixes #4926

@bep bep added this to the v0.49 milestone Sep 14, 2018

@bep bep reopened this Sep 18, 2018

@moorereason

This comment has been minimized.

Contributor

moorereason commented Sep 20, 2018

Status Update:

We committed PR #5170 to fix this issue, but it was reverted due to the fact that it broke stuff (see #5223). There seems to be some confusion in #5223 about testing against @onedrawingperday's test repo. I don't think I ever tested #5170 against that repo. My testing of that repo was against PR #4388.

I'm hoping to get some free time soon to take another look at these issues.

@onedrawingperday

This comment has been minimized.

Contributor

onedrawingperday commented Sep 20, 2018

I know that #5170 was not tested against my test repo (I never requested it and also I had removed the repo from GitHub).

Anyway it's a good thing that I am on the latest Hugo DEV so that I can see if PRs break things before they make it into a release.

I will keep an eye on this issue and any PR that will address it.

moorereason added a commit to moorereason/hugo that referenced this issue Sep 21, 2018

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 gohugoio#5223). Tests have been added to
check for this case.

Fixes gohugoio#4926

@bep bep modified the milestones: v0.49, v0.50 Sep 24, 2018

@bep bep closed this in #5245 Oct 3, 2018

bep added a commit that referenced this issue Oct 3, 2018

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

bep added a commit to bep/hugo that referenced this issue Oct 3, 2018

helpers: Consolidate MakgeSegment vs MakePathSanitized
In short:

* Avoid double tolower in MakeSegment
* Use MakePathSanitized for taxonomies in pageToPermalinkTitle; this matches what MakeSegment does.
* Move the "double hyphen and space" logic into UnicodeSanitize

The last bullet may be slightly breaking for some that now does not get the "--" in some URLs, but we need to reduce the amount of URL logic.

See gohugoio#4926

bep added a commit to bep/hugo that referenced this issue Oct 3, 2018

helpers: Consolidate MakeSegment vs MakePathSanitized
In short:

* Avoid double tolower in MakeSegment
* Use MakePathSanitized for taxonomies in pageToPermalinkTitle; this matches what MakeSegment does.
* Move the "double hyphen and space" logic into UnicodeSanitize

The last bullet may be slightly breaking for some that now does not get the "--" in some URLs, but we need to reduce the amount of URL logic.

See gohugoio#4926

bep added a commit to bep/hugo that referenced this issue Oct 3, 2018

helpers: Consolidate MakeSegment vs MakePathSanitized
In short:

* Avoid double tolower in MakeSegment
* Use MakePathSanitized for taxonomies in pageToPermalinkTitle; this matches what MakeSegment does.
* Move the "double hyphen and space" logic into UnicodeSanitize

The last bullet may be slightly breaking for some that now does not get the "--" in some URLs, but we need to reduce the amount of URL logic.

See gohugoio#4926

bep added a commit to bep/hugo that referenced this issue Oct 3, 2018

helpers: Consolidate MakeSegment vs MakePathSanitized
In short:

* Avoid double tolower in MakeSegment
* Use MakePathSanitized for taxonomies in pageToPermalinkTitle; this matches what MakeSegment does.
* Move the "double hyphen and space" logic into UnicodeSanitize

The last bullet may be slightly breaking for some that now does not get the "--" in some URLs, but we need to reduce the amount of URL logic.

See gohugoio#4926

bep added a commit to bep/hugo that referenced this issue Oct 3, 2018

helpers: Consolidate MakeSegment vs MakePathSanitized
In short:

* Avoid double tolower in MakeSegment
* Use MakePathSanitized for taxonomies in pageToPermalinkTitle; this matches what MakeSegment does.
* Move the "double hyphen and space" logic into UnicodeSanitize

The last bullet may be slightly breaking for some that now does not get the "--" in some URLs, but we need to reduce the amount of URL logic.

See gohugoio#4926

bep added a commit to bep/hugo that referenced this issue Oct 3, 2018

helpers: Consolidate MakeSegment vs MakePathSanitized
In short:

* Avoid double tolower in MakeSegment
* Use MakePathSanitized for taxonomies in pageToPermalinkTitle; this matches what MakeSegment does.
* Move the "double hyphen and space" logic into UnicodeSanitize

The last bullet may be slightly breaking for some that now does not get the "--" in some URLs, but we need to reduce the amount of URL logic.

See gohugoio#4926

bep added a commit that referenced this issue Oct 3, 2018

helpers: Consolidate MakeSegment vs MakePathSanitized
In short:

* Avoid double tolower in MakeSegment
* Use MakePathSanitized for taxonomies in pageToPermalinkTitle; this matches what MakeSegment does.
* Move the "double hyphen and space" logic into UnicodeSanitize

The last bullet may be slightly breaking for some that now does not get the "--" in some URLs, but we need to reduce the amount of URL logic.

See #4926
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment