From 2550cc09eb0374f6f9666a1cda9f94c7e805c5a7 Mon Sep 17 00:00:00 2001 From: Alex Grintsvayg Date: Tue, 6 Sep 2016 12:32:18 -0400 Subject: [PATCH] add error page footer, standardize some request things, stop throwing exception on 400s --- controller/Actions.class.php | 19 ------ controller/Controller.class.php | 2 +- controller/Request.class.php | 46 ++++++++++--- controller/action/BountyActions.class.php | 4 +- controller/action/ContentActions.class.php | 2 +- controller/action/DownloadActions.class.php | 12 ++-- controller/action/NavActions.class.php | 8 ++- controller/action/OpsActions.class.php | 64 ++++++++++++------- data/i18n/en.yaml | 6 +- .../HttpMethodNotAllowedException.class.php | 2 +- view/Response.class.php | 2 +- view/template/nav/_errorFooter.php | 5 ++ view/template/page/400.php | 8 +++ view/template/page/404.php | 1 + view/template/page/405.php | 3 +- web/index.php | 2 +- 16 files changed, 120 insertions(+), 66 deletions(-) create mode 100644 view/template/nav/_errorFooter.php create mode 100644 view/template/page/400.php diff --git a/controller/Actions.class.php b/controller/Actions.class.php index 466f5d5c..76d1dc0b 100644 --- a/controller/Actions.class.php +++ b/controller/Actions.class.php @@ -1,10 +1,5 @@ getAllowedMethods())); - return ['page/404']; + return ['page/405']; } } diff --git a/controller/Request.class.php b/controller/Request.class.php index 969e3ecd..1b8ef7cb 100644 --- a/controller/Request.class.php +++ b/controller/Request.class.php @@ -9,16 +9,39 @@ class Request protected static $method; + public static function getParam(string $key, $default = null) + { + return $_POST[$key] ?? $_GET[$key] ?? $default; + } + public static function getMethod(): string { if (!static::$method) { - $method = isset($_SERVER['REQUEST_METHOD']) ? strtoupper($_SERVER['REQUEST_METHOD']) : null; + $method = static::getHeader('REQUEST_METHOD') ? strtoupper(static::getHeader('REQUEST_METHOD')) : null; + static::$method = in_array($method, [static::GET, static::POST, static::HEAD, static::OPTIONS]) ? $method : static::GET; } return static::$method; } + protected static function getHeader(string $name, $default = null) + { + return $_SERVER[strtoupper($name)] ?? $default; + } + + public static function getHttpHeader(string $name, $default = null) + { + $header = 'HTTP_' . strtoupper(strtr($name, '-', '_')); + return isset($_SERVER[$header]) ? static::stripSlashes($_SERVER[$header]) : $default; + } + + protected static function stripSlashes($value) + { + return is_array($value) ? array_map(get_called_class() . '::stripSlashes', $value) : stripslashes($value); + } + + public static function isGet(): bool { return static::getMethod() == static::GET; @@ -36,29 +59,34 @@ class Request public static function getOriginalIp(): string { - return $_SERVER['HTTP_X_REAL_IP'] ?? - (isset($_SERVER['HTTP_X_FORWARDED_FOR']) ? trim(explode(',',$_SERVER['HTTP_X_FORWARDED_FOR'])[0]) : - ($_SERVER['REMOTE_ADDR'] ?? '')); + return static::getHttpHeader('X-Real-Ip') ?? + (static::getHttpHeader('X-Forwarded-For') ? trim(explode(',', static::getHttpHeader('X-Forwarded-For'))[0]) : + static::getHeader('REMOTE_ADDR', '')); } - + public static function getUserAgent(): string { - return $_SERVER['HTTP_USER_AGENT'] ?? ''; + return static::getHttpHeader('User-Agent') ?? ''; } public static function getHost(): string { // apparently trailing period is legal: http://www.dns-sd.org/TrailingDotsInDomainNames.html - return isset($_SERVER['HTTP_HOST']) ? rtrim($_SERVER['HTTP_HOST'], '.') : ''; + return static::getHttpHeader('Host') ? rtrim(static::getHttpHeader('Host'), '.') : ''; + } + + public static function getServerName(): string + { + return static::getHeader('SERVER_NAME'); } public static function getRelativeUri(): string { - return isset($_SERVER['REQUEST_URI']) ? parse_url($_SERVER['REQUEST_URI'], PHP_URL_PATH) : ''; + return static::getHeader('REQUEST_URI') ? parse_url(static::getHeader('REQUEST_URI'), PHP_URL_PATH) : ''; } public static function isGzipAccepted(): bool { - return isset($_SERVER['HTTP_ACCEPT_ENCODING']) && strpos(strtolower($_SERVER['HTTP_ACCEPT_ENCODING']), 'gzip') !== false; + return static::getHttpHeader('Accept-Encoding') && strpos(strtolower(static::getHttpHeader('Accept-Encoding')), 'gzip') !== false; } } \ No newline at end of file diff --git a/controller/action/BountyActions.class.php b/controller/action/BountyActions.class.php index cb227709..d483f1fa 100644 --- a/controller/action/BountyActions.class.php +++ b/controller/action/BountyActions.class.php @@ -30,8 +30,8 @@ class BountyActions extends Actions $allCategories = ['' => ''] + Post::collectMetadata($allBounties, 'category'); $allStatuses = ['' => ''] + array_merge(Post::collectMetadata($allBounties, 'status'), ['complete' => 'unavailable']); - $selectedStatus = static::param('status', 'available'); - $selectedCategory = static::param('category'); + $selectedStatus = Request::getParam('status', 'available'); + $selectedCategory = Request::getParam('category'); $filters = array_filter([ 'category' => $selectedCategory && isset($allCategories[$selectedCategory]) ? $selectedCategory : null, diff --git a/controller/action/ContentActions.class.php b/controller/action/ContentActions.class.php index 2a410a22..fa0b388d 100644 --- a/controller/action/ContentActions.class.php +++ b/controller/action/ContentActions.class.php @@ -81,7 +81,7 @@ class ContentActions extends Actions 'developer' => 'Developers', 'other' => 'Other Questions', ]); - $selectedCategory = static::param('category'); + $selectedCategory = Request::getParam('category'); $filters = array_filter([ 'category' => $selectedCategory && isset($allCategories[$selectedCategory]) ? $selectedCategory : null, ]); diff --git a/controller/action/DownloadActions.class.php b/controller/action/DownloadActions.class.php index 025ef910..29be474b 100644 --- a/controller/action/DownloadActions.class.php +++ b/controller/action/DownloadActions.class.php @@ -21,7 +21,7 @@ class DownloadActions extends Actions public static function executeGet() { - $email = static::param('e'); + $email = Request::getParam('e'); $user = []; if ($email) @@ -75,8 +75,8 @@ class DownloadActions extends Actions public static function executeSignup() { - $email = static::param('email'); - $code = static::param('code'); + $email = Request::getParam('email'); + $code = Request::getParam('code'); if (!$email || !filter_var($email, FILTER_VALIDATE_EMAIL)) { @@ -84,7 +84,7 @@ class DownloadActions extends Actions } else { - $referrerId = static::param('referrer_id'); + $referrerId = Request::getParam('referrer_id'); $failure = false; try { @@ -136,9 +136,9 @@ class DownloadActions extends Actions public static function prepareSignupPartial(array $vars) { return $vars + [ - 'defaultEmail' => static::param('e'), + 'defaultEmail' => Request::getParam('e'), 'allowInviteCode' => true, - 'referralCode' => static::param('r', '') + 'referralCode' => Request::getParam('r', '') ]; } diff --git a/controller/action/NavActions.class.php b/controller/action/NavActions.class.php index 772aa772..bf498c12 100644 --- a/controller/action/NavActions.class.php +++ b/controller/action/NavActions.class.php @@ -34,7 +34,13 @@ class NavActions extends Actions 'isDark' => true ]; } - + + public static function execute400(array $vars) + { + Response::setStatus(400); + return ['page/400', ['error' => $vars['error'] ?? null]]; + } + public static function execute404() { $uri = Request::getRelativeUri(); diff --git a/controller/action/OpsActions.class.php b/controller/action/OpsActions.class.php index 22ab35af..05252a13 100644 --- a/controller/action/OpsActions.class.php +++ b/controller/action/OpsActions.class.php @@ -1,25 +1,36 @@ 'No payload']); + } + + $payload = json_decode($payload, true); if ($payload['ref'] === 'refs/heads/master') { - Actions::returnErrorIf(!isset($_SERVER['HTTP_X_HUB_SIGNATURE']), "HTTP header 'X-Hub-Signature' is missing."); + $sig = Request::getHttpHeader('X-Hub-Signature'); + if (!$sig) + { + return NavActions::execute400(['error' => "HTTP header 'X-Hub-Signature' is missing."]); + } - list($algo, $hash) = explode('=', $_SERVER['HTTP_X_HUB_SIGNATURE'], 2) + array('', ''); - Actions::returnErrorIf(!in_array($algo, hash_algos(), TRUE), 'Invalid hash algorithm "' . $algo . '"'); + list($algo, $hash) = explode('=', Request::getHttpHeader('X-Hub-Signature'), 2) + ['', '']; + if (!in_array($algo, hash_algos(), true)) + { + return NavActions::execute400(['error' => 'Invalid hash algorithm "' . htmlspecialchars($algo) . '"']); + } $rawPost = file_get_contents('php://input'); - $secret = Config::get('github_key'); - Actions::returnErrorIf($hash !== hash_hmac($algo, $rawPost, $secret), 'Hash does not match. "' . $secret . '"' . ' algo: ' . $algo . '$'); + $secret = Config::get('github_key'); + if ($hash !== hash_hmac($algo, $rawPost, $secret)) + { + return NavActions::execute400(['error' => 'Hash does not match.']); + } file_put_contents(ROOT_DIR . '/data/writeable/NEEDS_UPDATE', ''); } @@ -27,19 +38,19 @@ class OpsActions extends Actions return [null, []]; } - public static function executeLogUpload() + public static function executeLogUpload(): array { $log = isset($_POST['log']) ? urldecode($_POST['log']) : null; if (isset($_POST['name'])) { - $name = substr(trim(urldecode($_POST['name'])),0,50); + $name = substr(trim(urldecode($_POST['name'])), 0, 50); } elseif (isset($_POST['date'])) { - $name = substr(trim(urldecode($_POST['date'])),0,20) . '_' . - substr(trim(urldecode($_POST['hash'])),0,20) . '_' . - substr(trim(urldecode($_POST['sys'])),0,50) . '_' . - substr(trim(urldecode($_POST['type'])),0,20); + $name = substr(trim(urldecode($_POST['date'])), 0, 20) . '_' . + substr(trim(urldecode($_POST['hash'])), 0, 20) . '_' . + substr(trim(urldecode($_POST['sys'])), 0, 50) . '_' . + substr(trim(urldecode($_POST['type'])), 0, 20); } else { @@ -48,17 +59,26 @@ class OpsActions extends Actions $name = preg_replace('/[^A-Za-z0-9_-]+/', '', $name); - Actions::returnErrorIf(!$log || !$name, "Required params: log, name"); + if (!$log || !$name) + { + return NavActions::execute400(['error' => "Required params: log, name"]); + } - $awsKey = Config::get('aws_log_access_key'); + $awsKey = Config::get('aws_log_access_key'); $awsSecret = Config::get('aws_log_secret_key'); - Actions::returnErrorIf(!$awsKey || !$awsSecret, "Missing AWS credentials"); + if (!$log || !$name) + { + throw new RuntimeException('Missing AWS credentials'); + } $tmpFile = tempnam(sys_get_temp_dir(), 'lbryinstalllog'); file_put_contents($tmpFile, $log); - Actions::returnErrorIf(filesize($tmpFile) > 1024*1024*2, "File is too large"); + if (filesize($tmpFile) > 1024 * 1024 * 2) + { + return NavActions::execute400(['error' => 'Log file is too large']); + } S3::$useExceptions = true; S3::setAuth($awsKey, $awsSecret); diff --git a/data/i18n/en.yaml b/data/i18n/en.yaml index ee88d4bd..767b44fa 100644 --- a/data/i18n/en.yaml +++ b/data/i18n/en.yaml @@ -93,7 +93,11 @@ news: next: Next prev: Previous page: - badmethod: HTTP method not allowed + badmethod: HTTP Method Not Allowed + badmethod_details: Did you GET when you should have POSTed? Or vice versa? + badrequest: Bad Request - Your browser sent a request that this server could not understand + badrequest_details: The client should not repeat the request without modifications. + error_contact_us: If this problem persists or requires immediate attention, please contact %email%. faq: back: Back to FAQ header: Frequently Asked Questions diff --git a/lib/routing/HttpMethodNotAllowedException.class.php b/lib/routing/HttpMethodNotAllowedException.class.php index 00a09e14..cc393a3c 100644 --- a/lib/routing/HttpMethodNotAllowedException.class.php +++ b/lib/routing/HttpMethodNotAllowedException.class.php @@ -6,7 +6,7 @@ class HttpMethodNotAllowedException extends HttpException { protected $allowedMethods; - public function __construct(array $allowedMethods, $message, $code, Exception $previous) + public function __construct(array $allowedMethods, $message = "", $code = 0, Exception $previous = null) { $this->allowedMethods = $allowedMethods; parent::__construct($message, $code, $previous); diff --git a/view/Response.class.php b/view/Response.class.php index dbfb485d..f134b780 100644 --- a/view/Response.class.php +++ b/view/Response.class.php @@ -129,7 +129,7 @@ class Response $content = static::getContent(); if (strlen($content) > 256) // not worth it for really short content { - $compressed = gzencode($content, 6); + $compressed = gzencode($content, 1); static::setContent($compressed); static::setHeader(static::HEADER_CONTENT_LENGTH, strlen($compressed)); static::setHeader(static::HEADER_CONTENT_ENCODING, 'gzip'); diff --git a/view/template/nav/_errorFooter.php b/view/template/nav/_errorFooter.php new file mode 100644 index 00000000..0eefea77 --- /dev/null +++ b/view/template/nav/_errorFooter.php @@ -0,0 +1,5 @@ +
+ '' . Config::HELP_CONTACT_EMAIL . '' + ]) ?> +
\ No newline at end of file diff --git a/view/template/page/400.php b/view/template/page/400.php new file mode 100644 index 00000000..5d45d5a7 --- /dev/null +++ b/view/template/page/400.php @@ -0,0 +1,8 @@ + false]) ?> +
+
+

{{page.badrequest}}

+

+ +
+
\ No newline at end of file diff --git a/view/template/page/404.php b/view/template/page/404.php index 3929880e..338826c3 100644 --- a/view/template/page/404.php +++ b/view/template/page/404.php @@ -3,5 +3,6 @@

{{page.notfound}}

{{page.funnier}}

+
\ No newline at end of file diff --git a/view/template/page/405.php b/view/template/page/405.php index bdb5c62e..d02260be 100644 --- a/view/template/page/405.php +++ b/view/template/page/405.php @@ -2,6 +2,7 @@

{{page.badmethod}}

-

{{page.funnier}}

+

{{page.badmethod_details}}

+
\ No newline at end of file diff --git a/web/index.php b/web/index.php index 8d30f40d..c45267cf 100644 --- a/web/index.php +++ b/web/index.php @@ -8,7 +8,7 @@ if (php_sapi_name() === 'cli-server' && is_file(__DIR__.preg_replace('#(\?.*)$#' include __DIR__ . '/../bootstrap.php'; -define('IS_PRODUCTION', $_SERVER['SERVER_NAME'] == 'lbry.io'); +define('IS_PRODUCTION', Request::getServerName() == 'lbry.io'); ini_set('display_errors', IS_PRODUCTION ? 'off' : 'on'); error_reporting(IS_PRODUCTION ? 0 : (E_ALL | E_STRICT));