View Issue Details

This bug affects 1 person(s).
 14
IDProjectCategoryView StatusLast Update
14601Bug reportsRemoteControlpublic2020-03-13 18:37
Reporterginosupport Assigned Toc_schmitz  
PrioritynoneSeverityblock 
Status closedResolutionreopened 
Product Version3.15.x 
Fixed in Version3.22.7 
Summary14601: AutitLog : RemoteControl delete_participant tries to execute invalid SQL
Description

After performing a comfortupdate from 3.14.9 to 3.15.9, the RemoteControl API call for delete_participant doesn't work anymore.
We're doing a call with iSurveyID = 315333, aTokenIDs = [4]
Which results in the following SQL-error:

syntax error at or near \"LIMIT\"\nLINE 1: SELECT * FROM \"lime_tokens_315333\" \"t\" WHERE tid= LIMIT 1\n

Seems like the tokenId variable isn't correctly filled

Steps To Reproduce

Create a survey in the web interface, I executed all following steps through the RemoteControl API

  • Activate the survey
  • Add some participants
  • Delete one of the partipants
Additional Information

This used to work fine in 3.14.9

TagsNo tags attached.
Attached Files
Bug heat14
Complete LimeSurvey version number (& build)3.15.9+190214
I will donate to the project if issue is resolvedNo
Browsernone (JSON-RPC API)
Database type & versionPostgres (pgsql 9.2.24)
Server OS (if known)Linux (RedHat)
Webserver software & version (if known)Apache
PHP Version 7.1.26

Relationships

related to 15651 closedadamzammit delete_participant fails to delete and gives no error back 
related to 14945 closeddominikvitt delete_participant fails to delete and gives no error back 

Users monitoring this issue

ginosupport

Activities

dominikvitt

dominikvitt

2019-03-05 17:06

developer   ~50783

I can't reproduce it on the latest 3.16.0 version, it works as expected.
Here is a sample of my script:
$myJSONRPCClient->delete_participants($sessionKey, $survey_id, [4, 5]);

delete_participants functions was last changed 15 months ago, so it should work fine.
Check your script, maybe you have mismatch for aTokenIDs variable or some strange characters.

ginosupport

ginosupport

2019-03-08 13:49

reporter   ~50849

Ok, i did some more investigating.

  • Calling delete_participants with an array of tokenIDs results in the SQL error. This is true for 1-element arrays but also for arrays with multiple elements
  • Replacing the 1-element-array of tokenIDs for a single value without array notation prevents the error, but doesn't actually delete anything
  • Calling a delete_participants with a tokenID that doesn't exist, correctly returns "Invalid token ID" without any errors
  • Adding tokenIDs to the array as a String or an int both return the same result

Updating from 3.15.9 to 3.16.0 (with comfortUpdate) doesn't make a difference. I still have an old instance with version 3.5.4 running that also still works (just like the 3.14.9 that i updated this limesurvey instance from originally)

ginosupport

ginosupport

2019-03-08 13:51

reporter   ~50850

By the way, i tried deleting a participant form a newly created survey instance (created after the update) and from an existing survey definition (created in 3.14.9). Both give the same results

dominikvitt

dominikvitt

2019-03-08 14:17

developer   ~50851

I tested it right now again, using postgresql database, but it still deletes records without any errors ( using 1 or more elements in an array).
Problem is probably related to JSON-RPC client you are using.
I'm using weberhofer/jsonrpcphp package.
Can you try using this package?
Or any other JSON-RPC client?

ginosupport

ginosupport

2019-03-08 15:09

reporter   ~50854

Ok, here's the result with Postman as my JSON-RPC client. (see attached files). These are the same actions as i described above.

result_postman_existing_id.png (86,866 bytes)   
result_postman_existing_id.png (86,866 bytes)   
dominikvitt

dominikvitt

2019-03-20 14:03

developer   ~51064

I switched to Rested extension for firefox and tried again.
When I used your syntax it failed, but it works when I provide parameters in key: value format.
See attached screenshot.
Try it like that.

ginosupport

ginosupport

2019-03-20 15:48

reporter   ~51070

Unfortunately, that didn't change anything. It seems like it does in fact do some resolving of the ID, because it can correctly identify when i'm trying to remove a single non-existing ID, but other cases just fail

other-syntax.png (68,179 bytes)   
other-syntax.png (68,179 bytes)   
other-syntax-no-array.png (79,947 bytes)   
other-syntax-no-array.png (79,947 bytes)   
ginosupport

ginosupport

2019-03-20 16:10

reporter   ~51071

Something that just came to mind. I'm doing this with an API user that only has the attached permissions.

Now that i think about it i think my initial description may have been too limited. This is what happens in more detail:

  • User 1 (with permissions described in permissions-creator.png) creates the survey in the limesurvey admin page
  • User 1 adds user 2 (different account, permissions described in permissions-api-user.png) to the survey in survey-permissions.
  • User 1 gives user 2 has all available permissions, see survey-permissions.png (user 1 is CJGR admin, user 2 is CJGR API user)
  • External application logs in through remotecontrol API, using credentials from user 2
  • User 2 activates the survey
  • User 2 adds participants
  • At some point user 2 can delete participants if needed. This is the step that fails.

For completeness: both users are in the same usergroup.

Note that i did not change the setup or the permissions recently. In 3.14.9 these steps worked fine with these permissions.

Additional note: The API user (user 2) should not be able to touch any surveys it has not explicitly been added to. I have experienced that the only way to achieve that is by only giving it create survey permissions (which i thought was weird, but it does work).

permissions-api-user.png (60,985 bytes)   
permissions-api-user.png (60,985 bytes)   
permissions-creator.png (61,695 bytes)   
permissions-creator.png (61,695 bytes)   
survey-permissions.png (20,468 bytes)   
survey-permissions.png (20,468 bytes)   
dominikvitt

dominikvitt

2019-03-20 16:46

developer   ~51073

Yes, giving user2 right to create surveys is only possible solution right now, because there isn't global permissions for tokens.

ginosupport

ginosupport

2019-03-26 15:19

reporter   ~51135

So, aside from the hint about the survey permissions (thanks for that), anything else i can try to prevent this SQL error or help you debug it? I have a feeling it's not so much related to the REST-interface, because if i provide an id that doesn't exist it returns exactly what i expect it to return. That to me means the interface itself is working just fine.

dominikvitt

dominikvitt

2019-03-26 15:25

developer   ~51136

Yes, you are correct, REST-interface works as expected when parameters are provided.
SQL error is shown only when aTokenIds parameter isn't accepted by REST-interface.
Correct parameter format is very important to be able to prevent this issue from happening.

DenisChenu

DenisChenu

2019-03-26 18:23

developer   ~51143

SQL error is shown only when aTokenIds parameter isn't accepted by REST-interface.

Then we must prevent this in API : if an invalid aTokenIds is set return error with status=>'Invalid aTokenIds in request'

ginosupport

ginosupport

2019-03-27 09:40

reporter   ~51147

But it is accepted. I did the exact same call twice, i only changed the value of the tokenid. Once with id 1 (SQL error), once with id 123 (correctly handled with a readable status-message). I don't see how that change would result in the REST-interface not accepting the parameter, because the difference in results depends on the actual database content. I feel like we're focussing on the wrong part of the application here. Like you stated, the REST-interface works as expected.

See https://bugs.limesurvey.org/file_download.php?file_id=11346&type=bug
vs
https://bugs.limesurvey.org/file_download.php?file_id=11347&type=bug

dominikvitt

dominikvitt

2019-03-27 11:07

developer   ~51152

Some JSON-RPC clients use parameters definition like this : $myJSONRPCClient->delete_participants($sessionKey, $survey_id, [3, 4, 6]);
But it is not a case for your client.

Can you try again using exactly the same structure as I used here: https://bugs.limesurvey.org/view.php?id=14601#c51064?
It looks that if browser REST extension is used for sending requests, you have to provide parameters in key: value format.

DenisChenu

DenisChenu

2019-03-27 11:34

developer   ~51156

I test with PostgreSQL 9.1.24lts2 to have a PG test : really can‘t reproduce Tested:

  • 1 : return empty string : OK
  • [1] : existing : OK deleted
  • [1] : exiting with response : OK deleted
  • [1] : not exist : OK return invalid
  • [1,2,3,4,5] some existing , some not exist : OK for all
DenisChenu

DenisChenu

2019-03-27 11:34

developer   ~51157

Set to Unable to reproduce on 3.16.1 : please update

dominikvitt

dominikvitt

2019-03-27 11:39

developer   ~51159

@DenisChenu: see my last note.
It looks like Postman use different parameter definition.
Can you try to reproduce using Postman?
https://bugs.limesurvey.org/view.php?id=14601#c51064

DenisChenu

DenisChenu

2019-03-27 11:39

developer   ~51160

OK : the fix is here : https://github.com/LimeSurvey/LimeSurvey/commit/3579c93e529792463e8b3fe44671830fe78d06f3

DenisChenu

DenisChenu

2019-03-27 11:44

developer   ~51161

Last edited: 2019-03-27 11:44

@dominikvitt the SQL error is duie to old code
$dlquery = 'DELETE FROM '.TokenDynamic::tableName().' WHERE tid=:tokenid';
, moved to clean code.

We must remove all this direct SQL instruction ;)

ginosupport

ginosupport

2019-05-16 17:14

reporter   ~51971

Even after updating to 3.17.3 this was still an issue. I've finally figured out what the problem is!

The issue is in the auditlog plugin. If i enable the auditlog plugin i get the SQL syntax error when deleting the survey. Disabling the auditlog plugin also fixes the error.

dominikvitt

dominikvitt

2019-06-25 17:56

developer   ~52552

@ginosupport:
I'll try to fix it this week.

ginosupport

ginosupport

2019-08-27 15:19

reporter   ~53302

Any progress on this (maybe on the 4.0.0 branch?)? We've been missing the auditlogging for a while now and would like to re-enable it.

andre_ars

andre_ars

2019-11-20 09:06

reporter   ~54700

I have the same problem in version 3.19.3+191023

ginosupport

ginosupport

2020-01-14 15:41

reporter   ~55239

Does https://bugs.limesurvey.org/view.php?id=15651 fix this too?

adamzammit

adamzammit

2020-01-17 01:22

developer   ~55340

It does for me - but can you please confirm it for you?

cdorin

cdorin

2020-02-22 20:46

reporter   ~56152

Waiting for confirmation that it has beens solved.

ginosupport

ginosupport

2020-03-04 10:56

reporter   ~56294

I've just checked on 3.22.7 and the problem is fixed now. Thanks!

Issue History

Date Modified Username Field Change
2019-03-05 13:16 ginosupport New Issue
2019-03-05 13:33 ginosupport Issue Monitored: ginosupport
2019-03-05 17:06 dominikvitt Assigned To => dominikvitt
2019-03-05 17:06 dominikvitt Status new => resolved
2019-03-05 17:06 dominikvitt Resolution open => unable to reproduce
2019-03-05 17:06 dominikvitt Note Added: 50783
2019-03-08 13:49 ginosupport Status resolved => feedback
2019-03-08 13:49 ginosupport Resolution unable to reproduce => reopened
2019-03-08 13:49 ginosupport Note Added: 50849
2019-03-08 13:51 ginosupport Note Added: 50850
2019-03-08 13:51 ginosupport Status feedback => assigned
2019-03-08 14:17 dominikvitt Note Added: 50851
2019-03-08 15:09 ginosupport File Added: result_postman_existing_id.png
2019-03-08 15:09 ginosupport File Added: result_postman_non-existing_id.png
2019-03-08 15:09 ginosupport File Added: sql_exception_postman_existing_id.png
2019-03-08 15:09 ginosupport Note Added: 50854
2019-03-20 14:03 dominikvitt File Added: Screenshot_2019-03-20_13-57-14.png
2019-03-20 14:03 dominikvitt Note Added: 51064
2019-03-20 15:48 ginosupport File Added: other-syntax.png
2019-03-20 15:48 ginosupport File Added: other-syntax-no-array.png
2019-03-20 15:48 ginosupport File Added: other-syntax-non-existing-id-in-array.png
2019-03-20 15:48 ginosupport Note Added: 51070
2019-03-20 16:10 ginosupport File Added: permissions-api-user.png
2019-03-20 16:10 ginosupport File Added: permissions-creator.png
2019-03-20 16:10 ginosupport File Added: survey-permissions.png
2019-03-20 16:10 ginosupport Note Added: 51071
2019-03-20 16:46 dominikvitt Note Added: 51073
2019-03-26 15:19 ginosupport Note Added: 51135
2019-03-26 15:25 dominikvitt Note Added: 51136
2019-03-26 18:23 DenisChenu Note Added: 51143
2019-03-27 09:40 ginosupport Note Added: 51147
2019-03-27 10:24 DenisChenu Assigned To dominikvitt => DenisChenu
2019-03-27 11:07 dominikvitt Note Added: 51152
2019-03-27 11:34 DenisChenu Status assigned => feedback
2019-03-27 11:34 DenisChenu Note Added: 51156
2019-03-27 11:34 DenisChenu Status feedback => closed
2019-03-27 11:34 DenisChenu Resolution reopened => unable to reproduce
2019-03-27 11:34 DenisChenu Note Added: 51157
2019-03-27 11:39 dominikvitt Note Added: 51159
2019-03-27 11:39 DenisChenu Note Added: 51160
2019-03-27 11:39 DenisChenu Fixed in Version => 3.15.x
2019-03-27 11:44 DenisChenu Note Added: 51161
2019-03-27 11:44 DenisChenu Note Edited: 51161
2019-05-16 17:14 ginosupport Status closed => feedback
2019-05-16 17:14 ginosupport Resolution unable to reproduce => reopened
2019-05-16 17:14 ginosupport Note Added: 51971
2019-05-16 19:04 DenisChenu Assigned To DenisChenu =>
2019-05-16 19:04 DenisChenu Summary RemoteControl delete_participant tries to execute invalid SQL => AutitLog : RemoteControl delete_participant tries to execute invalid SQL
2019-05-29 11:16 DenisChenu Relationship added related to 14945
2019-06-25 16:35 ginosupport Status feedback => new
2019-06-25 17:56 dominikvitt Note Added: 52552
2019-08-27 15:19 ginosupport Note Added: 53302
2019-11-20 09:06 andre_ars Note Added: 54700
2020-01-14 15:41 ginosupport Note Added: 55239
2020-01-14 15:57 cdorin Assigned To => cdorin
2020-01-14 15:57 cdorin Status new => assigned
2020-01-17 01:21 adamzammit Relationship added related to 15651
2020-01-17 01:22 adamzammit Note Added: 55340
2020-02-22 20:46 cdorin Assigned To cdorin =>
2020-02-22 20:46 cdorin Status assigned => feedback
2020-02-22 20:46 cdorin Note Added: 56152
2020-03-04 10:56 ginosupport Note Added: 56294
2020-03-04 10:56 ginosupport Status feedback => new
2020-03-04 12:05 ollehar Status new => resolved
2020-03-13 18:37 c_schmitz Assigned To => c_schmitz
2020-03-13 18:37 c_schmitz Status resolved => closed
2020-03-13 18:37 c_schmitz Fixed in Version 3.15.x => 3.22.7
2021-08-02 17:18 guest Bug heat 12 => 14