Posted by bfroehle on April 1, 2011 at 10:39pm
9 followers
| Project: | Drupal core |
| Version: | 7.x-dev |
| Component: | base system |
| Category: | bug report |
| Priority: | minor |
| Assigned: | Unassigned |
| Status: | closed (fixed) |
| Issue tags: | documentation, needs backport to D7, Novice |
Issue Summary
The comment in url_is_external() is wrong:
<?php
function url_is_external($path) {
$colonpos = strpos($path, ':');
// Only call the slow drupal_strip_dangerous_protocols() if $path contains a
// ':' before any / ? or #.
return $colonpos !== FALSE && !preg_match('![/?#]!', substr($path, 0, $colonpos)) && drupal_strip_dangerous_protocols($path) == $path;
}
?>In fact, we only call drupal_strip_dangerous_protocols() if $path does not contain a / ? or # before the first ':'.
A good explanation is in drupal_strip_dangerous_protocols()
// If a colon is preceded by a slash, question mark or hash, it cannot
// possibly be part of the URL scheme. This must be a relative URL, which
// inherits the (safe) protocol of the base document.
Comments
#1
Setting the priority to minor, as it's just a bug in the comments.
#2
I'm giving a shot at it.
#3
This is ready to go. Thanks, franz!
This review was done at DrupalCamp LA 2011 during a live session. You're famous!
#4
cool. =)
#5
+++ b/includes/common.inc@@ -2197,8 +2197,9 @@ function url($path = NULL, array $options = array()) {
+ // Avoid calling drupal_strip_dangerous_protocols() if there is any ¶
trailing white space
20 days to next Drupal core point release.
#6
#7
The last submitted patch, 1113702-url_is_external_comment.patch, failed testing.
#8
#6: 1113702-url_is_external_comment.patch queued for re-testing.
#9
The last submitted patch, 1113702-url_is_external_comment.patch, failed testing.
#10
#6: 1113702-url_is_external_comment.patch queued for re-testing.
#11
White space issue has been fixed.
#12
Shouldn't it be 'a URL' instead of 'an URL'?
#13
It seems to depend on how you pronnounce it: http://grammar.ccc.commnet.edu/grammar/abbreviations.htm (Using articles with abbreviations and acronyms) and http://grammar.ccc.commnet.edu/grammar/grammarlogs1/grammarlogs254.htm (Ctrl + F to searh for "an URL")
In my case I should've used "a URL" but I don't know about the standards adopted - if any - for this.
#14
However, my sources could be outdated, I checked a free online dictionary and pronunciation is like "yoo-arr-ell" (and not something ugly as "earl") so it should be "a URL".
#15
Simple change, doesn't need testing again.
#16
Committed to 8.x, back to 7.x for webchick.
#17
Committed and pushed to 7.x. Thanks!
#18
Automatically closed -- issue fixed for 2 weeks with no activity.