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(url_for/full_url_for): do not process absolute url #147

Merged
merged 1 commit into from Dec 2, 2019

Conversation

@curbengh
Copy link
Contributor

curbengh commented Dec 2, 2019

Noticed an edge case while testing hexojs/hexo-renderer-marked#124

const ctx = {
  config: {
    url: 'http://example.com',
    root: '/goal/'
  }
}

console.log(url_for.call(ctx, 'http://example.com/hash/bob.html'))
// /goal/http:/example.com/hash/bob.html

ReExp.test() is slightly faster than String.startsWith()/String.endsWith() and easier to add a new pattern.

@curbengh curbengh requested a review from hexojs/core Dec 2, 2019
@coveralls

This comment has been minimized.

Copy link

coveralls commented Dec 2, 2019

Coverage Status

Coverage increased (+0.02%) to 95.286% when pulling 9ed74b0 on curbengh:internal-url into ea43fba on hexojs:master.

@@ -4,7 +4,8 @@ const { parse, URL } = require('url');
const encodeURL = require('./encode_url');

function fullUrlForHelper(path = '/') {
if (path.startsWith('//')) return path;
const pathRegex = /^(\/\/|http(s)?:)/;

This comment has been minimized.

Copy link
@curbengh

curbengh Dec 2, 2019

Author Contributor

not sure if ignore case flag is needed, I don't mind adding it. I benchmarked the flag and I didn't notice any difference.

@SukkaW
SukkaW approved these changes Dec 2, 2019
Copy link
Member

SukkaW left a comment

LGTM!

@SukkaW SukkaW merged commit dc5543b into hexojs:master Dec 2, 2019
3 checks passed
3 checks passed
Travis CI - Pull Request Build Passed
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
coverage/coveralls Coverage increased (+0.02%) to 95.286%
Details
@curbengh curbengh deleted the curbengh:internal-url branch Dec 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.