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

[Issue] Fix SQL query quoting/casting when type is passed to where function #29590

Closed
4 tasks
m2-assistant bot opened this issue Aug 17, 2020 · 2 comments · Fixed by #27980
Closed
4 tasks

[Issue] Fix SQL query quoting/casting when type is passed to where function #29590

m2-assistant bot opened this issue Aug 17, 2020 · 2 comments · Fixed by #27980
Labels
Component: DB Fixed in 2.4.x The issue has been fixed in 2.4-develop branch Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development Priority: P3 May be fixed according to the position in the backlog. Progress: done Reproduced on 2.4.x The issue has been reproduced on latest 2.4-develop branch Severity: S4 Affects aesthetics, professional look and feel, “quality” or “usability”.

Comments

@m2-assistant
Copy link

m2-assistant bot commented Aug 17, 2020

This issue is automatically created based on existing pull request: #27980: Fix SQL query quoting/casting when type is passed to where function


Framework/DB/Select where function doesn't handle the "type" correctly.

Preconditions (*)

The $type variable can be both string or int, so before comparing it to
'TYPE_CONDITION' string it has to be casted to avoid comparing integer zero
with string (0 == 'TYPE_CONDITION') which will wrongly return true,
and remove the information about type.

Pass type provided to where function down the chain to allow automatic
casting of arrays of values e.g. to int.

This fixes following cases:
1)
$select-->where('attr_table.store_id IN (?)', $storeIds, Zend_Db::INT_TYPE);
2)
$select-->where('attr_table.store_id = ?', $storeId, Zend_Db::INT_TYPE);
In both cases now passed value is correctly casted to int
(either single value, or each value from array)

Related Pull Requests

Fixed Issues (if relevant)

Steps to reproduce:

  1. Make custom select like
    $select->from(['catalog_product_entity'], '*')->where('entity_id in (?)', ['1', 2, 3], \Zend_Db::INT_TYPE);
  2. Check sql
    $select->__toString()

Expected result (*)

SELECT catalog_product_entity.* FROM catalog_product_entity WHERE (entity_id in (1, 2, 3));

Actual result (*)

SELECT catalog_product_entity.* FROM catalog_product_entity WHERE (entity_id in ('1', 2, 3));

Questions or comments

Contribution checklist (*)

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All new or changed code is covered with unit/integration tests (if applicable)
  • All automated tests passed successfully (all builds are green)
@m2-assistant m2-assistant bot added Component: DB Severity: S2 Major restrictions or short-term circumventions are required until a fix is available. labels Aug 17, 2020
@magento-engcom-team magento-engcom-team added the Issue: Format is not valid Gate 1 Failed. Automatic verification of issue format is failed label Aug 17, 2020
@ghost ghost added Progress: PR in progress Priority: P2 A defect with this priority could have functionality issues which are not to expectations. and removed Progress: ready for QA labels Aug 17, 2020
@magento-engcom-team magento-engcom-team added Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed and removed Issue: Format is not valid Gate 1 Failed. Automatic verification of issue format is failed labels Sep 17, 2020
@engcom-Alfa engcom-Alfa added Reproduced on 2.4.x The issue has been reproduced on latest 2.4-develop branch Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed labels Sep 17, 2020
@magento-engcom-team
Copy link
Contributor

✅ Confirmed by @engcom-Alfa
Thank you for verifying the issue. Based on the provided information internal tickets MC-37711 were created

Issue Available: @engcom-Alfa, You will be automatically unassigned. Contributors/Maintainers can claim this issue to continue. To reclaim and continue work, reassign the ticket to yourself.

@magento-engcom-team magento-engcom-team added Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development Progress: ready for dev and removed Progress: PR in progress labels Sep 17, 2020
@ghost ghost added Priority: P3 May be fixed according to the position in the backlog. Severity: S4 Affects aesthetics, professional look and feel, “quality” or “usability”. and removed Priority: P2 A defect with this priority could have functionality issues which are not to expectations. Severity: S2 Major restrictions or short-term circumventions are required until a fix is available. labels Sep 22, 2020
@gabrieldagama gabrieldagama added the Fixed in 2.4.x The issue has been fixed in 2.4-develop branch label Sep 23, 2020
@gabrieldagama
Copy link
Contributor

Hi @m2-assistant[bot]. Thank you for your report.
The issue has been fixed in #27980 by @tmotyl in 2.4-develop branch
Related commit(s):

The fix will be available with the upcoming 2.4.2 release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: DB Fixed in 2.4.x The issue has been fixed in 2.4-develop branch Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development Priority: P3 May be fixed according to the position in the backlog. Progress: done Reproduced on 2.4.x The issue has been reproduced on latest 2.4-develop branch Severity: S4 Affects aesthetics, professional look and feel, “quality” or “usability”.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants