View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
16937 | Bug reports | Survey taking | public | 2020-12-31 15:24 | 2021-03-22 13:04 |
Reporter | gabrieljenik | Assigned To | ollehar | ||
Priority | normal | Severity | minor | ||
Status | assigned | Resolution | open | ||
Product Version | 4.4.0-RC1 | ||||
Summary | 16937: Easy way to launch CDBException | ||||
Description | Really easy to launch a CDbException publicly | ||||
Steps To Reproduce | Import included survey Copy the start link with newtest=Y Add &Q00=ABCDEFGHIJ Send the link | ||||
Additional Information | Seems prefilled value are not filtered with https://github.com/LimeSurvey/LimeSurvey/blob/bdd01aab12a6c9897cb777fbb8e3692a6f95858d/application/helpers/expressions/em_manager_helper.php#L10132 Here it's with mysql, but other have same issue (date for example) The 2 htmlk are done with debug or without Original solution for LS3 on 16481 needs to be ported or enhanced for LS4. | ||||
Tags | No tags attached. | ||||
Complete LimeSurvey version number (& build) | 4.4.0-RC1 | ||||
I will donate to the project if issue is resolved | No | ||||
Browser | not relevant | ||||
Database & DB-Version | mariadb | ||||
Server OS (if known) | fedora/linux | ||||
Webserver software & version (if known) | nginx | ||||
PHP Version | php7.3 | ||||
related to | 16481 | closed | DenisChenu | Easy way to launch CDBException |
PR: https://github.com/LimeSurvey/LimeSurvey/pull/1708 | |
@DenisChenu commented on the PR: > for master : we need a better solution : move after setMoveResult ? We have moved it: - from `StartSurvey()`, which is run inside `initFirstStep()` - to after `setMoveResult` This is the list of steps it does for starting up: ``` /** * Init session/params values depending of user moves * * - It init the needed variables for navigation: initFirstStep, initTotalAndMaxSteps, setMoveResult * - Then perform all the needed checks before moving: * + did the participant used browser navigation? * + did he pressed clear cancel, is he a confirmed quota? * + Is the previous step set? * + Is the survey finished? * + Are all the answer validated? (like: participant didn't answered to a mandatory question) */ private function initMove() { $this->initFirstStep(); // If it's the first time user load this survey, will init session and LEM $this->initTotalAndMaxSteps(); $this->checkIfUseBrowserNav(); // Check if user used browser navigation, or relaoded page if ($this->sMove != 'clearcancel' && $this->sMove != 'confirmquota') { $this->checkPrevStep(); // Check if prev step is set, else set it $this->setMoveResult(); $this->checkClearCancel(); $this->setPrevStep(); $this->checkIfFinished(); $this->setStep(); $this->saveStartingValues(); ``` Why do we need to move it after `setMoveResult()`? Shouldn't it be done before so the EM data is available for setting the move? |
|
To fix https://bugs.limesurvey.org/view.php?id=16482 prefilling value muts be done only when create survey by another way :) |
|
PS : Another solution : set it to defaultValue on EM ? |
|
I understand now. So now this ticket and 16482 (but for lsv4) are fixed. This approach is OK for me! |
|
Finally : i think you're right … Use default value system are (maybe) not the best solution … - Prefill : value was set at start of survey even if variables are in last group - Default : value was set when step happen (for exemple : in last group : set just before submit). I'm totally unsure on the best solution here … Using defaultvalue seems cool but unsure it the best solution. https://github.com/LimeSurvey/LimeSurvey/blob/68ce18e22194171e1c56c27f36ad7ce5b34adc8a/application/helpers/expressions/em_manager_helper.php#L7126 |
|
Ok, as to start fresh and avoid missunderstandings, I feel comfortable with the latest PR. What do you think? |
|
`if ($this->sSurveyMode != 'survey' && $_SESSION[$this->LEMsessid]['step'] == 0) return;` Yep : seems OK ! Maybe check `$this->aSurveyInfo['showwelcome']` ? https://github.com/LimeSurvey/LimeSurvey/blob/79656ae39499fafd8a36c6ee12c5c9649d74df70/application/helpers/SurveyRuntimeHelper.php#L788 |
|
New PR: https://github.com/LimeSurvey/LimeSurvey/pull/1726 | |
Waiting for PR to master branch. | |
New PR: https://github.com/LimeSurvey/LimeSurvey/pull/1754 | |
About how to reproduce: Import included survey Copy the start link with newtest=Y Add &Q00=ABCDEFGHIJ Send the link What happens after link is sent? What is the problem? I don't understand the issue title. |
|
500 error about DB See original issue https://bugs.limesurvey.org/view.php?id=16481 |
|
> Add &Q00=ABCDEFGHIJ How this this an issue if the URL is manually hacked? |
|
? Admin user can update question type for example. Moving for a long text to a single choice Can lauch a 500 erro, but i really think showing «CDbCommand failed to execute the SQL statement: SQLSTATE[22001]: String data, right truncated: 1406 Data too long for column '721221X2043X31723' at row 1» for public user is not the best way to do. 1. Log it as error : Yes 2. Throw an error or filter ? : Opinion, personally : i think it's better to filter silently. And **if throw an error : send a 400 : bad params, not a 500** 500 error can always be avoided. |
|
> Admin user can update question type for example. Moving for a long text to a single choice How exactly would this lead to a 500 exception? Genuine question. |
|
String data, right truncated … There are a LSA in the other issue : https://bugs.limesurvey.org/view.php?id=16481#c61441 |
|
> String data, right truncated … WITHOUT hacking the URL. :D No sane user would hack the URL and expect the software to still work flawlessly. |
|
Admin user create some link with ComeFrom=facebook ComeFrom=twitter ComeFrom=limesurvey Send this link … After : think : oh it's better if i have ComeFrom as single choice Then : ComeFrom=fb ComeFrom=tw ComeFrom=ls > No sane user would hack the URL and expect the software to still work flawlessly. 500 error still an issue … Another situation : Email reader add a parameter automatically (or a proxy) etc … A It's not a hacked string : it's just "more than 5 caracter" … And since we know it's bad aram : wwe can lauch a 400, not a 500 … > No sane user would hack the URL and expect the software to still work flawlessly. I don't care about the user entering url, i care about a clean error is show for the hoster. |
|
Calling checkValidityAnswer was missing on this version. Will add it. Then will try to reproduce the error again and see what kind of erro does it show. We can pick it up later from there. |
|
Considering the amount of blocking issues, maybe not spend too much time on this right now. ^^ | |
Date Modified | Username | Field | Change |
---|---|---|---|
2020-12-31 15:24 | gabrieljenik | New Issue | |
2020-12-31 15:24 | gabrieljenik | Issue generated from: 16481 | |
2020-12-31 15:25 | gabrieljenik | Additional Information Updated | View Revisions |
2020-12-31 15:25 | gabrieljenik | Relationship added | related to 16481 |
2021-01-05 14:25 | gabrieljenik | Note Added: 61432 | |
2021-01-11 17:11 | gabrieljenik | Note Added: 61481 | |
2021-01-11 17:18 | DenisChenu | Note Added: 61482 | |
2021-01-11 17:19 | DenisChenu | Note Added: 61483 | |
2021-01-12 21:55 | gabrieljenik | Note Added: 61507 | |
2021-01-12 21:56 | gabrieljenik | Note Edited: 61507 | View Revisions |
2021-01-13 08:04 | DenisChenu | Note Added: 61513 | |
2021-01-14 17:36 | gabrieljenik | Note Added: 61545 | |
2021-01-14 17:42 | DenisChenu | Note Added: 61546 | |
2021-01-24 22:36 | gabrieljenik | Note Added: 61657 | |
2021-02-08 17:37 | ollehar | Assigned To | => ollehar |
2021-02-08 17:37 | ollehar | Status | new => feedback |
2021-02-08 17:37 | ollehar | Note Added: 62034 | |
2021-02-08 20:48 | gabrieljenik | Note Added: 62051 | |
2021-02-08 20:48 | gabrieljenik | Status | feedback => assigned |
2021-02-11 15:56 | ollehar | Note Added: 62173 | |
2021-02-11 17:15 | DenisChenu | Note Added: 62180 | |
2021-02-11 17:15 | DenisChenu | File Added: Capture d’écran du 2021-02-11 17-14-28.png | |
2021-02-11 17:49 | ollehar | Note Added: 62182 | |
2021-02-11 17:55 | DenisChenu | Note Added: 62183 | |
2021-02-11 17:57 | DenisChenu | Note Edited: 62183 | View Revisions |
2021-02-11 18:52 | ollehar | Note Added: 62186 | |
2021-02-11 18:53 | ollehar | Note Edited: 62186 | View Revisions |
2021-02-11 19:01 | DenisChenu | Note Added: 62189 | |
2021-02-11 19:03 | ollehar | Note Added: 62191 | |
2021-02-11 20:55 | DenisChenu | Note Added: 62198 | |
2021-02-12 15:26 | gabrieljenik | Note Added: 62215 | |
2021-02-23 11:23 | ollehar | Note Added: 62460 | |
2021-02-23 11:24 | ollehar | Priority | none => normal |
2021-03-22 13:04 | c_schmitz | Severity | crash => minor |