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

Having the question mark character "?" in cleaner pre/post sql fields causes ERROR: Incorrect number of query parameters #105

Closed
nayabbukhari opened this issue Apr 24, 2020 · 4 comments
Assignees

Comments

@nayabbukhari
Copy link

Instruction:
Settings
1- Execute custom database query at post-wash
admin/settings.php?section=cleaner_custom_sql_post
2- Execute custom database query at pre-wash
admin/settings.php?section=cleaner_custom_sql_pre

If there is any ?/Placeholder On below boxes, there will be an error as shown below
Default exception handler: ERROR: Incorrect number of query parameters. Expected 1, got 0. Debug:
// error for admin/settings.php?section=cleaner_custom_sql_post
line 43 of /local/datacleaner/cleaner/custom_sql_post/classes/clean.php: call to local_datacleaner\clean::execute_sql()

  • line 145 of /local/datacleaner/cli/clean.php: call to cleaner_custom_sql_post\clean::execute()

Solution:
We can create separate input boxes for the password and decode/encrypt that field to store ?/placeholder.

Questions are welcome.

@kristian-94
Copy link

Probably this just needs some extra escaping to happen right before we run the sql, ie. here: https://github.com/catalyst/moodle-local_datacleaner/blob/master/cleaner/custom_sql_post/classes/clean.php#L43

@kristian-94
Copy link

@nayabbukhari We probably should also make a simple unit test for this, to feed in some test sql and see that it doesn't error

@kristian-94 kristian-94 changed the title Placeholder ? in cleaner_sutom_sql_post or cleaner_sutom_sql_pre fields Having the question mark character "?" in cleaner pre/post sql fields causes ERROR: Incorrect number of query parameters Apr 24, 2020
@nayabbukhari
Copy link
Author

nayabbukhari commented May 1, 2020

Hi All

Here is an update.

The Place holder issue is a bug related to PHP and is fixed for 7.4.

Issue Discussion: https://stackoverflow.com/questions/36173440/ ... when-using-pdo-with-postgresql
Official Bug discussion: https://bugs.php.net/bug.php?id=71885
Documentation: https://wiki.php.net/rfc/pdo_escape_placeholders
Git Diff: php/php-src#4217 ... 74929394c734d8b710d7a9ca3c9d3a

I will test with 7.4 and update the ticket & plugin issue with results.

Regards
Nayab

@brendanheywood
Copy link
Contributor

That doesn't sound right at all, moodle doesn't use the PDO drivers

brendanheywood added a commit that referenced this issue May 12, 2020
brendanheywood added a commit that referenced this issue May 12, 2020
brendanheywood added a commit that referenced this issue May 12, 2020
brendanheywood added a commit that referenced this issue May 12, 2020
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

No branches or pull requests

3 participants