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(is_external_link): handle invalid url #183

Merged
merged 2 commits into from
Mar 6, 2020

Conversation

SukkaW
Copy link
Member

@SukkaW SukkaW commented Feb 16, 2020

Details can be found at hexojs/hexo#4130

This PR introduce a try block and treat invalid URL as internal link (return false).

@SukkaW SukkaW requested review from curbengh and a team February 16, 2020 13:35
@coveralls
Copy link

coveralls commented Feb 16, 2020

Coverage Status

Coverage remained the same at 96.989% when pulling 2a9a5ba on SukkaW:fix-is-external-filter into 7e5633a on hexojs:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.009%) to 97.008% when pulling 12bdb3a on SukkaW:fix-is-external-filter into 7e084de on hexojs:master.

@SukkaW
Copy link
Member Author

SukkaW commented Feb 16, 2020

According to travis ci build, no performance impact detected.

@SukkaW
Copy link
Member Author

SukkaW commented Feb 16, 2020

is-absolute-url is also added for better performance.

Since it is a widely used packages on npm so I believe it is reliable enough.

@SukkaW SukkaW mentioned this pull request Feb 16, 2020
lib/is_external_link.js Outdated Show resolved Hide resolved
@SukkaW SukkaW force-pushed the fix-is-external-filter branch from d99e87b to 0ace953 Compare February 17, 2020 09:34
@SukkaW SukkaW requested a review from curbengh February 17, 2020 09:34
@SukkaW SukkaW force-pushed the fix-is-external-filter branch from 0ace953 to 2a9a5ba Compare February 20, 2020 19:19
@SukkaW SukkaW requested review from a team and curbengh and removed request for curbengh February 25, 2020 18:33
let data;
try {
data = new URL(input, `http://${sitehost}`);
} catch (e) { }
Copy link
Member

Choose a reason for hiding this comment

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

How about hexo.log.warn(...) to alert the user that there has an invalid URL?

Copy link
Member Author

Choose a reason for hiding this comment

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

Since hexo-util has no hexo-log's dependency, there is no way we can use hexo.log.warn in hexo-util.

@SukkaW SukkaW merged commit 2496d1f into hexojs:master Mar 6, 2020
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 this pull request may close these issues.

5 participants