View Issue Details

IDProjectCategoryView StatusLast Update
13950Bug reports[All Projects] Survey takingpublic2018-08-28 11:49
ReporterMerkOneAssigned To 
PrioritynoneSeverityblock 
Status newResolutionopen 
Product Version3.13.x 
Target VersionFixed in Version 
Summary13950: SQL Error when saving a response or getting a session token via API
Description

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[42000]: [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

PHP 7.0.30
PDO SQL: 5.2.0

Steps To Reproduce

Create and activate a survey, complete and attempt to save the response.

Call the get_session_key API Method.

Additional Information

I have tried to change the column type in the database from varbinary to nvarchar(max) but the error is still being raised.

TagsNo tags attached.
Complete LimeSurvey version number (& build)3.13.2+180709
I will donate to the project if issue is resolvedNo
BrowserChrome
Database & DB-Versionsqlsrv 12.0.5214.6
Operating System (Server)Linux 3.10.0-514.el7.x86_64 #1 SMP
Webserver software & versionApache/2.4.6 (Red Hat Enterprise Linux)
PHP Version7.0.30

Activities

MerkOne

MerkOne

2018-08-10 12:43

reporter   ~48770

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;

public function generateSessionRandomBlock()
{
    ini_set('session.entropy_length',20);       
    if(ini_get('session.entropy_length')!=20)
        return false;

    <other code>

}

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?

MerkOne

MerkOne

2018-08-10 15:08

reporter   ~48771

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.

LouisGac

LouisGac

2018-08-13 10:45

manager   ~48776

You're doing some great job in analyzing the bugs, feel free to submit some PR for the fixes.

MerkOne

MerkOne

2018-08-22 10:20

reporter   ~48843

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;

    $sDatabasetype = Yii::app()->db->getDriverName();
    // MSSQL delivers hex data (except for dblib driver)
    if ($sDatabasetype == 'sqlsrv' || $sDatabasetype == 'mssql') {
        $this->data = $this->hexToStr($this->data); 
    }

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.

LouisGac

LouisGac

2018-08-22 12:20

manager   ~48847

thank you very much for the PR

MerkOne

MerkOne

2018-08-22 13:01

reporter   ~48850

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.

LouisGac

LouisGac

2018-08-28 11:49

manager   ~48889

thx

Issue History

Date Modified Username Field Change
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