View Issue Details
|ID||Project||Category||View Status||Date Submitted||Last Update|
|13950||Bug reports||[All Projects] Survey taking||public||2018-08-09 12:38||2019-05-29 16:12|
|Target Version||Fixed in Version||3.17.x|
|Summary||13950: SQL Error when saving a response or getting a session token via API|
I am grabbing data & responses from the LimeSurvey installation via the API. On the demo versions hosted here, the API works fine. On my local installation, when I attempt to call the API (get_session_key) I receive the following error;
[i]CDbCommand failed to execute the SQL statement: SQLSTATE: [Microsoft][ODBC Driver 17 for SQL Server][SQL Server]Implicit conversion from data type nvarchar to varbinary(max) is not allowed. Use the CONVERT function to run this query.[/i]
The same error is also being thrown when a response is saved (related issue here: https://bugs.limesurvey.org/view.php?id=13946)
I have googled around and there's a suggestion this is either the PDO Version or the PHP version, but there have also been related issues fixed such as here: https://bugs.limesurvey.org/view.php?id=13700
I am currently running
|Steps To Reproduce|
Create and activate a survey, complete and attempt to save the response.
Call the get_session_key API Method.
I have tried to change the column type in the database from varbinary to nvarchar(max) but the error is still being raised.
|Tags||No tags attached.|
|Complete LimeSurvey version number (& build)||3.13.2+180709|
|I will donate to the project if issue is resolved||No|
|Database & DB-Version||sqlsrv 12.0.5214.6|
|Server OS (if known)||Linux 3.10.0-514.el7.x86_64 #1 SMP|
|Webserver software & version (if known)||Apache/2.4.6 (Red Hat Enterprise Linux)|
Further update - debugging through this and the original issue gets fixed by changing the column type to a NVARCHAR. However, a further error is thrown because the code is unable to generate a SessionKey so it saves the Session record in the database without an ID. The error is thrown when it tries to update this record using the blank id.
I've tracked it back to these lines in CSecurityManager.php;
The session.entropy_length never gets set so the generateSessionRandomBlock method returns a false. Setting the value in the php.ini doesnt seem to affect it either. From reading the php session docs, this setting was removed in PHP 7.1 : http://php.net/manual/en/session.configuration.php
If I remove the lines of code above from the function, then the function works and successfully returns the id. Note that any observations here apply to PHP 7.2 as my dev box is running 7.2 but our live server is running 7.0.3. I seem to be getting the initial issue both on PHP 7.2 AND 7.0.3.
I'm puzzled as to why the ini_set & ini_get may be failing in both PHP environments? Anyone have any ideas?
Another update :)
the ini_set & ini_get are only failing on my local dev machine (running PHP 7.2.0). I suspect that this is either a configuration issue or there is another issue here that only occurs with the PHP 7.2.0.
On my "live" machine (I have limited access to this box) it runs PHP 7.0.3 and I can see that its successfully creating the Session ID (by sniffing the data going into SQL Server). It isn't saving it to the database, but I suspect that is simply a change of column type - we had made this change but I think my colleague made it in the wrong database.
I intend on making the column change again and then retesting it. I'll update further when thats complete.
You're doing some great job in analyzing the bugs, feel free to submit some PR for the fixes.
Been away for a week, but I have a further update - the change of column type fixes the issue & sessions can be generated. However, I did hit a further code issue that I have a fix for and am preparing for a commit.
When I made the column change from varbinary to nvarchar I began hitting a further issue via the API. The error I was seeing is this: "CDbCommand failed to execute the SQL statement: SQLSTATE[IMSSP]: An error occurred translating string for input param 1 to UCS-2: Error code 0x54."
After a session is generated a record is created with an id (sessionToken) & data (username) fields which are stored in the Sessions table. Subsequent calls to the API use this sessionToken to lookup the username stored in the data field. In my scenario, after retrieving the value from the "data" column, the value was coming back in a different charset - it was appearing as a black diamond with question mark in the debugger.
While stepping through the code to prepare this update, I have found and fixed the issue. The issue is the afterFind() function in the Session class. This code;
Assumes that the data coming back from a SQL Server database is Hex and so attempts to convert the Hex to String. Its a valid assumption, however, as the column type has been changed to a nvarchar, this code is unnecessary. Removing this code means that it keeps the value from the data field, doesn't attempt to decode it and so works successfully.
I'm going to prepare a code fix and pull request (using my personal account githubLewis). I'll update this ticket when its completed.
thank you very much for the PR
PR complete and available here for review: https://github.com/LimeSurvey/LimeSurvey/pull/1113
In this PR I've also included some defensive coding changes that are required for my development environment (otherwise the code throws errors and is unusable), these aren't required to fix the reported issue.
@c_scmitz : since remote_control save ONLY user name in session database : https://github.com/LimeSurvey/LimeSurvey/blob/8bcc208f02549a848184dab165600ca1471506f3/application/helpers/remotecontrol/remotecontrol_handle.php#L55
Maybe the best solution are : create a remotecontrol model and DB. We just need a pk + timedate + a varchar for this.
If i make the patch for 3.16.X : are you OK to merge it ?
I test RC with manually updating data column to text : it's OK.
I can test session with MsSql DB, but don't do the fix if it's not accepted after.
Remind : i need to update DBVersion for all DB : then develop or master ?
To be tested
@MerkOne : can you check remote control AND DBsession with https://github.com/LimeSurvey/LimeSurvey/pull/1265 please.
Fix committed to master branch: http://bugs.limesurvey.org/plugin.php?page=Source/view&id=28970
LimeSurvey: master 17895257
2019-04-16 00:53:05Details Diff
|Fixed issue 13950: SQL Error when saving a response or getting a session token via API
Dev: use onBeforeSave Yii event
|mod - application/models/Session.php||Diff File|
LimeSurvey: master 1a28a463
Committer: GitHub Details Diff
|Fixed issue 14700: DbHttpSession broke (mariadb) with data too long
Fixed issue 13950: (MS)SQL Error when saving getting a session token via API
Dev: update data transparently (reset after allow usage after)
Dev: usage of iSessionExpirationTime
|mod - application/config/version.php||Diff File|
|mod - application/controllers/survey/index.php||Diff File|
|mod - application/core/db/MssqlSchema.php||Diff File|
|mod - application/core/db/PgsqlSchema.php||Diff File|
|mod - application/core/web/DbHttpSession.php||Diff File|
|mod - application/helpers/remotecontrol/remotecontrol_handle.php||Diff File|
|mod - application/helpers/update/updatedb_helper.php||Diff File|
|mod - application/models/Session.php||Diff File|
|mod - installer/create-database.php||Diff File|
|2018-08-09 12:38||MerkOne||New Issue|
|2018-08-10 12:43||MerkOne||Note Added: 48770|
|2018-08-10 15:08||MerkOne||Note Added: 48771|
|2018-08-13 10:45||LouisGac||Note Added: 48776|
|2018-08-22 10:20||MerkOne||Note Added: 48843|
|2018-08-22 12:20||LouisGac||Note Added: 48847|
|2018-08-22 13:01||MerkOne||Note Added: 48850|
|2018-08-28 11:49||LouisGac||Note Added: 48889|
|2019-03-19 12:11||DenisChenu||Note Added: 51052|
|2019-03-25 19:03||DenisChenu||Relationship added||has duplicate 14687|
|2019-03-25 19:05||DenisChenu||Assigned To||=> c_schmitz|
|2019-03-25 19:05||DenisChenu||Status||new => feedback|
|2019-03-25 19:05||DenisChenu||Note Added: 51121|
|2019-03-26 14:58||DenisChenu||Note View State: 51052: private|
|2019-03-26 14:58||DenisChenu||Note View State: 51052: public|
|2019-04-16 00:52||DenisChenu||Assigned To||c_schmitz => DenisChenu|
|2019-04-16 01:06||DenisChenu||Note Added: 51475|
|2019-04-16 11:29||DenisChenu||Note Added: 51478|
|2019-04-27 07:41||DenisChenu||Status||feedback => resolved|
|2019-04-27 07:41||DenisChenu||Resolution||open => fixed|
|2019-04-27 07:41||DenisChenu||Fixed in Version||=> 3.17.x|
|2019-04-27 07:41||DenisChenu||Note Added: 51604|
|2019-04-30 09:10||c_schmitz||Status||resolved => closed|
|2019-05-29 16:12||SondagesPro||Changeset attached||=> LimeSurvey master 1a28a463|
|2019-05-29 16:12||DenisChenu||Changeset attached||=> LimeSurvey master 17895257|
|2019-05-29 16:12||DenisChenu||Note Added: 52200|