Merge pull request #5282 from patricknelson/issue-5279-director-code-quality-master

FIX for #5279 Addressing only a few PSR-2 items in one file, but primarily targeting Director::is_https() and invalid URL's.
This commit is contained in:
Damian Mooyman 2016-04-11 08:48:35 +12:00
commit fcaa579572
2 changed files with 113 additions and 111 deletions

View File

@ -349,7 +349,11 @@ class Director implements TemplateGlobalProvider {
$_SERVER['REQUEST_URI'] = Director::baseURL() . $urlWithQuerystring; $_SERVER['REQUEST_URI'] = Director::baseURL() . $urlWithQuerystring;
$request = new SS_HTTPRequest($httpMethod, $url, $getVars, $postVars, $body); $request = new SS_HTTPRequest($httpMethod, $url, $getVars, $postVars, $body);
if($headers) foreach($headers as $k => $v) $request->addHeader($k, $v); if ($headers) {
foreach($headers as $k => $v) {
$request->addHeader($k, $v);
}
}
// Pre-request filtering // Pre-request filtering
// @see issue #2517 // @see issue #2517
@ -548,11 +552,15 @@ class Director implements TemplateGlobalProvider {
return Director::protocol() . $_SERVER['HTTP_HOST']; return Director::protocol() . $_SERVER['HTTP_HOST'];
} else { } else {
global $_FILE_TO_URL_MAPPING; global $_FILE_TO_URL_MAPPING;
if(Director::is_cli() && isset($_FILE_TO_URL_MAPPING)) $errorSuggestion = ' You probably want to define '. if (Director::is_cli() && isset($_FILE_TO_URL_MAPPING)) {
$errorSuggestion = ' You probably want to define ' .
'an entry in $_FILE_TO_URL_MAPPING that covers "' . Director::baseFolder() . '"'; 'an entry in $_FILE_TO_URL_MAPPING that covers "' . Director::baseFolder() . '"';
else if(Director::is_cli()) $errorSuggestion = ' You probably want to define $_FILE_TO_URL_MAPPING in '. } elseif (Director::is_cli()) {
$errorSuggestion = ' You probably want to define $_FILE_TO_URL_MAPPING in ' .
'your _ss_environment.php as instructed on the "sake" page of the doc.silverstripe.com wiki'; 'your _ss_environment.php as instructed on the "sake" page of the doc.silverstripe.com wiki';
else $errorSuggestion = ""; } else {
$errorSuggestion = "";
}
user_error("Director::protocolAndHost() lacks sufficient information - HTTP_HOST not set." user_error("Director::protocolAndHost() lacks sufficient information - HTTP_HOST not set."
. $errorSuggestion, E_USER_WARNING); . $errorSuggestion, E_USER_WARNING);
@ -576,10 +584,8 @@ class Director implements TemplateGlobalProvider {
* @return bool * @return bool
*/ */
public static function is_https() { public static function is_https() {
$return = false;
// See https://en.wikipedia.org/wiki/List_of_HTTP_header_fields // See https://en.wikipedia.org/wiki/List_of_HTTP_header_fields
// See https://support.microsoft.com/?kbID=307347 // See https://support.microsoft.com/en-us/kb/307347
$headerOverride = false; $headerOverride = false;
if (TRUSTED_PROXY) { if (TRUSTED_PROXY) {
$headers = (defined('SS_TRUSTED_PROXY_PROTOCOL_HEADER')) ? array(SS_TRUSTED_PROXY_PROTOCOL_HEADER) : null; $headers = (defined('SS_TRUSTED_PROXY_PROTOCOL_HEADER')) ? array(SS_TRUSTED_PROXY_PROTOCOL_HEADER) : null;
@ -597,18 +603,16 @@ class Director implements TemplateGlobalProvider {
} }
if ($protocol = Config::inst()->get('Director', 'alternate_protocol')) { if ($protocol = Config::inst()->get('Director', 'alternate_protocol')) {
$return = ($protocol == 'https'); return ($protocol == 'https');
} elseif ($headerOverride) { } elseif ($headerOverride) {
$return = true; return true;
} elseif ((!empty($_SERVER['HTTPS']) && $_SERVER['HTTPS'] != 'off')) { } elseif ((!empty($_SERVER['HTTPS']) && $_SERVER['HTTPS'] != 'off')) {
$return = true; return true;
} elseif (isset($_SERVER['SSL'])) { } elseif (isset($_SERVER['SSL'])) {
$return = true; return true;
} else { } else {
$return = false; return false;
} }
return $return;
} }
/** /**
@ -699,11 +703,9 @@ class Director implements TemplateGlobalProvider {
// If we are already looking at baseURL, return '' (substr will return false) // If we are already looking at baseURL, return '' (substr will return false)
if ($url == $base1) { if ($url == $base1) {
return ''; return '';
} } elseif (substr($url, 0, strlen($base1)) == $base1) {
else if(substr($url,0,strlen($base1)) == $base1) {
return substr($url, strlen($base1)); return substr($url, strlen($base1));
} } elseif (substr($base1, -1) == "/" && $url == substr($base1, 0, -1)) {
else if(substr($base1,-1)=="/" && $url == substr($base1,0,-1)) {
// Convert http://www.mydomain.com/mysitedir to '' // Convert http://www.mydomain.com/mysitedir to ''
return ""; return "";
} }

View File

@ -164,7 +164,7 @@ class ParameterConfirmationToken {
// Are we http or https? Replicates Director::is_https() without its dependencies/ // Are we http or https? Replicates Director::is_https() without its dependencies/
$proto = 'http'; $proto = 'http';
// See https://en.wikipedia.org/wiki/List_of_HTTP_header_fields // See https://en.wikipedia.org/wiki/List_of_HTTP_header_fields
// See https://support.microsoft.com/?kbID=307347 // See https://support.microsoft.com/en-us/kb/307347
$headerOverride = false; $headerOverride = false;
if(TRUSTED_PROXY) { if(TRUSTED_PROXY) {
$headers = (defined('SS_TRUSTED_PROXY_PROTOCOL_HEADER')) ? array(SS_TRUSTED_PROXY_PROTOCOL_HEADER) : null; $headers = (defined('SS_TRUSTED_PROXY_PROTOCOL_HEADER')) ? array(SS_TRUSTED_PROXY_PROTOCOL_HEADER) : null;