Geeklog-Core / geeklog

Geeklog - The Secure CMS.
https://www.geeklog.net
24 stars 19 forks source link

Slashes \ go Missing when saving Staticpage (and other slash \ issues) #1011

Open eSilverStrike opened 4 years ago

eSilverStrike commented 4 years ago

Create a staticpage with the following title, page title, content, etc...

This's is a test\ of slashes / the end as well as & andpersands. 'Test' and "Test"!

and the \ by test will disappear when you view the page.

eSilverStrike commented 4 years ago

Polls, comments, and articles seem to work fine but haven't tested anything else.

Correction

eSilverStrike commented 4 years ago

Also take a look at comments when articles have the title:

This's is a test\ of slashes / the end as well as & andpersands.

They don't display right in the comment editor when the comment editor is displayed with the content.

Also the Admin Comment Manager the column "Parent or Comment" doesn't display correctly for comments on articles with this title.

eSilverStrike commented 2 years ago

@mystralkk So I was checking this issue out and it looks like Geeklog and plugins (including 3rd party plugins like forum and media gallery) still uses these PHP functions in a lot of places

addslashes() - used over 50 times stripslashes() - used over 240 times

It's these functions that are causing the problems mentioned in this issue when stripslashes for example is being applied to a title before it is being displayed.

Now I am sure a limited number of these are needed (like a few spots in the autotags plugins and maybe elsewhere)

When we updated Geeklog to use Input Class instead of Com_applyFilters (issue #700) and with the use of DB_escapeString to escape the content before it is saved to the database is there any reason why we left all these stripslashes functions?

I feel like I am missing something obvious. The only reason I can see maybe is backwards compatibility for when addslashes was used in older content before we depreciated COM_stripslashes (issue #168). For at least articles I believe that is what the gltext.class is supposed to handle (among other things) when articles where saved before these newer filtering and escape functions was created.

I think we are the point where we can just remove the stripslashes and just require people if they have older content (like blocks, comments, staticpages, etc) have to update the content manual if needed. Articles still have the gltext.class to handle things and other plugins like the forum we may have to figure something else out (or not) for older content.

I guess this issue is also related to

Make sure the following Plugins Supports Latest Library Code Changes Issue #825

as we did not fully remove the stripslashes when we started adding in the use of Geeklog Input Class and DB_escapeString. I guess all those plugins marked fixed should be double checked for stripslashes, addslashes.

Thoughts?

mystralkk commented 2 years ago

When I introduced Input class, I removed some of addslashes and stripslashes, and left the others, since it seems to me that the latter are intended to emulate magic_quotes_gpc is on. However, I might have removed addslahes that shouldn't be or might not have removed those that should be. To solve the issues completely, I introduced EntityBase and UserAttributeEntity classes to handle $_TABLES['user*'] tables. In the future, I will introduce similar classes. What do you think of this, @eSilverStrike ?

eSilverStrike commented 2 years ago

That's better than my solution. I will leave it as is then.

mystralkk commented 2 years ago

I will handle this issue in Geeklog 2.2.3 or 2.3.0.

eSilverStrike commented 2 years ago

Sounds good. It is a big change.