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

check if relative URL contains a colon #5069

Merged
merged 1 commit into from Jul 13, 2016

Conversation

Projects
None yet
6 participants
@Crunch09
Member

Crunch09 commented Jul 8, 2016

fixes #2834

screen shot 2016-07-08 at 20 35 47

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Jul 9, 2016

Member

Yes! Perfect! This LGTM. @jekyll/core, what say you?

Member

parkr commented Jul 9, 2016

Yes! Perfect! This LGTM. @jekyll/core, what say you?

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Jul 11, 2016

Member

@envygeeks do you want to take a look at this, or?

Member

parkr commented Jul 11, 2016

@envygeeks do you want to take a look at this, or?

@parkr parkr added fix ux labels Jul 11, 2016

@envygeeks

This comment has been minimized.

Show comment
Hide comment
@envygeeks

envygeeks Jul 13, 2016

Contributor

LGTM! :shipit:

Contributor

envygeeks commented Jul 13, 2016

LGTM! :shipit:

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Jul 13, 2016

Member

@jekyllbot: merge +minor

Member

parkr commented Jul 13, 2016

@jekyllbot: merge +minor

@jekyllbot jekyllbot merged commit 1f36673 into jekyll:master Jul 13, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

jekyllbot added a commit that referenced this pull request Jul 13, 2016

stevecheckoway added a commit to stevecheckoway/jekyll that referenced this pull request Jul 24, 2016

@akoeplinger

This comment has been minimized.

Show comment
Hide comment
@akoeplinger

akoeplinger Aug 23, 2016

Contributor

This breaks using URLs with colons in the jekyll-redirect-from plugin: jekyll/jekyll-redirect-from#119

Contributor

akoeplinger commented Aug 23, 2016

This breaks using URLs with colons in the jekyll-redirect-from plugin: jekyll/jekyll-redirect-from#119

@icai

This comment has been minimized.

Show comment
Hide comment
@icai

icai Sep 9, 2016

I don't think this is a good idea which check including colons(:) or not.
should use blacklist(year, month...) with colons to check URL, but not if sanitized_url.include?(":")

#5261
#2834

icai commented Sep 9, 2016

I don't think this is a good idea which check including colons(:) or not.
should use blacklist(year, month...) with colons to check URL, but not if sanitized_url.include?(":")

#5261
#2834

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