[Bug] GraphQL SearchCriteriaValidator not loaded — removed by module-graph-ql di.xml array override#40742
[Bug] GraphQL SearchCriteriaValidator not loaded — removed by module-graph-ql di.xml array override#40742mtytula wants to merge 7 commits intomagento:2.4-developfrom
Conversation
|
Hi @mtytula. Thank you for your contribution!
Allowed build names are:
You can find more information about the builds here For more details, review the Code Contributions documentation. |
|
The security team has been informed about this pull request due to the presence of risky security keywords. For security vulnerability reports, please visit Adobe's vulnerability disclosure program on HackerOne or email psirt@adobe.com. |
1 similar comment
|
The security team has been informed about this pull request due to the presence of risky security keywords. For security vulnerability reports, please visit Adobe's vulnerability disclosure program on HackerOne or email psirt@adobe.com. |
|
The security team has been informed about this pull request due to the presence of risky security keywords. For security vulnerability reports, please visit Adobe's vulnerability disclosure program on HackerOne or email psirt@adobe.com. |
|
@magento run all tests |
|
The security team has been informed about this pull request due to the presence of risky security keywords. For security vulnerability reports, please visit Adobe's vulnerability disclosure program on HackerOne or email psirt@adobe.com. |
1 similar comment
|
The security team has been informed about this pull request due to the presence of risky security keywords. For security vulnerability reports, please visit Adobe's vulnerability disclosure program on HackerOne or email psirt@adobe.com. |
|
@magento create issue |
|
The security team has been informed about this pull request due to the presence of risky security keywords. For security vulnerability reports, please visit Adobe's vulnerability disclosure program on HackerOne or email psirt@adobe.com. |
|
@magento add to contributors team |
|
@magento run all tests |
|
@magento run all tests |
|
The security team has been informed about this pull request due to the presence of risky security keywords. For security vulnerability reports, please visit Adobe's vulnerability disclosure program on HackerOne or email psirt@adobe.com. |
1 similar comment
|
The security team has been informed about this pull request due to the presence of risky security keywords. For security vulnerability reports, please visit Adobe's vulnerability disclosure program on HackerOne or email psirt@adobe.com. |
|
@magento run all tests |
|
@magento run all tests |
|
My tests passed 🚀 |
engcom-Hotel
left a comment
There was a problem hiding this comment.
Thank you @mtytula for making the changes, approving this PR for further process
Preconditions
magento/module-graph-ql100.4.7-p9)pageSizeorcurrentPageargumentsSteps to reproduce
pageSizeexceeding the configured limit, e.g.:{ products(search: "test", pageSize: 999) { items { sku } } }Expected result
GraphQL returns a validation error:
{ "data": { "products": null }, "errors": [ { "message": "Maximum pageSize is 10", "path": [ "products" ], "locations": [ { "line": 2, "column": 5 } ], "extensions": { "category": "graphql-input" } } ] }(
Maximum pageSize is X— where X is the configuredmaxPageSizevalue, default 300)Actual result
Query executes without validation.
pageSize: 999is accepted silently. No error is returned.Root cause
SearchCriteriaValidator(which enforcesmaxPageSize: 300) is registeredin
magento2-base/app/etc/di.xml(primary config):module-graph-ql/etc/di.xml(module config) also definesvalidatorsfor thesame type, but only with
backpressureValidator:Module-level DI configuration has higher priority than primary config.
When module config defines the same
xsi:type="array"argument, it replacesthe primary config's array — it does not merge items. As a result,
searchCriteriaValidatoris silently dropped.Verification at runtime:
Proposed fix
Move
searchCriteriaValidatorregistration frommagento2-base/app/etc/di.xmlto
module-graph-ql/etc/di.xml. Both validators should live at module levelso that future validators added by third-party modules can safely merge via
xsi:type="array".module-graph-ql/etc/di.xml— after fixmagento2-base/app/etc/di.xml— remove this blockContribution checklist (*)
Temporary workaround (composer patch)
Until the upstream fix is merged — apply a patch via
cweagans/composer-patches.1.
composer.json2.
patches/magento/module-graph-ql/fix-search-validator.diff3. Apply
composer require cweagans/composer-patches --no-update composer update magento/module-graph-ql # or simply: composer installAffected repositories
magento/magento2—app/etc/di.xml(magento2-base)magento/module-graph-ql—etc/di.xmlAffected versions
All versions of
magento/module-graph-qlthat introducedbackpressureValidator(i.e. all versions that define
CompositeValidatortype in theiretc/di.xml).Resolved issues: