Skip to content

Commit

Permalink
Have url::site() and other methods return a SafeString, just as t() a…
Browse files Browse the repository at this point in the history
…nd t2().

Benefits:
 - url::site() is often used in views and we can ensure in the url class that returned strings are indeed safe for use in HTML. Makes the list of vars of unknown safety status shorter.
 - url::site() is often used as message parameter to t() and t2(). The parameter would be HTML-escaped if it wasn't marked as safe HTML already. Makes the usage simpler / shorter.
  • Loading branch information
andyst committed Aug 29, 2009
1 parent 020281d commit 1d63345
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 5 deletions.
24 changes: 23 additions & 1 deletion modules/gallery/helpers/MY_url.php
Expand Up @@ -30,7 +30,8 @@ static function site($uri, $protocol=false) {
if ($parts[0] == "albums" || $parts[0] == "photos") {
$uri = model_cache::get("item", $parts[1])->relative_path();
}
return parent::site($uri . $query, $protocol);
$url = parent::site($uri . $query, $protocol);
return SafeString::of($url)->mark_html_safe();
}

static function parse_url() {
Expand Down Expand Up @@ -99,4 +100,25 @@ static function abs_site($path) {
static function abs_current($qs=false) {
return self::abs_site(url::current($qs));
}

public static function base($index=false, $protocol=false) {
$url = parent::base($index, $protocol);
return SafeString::of($url)->mark_html_safe();
}

public static function current($qs=false) {
$url = parent::current($qs);
return SafeString::of($url)->mark_html_safe();
}

public static function file($file, $index=false) {
$url = parent::file($file, $index);
return SafeString::of($url)->mark_html_safe();
}

public static function merge(array $arguments) {
$url = parent::merge($arguments);
return SafeString::of($url)->mark_html_safe();
}

}
24 changes: 20 additions & 4 deletions modules/gallery/tests/Xss_Security_Test.php
Expand Up @@ -126,7 +126,23 @@ public function find_unescaped_variables_in_views_test() {
$token_number++;
$token = $tokens[$token_number];
}
}
} else if ($token[1] == "url") {
// url methods return a SafeString
if (self::_token_matches(array(T_DOUBLE_COLON, "::"), $tokens, $token_number + 1) &&
self::_token_matches(array(T_STRING), $tokens, $token_number + 2) &&
in_array($tokens[$token_number + 2][1],
array("site", "current", "base", "file", "abs_site", "abs_current",
"abs_file", "merge")) &&
self::_token_matches("(", $tokens, $token_number + 3)) {
$frame->is_safestring(true);

$method = $tokens[$token_number + 2][1];
$frame->expr_append("::$method(");

$token_number += 3;
$token = $tokens[$token_number];
}
}
} else if ($frame && $token[0] == T_OBJECT_OPERATOR) {
$frame->expr_append($token[1]);

Expand Down Expand Up @@ -155,8 +171,9 @@ public function find_unescaped_variables_in_views_test() {
}
}

// Generate the report.
/*
* Generate the report
*
* States for uses of < ? = X ? >:
* JS_XSS:
* In <script> block
Expand All @@ -166,7 +183,7 @@ public function find_unescaped_variables_in_views_test() {
* X can be anything without a call to ->for_html() or ->purified_html()
* CLEAN:
* Outside <script> block:
* X = t() or t2()
* X = is SafeString (t(), t2(), url::site())
* X = * and for_html() or purified_html() is called
* Inside <script> block:
* X = * with ->for_js() or json_encode(...)
Expand All @@ -192,7 +209,6 @@ public function find_unescaped_variables_in_views_test() {
}
}
fclose($fd);
exit;

// Compare with the expected report from our golden file.
$canonical = MODPATH . "gallery/tests/xss_data.txt";
Expand Down

0 comments on commit 1d63345

Please sign in to comment.