From 76da05aa2b18555df3167acfe2b047dd8363e30a Mon Sep 17 00:00:00 2001 From: Hannes Papenberg Date: Wed, 28 Aug 2024 13:11:04 +0200 Subject: [PATCH 1/5] SEF: Fix URLs when preprocessing --- components/com_contact/src/Service/Router.php | 16 +- components/com_content/src/Service/Router.php | 16 +- .../com_newsfeeds/src/Service/Router.php | 16 +- .../Router/Rules/PreprocessRules.php | 172 ++++++++++++++++++ 4 files changed, 184 insertions(+), 36 deletions(-) create mode 100644 libraries/src/Component/Router/Rules/PreprocessRules.php diff --git a/components/com_contact/src/Service/Router.php b/components/com_contact/src/Service/Router.php index 1ad77acca8a4e..7e3d51ea6752f 100644 --- a/components/com_contact/src/Service/Router.php +++ b/components/com_contact/src/Service/Router.php @@ -18,6 +18,7 @@ use Joomla\CMS\Component\Router\RouterViewConfiguration; use Joomla\CMS\Component\Router\Rules\MenuRules; use Joomla\CMS\Component\Router\Rules\NomenuRules; +use Joomla\CMS\Component\Router\Rules\PreprocessRules; use Joomla\CMS\Component\Router\Rules\StandardRules; use Joomla\CMS\Menu\AbstractMenu; use Joomla\Database\DatabaseInterface; @@ -99,6 +100,9 @@ public function __construct(SiteApplication $app, AbstractMenu $menu, CategoryFa parent::__construct($app, $menu); + $preprocess = new PreprocessRules($contact, '#__contact_details', 'id', 'catid'); + $preprocess->setDatabase($this->db); + $this->attachRule($preprocess); $this->attachRule(new MenuRules($this)); $this->attachRule(new StandardRules($this)); $this->attachRule(new NomenuRules($this)); @@ -155,18 +159,6 @@ public function getCategoriesSegment($id, $query) */ public function getContactSegment($id, $query) { - if (!strpos($id, ':')) { - $id = (int) $id; - $dbquery = $this->db->getQuery(true); - $dbquery->select($this->db->quoteName('alias')) - ->from($this->db->quoteName('#__contact_details')) - ->where($this->db->quoteName('id') . ' = :id') - ->bind(':id', $id, ParameterType::INTEGER); - $this->db->setQuery($dbquery); - - $id .= ':' . $this->db->loadResult(); - } - if ($this->noIDs) { list($void, $segment) = explode(':', $id, 2); diff --git a/components/com_content/src/Service/Router.php b/components/com_content/src/Service/Router.php index 2bbcda641cca4..dafc8c011603d 100644 --- a/components/com_content/src/Service/Router.php +++ b/components/com_content/src/Service/Router.php @@ -18,6 +18,7 @@ use Joomla\CMS\Component\Router\RouterViewConfiguration; use Joomla\CMS\Component\Router\Rules\MenuRules; use Joomla\CMS\Component\Router\Rules\NomenuRules; +use Joomla\CMS\Component\Router\Rules\PreprocessRules; use Joomla\CMS\Component\Router\Rules\StandardRules; use Joomla\CMS\Menu\AbstractMenu; use Joomla\Database\DatabaseInterface; @@ -100,6 +101,9 @@ public function __construct(SiteApplication $app, AbstractMenu $menu, CategoryFa parent::__construct($app, $menu); + $preprocess = new PreprocessRules($article, '#__content', 'id', 'catid'); + $preprocess->setDatabase($this->db); + $this->attachRule($preprocess); $this->attachRule(new MenuRules($this)); $this->attachRule(new StandardRules($this)); $this->attachRule(new NomenuRules($this)); @@ -156,18 +160,6 @@ public function getCategoriesSegment($id, $query) */ public function getArticleSegment($id, $query) { - if (!strpos($id, ':')) { - $id = (int) $id; - $dbquery = $this->db->getQuery(true); - $dbquery->select($this->db->quoteName('alias')) - ->from($this->db->quoteName('#__content')) - ->where($this->db->quoteName('id') . ' = :id') - ->bind(':id', $id, ParameterType::INTEGER); - $this->db->setQuery($dbquery); - - $id .= ':' . $this->db->loadResult(); - } - if ($this->noIDs) { list($void, $segment) = explode(':', $id, 2); diff --git a/components/com_newsfeeds/src/Service/Router.php b/components/com_newsfeeds/src/Service/Router.php index f72c3af43f950..a2ba96d266f6f 100644 --- a/components/com_newsfeeds/src/Service/Router.php +++ b/components/com_newsfeeds/src/Service/Router.php @@ -18,6 +18,7 @@ use Joomla\CMS\Component\Router\RouterViewConfiguration; use Joomla\CMS\Component\Router\Rules\MenuRules; use Joomla\CMS\Component\Router\Rules\NomenuRules; +use Joomla\CMS\Component\Router\Rules\PreprocessRules; use Joomla\CMS\Component\Router\Rules\StandardRules; use Joomla\CMS\Menu\AbstractMenu; use Joomla\Database\DatabaseInterface; @@ -95,6 +96,9 @@ public function __construct(SiteApplication $app, AbstractMenu $menu, CategoryFa parent::__construct($app, $menu); + $preprocess = new PreprocessRules($newsfeed, '#__newsfeeds', 'id', 'catid'); + $preprocess->setDatabase($this->db); + $this->attachRule($preprocess); $this->attachRule(new MenuRules($this)); $this->attachRule(new StandardRules($this)); $this->attachRule(new NomenuRules($this)); @@ -151,18 +155,6 @@ public function getCategoriesSegment($id, $query) */ public function getNewsfeedSegment($id, $query) { - if (!strpos($id, ':')) { - $id = (int) $id; - $dbquery = $this->db->getQuery(true); - $dbquery->select($this->db->quoteName('alias')) - ->from($this->db->quoteName('#__newsfeeds')) - ->where($this->db->quoteName('id') . ' = :id') - ->bind(':id', $id, ParameterType::INTEGER); - $this->db->setQuery($dbquery); - - $id .= ':' . $this->db->loadResult(); - } - if ($this->noIDs) { list($void, $segment) = explode(':', $id, 2); diff --git a/libraries/src/Component/Router/Rules/PreprocessRules.php b/libraries/src/Component/Router/Rules/PreprocessRules.php new file mode 100644 index 0000000000000..5c9e58526c59a --- /dev/null +++ b/libraries/src/Component/Router/Rules/PreprocessRules.php @@ -0,0 +1,172 @@ + + * @license GNU General Public License version 2 or later; see LICENSE.txt + */ + +namespace Joomla\CMS\Component\Router\Rules; + +use Joomla\CMS\Component\ComponentHelper; +use Joomla\CMS\Component\Router\RouterView; +use Joomla\CMS\Component\Router\RouterViewConfiguration; +use Joomla\CMS\Language\Multilanguage; +use Joomla\Database\DatabaseAwareTrait; +use Joomla\Database\ParameterType; + +// phpcs:disable PSR1.Files.SideEffects +\defined('_JEXEC') or die; +// phpcs:enable PSR1.Files.SideEffects + +/** + * Rule to prepare the query and add missing information + * + * This rule adds the alias to an ID query parameter and the + * category ID if either of them is missing. This requires that + * the db table contains an alias column. + * This fixes sloppy URLs in the code, but doesn't mean you can + * simply drop the alias from the &id= in the future. Cleaning up + * every request with this would mean a significant performance impact + * + * @since __DEPLOY_VERSION__ + */ +class PreprocessRules implements RulesInterface +{ + use DatabaseAwareTrait; + + /** + * View to prepare + * + * @var RouterViewConfiguration + * @since __DEPLOY_VERSION__ + */ + protected $view; + + /** + * DB Table to read the information from + * + * @var string + * @since __DEPLOY_VERSION__ + */ + protected $table; + + /** + * ID column in the table to read the information from + * + * @var string + * @since __DEPLOY_VERSION__ + */ + protected $key; + + /** + * Parent ID column in the table to read the information from + * + * @var string + * @since __DEPLOY_VERSION__ + */ + protected $parent_key; + + /** + * Class constructor. + * + * @param RouterViewConfiguration $view View to act on + * @param string $table Table name for the views information + * @param string $key Key in the table to get the information + * @param string $parent_key Column name of the parent key + * + * @since __DEPLOY_VERSION__ + */ + public function __construct(RouterViewConfiguration $view, $table, $key = null, $parent_key = null) + { + $this->view = $view; + $this->table = $table; + $this->key = $key; + $this->parent_key = $parent_key; + } + + /** + * Finds the right Itemid for this query + * + * @param array &$query The query array to process + * + * @return void + * + * @since __DEPLOY_VERSION__ + */ + public function preprocess(&$query) + { + // We only work for URLs with the view we have been setup for + if (!isset($query['view']) || $query['view'] != $this->view->name) { + return; + } + + $key = $this->view->key; + $parent_key = $this->view->parent_key; + + // We have to have at least the ID or something to repair + if (!isset($query[$key]) || (strpos($query[$key], ':') && isset($query[$parent_key]))) { + return; + } + + $dbquery = $this->getDatabase()->getQuery(true); + + $dbquery->select($dbquery->quoteName('alias')) + ->from($this->table) + ->where($dbquery->quoteName($this->key) . ' = :key') + ->bind(':key', $query[$key], ParameterType::INTEGER); + + // Do we have a parent key? + if ($parent_key && $this->parent_key) { + $dbquery->select($dbquery->quoteName($this->parent_key)); + } + + $obj = $this->getDatabase()->setQuery($dbquery)->loadObject(); + + // We haven't found the item in the database. Abort. + if (!$obj) { + return; + } + + // Lets fix the slug (id:alias) + if (!strpos($query[$key], ':')) { + $query[$key] .= ':' . $obj->alias; + } + + // If we have a parent key and it is missing, lets add it + if ($parent_key && $this->parent_key && !isset($query[$parent_key])) { + $query[$parent_key] = $obj->{$this->parent_key}; + } + } + + /** + * Dummy method to fulfil the interface requirements + * + * @param array &$segments The URL segments to parse + * @param array &$vars The vars that result from the segments + * + * @return void + * + * @since __DEPLOY_VERSION__ + * @codeCoverageIgnore + */ + public function parse(&$segments, &$vars) + { + } + + /** + * Dummy method to fulfil the interface requirements + * + * @param array &$query The vars that should be converted + * @param array &$segments The URL segments to create + * + * @return void + * + * @since __DEPLOY_VERSION__ + * @codeCoverageIgnore + */ + public function build(&$query, &$segments) + { + } +} From da8002a865a6eebb53354b85e107fa81838db60e Mon Sep 17 00:00:00 2001 From: Hannes Papenberg Date: Wed, 28 Aug 2024 13:27:46 +0200 Subject: [PATCH 2/5] Remove obsolete imports --- libraries/src/Component/Router/Rules/PreprocessRules.php | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/libraries/src/Component/Router/Rules/PreprocessRules.php b/libraries/src/Component/Router/Rules/PreprocessRules.php index 5c9e58526c59a..8ebdb86771af0 100644 --- a/libraries/src/Component/Router/Rules/PreprocessRules.php +++ b/libraries/src/Component/Router/Rules/PreprocessRules.php @@ -9,10 +9,7 @@ namespace Joomla\CMS\Component\Router\Rules; -use Joomla\CMS\Component\ComponentHelper; -use Joomla\CMS\Component\Router\RouterView; use Joomla\CMS\Component\Router\RouterViewConfiguration; -use Joomla\CMS\Language\Multilanguage; use Joomla\Database\DatabaseAwareTrait; use Joomla\Database\ParameterType; @@ -78,7 +75,7 @@ class PreprocessRules implements RulesInterface * * @since __DEPLOY_VERSION__ */ - public function __construct(RouterViewConfiguration $view, $table, $key = null, $parent_key = null) + public function __construct(RouterViewConfiguration $view, $table, $key, $parent_key = null) { $this->view = $view; $this->table = $table; From 6047bcab55e7fff73eb8c0d50713e9ecd9fc13c2 Mon Sep 17 00:00:00 2001 From: Hannes Papenberg Date: Thu, 29 Aug 2024 09:49:11 +0200 Subject: [PATCH 3/5] Adding a check --- components/com_contact/src/Service/Router.php | 2 +- components/com_content/src/Service/Router.php | 2 +- components/com_newsfeeds/src/Service/Router.php | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/components/com_contact/src/Service/Router.php b/components/com_contact/src/Service/Router.php index 7e3d51ea6752f..34969b8e99e67 100644 --- a/components/com_contact/src/Service/Router.php +++ b/components/com_contact/src/Service/Router.php @@ -159,7 +159,7 @@ public function getCategoriesSegment($id, $query) */ public function getContactSegment($id, $query) { - if ($this->noIDs) { + if ($this->noIDs && strpos($id, ':')) { list($void, $segment) = explode(':', $id, 2); return [$void => $segment]; diff --git a/components/com_content/src/Service/Router.php b/components/com_content/src/Service/Router.php index dafc8c011603d..fd4ecec0530db 100644 --- a/components/com_content/src/Service/Router.php +++ b/components/com_content/src/Service/Router.php @@ -160,7 +160,7 @@ public function getCategoriesSegment($id, $query) */ public function getArticleSegment($id, $query) { - if ($this->noIDs) { + if ($this->noIDs && strpos($id, ':')) { list($void, $segment) = explode(':', $id, 2); return [$void => $segment]; diff --git a/components/com_newsfeeds/src/Service/Router.php b/components/com_newsfeeds/src/Service/Router.php index a2ba96d266f6f..4afcb63514fde 100644 --- a/components/com_newsfeeds/src/Service/Router.php +++ b/components/com_newsfeeds/src/Service/Router.php @@ -155,7 +155,7 @@ public function getCategoriesSegment($id, $query) */ public function getNewsfeedSegment($id, $query) { - if ($this->noIDs) { + if ($this->noIDs && strpos($id, ':')) { list($void, $segment) = explode(':', $id, 2); return [$void => $segment]; From 15c3b7ad12efbf51349811fb47ac5cda57ccf38b Mon Sep 17 00:00:00 2001 From: Hannes Papenberg Date: Sat, 31 Aug 2024 21:51:13 +0200 Subject: [PATCH 4/5] Update libraries/src/Component/Router/Rules/PreprocessRules.php Co-authored-by: Brian Teeman --- libraries/src/Component/Router/Rules/PreprocessRules.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/src/Component/Router/Rules/PreprocessRules.php b/libraries/src/Component/Router/Rules/PreprocessRules.php index 8ebdb86771af0..5fe8720687420 100644 --- a/libraries/src/Component/Router/Rules/PreprocessRules.php +++ b/libraries/src/Component/Router/Rules/PreprocessRules.php @@ -84,7 +84,7 @@ public function __construct(RouterViewConfiguration $view, $table, $key, $parent } /** - * Finds the right Itemid for this query + * Finds the correct Itemid for this query * * @param array &$query The query array to process * From 894383eb73af662cad44c902034b987c85456c03 Mon Sep 17 00:00:00 2001 From: Hannes Papenberg Date: Thu, 12 Sep 2024 12:16:06 +0200 Subject: [PATCH 5/5] Fixing system tests --- .../integration/site/components/com_contact/Contact.cy.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/System/integration/site/components/com_contact/Contact.cy.js b/tests/System/integration/site/components/com_contact/Contact.cy.js index 93d8794df5bf9..5066cc5e7cd93 100644 --- a/tests/System/integration/site/components/com_contact/Contact.cy.js +++ b/tests/System/integration/site/components/com_contact/Contact.cy.js @@ -2,7 +2,7 @@ describe('Test in frontend that the contact details view', () => { it('can display a form', () => { cy.db_getUserId().then((id) => cy.db_createContact({ name: 'contact 1', user_id: id })) .then((contact) => { - cy.visit(`/index.php?option=com_contact&view=contact&id='${contact.id}'`); + cy.visit(`/index.php?option=com_contact&view=contact&id=${contact.id}`); cy.contains('Contact Form'); cy.get('.m-0').should('exist'); @@ -17,7 +17,7 @@ describe('Test in frontend that the contact details view', () => { .then(() => cy.db_getUserId()) .then((userId) => cy.db_createContact({ name: 'automated test contact 1', user_id: userId })) .then((contact) => { - cy.visit(`/index.php?option=com_contact&view=contact&id='${contact.id}'`); + cy.visit(`/index.php?option=com_contact&view=contact&id=${contact.id}`); cy.contains('automated test_field group').should('exist'); cy.contains('test field').should('exist');