Post by SoulReceiver » Tue Feb 27, 2018 5:16 am

So I'm new to OpenCart and I am really happy with the platform. I found it supported almost everything I need it to do, and anything it doesn't is easy to implement using custom PHP.

However, I've found something that surprised me. When I add products (or any data really) that contains certain characters (such as double quotes), I found that these were being HTML encoded when stored in the database. Now, I know why. This is because keeping them in their original state would break the Edit Form when you decided to edit the product again (as the source HTML would break out of the `value` attribute of the `input` element, showing only a portion of the original value). There are better ways of dealing with this though.

I'm a developer myself, and one of the first lessons I was taught is never to encode data when it's being inserted. Always encode when it's being outputted instead. (This doesn't mean you shouldn't verify and clean data before it goes in, but that's a different matter.) The database should be the source of truth, and by HTML encoding the data it essentially means that the application is assuming that the data will only ever be displayed in an HTML context, which isn't always true. At some point I want to develop an API for my OpenCart instance and supply the data as JSON strings, which can be potentially used for anything. But if I have to HTML decode everything just to get it back to its original form, then there's room for issues to occur and forcing me to perform a task that really shouldn't be necessary.

So, why was the decision made to encode data going into the database?

Newbie

Posts

Joined
Wed Jan 03, 2018 4:03 am

Post by Johnathan » Wed Feb 28, 2018 12:09 am

Good question, but I hate to say, you're probably not going to get a reply. Daniel (the author of OpenCart) doesn't check the forums often, so he probably will not see or respond to this. He's the only one who could answer this question, since the decision was his so many years ago.

If he does see this and reply, I'd be curious to know the answer as well. I don't have any preference on encoding before or after sending to the database, but I always find people's reasons interesting.

Image Image Image Image Image


User avatar
Administrator

Posts

Joined
Fri Dec 18, 2009 3:08 am


Post by qahar » Wed Feb 28, 2018 9:22 am

OpenCart not use prepared statement, thus it's require to escape input in order to prevent sql injection

User avatar
Expert Member

Posts

Joined
Tue Jun 29, 2010 10:24 pm
Location - Indonesia

Post by Stylesupplier » Wed Feb 28, 2018 9:39 am

qahar wrote:
Wed Feb 28, 2018 9:22 am
OpenCart not use prepared statement, thus it's require to escape input in order to prevent sql injection
Wow, I think you are correct. Thanks!

Newbie

Posts

Joined
Thu Feb 15, 2018 12:20 pm

Post by SoulReceiver » Wed Feb 28, 2018 6:21 pm

qahar wrote:
Wed Feb 28, 2018 9:22 am
OpenCart not use prepared statement, thus it's require to escape input in order to prevent sql injection
Whilst OpenCart doesn't use prepared statements, it still uses the built-in MySQLi function `mysqli_real_escape_string` to escape the input to allow it to be used as part of an inline SQL query. So the HTML encoding aspect is redundant, as the `clean()` function that runs over the request data only encodes double quotes and leaves single quotes alone:

Code: Select all

htmlspecialchars($data, ENT_COMPAT, 'UTF-8');

Newbie

Posts

Joined
Wed Jan 03, 2018 4:03 am

Post by qahar » Fri Mar 02, 2018 3:28 pm

The clean() code can be traced far back to version 1.4, at least that is when I know OpenCart.

Opencart heavily use and transparantly pass $this->request->get and $this ->request->post to view. Instead of escaping all this output, and afraid the developer forgot escape it, cleaning the input is the most efficient to prevent XSS. I thnk this is the main reason why OpenCart escaping the input automatically.

I know that Twig is auto escape by default but Twig is introduced recently in v3. Even in v3, OpenCart still have option to use PHP templating which IMO no one will used it.
Last edited by qahar on Fri Mar 02, 2018 3:31 pm, edited 1 time in total.
Reason: why inline code not work, sigh

User avatar
Expert Member

Posts

Joined
Tue Jun 29, 2010 10:24 pm
Location - Indonesia

Post by SoulReceiver » Tue Mar 06, 2018 4:54 am

Ah, this is all making a lot more sense now. Thanks for your responses all. Although I still stand by original argument that encoding into the database is not the right way of doing it, and when OpenCart transitioned to Twig it was the perfect opportunity to remedy this (although perhaps not from a migration perspective, handling data from older OpenCart instances would be potentially problematic). However, I can see now why it is how it is.

Newbie

Posts

Joined
Wed Jan 03, 2018 4:03 am
Who is online

Users browsing this forum: No registered users and 10 guests