berlindb / core

All of the required core code
MIT License
254 stars 28 forks source link

Remove applying stripslashes on string content columns before saving them #97

Closed engahmeds3ed closed 3 years ago

engahmeds3ed commented 3 years ago

Introduction

Here https://github.com/berlindb/core/blob/master/query.php#L2057

We are checking if the type of the value being saved is a string then we strip slashes from it.

The problem

This may be a problem when the content contains a backslash like in the following CSS file:

https://themes.trac.wordpress.org/browser/storefront/3.5.1/assets/css/base/icons.css as mentioned in this issue: https://github.com/woocommerce/storefront/issues/1662

so we need to save the content as is without any stripping.

Proposed solution

We don't think that we need this stripslashes at all and we need to save into the DB the content as is without any restriction. and if this was added here because of old versions of WP we may check the version of WP being used and apply this stripslashes on those old versions only.

ashleyfae commented 3 years ago

I also had this problem in RCP when trying to save a namespaced class (which contained a backslash).

Although I appreciate BerlinDB is trying to fix a potential gotcha with stripslashes, I personally feel like it should be up to the person implementing BerlinDB to do that themselves prior to the database layer. (In their own sanitization.)

JJJ commented 3 years ago

I personally feel like it should be up to the person implementing BerlinDB to do that themselves prior to the database layer. (In their own sanitization.)

You are right, and I do agree with you.

if this was added here because of old versions of WP we may check the version of WP being used and apply this stripslashes on those old versions only.

Unfortunately it's all WordPress versions, due to PHP versions older than 5.4 and magic quotes.

Relevant reading to explain my paranoia regarding removing it:


Will put together a pull request so everyone can test. 👍