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

Bug: colon issue in query binding #4595

Closed
tibszabo-dev opened this issue Apr 21, 2021 · 4 comments
Closed

Bug: colon issue in query binding #4595

tibszabo-dev opened this issue Apr 21, 2021 · 4 comments
Labels
bug Verified issues on the current code behavior or pull requests that will fix them

Comments

@tibszabo-dev
Copy link

tibszabo-dev commented Apr 21, 2021

Describe the bug
Query binding fails when using date_format with hours in it (colon issue)

If there is a colon in the query, the query builder thinks it is a named bind, and handles the query in that way, even if you have ? bindings.

CodeIgniter 4 version
v4.1.1

Affected module(s)
system/Database/Query.php
Line 352

Expected behavior, and steps to reproduce if appropriate
Old version:
$hasNamedBinds = strpos($sql, ':') !== false && strpos($sql, ':=') === false;
This returns true when query has date_format(now(), '%d-%m-%Y %H:%i'), but in the query I only use ? as binding, so it won't find any :mybind: binding.

suggestion: $hasNamedBinds = preg_match('/:[a-z._-]+:/',$sql);

With this fix, all my queries are working.

Context

  • OS: debian 10
  • Web server: Apache 2.4.38
  • PHP version: 7.3.27
@tibszabo-dev tibszabo-dev added the bug Verified issues on the current code behavior or pull requests that will fix them label Apr 21, 2021
@tibszabo-dev
Copy link
Author

I saw a fix in the develop branch, I think it must be a quick patch, because 4.1.1 is getting old.

@paulbalandan
Copy link
Member

I saw a fix in the develop branch, I think it must be a quick patch, because 4.1.1 is getting old.

Yes, there's already a fix for the bindings. Can you check that out if it will pass with your use case?

@tibszabo-dev
Copy link
Author

I saw a fix in the develop branch, I think it must be a quick patch, because 4.1.1 is getting old.

Yes, there's already a fix for the bindings. Can you check that out if it will pass with your use case?

Yes, it is working well with that fix!

@paulbalandan
Copy link
Member

Thanks for confirming. Closing then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Verified issues on the current code behavior or pull requests that will fix them
Projects
None yet
Development

No branches or pull requests

2 participants