add error page footer, standardize some request things, stop throwing exception on 400s

controller/Actions.class.php

@@ -1,10 +1,5 @@
 <?php
 
-/**
- * Description of Actions
- *
- * @author jeremy
- */
 class Actions
 {
   public static function param($key, $default = null)
@@ -12,20 +7,6 @@ class Actions
     return $_POST[$key] ?? $_GET[$key] ?? $default;
   }
 
-  //this is dumb
-  protected static function returnError($error)
-  {
-    throw new ErrorException($error);
-  }
-
-  protected static function returnErrorIf($condition, $error)
-  {
-    if ($condition)
-    {
-      Actions::returnError($error);
-    }
-  }
-
   protected static function isForRobot()
   {
     $bots = [

controller/Controller.class.php

@@ -69,7 +69,7 @@ class Controller
     {
       Response::setStatus(405);
       Response::setHeader('Allow', implode(', ', $e->getAllowedMethods()));
-      return ['page/404'];
+      return ['page/405'];
     }
   }
 

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'] ??
+    return static::getHttpHeader('X-Real-Ip') ??
-      (isset($_SERVER['HTTP_X_FORWARDED_FOR']) ? trim(explode(',',$_SERVER['HTTP_X_FORWARDED_FOR'])[0]) :
+           (static::getHttpHeader('X-Forwarded-For') ? trim(explode(',', static::getHttpHeader('X-Forwarded-For'))[0]) :
-        ($_SERVER['REMOTE_ADDR'] ?? ''));
+             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;
   }
 }

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');
+    $selectedStatus = Request::getParam('status', 'available');
-    $selectedCategory = static::param('category');
+    $selectedCategory = Request::getParam('category');
 
     $filters = array_filter([
       'category' => $selectedCategory && isset($allCategories[$selectedCategory]) ? $selectedCategory : null,

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,
       ]);

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');
+    $email = Request::getParam('email');
-    $code  = static::param('code');
+    $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', '')
     ];
   }
 

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();

controller/action/OpsActions.class.php

@@ -1,25 +1,36 @@
 <?php
 
-/**
- * Description of OpsActions
- *
- * @author jeremy
- */
 class OpsActions extends Actions
 {
-  public static function executePostCommit()
+  public static function executePostCommit(): array
   {
-    $payload = json_decode($_REQUEST['payload'], true);
+    $payload = Request::getParam('payload');
+    if (!$payload)
+    {
+      return NavActions::execute400(['error' => '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('', '');
+      list($algo, $hash) = explode('=', Request::getHttpHeader('X-Hub-Signature'), 2) + ['', ''];
-      Actions::returnErrorIf(!in_array($algo, hash_algos(), TRUE), 'Invalid hash algorithm "' . $algo . '"');
+      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');
+      $secret  = Config::get('github_key');
-      Actions::returnErrorIf($hash !== hash_hmac($algo, $rawPost, $secret), 'Hash does not match. "' . $secret . '"' . ' algo: ' . $algo . '$');
+      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) . '_' .
+      $name = substr(trim(urldecode($_POST['date'])), 0, 20) . '_' .
-              substr(trim(urldecode($_POST['hash'])),0,20) . '_' .
+              substr(trim(urldecode($_POST['hash'])), 0, 20) . '_' .
-              substr(trim(urldecode($_POST['sys'])),0,50)  . '_' .
+              substr(trim(urldecode($_POST['sys'])), 0, 50) . '_' .
-              substr(trim(urldecode($_POST['type'])),0,20);
+              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);

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

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);

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');

view/template/nav/_errorFooter.php

@@ -0,0 +1,5 @@
+<div>
+  <?php echo __('page.error_contact_us', [
+    '%email%' => '<a class="link-primary" href="mailto:' . Config::HELP_CONTACT_EMAIL . '">' . Config::HELP_CONTACT_EMAIL . '</a>'
+  ]) ?>
+</div>

view/template/page/400.php

@@ -0,0 +1,8 @@
+<?php echo View::render('nav/_header', ['isDark' => false]) ?>
+<main>
+  <div class="content">
+    <h1>{{page.badrequest}}</h1>
+    <p><?php echo $error ?? __('page.badrequest_details') ?></p>
+    <?php echo View::render('nav/_errorFooter') ?>
+  </div>
+</main>

view/template/page/404.php

@@ -3,5 +3,6 @@
   <div class="content">
     <h1>{{page.notfound}}</h1>
     <p>{{page.funnier}}</p>
+    <?php echo View::render('nav/_errorFooter') ?>
   </div>
 </main>

view/template/page/405.php

@@ -2,6 +2,7 @@
 <main>
   <div class="content">
     <h1>{{page.badmethod}}</h1>
-    <p>{{page.funnier}}</p>
+    <p>{{page.badmethod_details}}</p>
+    <?php echo View::render('nav/_errorFooter') ?>
   </div>
 </main>

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));