Skip to content

Commit

Permalink
Fixed issue #17221: Check data integrity crash with SQL error when # …
Browse files Browse the repository at this point in the history
…is use in the name of field
  • Loading branch information
olleharstedt committed Apr 3, 2021
1 parent 97b8a94 commit 6a82960
Showing 1 changed file with 4 additions and 1 deletion.
5 changes: 4 additions & 1 deletion application/controllers/admin/checkintegrity.php
Original file line number Diff line number Diff line change
Expand Up @@ -597,14 +597,17 @@ protected function _checkintegrity()
// QID field can be more than just QID, like: 886other or 886A1
// So we clean it by finding the first alphabetical character
$sDirtyQid = $aFields[2];
preg_match('~[a-zA-Z_]~i', $sDirtyQid, $match, PREG_OFFSET_CAPTURE);
preg_match('~[a-zA-Z_#]~i', $sDirtyQid, $match, PREG_OFFSET_CAPTURE);

if (isset($match[0][1])) {
$sQID = substr($sDirtyQid, 0, $match[0][1]);
} else {
// It was just the QID....
$sQID = $sDirtyQid;

This comment has been minimized.

Copy link
@Shnoulle

Shnoulle Apr 6, 2021

Collaborator

And with subquestion with code 1, 2 etc … and for ranking ?

We still unsure it's a qid

This comment has been minimized.

Copy link
@olleharstedt

olleharstedt Apr 7, 2021

Author Collaborator

Good question. Ugh.

This comment has been minimized.

Copy link
@Shnoulle

Shnoulle Apr 7, 2021

Collaborator

It's commented after

// So for certain question types (for example Text Array) the field name cannot be properly derived

Dev know it's a bad solution ;)

}
if ((string) intval($sQID) !== $sQID) {
throw new \Exception('sQID is not an integer: ' . $sQID);

This comment has been minimized.

Copy link
@Shnoulle

Shnoulle Apr 6, 2021

Collaborator

Maybe log it ? In mariadb ist's a dev issue , no need an Exception

This comment has been minimized.

Copy link
@olleharstedt

olleharstedt Apr 7, 2021

Author Collaborator

Silent fail is the worst fail.

This comment has been minimized.

Copy link
@olleharstedt

olleharstedt Apr 7, 2021

Author Collaborator

Who would read the logs, though?

This comment has been minimized.

Copy link
@Shnoulle

Shnoulle Apr 7, 2021

Collaborator

Yes, but here : some real user without development knowledge can not check integrity if we made an error.

Before the fix : they can delete old table (for example), after the fix : it break

Maybe with debug ?

This comment has been minimized.

Copy link
@olleharstedt

olleharstedt Apr 7, 2021

Author Collaborator

If we made an error, then we want them to report the bug...?

This comment has been minimized.

Copy link
@Shnoulle

Shnoulle Apr 7, 2021

Collaborator

🤣

OK 🤔

}

// Here, we get the question as defined in backend
$oQuestion = Question::model()->findByAttributes([ 'qid' => $sQID , 'sid' => $oSurvey->sid ]);
Expand Down

0 comments on commit 6a82960

Please sign in to comment.