Problem/Motivation
According to the filtering documentation the labels assigned to filters can be a string or even an integer, but we found that filters with "0" as a label throw the following error:
'@root' is a reserved filter id.
Because of the way that PHP makes comparisons between the integer 0 and strings (https://www.php.net/manual/en/types.comparisons.php), the following piece of code is evaluated to true in the Filter class.
// Throw an exception if the query uses the reserved filter id for the
// root group.
if ($key == static::ROOT_ID) {
$msg = sprintf("'%s' is a reserved filter id.", static::ROOT_ID);
throw new \UnexpectedValueException($msg);
}
Steps to reproduce
- Enable jsonapi.
- Make a filtered request to a collection endpoint with "0" as a label of one of the filters. For example:
/jsonapi/node/article?filter[0][path]=status&filter[0][value]=1
Proposed resolution
Make a strict comparison between the filters' label and the reserved filter id.
Comments
Comment #2
guillerecalde commentedComment #3
mohit.bansal623 commentedComment #4
abhijith s commentedApplied patch #3 in 8.9.x and it works fine.The error is gone after applying this patch.Adding screenshots below,
Before:

After patch:

Comment #5
ilgnerfagundes commentedI applied patch # 3 on drupal 8.9 and it's working, I'll leave prints showing
Comment #6
anacolautti commentedHi there! Out of curiosity, as far as I see both patches have the exact same contents. Why do we have 2 patches here?
Comment #7
alexpottThanks for filing this bug report and for fixing the issue. Bug fixing is very valuable. In order to commit a bug fix, we need an automated test to prove that we've fixed the bug and ensure that we don't break it again in the future. For more information about writing tests in Drupal, see the following links:
Comment #8
mohit_aghera commented- Adding test cases and test-only patch to replicate the bug.
- I can see the error
'@root' is a reserved filter id.on local when I run the test case.- Bump up version to 9.3.x as we are going to address bugs in that release.
Comment #9
mohit_aghera commentedForgot to remove the quotes around the key from test cases.
Test case is same and still test-only patch failing on local.
Let's see how it goes.
Comment #10
bbralaHi there, the test-only patch should be failing to prove the bug, i know it's a hassle since the testing takes so long. I wonder if perhaps PHP version has something to do with is?
Comment #15
smustgrave commentedThis came up as a daily BSI
Think it would be good to determine if this is still an issue.
See we had a patch with test coverage we could add that to an MR now.