-
Notifications
You must be signed in to change notification settings - Fork 7.6k
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
Conversation
{ | ||
$uri = site_url($uri); | ||
} | ||
$uri = preg_match('!^(\w+:)?//! i', $uri) ? $uri : site_url($uri); |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
Nice addition @aaronadamsTO. Apart from my comment on the diff - could you add a changelog entry for this? |
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 ( |
{ | ||
$uri = site_url($uri); | ||
} | ||
$uri = preg_match('!^(\w+:)?//!i', $uri) ? $uri : site_url($uri); |
There was a problem hiding this comment.
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).
There was a problem hiding this comment.
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>
All cleaned up! |
Fix URL helpers to recognize protocol-relative URLs.
Fix URL helpers to recognize protocol-relative URLs.
While most of CodeIgniter supports protocol-relative URLs, a few URL helpers do not.
Most notably,
redirect('//www.facebook.com/aaronadams')
led my browser tohttps://aaronadams.ca/index.php/www.facebook.com/aaronadams
.In this commit, I have fixed the
header()
helper, along with theanchor()
andanchor_popup()
helpers, to be compatible with protocol-relative URLs.Signed-off-by: Aaron Adams aaron@aaronadams.ca