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 helpers to recognize protocol-relative URLs. #2059

Merged
merged 2 commits into from
Dec 8, 2012
Merged

Fix URL helpers to recognize protocol-relative URLs. #2059

merged 2 commits into from
Dec 8, 2012

Conversation

aaronadamsCA
Copy link
Contributor

While most of CodeIgniter supports protocol-relative URLs, a few URL helpers do not.

Most notably, redirect('//www.facebook.com/aaronadams') led my browser to https://aaronadams.ca/index.php/www.facebook.com/aaronadams.

In this commit, I have fixed the header() helper, along with the anchor() and anchor_popup() helpers, to be compatible with protocol-relative URLs.

Signed-off-by: Aaron Adams aaron@aaronadams.ca

{
$uri = site_url($uri);
}
$uri = preg_match('!^(\w+:)?//! i', $uri) ? $uri : site_url($uri);
Copy link
Contributor

Choose a reason for hiding this comment

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

A nitpick, but could you remove that space between the delimiter and the 'i' flag?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will do! It seemed strange to me as well, I just didn't want to change more than what was necessary.

@narfbg
Copy link
Contributor

narfbg commented Dec 7, 2012

Nice addition @aaronadamsTO. Apart from my comment on the diff - could you add a changelog entry for this?

@aaronadamsCA
Copy link
Contributor Author

All done! Let me know if I did anything wrong – it's my first pull request, and I screwed up at least a couple of times (commit without -s, pull origin without --rebase). Pretty sure it's all fixed now.

{
$uri = site_url($uri);
}
$uri = preg_match('!^(\w+:)?//!i', $uri) ? $uri : site_url($uri);
Copy link
Contributor

Choose a reason for hiding this comment

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

Will do! It seemed strange to me as well, I just didn't want to change more than what was necessary.

Hm ... now that I look more carefully - you did change more that it was needed.
The old code only modifies $uri if needed, while your code will always re-assign its value (even if it will assign to its own value).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fair – I was just going for consistency with the previous code, but I suppose that _is_ a separate issue. Will change!

Signed-off-by: Aaron Adams <aaron@aaronadams.ca>
…helpers do not.

Most notably, redirect('//www.facebook.com/aaronadams') led my browser to https://aaronadams.ca/index.php/www.facebook.com/aaronadams.

In this commit, I have fixed the header() helper, along with the anchor() and anchor_popup() helpers, to be compatible with protocol-relative URLs.

Signed-off-by: Aaron Adams <aaron@aaronadams.ca>
@aaronadamsCA
Copy link
Contributor Author

All cleaned up!

narfbg added a commit that referenced this pull request Dec 8, 2012
Fix URL helpers to recognize protocol-relative URLs.
@narfbg narfbg merged commit 545a7c8 into bcit-ci:develop Dec 8, 2012
nonchip pushed a commit to nonchip/CodeIgniter that referenced this pull request Jun 29, 2013
Fix URL helpers to recognize protocol-relative URLs.
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.

2 participants