Bug #95485
closedSlug generation fails for non pages records when prefixParentPageSlug is activated in generatorOptions
100%
Description
Following TCA configuration for non pages records results in a Segmentation Fault because Slughelper::generate() is called infinitively.
This behavior was introduced with https://review.typo3.org/c/Packages/TYPO3.CMS/+/71059
'slug' => [ 'config' => [ 'type' => 'slug', 'generatorOptions' => [ 'prefixParentPageSlug' => true, ], ], ]
The problem is, that for non pages records the $this->tableName
is set to something else than pages
. As a result the if-clause at the beginning of generate
will not become true.
It do not know what the best solution might be. Maybe set $this->tableName = 'pages'
after calling $this->resolveParentPageRecord()
? But I am not sure whether this would always be true? Please let me know I would like to fix it!
I have created a test to reproduce the problem:
... namespace TYPO3\CMS\Core\Tests\Unit\DataHandling ... class SlugHelperTest extends UnitTestCase { /** * @return array */ public function generatePrependsSlugsForNonPagesDataProvider(): array { return [ 'simple title' => [ 'Product Name', '/parent-page/product-name', [ 'generatorOptions' => [ 'fields' => ['title'], 'prefixParentPageSlug' => true, ], ], ], ]; } /** * @dataProvider generatePrependsSlugsForNonPagesDataProvider * @param string $input * @param string $expected * @test */ public function generatePrependsSlugsForNonPages(string $input, string $expected, array $options): void { $GLOBALS['dummyTable']['ctrl'] = []; $parentPage = [ 'uid' => '0', 'pid' => null, ]; $subject = $this->getAccessibleMock( SlugHelper::class, ['resolveParentPageRecord'], [ 'another_table', 'slug', $options, ] ); $subject->expects(self::any()) ->method('resolveParentPageRecord') ->withAnyParameters() ->willReturn($parentPage); self::assertEquals( $expected, $subject->generate(['title' => $input, 'uid' => 13], 13) ); } }
Updated by Stefan Bürk about 3 years ago
Created WIP patch for now adding the provided test case:
72168: [WIP][BUGFIX] Slug generation fails for non page records | https://review.typo3.org/c/Packages/TYPO3.CMS/+/72168
Updated by Stefan Bürk about 3 years ago
- Related to Bug #94655: Slug generation for records with pid=0 always returns "/" added
Updated by Gerrit Code Review about 3 years ago
- Status changed from New to Under Review
Patch set 2 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/72168
Updated by Stefan Bürk about 3 years ago
- Status changed from Under Review to Needs Feedback
Thanks for reporting this, Gerald Rintisch.
At first, where did you get the information from to use `prefixParentSlug` for non page records? Or do you really have the use-case that page slugs have to been added as prefix ?
Reading the documentation this field was only intended for use on pages itself:
The slugs of parent pages will be prefixed to the slug for the page itself. Disable it for shorter URLs, but take the higher chance of collision into consideration.
Anyway it should not end in a endless recursive loop, which occurs for records on pid = 0 or on pages with no site root up in the tree since #94655 .
Updated by Gerrit Code Review about 3 years ago
- Status changed from Needs Feedback to Under Review
Patch set 3 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/72168
Updated by Gerrit Code Review about 3 years ago
Patch set 4 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/72168
Updated by Stefan Bürk about 3 years ago
- Status changed from Under Review to Needs Feedback
Updated by Gerald Rintisch about 3 years ago
Stefan Bürk wrote in #note-5:
At first, where did you get the information from to use `prefixParentSlug` for non page records? Or do you really have the use-case that page slugs have to been added as prefix ?
This is not a real use-case, it happened due to a copy-paste error. But as you also already said even if it's used falsy this should not crashing the whole system and that's the reason why I reported that error.
Updated by Stefan Bürk about 3 years ago
Thanks for the answer, Gerald Rintisch.
Is it possible for you to try out v11 master + the patch from gerrit if it solves this issue for you (latest patchset)? (Should do it). Or at least as composer patch ?
Updated by Gerrit Code Review about 3 years ago
- Status changed from Needs Feedback to Under Review
Patch set 5 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/72168
Updated by Gerrit Code Review about 3 years ago
Patch set 6 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/72168
Updated by Gerrit Code Review about 3 years ago
Patch set 7 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/72168
Updated by Gerrit Code Review almost 3 years ago
Patch set 1 for branch 11.5 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/72256
Updated by Gerrit Code Review almost 3 years ago
Patch set 1 for branch 10.4 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/72257
Updated by Stefan Bürk almost 3 years ago
- Status changed from Under Review to Resolved
- % Done changed from 0 to 100
Applied in changeset f75945cc074633389bb88a1cb6a98dbba2af9a97.
Updated by Gerald Rintisch almost 3 years ago
Stefan Bürk wrote in #note-10:
Is it possible for you to try out v11 master + the patch from gerrit if it solves this issue for you (latest patchset)? (Should do it). Or at least as composer patch ?
Thanks for your fix, Stefan Bürk!
Unfortunately I was not able to test it, sorry for not answering in time :-/