? ? Pending

User tests: Successful: Unsuccessful:

avatar csthomas
csthomas
9 Jan 2017

Pull Request for Issue #13507 .

Summary of Changes

SQL Server database always escape backslash in result.
This PR stripslashes for each column from result query.

Testing Instructions

TEST 1

Before patch

  1. Go to article, change some text or nothing, save article a few times.
  2. Go to "Versions" (button at the top), there should open a modal window with list of dates and versions.
  3. Click on some date row which open popup window.
  4. You should see warnings and no data information about changes between versions.
Warning: Invalid argument supplied for foreach() in .../administrator/components/com_contenthistory/helpers/contenthistory.php on line 32

Warning: Invalid argument supplied for foreach() in .../administrator/components/com_contenthistory/helpers/contenthistory.php on line 295

After patch the warning does not appears and you see some information.

TEST 2

Before patch

  1. Edit new article, and write two chars in full text: \"
  2. Re-save a few times without changing anything more.
  3. You should see more and more backslashes, ignore html tags in ex. \\", next \\\\", etc.

After patch the number of backslashes are not increased on each save.

TEST 3

Before patch:

  1. Edit article and add to Link A Text (Images and Links Tab): A""B.
  2. After you save article you get Error decoding JSON data: Syntax error.

After patch

  1. Do the same and save and load article works as expected.

Documentation Changes Required

N/A

avatar csthomas csthomas - open - 9 Jan 2017
avatar csthomas csthomas - change - 9 Jan 2017
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 9 Jan 2017
Category MS SQL Libraries
avatar csthomas csthomas - change - 9 Jan 2017
Labels Added: ?
avatar RonakParmar
RonakParmar - comment - 10 Jan 2017

Missing testing instruction.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/13534.

avatar csthomas csthomas - change - 10 Jan 2017
The description was changed
avatar csthomas csthomas - edited - 10 Jan 2017
avatar csthomas
csthomas - comment - 10 Jan 2017

Test instruction added.

There is a lots of problems with mssql. This PR only fix a few of them.
There is a lots of places where something does not work this not means this PR is buggy.

"This PR fix only SELECT queries".

avatar waader
waader - comment - 10 Jan 2017

I have tested this item successfully on f57b5ff

Thanks!


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/13534.

avatar waader waader - test_item - 10 Jan 2017 - Tested successfully
avatar alikon
alikon - comment - 11 Jan 2017

I have tested this item successfully on f57b5ff


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/13534.

avatar alikon alikon - test_item - 11 Jan 2017 - Tested successfully
avatar jeckodevelopment jeckodevelopment - change - 11 Jan 2017
Status Pending Ready to Commit
avatar jeckodevelopment
jeckodevelopment - comment - 11 Jan 2017

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/13534.

avatar rdeutz
rdeutz - comment - 11 Jan 2017

Just one question, you removed loadResult from the class because the JDatabaseDriver class does the job ok?

avatar csthomas
csthomas - comment - 11 Jan 2017

yes

avatar rdeutz rdeutz - change - 11 Jan 2017
Status Ready to Commit Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2017-01-11 19:44:54
Closed_By rdeutz
Labels Added: ?
avatar rdeutz rdeutz - close - 11 Jan 2017
avatar rdeutz rdeutz - merge - 11 Jan 2017
avatar dgt41
dgt41 - comment - 11 Jan 2017

@rdeutz isn't the removal of a public function a b/c break?

avatar csthomas
csthomas - comment - 11 Jan 2017

It is not removed because it is inherited from JDatabaseDriver

avatar dgt41
dgt41 - comment - 11 Jan 2017

@csthomas in such case ignore my comment

avatar csthomas
csthomas - comment - 14 Jan 2017

After more deep searching I found this PR fixed errors but does not do it in the right way.
It was weird to me that I have to use stripslashes() but it worked.

At the end I have found that $db->quote() does not work correctly and generate additional backslashes that are saved to database (specially to columns with encoded json). After that stripslashes() is needed.

After I changed $db->quote() code then stripslashes() is not needed.
Take a look at PR #13585

Add a Comment

Login with GitHub to post a comment