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.

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: encoding of quotes in Page Sharer URL #2726

Closed
wants to merge 2 commits into from

Conversation

cbdm
Copy link

@cbdm cbdm commented May 25, 2022

Purpose

Change the way url and title are encoded for sharing posts. Originally htmlEscape was used; however, it only escapes few characters and it breaks the generated urls.

For example, if a post title was "What's in today?", the quote ' would be encoded as ' which broke the link.
Besides that, htmlEscape only tried to fix few characters.

So instead of using htmlEscape we should use urlquery which encodes all characters that are required in the string.
However, urlquery encodes + as %2B and (i.e., space) as +:
https://cs.opensource.google/go/go/+/refs/tags/go1.18.2:src/net/url/url.go;drc=7791e934c882fd103357448aee0fd577b20013ce;l=285
Because of this mailto links were still having trouble with + and spaces, so we re-encode the "+spaces" into %20, so they decode correctly for mailto.

@netlify
Copy link

netlify bot commented May 25, 2022

Deploy Preview for hugo-portfolio-theme ready!

Name Link
🔨 Latest commit 67942fd
🔍 Latest deploy log https://app.netlify.com/sites/hugo-portfolio-theme/deploys/62f8f4d78175d50008bcdc52
😎 Deploy Preview https://deploy-preview-2726--hugo-portfolio-theme.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@netlify
Copy link

netlify bot commented May 25, 2022

Deploy Preview for markdown-slides ready!

Name Link
🔨 Latest commit 67942fd
🔍 Latest deploy log https://app.netlify.com/sites/markdown-slides/deploys/62f8f4d747e888000871260a
😎 Deploy Preview https://deploy-preview-2726--markdown-slides.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@gcushen
Copy link
Collaborator

gcushen commented May 25, 2022

Thanks for spotting this issue with using Hugo's htmlEscape and proposing using urlquery instead.

Note: the urlquery function appears to be an undocumented Hugo feature as well as a Go function: gohugoio/hugoDocs#1627 (comment)

@cbdm
Copy link
Author

cbdm commented May 25, 2022

Yep, I couldn't find anything on https://gohugo.io/functions/
Then when I was implementing this same functionality I saw the link for the go built-ins available (https://pkg.go.dev/text/template#hdr-Functions) and urlquery was there :)

EDIT: just saw you said it's also an undocumented Hugo feature! That I didn't know haha

@rodrigoalcarazdelaosa
Copy link
Contributor

Does this fix this issue without doing anything extra?

@cbdm
Copy link
Author

cbdm commented May 26, 2022

Does this fix this issue without doing anything extra?

Yes! As far as I tested that should be enough to fix that.

@gcushen gcushen changed the title Replace htmlEscape with urlquery to fix sharing urls. fix: encoding of quotes in Page Sharer URL Aug 14, 2022
@gcushen gcushen closed this in 22e2a7c Aug 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants