Skip to content

Fix user session on mssql server and a new $db->quoteBinary() method#23213

Merged
HLeithner merged 1 commit intojoomla:stagingfrom
csthomas:quoteBinary
Sep 17, 2019
Merged

Fix user session on mssql server and a new $db->quoteBinary() method#23213
HLeithner merged 1 commit intojoomla:stagingfrom
csthomas:quoteBinary

Conversation

@csthomas
Copy link
Contributor

@csthomas csthomas commented Nov 30, 2018

Summary of Changes

From J3.9 the type of session_id column is varbinary(192) and it works OK for mysql and PostgreSQL database.

In the case of mssql, this column requires explicitly cast to binary value to work properly.

See https://docs.microsoft.com/en-us/sql/t-sql/data-types/data-type-conversion-database-engine?view=sql-server-2017

Testing Instructions

Test user session, login. logout, action logs, etc.

Probably no one will test it on mssql server, please test it at least on mysql.

Expected result

Joomla works (after installing or updating) on the mssql server.

Actual result

Joomla does not work (after installing or updating) on the mssql server.

Documentation Changes Required

Yes.
A new method $db->quoteBinary($data).

@alikon
Copy link
Contributor

alikon commented Dec 3, 2018

Probably no one will test it on mssql server, please test it at least on mysql.

🤦‍♂️

on postgresql only 😄

@alikon
Copy link
Contributor

alikon commented Dec 3, 2018

shouldn't be ported on https://github.com/joomla-framework/database too ?

@alikon
Copy link
Contributor

alikon commented Dec 3, 2018

I have tested this item ✅ successfully on 958f312


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

@csthomas
Copy link
Contributor Author

csthomas commented Dec 3, 2018

shouldn't be ported on https://github.com/joomla-framework/database too ?

Yes. I will do it, I start here.

@csthomas
Copy link
Contributor Author

I'm closing it for now

@csthomas csthomas closed this Dec 21, 2018
@SharkyKZ
Copy link
Contributor

SharkyKZ commented Mar 5, 2019

@csthomas Can you re-open this please?

@HLeithner HLeithner reopened this Jul 12, 2019
@HLeithner
Copy link
Member

I have tested this item ✅ successfully on 958f312

Works on mssql server 2017 (Linux)


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

@alikon
Copy link
Contributor

alikon commented Jul 15, 2019

RTC


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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Jul 15, 2019
@HLeithner HLeithner removed the RTC This Pull Request is Ready To Commit label Jul 17, 2019
@HLeithner
Copy link
Member

I see only one test on an unkown database (alikon) and my test on mssql so we need one for mysql and one for pgsql before this is RTC.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Jul 17, 2019
@ghost ghost removed the RTC This Pull Request is Ready To Commit label Jul 17, 2019
@ghost
Copy link

ghost commented Jul 17, 2019

Status back on Pending.

@alikon
Copy link
Contributor

alikon commented Jul 17, 2019

my test was on postgresql

@ghost
Copy link

ghost commented Jul 20, 2019

Status "Ready To Commit" as Test by @alikon was on postgresql.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Jul 20, 2019
@HLeithner
Copy link
Member

So we need at least one on mysql

@HLeithner HLeithner removed the RTC This Pull Request is Ready To Commit label Jul 20, 2019
@SharkyKZ
Copy link
Contributor

Is MariaDB OK?

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Jul 20, 2019
@HLeithner
Copy link
Member

HLeithner commented Jul 20, 2019 via email

@SharkyKZ
Copy link
Contributor

10.1.40 and 10.3.16.

@HLeithner
Copy link
Member

A mysql 5.5 test would be great...

@HLeithner
Copy link
Member

Thanks for adding binary encoding support to Joomla.

@HLeithner HLeithner merged commit 7d8b1b8 into joomla:staging Sep 17, 2019
@HLeithner HLeithner added this to the Joomla! 3.9.12 milestone Sep 17, 2019
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Sep 17, 2019
@csthomas csthomas deleted the quoteBinary branch September 20, 2019 18:55
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

Successfully merging this pull request may close these issues.

5 participants