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.
Currently in pathauto's alias cleaner it looks to avoid url paths with the function
public function cleanTokenValues(&$replacements, $data = array(), $options = array()) {
foreach ($replacements as $token => $value) {
// Only clean non-path tokens.
if (!preg_match('/(path|alias|url|url-brief)\]$/', $token)) {
$replacements[$token] = $this->cleanString($value, $options);
}
}
}
however this preg_match is way too broad, if you create your own field named, for example, field_url, then cleanString will not be passed over it. If you create your own field called field_curling (and so on....) then it will also not work because 'curling' contains 'url'.
What are the actual fields that this preg_match is trying to prevent being cleaned? Would a pattern like '/(^path|^alias|^url|^url-brief)\]$/' be more appropriate?
Comment | File | Size | Author |
---|---|---|---|
#24 | pathauto-path-safe-tokens-2773573-24-interdiff.txt | 1.86 KB | Berdir |
#24 | pathauto-path-safe-tokens-2773573-24.patch | 6.68 KB | Berdir |
| |||
#21 | pathauto-path-safe-tokens-2773573-21.patch | 5.46 KB | Primsi |
#19 | pathauto-path-safe-tokens-2773573-17-interdiff.txt | 1.02 KB | Berdir |
#19 | pathauto-path-safe-tokens-2773573-17.patch | 5.77 KB | Berdir |
Comments
Comment #2
cburschkaNote: The "curling" field wouldn't break because the pattern looks for the \] at the end.
After looking at the other available tokens, it seems that the targeted tokens are ones like site:url, node:url, site:url-brief and site:login-url. The latter seems to be the only token matching url] but not :url], so it could be added explicitly.
Based on this and the fact that field machine names cannot contain :, I would suggest the following patch.
Comment #4
cburschkaIf the unit test is correct in wanting [array:join-path] to bypass cleaning, then we could look for [:-] instead of just :. This is still safe because field names also can't contain -.
However, imho this is pretty vague, and it would probably be a better idea to explicitly list the non-transliterable tokens instead of relying on magic strings.
Comment #5
BerdirYes, this is tricky. Thanks for working on this.
I've also seen problems related to doing something like ...:url:relative] when url isn't the last part but the thing that comes later should inherit this thing. So if we explicitly switch to :url, we could maybe look for :url at the end or :url: anywhere in the token?
Comment #6
cburschkaWould
/:(alias|path|login-url|url|url-brief)(:|\]$)/
be a good idea? This matches tokens that have alias,path,login-url,url,url-brief anywhere in the name, delimited by :Comment #8
cburschkaAnd this one updates the unit test to check :path instead of :join-path.
(It might be worth making this configurable - if not as a full regex, then at least as a list of names X,Y,Z that are matched as
:(X|Y|Z):
or:(X|Y|Z)\]$
. But that's a bigger feature addition.)Comment #9
cburschkaComment #11
cburschkaDerp - I missed the fact that the join-path token isn't arbitrary but from tokens.inc. Including it in the regex now.
Comment #12
BerdirNice. It's good that the existing tests are passing but it would be really important to have some additional tests to ensure that additioanal things that we are now supporting also work.
We don't actually need to set up things like nodes to have real token replacements working, we can just create a fake $replacements array like in \Drupal\Tests\pathauto\Kernel\PathautoTokenTest::testPathautoTokens() with various examples, for example [node:field_url], [node:url], [node:url:relative] and so on and pass them to that function to make sure they work as expected.
Comment #13
BerdirTurns out we could use this to be configurable in a project, so using this to also add a setting as well as tests.
Comment #15
BerdirBad patch.
Comment #16
Berdiranother typo in update function name fixed.
Comment #19
BerdirAdding a space in UI and removing it again on save. We also have no tests yet for the UI, but tested in our project and it works.
Comment #21
Primsi CreditAttribution: Primsi at MD Systems GmbH for MD Systems GmbH commentedRe-rolled.
Comment #22
Primsi CreditAttribution: Primsi at MD Systems GmbH for MD Systems GmbH commentedComment #24
BerdirFixed that new test.
Comment #26
BerdirCommitted.
Comment #28
karenann CreditAttribution: karenann as a volunteer commentedI recently updated from pathauto_8.x-1.2 to 8.x-1.3.
I have a content type that uses the path pattern [node:menu-link:parents:join-path]/[node:title]
With this update, I noticed that my URLs were updating from, e.g., /community/campus-map/parking to /communitycampus-map/parking.
I tested with other pages and found the same thing - the slash between the first and second argument was being removed.
I went through the repository and found this commit:
https://cgit.drupalcode.org/pathauto/commit/src/AliasCleaner.php?id=2705...
When I reverted this single commit, my problem disappeared.
I was unsure if this would be best suited to be a part of this issue or if I should create a new issue. I figured that I'd start here and go from there.
Thanks! Karen
Comment #29
BerdirSee #3006242-4: First slash in token [term:parents:join-path] gets stripped for example. This added an update function to update configuration, you probably reverted that with a config import.
Comment #30
karenann CreditAttribution: karenann as a volunteer commentedThanks, Berdir. Following the instructions in the comment you linked me to solved my issue.
Comment #31
sker101 CreditAttribution: sker101 as a volunteer commentedAfter updating the module using the update hook "pathauto_update_8107" provided in the patch, I'm seeing a duplicate value `alias` in the safe_token array.
Is the duplicated `alias` expected to be in the configuration?