Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Steps to Reproduce
- Add a field with a very long title/name like field_01234567890123456789012345 to any content type.
- Create a View with a page and a menu item that uses this field as a Contextual Filter
- Click save
- Site will return the below error and stop working
Warning: preg_match(): Compilation failed: subpattern name is too long (maximum 32 characters) at offset 56 in Drupal\Core\Routing\RouteProvider->getRoutesByPath() (line 257 of core/lib/Drupal/Core/Routing/RouteProvider.php).
Notes
- The regex pattern that is failing to compile is
#^/test(?:/(?P<arg_field_01234567890123456789012345_value>[^/]++))?$#s
- The regex pattern contains a subpattern greater than 32 characters because the 32 character field name is prefixed with 'arg_' and suffixed with '_value'
- The subpattern suffix '_value' can change depending on the type field and requested data.
- There is no way to allow 32 character subpatterns in regex.
- This is a known Symfony issue, but it should not be bringing down a Drupal website.
- I have searched Drupal.org and Google and this does not seem like a very common issue.
- My guess is that Views' contextual filters is the only place in core that is going to run into this problem.
- Marking this as major since it will crash a website.
Workarounds
- Don't use long field names. Field names with less than 20 characters will avoid this issue.
Possible Solutions
- Use subpatterns based on field names.
- Pass all subpatterns through md5() which returns a 32 character string... this is an ugly solution.
- Use an arguments index instead of its name
References
Comment | File | Size | Author |
---|---|---|---|
#16 | 2161727-16-PASS.patch | 11.59 KB | damiankloip |
#16 | 2161727-16-test-only-FAIL.patch | 2.03 KB | damiankloip |
Comments
Comment #1
dawehnerThis is indeed an interesting bug.
This is just a rough ID how to fix it.
Comment #3
jrockowitz CreditAttribution: jrockowitz commentedThanks for taking a crack at this one,
I tested the patch ad it is not working because the 4 character 'arg_' prepended to the 32 character md5() hash is 36 characters long. This still throws a regex compilation error.
When I remove the 'arg_' prefix from
'arg_' . md5($arg_id);
, the site loaded with no errors but I could not get contextual arguments for the 32 character field to work.I am not very familiar with D8 and Symfony but it seems that the
$view_argument_map
is being created but not used to map the 'md5' parameters to their names when a request is coming in.Finally, this is a very minor tweak but since this issue may affect contrib modules as well, the '$view_argument_map' could be call just '$argument_map'.
Comment #4
jrockowitz CreditAttribution: jrockowitz commentedDUPLICATEComment #5
dawehnershould we just truncate the md5 generated hash? We use arg_ in other places.
I don't really care ...
Feel free to work on this patch, I just wanted to get some idea out, so other people don't have to research where the change would be needed.
Comment #6
jrockowitz CreditAttribution: jrockowitz commenteddawehner, you are right, I just need to bite the bullet and see what I can do to help fix this issue.
Your patch definitely pointed me in the right direction.
Basically the below objects and their related tests need to be tweaked to fix this issue.
I think my suggestion to use an md5() hash to limit the parameter name's length is the wrong solution. Since a views arguments are 'indexed' and this order will never change, it is possible that the arg's index can be used as the route's parameter name instead of the view full argument name.
For example, in the current view tests the 'test_id' argument converts to `test_route/{arg_test_id}`, I think it can be simplified to just be `test_route/{arg_0}`
Attached is a patch implementing this solution with all the existing tests updated to look for arg_0 instead arg_{name}. I did remove your test since I am no longer using the '_view_argument_map' option. I am unsure if a new test is needed since this approach is not adding new functionality.
I just noticed after I uploaded my patch that PHPStorm added some extra data to the file. Since I can't delete the uploaded patch, I am going to let the 'test bot' test it and then I will make sure to upload a cleaner patch.
Comment #7
jrockowitz CreditAttribution: jrockowitz commentedComment #9
jrockowitz CreditAttribution: jrockowitz commentedUploading same patch generated using git command instead of PHPStorm IDE.
Comment #11
jrockowitz CreditAttribution: jrockowitz commentedFixing broken tests in Drupal\views\Tests\Plugin\DisplayPageTest
Comment #12
dawehnerYeah this seems to be the only really possible solution, don't use the argument ID but just some index in the pattern.
Comment #13
xjmRelated issue: #1709960: declare a maximum length for entity and bundle machine names
Comment #14
xjmAlso #2120003: [META] Create sensible limits for the maximum length of configuration object filename components.
Comment #15
xjmReviewed with @Dries, @webchick, and @effulgentsia. The approach in this patch looks like a good solution; the other issues I've linked wouldn't solve this even if they were resolved. However, this issue is missing an explicit regression test to prove the functional bug with long fieldnames is now resolved. So let's get an automated test that fails before the patch and passes after. Thanks!
Comment #16
damiankloip CreditAttribution: damiankloip commentedI don't see too much point in adding this, as the internal implementation is completely changing, making the argument ID irrelevant. Anyway, here's a test for that.
No interdiff as the test only patch is the interdiff.
Comment #17
damiankloip CreditAttribution: damiankloip commentedComment #19
dawehnerI really hope that this is high enough in the chain.
Comment #20
webchickCommitted and pushed to 8.x.