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

url_for helper doesn't work for mailto link #122

Closed
lookis opened this issue Nov 7, 2019 · 6 comments · Fixed by #130
Closed

url_for helper doesn't work for mailto link #122

lookis opened this issue Nov 7, 2019 · 6 comments · Fixed by #130

Comments

@lookis
Copy link

lookis commented Nov 7, 2019

https://github.com/hexojs/hexo-util/blob/master/lib/url_for.js#L28-L31

it will return origin path when met external link like http://example.com, but will not return origin path for mailto: someone@example.com. and lead to an issue when relative_link was set.

url_for('/category') => /relative_link/category <= OK!
url_for('http://example.com') => http://example.com <= OK!
url_for('mailto:someone@example.com') => /relative_link/mailto:someone@example.com <= Ops!

@SukkaW
Copy link
Member

SukkaW commented Nov 9, 2019

There is no better way to fix this. We might need to hardcode some whitelist protocols inside hexo-util, just as one of contributor of nodejs gives: nodejs/node#28482

https://github.com/nodejs/node/blob/49a5fa0d8b15cc50df833848bfc01d2260930dfd/lib/internal/url.js#L340-L348

@SukkaW
Copy link
Member

SukkaW commented Nov 10, 2019

@curbengh Any idea?

In isExternalLink() we use new URL(path)).origin === 'null' and consider mailto: javascript: links as an internal link, so that they won't be processed by external_link filter. But when at url_for and full_url_for, it requires to consider them as external links.

Currently I only come up with the idea of whitelist protocols.

@curbengh
Copy link
Contributor

curbengh commented Nov 10, 2019

https://github.com/hexojs/hexo-util/blob/master/lib/url_for.js#L28-L31

The workaround is necessary to support path with a semicolon.

However, since encodeURL() never support path with semicolon, I'm ok dropping it from url_for() and full_url_for().

I'm assuming mailto: is more common than path with semicolon.

@curbengh
Copy link
Contributor

When I created #114, I assumed url_for() would only be used for relative url, not data url (like mailto:) nor absolute url, which is why I made a leeway to support path with semicolon.

In relation to theme-next/hexo-theme-next#1260, I wonder why url_for(mailto:) exists (my apology, I don't understand 社交栏 and 检查单). I assume the theme dev also share my assumption.

Btw, this issue is still going to be fixed.

@lookis
Copy link
Author

lookis commented Nov 10, 2019

thx @curbengh
社交栏 means social account such as facebook or twitter account
检查单 is a check list for user to submit an issue

and I am not the author of theme-next, maybe they wrap every link with url_for :(

@stevenjoezhang
Copy link
Member

https://github.com/theme-next/hexo-theme-next/blob/45148b265df579977a18bde6a908cf4977a75797/scripts/helpers/next-url.js#L16

theme-next is using a custom helper next_url. It is used to generate an <a> tag with a custom attributes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants