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.Problem/Motivation
The provided argument in my case already includes the leading slash and cannot be modified on the client side. The code assumes that there is no leading slash, so the path argument ends up as //path/alias
Proposed resolution
Check the provided argument and static segment for a slash before initializing the $alias variable.
| Comment | File | Size | Author |
|---|---|---|---|
| #7 | leading-slash-check-3133621-7.patch | 2.53 KB | drewble |
| |||
| #6 | 3133621-6.patch | 2.04 KB | heddn |
| |||
| #5 | leading-slash-check-3133621-5.patch | 763 bytes | drewble |
| |||
| #3 | 3133621-3.patch | 697 bytes | heddn |
| |||
Comments
Comment #2
drewble CreditAttribution: drewble commentedI've rolled a patch against 8.x-1.1 that does the following.
API changes
• Add a private method to check both the provided static segment and the provided argument for a leading slash before assignment to a variable, providing one if a leading slash is not present.
• Initialize the
$aliasvariable with the value of the argument and prepend the static segment, if provided.Comment #3
heddnThis might be an easier solution.
Comment #4
heddnComment #5
drewble CreditAttribution: drewble commented@heddn I don't know that #3 solves the problem for an argument with a leading slash.
Say I provide a static segment
path.After the first condition, the value of
$aliasispath/at line 115.If my argument has a leading slash, the next mutation of
$aliasat line 120 changes the value topath//argument.The final mutation at line 121 changes
$aliasto/path//argument#3 would solve the issue for a provided segment with a leading slash as the last mutation would remove the leading slash from the provided segment.
You could get away with
$alias .= '/' . ltrim($argument, '/');at line 120 and deleting line 121. If you wanted to make the handling for the provided segment more robust, you could give the same treatment to line 117$alias .= ltrim($this->options['segments'], '/') . '/';I still like the fewer mutations of
$aliasin the patch in #2, but I can see the sense in your approach.Here's a patch with my suggested changes to your approach.
Comment #6
heddnDon't you have control over the segments in the view setup? Why can't that be fixed? If we need to, we could (should?) add validation that the segments don't have a leading slash.
Actually, here's a patch that adds form validation. If that doesn't make sense, please be patient with me and explain more of your use case and why the segments must include leading/trailing slashes.
Comment #7
drewble CreditAttribution: drewble commentedYou're right. The segment value is totally controlled. I do like the addition of validation. I apologize if I came across as impatient.
With the validation, line 127 can drop the
ltrim()Here's a new patch combining the two.
Comment #8
heddnIs #7 needed at all? I don't think it is.
Comment #9
drewble CreditAttribution: drewble commentedI thought it made sense for the convenience of anyone who has the same issue down the line, so they could add a single patch. If it is clutter I suppose it can be removed.
Comment #11
heddnThe warning message for the form already said not to prefix or suffix with a slash. Now we do proper validation. I think that's probably good enough. Landed this. Thanks for noticing and bringing it up. And posting the first patches.