Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes cron crash due to prepared statements #29

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

michael-bentz
Copy link

Adds empty array ExternalModules::query(). Closes #28

@ChemiKyle
Copy link

This arg is actually REDCap version dependent, you'll want to bump the redcap-version-min in config.json to avoid downloads of this module on incompatible systems. Unfortunately, easy access to which REDCap version introduced the need for a parameter array disappeared when the External Modules repo went private.

@michael-bentz
Copy link
Author

michael-bentz commented Mar 9, 2021

Even though https://community.projectredcap.org/articles/76538/prepared-statements-parameterized-query-supportreq.html describes prepared statements being enforced in REDCap 9.7.8 with framework version 4, the code to enforce prepared statements already exists in REDCap 9.6.2.

The changes to provide empty arrays is supported by REDCap >= 9.5.0. See query signature summary below.

query() signature summary

REDCap Version query() signature prepared statement enforcement
9.4 public static function query($sql) N/A
9.5 public static function query($sql, $parameterValues = null, $retriesLeft = 2) N/A
9.6.2 public static function query($sql, $parameters = null, $retriesLeft = 2) if($parameters === null){ throw new Exception(ExternalModules::tt('em_errors_117')); }

@ChemiKyle
Copy link

The root of this issue (and my mischaracterization of it) was the use of ExternalModules::query rather than $this->framework->query. The former is not likely to be accepted to the repo.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants