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

Missing validation for SEO keywords #1632

Closed
mindplay-dk opened this issue Jul 14, 2014 · 9 comments
Closed

Missing validation for SEO keywords #1632

mindplay-dk opened this issue Jul 14, 2014 · 9 comments

Comments

@mindplay-dk
Copy link

Neither categories nor products have any sort of validation for SEO keywords, which means you're allowed to create duplicates - on the product admin page, if a duplicate keyword if used in the product's category path, the admin page will crash:

Warning: mysqli::query() [mysqli.query]: (21000/1242): Subquery returns more than 1 row in C:\workspace\xxx\webroot\system\database\mysqli.php on line 17 Fatal error: Uncaught exception 'ErrorException' with message 'Error: Subquery returns more than 1 row<br />Error No: 1242<br />SELECT DISTINCT *, (SELECT GROUP_CONCAT(cd1.name ORDER BY level SEPARATOR ' &gt; ') FROM category_path cp LEFT JOIN category_description cd1 ON (cp.path_id = cd1.category_id AND cp.category_id != cp.path_id) WHERE cp.category_id = c.category_id AND cd1.language_id = '2' GROUP BY cp.category_id) AS path, (SELECT keyword FROM url_alias WHERE query = 'category_id=14') AS keyword FROM category c LEFT JOIN category_description cd2 ON (c.category_id = cd2.category_id) WHERE c.category_id = '14' AND cd2.language_id = '2'' in C:\workspace\xxx\webroot\system\database\mysqli.php:41 Stack trace:
#0 C:\workspace\xxx\webroot\system\library\db.php(20): DBMySQLi->query('SELECT DISTINCT...')
#1 C:\workspace\xxx\webroot\admin\model\catalog\category.php(202): DB->query('SELECT DISTINCT...')
#2 C:\workspace\xxx\webroot\admin\controller\catalog\product.php(1044): ModelCatalogCategory- in C:\workspace\xxx\webroot\system\database\mysqli.php on line 41
@danielkerr
Copy link
Member

does not crash with me

@mindplay-dk
Copy link
Author

It will crash under the right circumstances - the subquery (SELECT keyword FROM url_alias WHERE query = 'category_id=14') AS keyword will return more than one row if there is more than one entry where query = 'category_id=14' and you will get the exception quoted above.

The problem is not the query however, but the fact that you're allowed to create conflicting entries because the input isn't being validated.

Even if this query could execute properly with corrupted/invalid data allowed into the database, e.g. if you were to add a LIMIT clause, you would simply get a random row and a random result.

How would I even know as an administrator if I've created a duplicate? Manually page through 400 products and look at every SEO keyword and memorize them? :-)

@mMoovs
Copy link

mMoovs commented Feb 9, 2016

Why was this closed without any solution? I have one OC install where this exact error happens.

If I try to edit a product or category all I get is a blank page with the error text, with product it is:

Warning: mysql_query(): Unable to save result set in /var/www/clients/xxx/yyy/web/system/database/mysql.php on line 22Notice: Error: Subquery returns more than 1 row
Error No: 1242
SELECT DISTINCT *, (SELECT keyword FROM oc_url_alias WHERE query = 'product_id=856') AS keyword FROM oc_product p LEFT JOIN oc_product_description pd ON (p.product_id = pd.product_id) WHERE p.product_id = '856' AND pd.language_id = '2' in /var/www/clients/xxx/yyy/web/system/database/mysql.php on line 50

It's a webshop with two languages.I think this was working few weeks ago but after I removed Opencart SEO Pack VQMod files (because those were making trouble elsewhere) I can't get to edit product or category.

I found some fixes for this but none of them worked. Some fix suggested that I should remove the duplicate values from database but doesn't that mean I'll loose the links for the other language?

@thecotne
Copy link

thecotne commented Dec 5, 2016

anyone here you better start using something that works!

still not fixed in latest version of opencart (2.3.0.2)

Why? Because Fuck You, That's Why.

as a workaround you can change "oc_url_alias.query" to unique (this may or may not create error messages but will prevent corrupting data)

@trueliarx
Copy link

trueliarx commented Feb 27, 2017

houston, we have the same problem (without any extension!)

@OpenCartAddons
Copy link
Contributor

@trueliarx How did you input your product information? Did you use the product edit forms, or an product importer?

When editing a product via the OC interface, the SEO URL entry is removed from the table url_alias prior to the new keyword being added. This should prevent any duplicate entries.

$this->db->query("DELETE FROM " . DB_PREFIX . "url_alias WHERE query = 'product_id=" . (int)$product_id . "'");

If you're using a 3rd party product importer, you will need to contact the extension developer to ensure the same process is used to prevent duplicate entries.

@cartbinder
Copy link

$this->db->query("DELETE FROM " . DB_PREFIX . "url_alias WHERE query = 'product_id=" . (int)$product_id . "'");

This shall only delete for that product id.

But the duplicate keyword should be checked if it exists. As main thing is preventing duplicate keywords in url_alias table. As on front end the checking can only be done with keyword coming from url.

So adding a check while adding products is a good idea.

@OpenCartAddons
Copy link
Contributor

The system looks for duplicate keywords when you attempt to save the product form. If the keyword already exists the user is given the error message SEO URL already in use!

		if (utf8_strlen($this->request->post['keyword']) > 0) {
			$this->load->model('catalog/url_alias');
			$url_alias_info = $this->model_catalog_url_alias->getUrlAlias($this->request->post['keyword']);
			if ($url_alias_info && isset($this->request->get['product_id']) && $url_alias_info['query'] != 'product_id=' . $this->request->get['product_id']) {
				$this->error['keyword'] = sprintf($this->language->get('error_keyword'));
			}
			if ($url_alias_info && !isset($this->request->get['product_id'])) {
				$this->error['keyword'] = sprintf($this->language->get('error_keyword'));
			}
		}

@4pxs
Copy link
Contributor

4pxs commented Feb 28, 2017

look at my implementation of url alias model.
https://github.com/4pxs/opencart/blob/admincart-dev/public/admin/model/catalog/url_alias.php
it works without duplicates, and autogenerate slug
use in anywhere model
example
$this->model_catalog_url_alias->addURLAlias('category_id=1', 'Name category');
and this method save slug

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

8 participants