Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

not a valid URL error when using 3.1.2 #16

Closed
sevfurneaux opened this issue Dec 3, 2018 · 12 comments
Closed

not a valid URL error when using 3.1.2 #16

sevfurneaux opened this issue Dec 3, 2018 · 12 comments

Comments

@sevfurneaux
Copy link

sevfurneaux commented Dec 3, 2018

I'm receiving this error when hitting http using a Restricted URL Segments of /:

yii\base\ErrorException: / is not a valid URL in /app/vendor/yiisoft/yii2/base/ErrorException.php:43

Looks like this relates to the previous closed issue: #9

However, this is still happening for me with 3.1.2?

@selvinortiz
Copy link
Collaborator

@sevfurneaux Thank you for the report.

Do you mind sharing your Patrol config and what your siteUrl is for the environment that this is happening on?

@sevfurneaux
Copy link
Author

@selvinortiz Thanks for the super quick response 😌

Sure, here is the patrol.php:

<?php

return [
    '*' => [
        'sslRoutingEnabled' => true
    ],
    'dev' => [
        'sslRoutingEnabled' => false
    ]
];

And here is general.php settings for staging and production:

    // Staging environment settings
    'staging' => [
        // Base site URL
        'siteUrl' => null,

        'devMode' => true,
    ],

    // Production environment settings
    'production' => [
        // Base site URL
        'siteUrl' => null,
    ],

Assume the null values are the issue? This hasn’t been a problem with previous versions.

@selvinortiz
Copy link
Collaborator

Thank you, @sevfurneaux

Let me take a look at it tonight or tomorrow and I'll get back to you.

@sevfurneaux
Copy link
Author

Thanks @selvinortiz 😌

@selvinortiz
Copy link
Collaborator

@sevfurneaux I apologize for the delay.

I went through a few different test cases to attempt to reproduce the issue you outlined above, using your Patrol and General configs.

I did not succeed. Patrol continue to redirect as expected.

If you haven't already, can you disable the plugin to make sure that the error does not occur with Patrol disabled or uninstalled?

@sevfurneaux
Copy link
Author

sevfurneaux commented Dec 5, 2018

Thanks for trying @selvinortiz. Yes, I had to disable the plugin to remove error. Enabling the plugin shows the error again. Seems like it is plugin specific 🤔

Would there be an issue with the Craft version? Currently on 3.0.20.

@jeffaglenn
Copy link

I'm having this issue as well. Same .env config as above. Running Craft 3.0.34 & Patrol 3.1.2. Sorry I commented on the closed issue (#9) before I saw this one. It seems there is now a / in front of the URL resulting in this error:

2018-12-11 15:14:46 [-][-][-][error][yii\base\ErrorException:1] yii\base\ErrorException: /https://thecitizenapts.com is not a valid URL in /home/the-citizen/webapps/the-citizen/vendor/yiisoft/yii2/base/ErrorException.php:43
Stack trace:
#0 /home/the-citizen/webapps/the-citizen/vendor/selvinortiz/patrol/src/services/PatrolService.php(134): selvinortiz\patrol\services\PatrolService->forceSsl()
#1 /home/the-citizen/webapps/the-citizen/vendor/selvinortiz/patrol/src/services/PatrolService.php(65): selvinortiz\patrol\services\PatrolService->handleSslRouting()
#2 /home/the-citizen/webapps/the-citizen/vendor/selvinortiz/patrol/src/Patrol.php(47): selvinortiz\patrol\services\PatrolService->watch()
#3 /home/the-citizen/webapps/the-citizen/vendor/yiisoft/yii2/base/BaseObject.php(109): selvinortiz\patrol\Patrol->init()
#4 /home/the-citizen/webapps/the-citizen/vendor/yiisoft/yii2/base/Module.php(158): yii\base\BaseObject->__construct(Array)
#5 /home/the-citizen/webapps/the-citizen/vendor/craftcms/cms/src/base/Plugin.php(113): yii\base\Module->__construct('patrol', Object(craft\web\Application), Array)
#6 [internal function]: craft\base\Plugin->__construct('patrol', Object(craft\web\Application), Array)
#7 /home/the-citizen/webapps/the-citizen/vendor/yiisoft/yii2/di/Container.php(383): ReflectionClass->newInstanceArgs(Array)
#8 /home/the-citizen/webapps/the-citizen/vendor/yiisoft/yii2/di/Container.php(156): yii\di\Container->build('selvinortiz\\pat...', Array, Array)
#9 /home/the-citizen/webapps/the-citizen/vendor/yiisoft/yii2/BaseYii.php(349): yii\di\Container->get('selvinortiz\\pat...', Array, Array)
#10 /home/the-citizen/webapps/the-citizen/vendor/craftcms/cms/src/services/Plugins.php(787): yii\BaseYii::createObject(Array, Array)
#11 /home/the-citizen/webapps/the-citizen/vendor/craftcms/cms/src/services/Plugins.php(199): craft\services\Plugins->createPlugin('patrol', Array)
#12 /home/the-citizen/webapps/the-citizen/vendor/craftcms/cms/src/base/ApplicationTrait.php(1148): craft\services\Plugins->loadPlugins()
#13 /home/the-citizen/webapps/the-citizen/vendor/craftcms/cms/src/web/Application.php(109): craft\web\Application->_postInit()
#14 /home/the-citizen/webapps/the-citizen/vendor/yiisoft/yii2/base/BaseObject.php(109): craft\web\Application->init()
#15 /home/the-citizen/webapps/the-citizen/vendor/yiisoft/yii2/base/Application.php(206): yii\base\BaseObject->__construct(Array)
#16 /home/the-citizen/webapps/the-citizen/vendor/craftcms/cms/src/web/Application.php(97): yii\base\Application->__construct(Array)
#17 [internal function]: craft\web\Application->__construct(Array)
#18 /home/the-citizen/webapps/the-citizen/vendor/yiisoft/yii2/di/Container.php(383): ReflectionClass->newInstanceArgs(Array)
#19 /home/the-citizen/webapps/the-citizen/vendor/yiisoft/yii2/di/Container.php(156): yii\di\Container->build('craft\\web\\Appli...', Array, Array)
#20 /home/the-citizen/webapps/the-citizen/vendor/yiisoft/yii2/BaseYii.php(349): yii\di\Container->get('craft\\web\\Appli...', Array, Array)
#21 /home/the-citizen/webapps/the-citizen/vendor/craftcms/cms/bootstrap/bootstrap.php(252): yii\BaseYii::createObject(Array)
#22 /home/the-citizen/webapps/the-citizen/vendor/craftcms/cms/bootstrap/web.php(42): require('/home/the-citiz...')
#23 /home/the-citizen/webapps/the-citizen/web/index.php(20): require('/home/the-citiz...')
#24 {main}

@selvinortiz
Copy link
Collaborator

Ha! I ended up commenting on that one too 😂

Anyway, I'm looking into it.

@selvinortiz
Copy link
Collaborator

@jeffaglenn @sevfurneaux I'm having a hard time testing on my side and I'd love your help.

When you are able, would you please replace the code in PatrolService.php with the patched versionn below and let me know if this fixes the issue for you?

If it does, I will cut a new release.

<?php
namespace selvinortiz\patrol\services;

use yii\web\HttpException;
use yii\base\ErrorException;

use Craft;
use craft\base\Component;
use craft\helpers\UrlHelper;

use selvinortiz\patrol\Patrol;
use selvinortiz\patrol\models\SettingsModel;

/**
 * Class PatrolService
 *
 * @package selvinortiz\patrol\services
 */
class PatrolService extends Component
{
    /**
     * @var SettingsModel
     */
    protected $settings;

    /**
     * Key/value pairs used when parsing restricted areas like {cpTrigger}
     *
     * @var array
     */
    protected $dynamicParams;

    public function init()
    {
        parent::init();

        $this->settings = Patrol::getInstance()->getSettings();
    }

    public function allow()
    {
        $requestToken = Craft::$app->request->getQueryParam('access');
        $requestingIp = Craft::$app->request->getUserIp();

        if (!empty($requestToken) && in_array($requestToken, $this->settings->maintenanceModeAccessTokens))
        {
            if (!in_array($requestingIp, $this->settings->maintenanceModeAuthorizedIps))
            {
                $this->settings->maintenanceModeAuthorizedIps[] = $requestingIp;

                Craft::$app->plugins->savePluginSettings(Patrol::getInstance(), $this->settings->getAttributes());
            }
        }
    }

    /**
     * @throws ErrorException
     * @throws HttpException
     * @throws \yii\base\Exception
     * @throws \yii\base\InvalidConfigException
     */
    public function watch()
    {
        $this->handleRouting();
        $this->handleSslRouting();
        $this->handleMaintenanceMode();
    }

    /**
     * @throws ErrorException
     * @throws \yii\base\Exception
     * @throws \yii\base\InvalidConfigException
     */
    public function handleRouting()
    {
        if (!is_string($this->settings->primaryDomain) || empty($this->settings->primaryDomain))
        {
            return;
        }

        $primaryDomain   = mb_strtolower(trim($this->settings->primaryDomain));
        $requestedDomain = mb_strtolower(trim(Craft::$app->request->getHostName()));

        if (empty($primaryDomain) || $primaryDomain === '*')
        {
            return;
        }

        if ($primaryDomain === $requestedDomain)
        {
            return;
        }

        $http = Craft::$app->request->getIsSecureConnection() ? 'https://' : 'http://';

        Craft::$app->request->setHostInfo($http.$primaryDomain);
        Craft::$app->response->redirect(Craft::$app->request->getUrl(), $this->settings->redirectStatusCode);
    }

    /**
     * Forces SSL based on restricted URLs
     * The environment settings take priority over those defined in the control panel
     *
     * @return bool
     *
     * @throws ErrorException
     * @throws \yii\base\Exception
     * @throws \yii\base\InvalidConfigException
     */
    public function handleSslRouting()
    {
        if ($this->settings->sslRoutingEnabled && !Craft::$app->getRequest()->getIsConsoleRequest())
        {
            $requestedUrl   = Craft::$app->request->getUrl();
            $restrictedUrls = $this->settings->sslRoutingRestrictedUrls;

            if (!Craft::$app->request->isSecureConnection)
            {
                foreach ($restrictedUrls as $restrictedUrl)
                {
                    // Parse dynamic variables like /{cpTrigger}
                    if (stripos($restrictedUrl, '{') !== false)
                    {
                        $restrictedUrl = Craft::$app->view->renderObjectTemplate(
                            $restrictedUrl,
                            $this->getDynamicParams()
                        );
                    }

                    $restrictedUrl = '/'.ltrim($restrictedUrl, '/');

                    if (stripos($requestedUrl, $restrictedUrl) === 0)
                    {
                        $this->forceSsl();
                    }
                }

                return true;
            }
        }

        return false;
    }

    /**
     * Returns a list of dynamic parameters and their values that can be used in restricted area settings
     *
     * @return array
     *
     * @throws \yii\base\Exception
     */
    protected function getDynamicParams()
    {
        if (is_null($this->dynamicParams))
        {
            $this->dynamicParams = [
                'siteUrl'       => UrlHelper::siteUrl(),
                'cpTrigger'     => Craft::$app->config->general->cpTrigger,
                'actionTrigger' => Craft::$app->config->general->actionTrigger,
            ];
        }

        return $this->dynamicParams;
    }

    /**
     * Redirects to the HTTPS version of the requested URL
     *
     * @throws ErrorException
     * @throws \yii\base\Exception
     * @throws \yii\base\InvalidConfigException
     */
    protected function forceSsl()
    {
        // Define and trim base URL
        $baseUrl = trim($this->settings->sslRoutingBaseUrl);

        // Parse dynamic params in SSL routing URL
        if (mb_stripos($baseUrl, '{') !== false)
        {
            $baseUrl = Craft::$app->view->renderObjectTemplate(
                $this->settings->sslRoutingBaseUrl,
                $this->getDynamicParams()
            );
        }

        // Protect against invalid base URL
        if (empty($baseUrl) || $baseUrl == '/')
        {
            $baseUrl = trim(Craft::$app->request->hostInfo);
        }

        // Define and trim URI to append to the base URL
        $requestUri = trim(Craft::$app->request->getUrl());

        // Base URL should now be set to 'http://domain.ext' or 'http://domain.ext/'
        // Request URI can could be empty or '/page?key=val'

        // Define the final URL formed by the host portion and the URI portion
        $url = sprintf('%s%s', rtrim($baseUrl, '/'), $requestUri);

        // Ensure https is used
        $url = str_replace('http:', 'https:', $url);

        if (!filter_var($url, FILTER_VALIDATE_URL))
        {
            throw new ErrorException(
                Craft::t('patrol', '{url} is not a valid URL', ['url' => $url])
            );
        }

        Craft::$app->response->redirect(
            $url,
            $this->settings->redirectStatusCode
        );
    }

    /**
     * Restricts accessed based on authorizedIps
     *
     * @return bool
     *
     * @throws HttpException
     * @throws \yii\base\InvalidConfigException
     */
    public function handleMaintenanceMode()
    {
        // Authorize logged in admins on the fly
        if ($this->doesCurrentUserHaveAccess())
        {
            return true;
        }

        if (Craft::$app->request->isSiteRequest && $this->settings->maintenanceModeEnabled)
        {
            $requestingIp   = $this->getRequestingIp();
            $authorizedIps  = $this->settings->maintenanceModeAuthorizedIps;
            $maintenanceUrl = $this->settings->maintenanceModePageUrl;

            if ($maintenanceUrl == Craft::$app->request->getUrl())
            {
                return true;
            }

            if (empty($authorizedIps))
            {
                $this->forceRedirect($maintenanceUrl);
            }

            if (is_array($authorizedIps) && count($authorizedIps))
            {
                if (in_array($requestingIp, $authorizedIps))
                {
                    return true;
                }

                foreach ($authorizedIps as $authorizedIp)
                {
                    $authorizedIp = str_replace('*', '', $authorizedIp);

                    if (stripos($requestingIp, $authorizedIp) === 0)
                    {
                        return true;
                    }
                }

                $this->forceRedirect($maintenanceUrl);
            }
        }
    }

    /**
     * Returns whether or not the current user has access during maintenance mode
     */
    protected function doesCurrentUserHaveAccess()
    {
        // Admins have access by default
        if (Craft::$app->user->getIsAdmin())
        {
            return true;
        }

        // User has the right permission
        if (Craft::$app->user->checkPermission(Patrol::MAINTENANCE_MODE_BYPASS_PERMISSION))
        {
            return true;
        }

        return false;
    }

    /**
     * Ensures that we get the right IP address even if behind CloudFlare or most proxies
     *
     * @return string
     */
    public function getRequestingIp()
    {
        if (isset($_SERVER['HTTP_CF_CONNECTING_IP'])) {

            return $_SERVER['HTTP_CF_CONNECTING_IP'];
        }
        elseif (isset($_SERVER['HTTP_X_FORWARDED_FOR']))
        {
            return isset($_SERVER['HTTP_X_FORWARDED_FOR']);
        }
        elseif (isset($_SERVER['HTTP_X_REAL_IP']))
        {
            return isset($_SERVER['HTTP_X_REAL_IP']);
        }
        else
        {
            return $_SERVER['REMOTE_ADDR'];
        }
    }

    /**
     * @param string $redirectTo
     *
     * @throws HttpException
     */
    protected function forceRedirect($redirectTo = '')
    {
        if (empty($redirectTo))
        {
            $this->runDefaultBehavior();
        }

        Craft::$app->response->redirect($redirectTo, $this->settings->redirectStatusCode);
    }

    /**
     * @throws HttpException
     */
    protected function runDefaultBehavior()
    {
        throw new HttpException($this->settings->maintenanceModeResponseStatusCode);
    }

    /**
     * Parses authorizedIps to ensure they are valid even when created from a string
     *
     * @param array|string $ips
     *
     * @return array
     */
    public function parseAuthorizedIps($ips)
    {
        $ips = trim($ips);

        if (is_string($ips) && !empty($ips))
        {
            $ips = explode(PHP_EOL, $ips);
        }

        return $this->filterOutArrayValues(
            $ips, function($val) {
            return preg_match('/^[0-9\.\*]{5,15}$/i', $val);
        }
        );
    }

    /**
     * Filters out array values by using a custom filter
     *
     * @param array|string|null $values
     * @param callable|\Closure $filter
     * @param bool              $preserveKeys
     *
     * @return array
     */
    protected function filterOutArrayValues($values = null, \Closure $filter = null, $preserveKeys = false)
    {
        $data = [];

        if (is_array($values) && count($values))
        {
            foreach ($values as $key => $value)
            {
                $value = trim($value);

                if (!empty($value))
                {
                    if (is_callable($filter) && $filter($value))
                    {
                        $data[$key] = $value;
                    }
                }
            }

            if (!$preserveKeys)
            {
                $data = array_values($data);
            }
        }

        return $data;
    }

    /**
     * Parse restricted areas to ensure they are valid even when created from a string
     *
     * @param array|string $areas
     *
     * @return array
     */
    public function parseRestrictedAreas($areas)
    {
        if (is_string($areas) && !empty($areas))
        {
            $areas = trim($areas);
            $areas = explode(PHP_EOL, $areas);
        }

        return $this->filterOutArrayValues(
            $areas,
            function($val) {
                $valid = preg_match('/^[\/\{\}a-z\_\-\?\=]{1,255}$/i', $val);

                if (!$valid)
                {
                    return false;
                }

                return true;
            }
        );
    }
}

@jeffaglenn
Copy link

This seems to have fixed it for me. I've only tried with one site so far but this is looking good! Thank you so much for your help and the plugin!

@selvinortiz
Copy link
Collaborator

@jeffaglenn Good to hear!

I believe that one of the PRs that I merged was incorrectly trimming and inserting with ltrim() rather than rtrim(). However, that piece of code was getting a little smelly, so I decided to refactor it.

I'll cut a new release today, tomorrow at the latest.

selvinortiz added a commit that referenced this issue Dec 19, 2018
- Fix issue #16 where `sslRoutingBaseUrl` was exploding when set to `empty` or `/`
@selvinortiz
Copy link
Collaborator

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants