? ? Failure

User tests: Successful: Unsuccessful:

avatar Yiannistaos
Yiannistaos
21 Jul 2020

This fix is when a plugin or template developer would like to create a custom 404 error page by using different names than the default "404.php".

For example, without this fix, the below example works only if in the template "protostar-test" I have the file "error.php" only. But, it's not working if I rename the file to "another-error404-page.php".

public static function renderCustom404ErrorPage($error)
{
    $expectedClass = PHP_MAJOR_VERSION >= 7 ? '\Throwable' : '\Exception';
    $isException   = $error instanceof $expectedClass;

    // In PHP 5, the $error object should be an instance of \Exception; PHP 7 should be a Throwable implementation
    if ($isException)
    {
        try
        {
            // Try to log the error, but don't let the logging cause a fatal error
            try
            {
                \JLog::add(
                    sprintf(
                        'Uncaught %1$s of type %2$s thrown. Stack trace: %3$s',
                        $expectedClass,
                        get_class($error),
                        $error->getTraceAsString()
                    ),
                    \JLog::CRITICAL,
                    'error'
                );
            }
            catch (\Throwable $e)
            {
                // Logging failed, don't make a stink about it though
            }
            catch (\Exception $e)
            {
                // Logging failed, don't make a stink about it though
            }

            $app = \JFactory::getApplication();

            // If site is offline and it's a 404 error, just go to index (to see offline message, instead of 404)
            if ($error->getCode() == '404' && $app->get('offline') == 1)
            {
                $app->redirect('index.php');
            }

            $attributes = array(
                'charset'   => 'utf-8',
                'lineend'   => 'unix',
                'tab'       => "\t",
                'language'  => 'en-GB',
                'direction' => 'ltr',
            );

            // If there is a \JLanguage instance in \JFactory then let's pull the language and direction from its metadata
            if (\JFactory::$language)
            {
                $attributes['language']  = \JFactory::getLanguage()->getTag();
                $attributes['direction'] = \JFactory::getLanguage()->isRtl() ? 'rtl' : 'ltr';
            }

            $document = \JDocument::getInstance('error', $attributes);

            if (!$document)
            {
                // We're probably in an CLI environment
                jexit($error->getMessage());
            }


            // Get the current template from the application
            $template = 'protostar-test'; //$app->getTemplate();
            $app->set('theme', 'protostar-test');

            // Push the error object into the document
            $document->setError($error);

            if (ob_get_contents())
            {
                ob_end_clean();
            }

            $document->setTitle(\JText::_('ERROR') . ': ' . $error->getCode());

            $document_options = array(
                'template'  => $app->get('theme'),
                'file'		=> $app->get('themeFile', 'another-error404-page.php'),
                'directory' => JPATH_THEMES,
                'params'  	=> $app->get('themeParams'),
                'debug'     => JDEBUG,
            );

            $document->parse($document_options);

            $data = $document->render(
                false,
                $document_options
            );

            // Do not allow cache
            $app->allowCache(false);

            // If nothing was rendered, just use the message from the Exception
            if (empty($data))
            {
                $data = $error->getMessage();
            }

            $app->setBody($data);

            echo $app->toString();

            $app->close(0);

            // This return is needed to ensure the test suite does not trigger the non-Exception handling below
            return;
        }
        catch (\Throwable $e)
        {
            // Pass the error down
        }
        catch (\Exception $e)
        {
            // Pass the error down
        }
    }

    // This isn't an Exception, we can't handle it.
    if (!headers_sent())
    {
        header('HTTP/1.1 500 Internal Server Error');
    }

    $message = 'Error';

    if ($isException)
    {
        // Make sure we do not display sensitive data in production environments
        if (ini_get('display_errors'))
        {
            $message .= ': ';

            if (isset($e))
            {
                $message .= $e->getMessage() . ': ';
            }

            $message .= $error->getMessage();
        }
    }

    echo $message;

    jexit(1);
}
avatar Yiannistaos Yiannistaos - open - 21 Jul 2020
avatar Yiannistaos Yiannistaos - change - 21 Jul 2020
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 21 Jul 2020
Category Libraries
avatar ReLater
ReLater - comment - 21 Jul 2020

Could you please provide a code snippet where we see how and where we have to define the 'file' inside $params. I have absolutely no idea how to test this pr in a "living Joomla".

BTW: I personally wouldn't use 'file' but something more specific like 'errorTmpl'

(?????)
And I think this is a new feature. Joomla 3 has a feature lock. You should rebase the pr for Joomla 4.
(?????)

avatar Fedik
Fedik - comment - 21 Jul 2020

I suspect you doing some hack to use own exception handler? :)

You also have to check $file exists, and always fallback to error.php.

But, if all that you want is to have an error layout per "status code", then you can just improve ErrorDocuemnt to check these files:
eg "Error 500" => search for error-500.php => or fallback to error.php

And as @ReLater already said, you need to provide a testing instruction. Little code snippet that anyone can copy/paste and run.

avatar zero-24
zero-24 - comment - 21 Jul 2020

hmm RIPS also just reported an possible Remote File Inclusion issue following this changes here. So before this here get finalised & merged please tag the JSST for a final review. Thanks.

avatar Quy
Quy - comment - 6 Sep 2020
avatar SniperSister
SniperSister - comment - 6 Sep 2020

@zero-24

hmm RIPS also just reported an possible Remote File Inclusion issue following this changes here. So before this here get finalised & merged please tag the JSST for a final review. Thanks.

$params is not user supplied input, so can't see a RFI here. Do I miss anything?

avatar zero-24
zero-24 - comment - 6 Sep 2020

$params is not user supplied input, so can't see a RFI here. Do I miss anything?

$params comes form here ($this->docOptions): https://github.com/joomla/joomla-cms/blob/staging/libraries/src/Application/CMSApplication.php#L1044

$this->docOptions['file'] comes from here: https://github.com/joomla/joomla-cms/blob/staging/libraries/src/Application/CMSApplication.php#L1016

And themeFile can be set via an input parameter:

$file = $input->getCmd('tmpl', 'index');
if ($component === 'com_login')
{
$file = 'login';
}
$this->set('themeFile', $file . '.php');

So that option can be usersupplied (but still be CMD filtered) atleast in the core. I guess thats the thing that RIPS found. The question now is whether this is a issue or not.

avatar zero-24
zero-24 - comment - 12 Jun 2022

as mentiond above we still have not got any info how to use it. i will close this PR.

avatar zero-24 zero-24 - change - 12 Jun 2022
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2022-06-12 17:53:23
Closed_By zero-24
Labels Added: ? ?
Removed: ?
avatar zero-24 zero-24 - close - 12 Jun 2022

Add a Comment

Login with GitHub to post a comment