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.

Sign up for GitHub

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

Jump to bottom

Choose the correct protocol in route::link when force https is disabled #24089

Merged
merged 11 commits into from Jun 5, 2019
Merged

Choose the correct protocol in route::link when force https is disabled #24089

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
f99e3da
Choose the correct protocol in route::link when force https is disabled.
HLeithner Mar 4, 2019
f549b20
Add absolute parameter to route::_, link and introduce new TLS constants
HLeithner Mar 5, 2019
e814241
cs
HLeithner Mar 5, 2019
bb54c76
Added deprecation tag
HLeithner Mar 5, 2019
1fe652e
Add since tag for constants
HLeithner Apr 25, 2019
93fe901
Merge branch 'staging' into fix-route-link-ssl
HLeithner Apr 25, 2019
4610fe4
Include namespaces
HLeithner Apr 25, 2019
05226d3
Merge branch 'staging' into fix-route-link-ssl
jeckodevelopment Apr 26, 2019
3194253
Merge branch 'staging' into fix-route-link-ssl
zero-24 Apr 27, 2019
2bf9019
Add deprecation todo
HLeithner May 1, 2019
7dba0f1
Merge branch 'staging' into fix-route-link-ssl
HLeithner May 10, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion administrator/components/com_actionlogs/helpers/actionlogs.php
View file
Open in desktop
Expand Up @@ -193,7 +193,7 @@ public static function getHumanReadableLogMessage($log, $generateLinks = true)
$messageData['extension_name'] = Text::_($messageData['extension_name']);
}

$linkMode = Factory::getApplication()->get('force_ssl', 0) >= 1 ? 1 : -1;
$linkMode = Factory::getApplication()->get('force_ssl', 0) >= 1 ? Route::TLS_FORCE : Route::TLS_IGNORE;

foreach ($messageData as $key => $value)
{
Expand Down
4 changes: 2 additions & 2 deletions administrator/components/com_messages/models/message.php
View file
Open in desktop
Expand Up @@ -359,10 +359,10 @@ public function save($data)

// Build the email subject and message
$app = JFactory::getApplication();
$linkMode = $app->get('force_ssl', 0) >= 1 ? 1 : -1;
$linkMode = $app->get('force_ssl', 0) >= 1 ? Route::TLS_FORCE : Route::TLS_IGNORE;
$sitename = $app->get('sitename');
$fromName = $fromUser->get('name');
$siteURL = JRoute::link('administrator', 'index.php?option=com_messages&view=message&message_id=' . $table->message_id, false, $linkMode);
$siteURL = JRoute::link('administrator', 'index.php?option=com_messages&view=message&message_id=' . $table->message_id, false, $linkMode, true);
$subject = html_entity_decode($table->subject, ENT_COMPAT, 'UTF-8');
$message = strip_tags(html_entity_decode($table->message, ENT_COMPAT, 'UTF-8'));

Expand Down
2 changes: 1 addition & 1 deletion administrator/components/com_privacy/models/dashboard.php
View file
Open in desktop
Expand Up @@ -117,7 +117,7 @@ public function getRequestFormPublished()
}
}

$linkMode = JFactory::getApplication()->get('force_ssl', 0) == 2 ? 1 : -1;
$linkMode = JFactory::getApplication()->get('force_ssl', 0) == 2 ? Route::TLS_FORCE : Route::TLS_IGNORE;

if (!$menuItem)
{
Expand Down
6 changes: 3 additions & 3 deletions administrator/components/com_privacy/models/request.php
View file
Open in desktop
Expand Up @@ -311,13 +311,13 @@ public function notifyUserAdminCreatedRequest($id)
{
$app = JFactory::getApplication();

$linkMode = $app->get('force_ssl', 0) == 2 ? 1 : -1;
$linkMode = $app->get('force_ssl', 0) == 2 ? Route::TLS_FORCE : Route::TLS_IGNORE;

$substitutions = array(
'[SITENAME]' => $app->get('sitename'),
'[URL]' => JUri::root(),
'[TOKENURL]' => JRoute::link('site', 'index.php?option=com_privacy&view=confirm&confirm_token=' . $token, false, $linkMode),
'[FORMURL]' => JRoute::link('site', 'index.php?option=com_privacy&view=confirm', false, $linkMode),
'[TOKENURL]' => JRoute::link('site', 'index.php?option=com_privacy&view=confirm&confirm_token=' . $token, false, $linkMode, true),
'[FORMURL]' => JRoute::link('site', 'index.php?option=com_privacy&view=confirm', false, $linkMode, true),
'[TOKEN]' => $token,
'\\n' => "\n",
);
Expand Down
6 changes: 3 additions & 3 deletions components/com_privacy/models/request.php
View file
Open in desktop
Expand Up @@ -125,13 +125,13 @@ public function createRequest($data)
{
$app = JFactory::getApplication();

$linkMode = $app->get('force_ssl', 0) == 2 ? 1 : -1;
$linkMode = $app->get('force_ssl', 0) == 2 ? Route::TLS_FORCE : Route::TLS_IGNORE;

$substitutions = array(
'[SITENAME]' => $app->get('sitename'),
'[URL]' => JUri::root(),
'[TOKENURL]' => JRoute::link('site', 'index.php?option=com_privacy&view=confirm&confirm_token=' . $token, false, $linkMode),
'[FORMURL]' => JRoute::link('site', 'index.php?option=com_privacy&view=confirm', false, $linkMode),
'[TOKENURL]' => JRoute::link('site', 'index.php?option=com_privacy&view=confirm&confirm_token=' . $token, false, $linkMode, true),
'[FORMURL]' => JRoute::link('site', 'index.php?option=com_privacy&view=confirm', false, $linkMode, true),
'[TOKEN]' => $token,
'\\n' => "\n",
);
Expand Down
12 changes: 6 additions & 6 deletions components/com_users/models/registration.php
View file
Open in desktop
Expand Up @@ -108,14 +108,14 @@ public function activate($token)
// Admin activation is on and user is verifying their email
if (($userParams->get('useractivation') == 2) && !$user->getParam('activate', 0))
{
$linkMode = $config->get('force_ssl', 0) == 2 ? 1 : -1;
$linkMode = $config->get('force_ssl', 0) == 2 ? Route::TLS_FORCE : Route::TLS_IGNORE;

// Compile the admin notification mail values.
$data = $user->getProperties();
$data['activation'] = JApplicationHelper::getHash(JUserHelper::genRandomPassword());
$user->set('activation', $data['activation']);
$data['siteurl'] = JUri::base();
$data['activate'] = JRoute::link('site', 'index.php?option=com_users&task=registration.activate&token=' . $data['activation'], false, $linkMode);
$data['activate'] = JRoute::link('site', 'index.php?option=com_users&task=registration.activate&token=' . $data['activation'], false, $linkMode, true);

$data['fromname'] = $config->get('fromname');
$data['mailfrom'] = $config->get('mailfrom');
Expand Down Expand Up @@ -471,9 +471,9 @@ public function register($temp)
if ($useractivation == 2)
{
// Set the link to confirm the user email.
$linkMode = $config->get('force_ssl', 0) == 2 ? 1 : -1;
$linkMode = $config->get('force_ssl', 0) == 2 ? Route::TLS_FORCE : Route::TLS_IGNORE;

$data['activate'] = JRoute::link('site', 'index.php?option=com_users&task=registration.activate&token=' . $data['activation'], false, $linkMode);
$data['activate'] = JRoute::link('site', 'index.php?option=com_users&task=registration.activate&token=' . $data['activation'], false, $linkMode, true);

$emailSubject = JText::sprintf(
'COM_USERS_EMAIL_ACCOUNT_DETAILS',
Expand Down Expand Up @@ -508,9 +508,9 @@ public function register($temp)
elseif ($useractivation == 1)
{
// Set the link to activate the user account.
$linkMode = $config->get('force_ssl', 0) == 2 ? 1 : -1;
$linkMode = $config->get('force_ssl', 0) == 2 ? Route::TLS_FORCE : Route::TLS_IGNORE;

$data['activate'] = JRoute::link('site', 'index.php?option=com_users&task=registration.activate&token=' . $data['activation'], false, $linkMode);
$data['activate'] = JRoute::link('site', 'index.php?option=com_users&task=registration.activate&token=' . $data['activation'], false, $linkMode, true);

$emailSubject = JText::sprintf(
'COM_USERS_EMAIL_ACCOUNT_DETAILS',
Expand Down
86 changes: 61 additions & 25 deletions libraries/src/Router/Route.php
View file
Open in desktop
Expand Up @@ -20,6 +20,21 @@
*/
class Route
{
/**
* No change, use the protocol currently used.
HLeithner marked this conversation as resolved.
Show resolved Hide resolved
*/
const TLS_IGNORE = 0;

/**
* Make URI secure using http over TLS (https).
*/
const TLS_FORCE = 1;

/**
* Make URI unsecure using plain http (http).
*/
const TLS_DISABLE = 2;

/**
* The route object so we don't have to keep fetching it.
*
Expand All @@ -31,25 +46,31 @@ class Route
/**
* Translates an internal Joomla URL to a humanly readable URL. This method builds links for the current active client.
*
* @param string $url Absolute or Relative URI to Joomla resource.
* @param boolean $xhtml Replace & by & for XML compliance.
* @param integer $ssl Secure state for the resolved URI.
* 0: (default) No change, use the protocol currently used in the request
* 1: Make URI secure using global secure site URI.
* 2: Make URI unsecure using the global unsecure site URI.
* @param string $url Absolute or Relative URI to Joomla resource.
* @param boolean $xhtml Replace & by & for XML compliance.
* @param integer $tls Secure state for the resolved URI. Use Route::TLS_* constants
* 0: (default) No change, use the protocol currently used in the request
* 1: Make URI secure using global secure site URI.
* 2: Make URI unsecure using the global unsecure site URI.
* @param boolean $absolute Return an absolute URL
*
* @return string The translated humanly readable URL.
*
* @since 1.7.0
*/
public static function _($url, $xhtml = true, $ssl = null)
public static function _($url, $xhtml = true, $tls = self::TLS_IGNORE, $absolute = false)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really need an $absolute param here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, route::_ is now a short cut to the current client router so should have the same features as route::link

Copy link
Contributor

@SharkyKZ SharkyKZ Mar 5, 2019 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New parameter is not necessary. Just have 4 acceptable values for $ssl. I.e. 0 keep relative URL, 1 force HTTPS, 2 force HTTP, 3 use protocol from request.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thats not true, its a pain everytime you try to create a link that can't be relative (like all I added true in this PR) or if you create a link for an external system (api, json, mail you name it)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I mean is you just need another constant/value for absolute URL with protocol from request, e.g.:

RELATIVE = 0;
HTTPS = 1;
HTTP = 2;
REQUEST = 3;

And then in the code use:

$linkMode = $config->get('force_ssl', 0) == 2 ? Route::HTTPS : Route::REQUEST;

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my opinion this are 2 complete different things. Thats the reason for an extra parameter.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having another constant would be cleaner, in my opinion. A separate parameter won't have any effect when $tls is 1 or 2. But you're the lead, you decide. If you do go with a parameter, please add a note that it only takes effect when $tls = 0.

Now fix the conflicts please and let's have this tested.

{
try
{
// Before __DEPLOY_VERSION__ this method accepted -1. This B/C will be removed in Joomla 4.0.
HLeithner marked this conversation as resolved.
Show resolved Hide resolved
if ($tls == -1) {
$tls = self::TLS_DISABLE;
HLeithner marked this conversation as resolved.
Show resolved Hide resolved
}

$app = Factory::getApplication();
$client = $app->getName();

return static::link($client, $url, $xhtml, $ssl);
return static::link($client, $url, $xhtml, $tls, $absolute);
}
catch (\RuntimeException $e)
{
Expand All @@ -62,21 +83,22 @@ public static function _($url, $xhtml = true, $ssl = null)
* Translates an internal Joomla URL to a humanly readable URL.
* NOTE: To build link for active client instead of a specific client, you can use <var>JRoute::_()</var>
*
* @param string $client The client name for which to build the link.
* @param string $url Absolute or Relative URI to Joomla resource.
* @param boolean $xhtml Replace & by &amp; for XML compliance.
* @param integer $ssl Secure state for the resolved URI.
* 0: (default) No change, use the protocol currently used in the request
* 1: Make URI secure using global secure site URI.
* 2: Make URI unsecure using the global unsecure site URI.
* @param string $client The client name for which to build the link.
* @param string $url Absolute or Relative URI to Joomla resource.
* @param boolean $xhtml Replace & by &amp; for XML compliance.
* @param integer $tls Secure state for the resolved URI. Use Route::TLS_* constants
* 0: (default) No change, use the protocol currently used in the request
* 1: Make URI secure using global secure site URI.
* 2: Make URI unsecure using the global unsecure site URI.
* @param boolean $absolute Return an absolute URL
*
* @return string The translated humanly readable URL.
*
* @throws \RuntimeException
*
* @since 3.9.0
*/
public static function link($client, $url, $xhtml = true, $ssl = null)
public static function link($client, $url, $xhtml = true, $tls = self::TLS_IGNORE, $absolute = false)
{
// If we cannot process this $url exit early.
if (!is_array($url) && (strpos($url, '&') !== 0) && (strpos($url, 'index.php') !== 0))
Expand Down Expand Up @@ -109,20 +131,34 @@ public static function link($client, $url, $xhtml = true, $ssl = null)
* https and need to set our secure URL to the current request URL, if not, and the scheme is
* 'http', then we need to do a quick string manipulation to switch schemes.
*/
if ((int) $ssl || $uri->isSsl())
if ($tls === self::TLS_FORCE)
{
$uri->setScheme('https');
}
elseif ($tls === self::TLS_DISABLE)
{
$uri->setScheme('http');
}

// Set scheme if requested or
if ($absolute || $tls > 0)
{
static $host_port;
static $scheme_host_port;

if (!is_array($host_port))
if (!is_array($scheme_host_port))
{
$uri2 = Uri::getInstance();
$host_port = array($uri2->getHost(), $uri2->getPort());
$uri2 = Uri::getInstance();
$scheme_host_port = array($uri2->getScheme(), $uri2->getHost(), $uri2->getPort());
}

// Determine which scheme we want.
$uri->setScheme(((int) $ssl === 1 || $uri->isSsl()) ? 'https' : 'http');
$uri->setHost($host_port[0]);
$uri->setPort($host_port[1]);
if (is_null($uri->getScheme()))
{
$uri->setScheme($scheme_host_port[0]);
}

$uri->setHost($scheme_host_port[1]);
$uri->setPort($scheme_host_port[2]);

$scheme = array_merge($scheme, array('host', 'port', 'scheme'));
}

Expand Down
6 changes: 3 additions & 3 deletions plugins/system/privacyconsent/privacyconsent.php
View file
Open in desktop
Expand Up @@ -568,7 +568,7 @@ private function remindExpiringConsents()
}

$app = JFactory::getApplication();
$linkMode = $app->get('force_ssl', 0) == 2 ? 1 : -1;
$linkMode = $app->get('force_ssl', 0) == 2 ? Route::TLS_FORCE : Route::TLS_IGNORE;

foreach ($users as $user)
{
Expand All @@ -581,8 +581,8 @@ private function remindExpiringConsents()
$substitutions = array(
'[SITENAME]' => $app->get('sitename'),
'[URL]' => JUri::root(),
'[TOKENURL]' => JRoute::link('site', 'index.php?option=com_privacy&view=remind&remind_token=' . $token, false, $linkMode),
'[FORMURL]' => JRoute::link('site', 'index.php?option=com_privacy&view=remind', false, $linkMode),
'[TOKENURL]' => JRoute::link('site', 'index.php?option=com_privacy&view=remind&remind_token=' . $token, false, $linkMode, true),
'[FORMURL]' => JRoute::link('site', 'index.php?option=com_privacy&view=remind', false, $linkMode, true),
'[TOKEN]' => $token,
'\\n' => "\n",
);
Expand Down