Project

General

Profile

Actions

Bug #95485

closed

Slug generation fails for non pages records when prefixParentPageSlug is activated in generatorOptions

Added by Gerald Rintisch over 2 years ago. Updated over 1 year ago.

Status:
Closed
Priority:
Should have
Assignee:
Category:
Link Handling, Site Handling & Routing
Target version:
-
Start date:
2021-10-06
Due date:
% Done:

100%

Estimated time:
TYPO3 Version:
11
PHP Version:
7.4
Tags:
generatorOptions, prefixParentPageSlug
Complexity:
easy
Is Regression:
No
Sprint Focus:

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)
        );
    }
}


Related issues 1 (0 open1 closed)

Related to TYPO3 Core - Bug #94655: Slug generation for records with pid=0 always returns "/"Closed2021-07-28

Actions
Actions #1

Updated by Stefan Bürk over 2 years ago

  • Assignee set to Stefan Bürk
Actions #2

Updated by Stefan Bürk over 2 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

Actions #3

Updated by Stefan Bürk over 2 years ago

  • Related to Bug #94655: Slug generation for records with pid=0 always returns "/" added
Actions #4

Updated by Gerrit Code Review over 2 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

Actions #5

Updated by Stefan Bürk over 2 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:

https://docs.typo3.org/m/typo3/reference-tca/master/en-us/ColumnsConfig/Type/Slug/Properties/GeneratorOptions.html#confval-generatorOptions:prefixParentPageSlug

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 .

Actions #6

Updated by Gerrit Code Review over 2 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

Actions #7

Updated by Gerrit Code Review over 2 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

Actions #8

Updated by Stefan Bürk over 2 years ago

  • Status changed from Under Review to Needs Feedback
Actions #9

Updated by Gerald Rintisch over 2 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.

Actions #10

Updated by Stefan Bürk over 2 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 ?

See: https://review.typo3.org/c/Packages/TYPO3.CMS/+/72168

Actions #11

Updated by Gerrit Code Review over 2 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

Actions #12

Updated by Gerrit Code Review over 2 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

Actions #13

Updated by Gerrit Code Review over 2 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

Actions #14

Updated by Gerrit Code Review over 2 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

Actions #15

Updated by Gerrit Code Review over 2 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

Actions #16

Updated by Stefan Bürk over 2 years ago

  • Status changed from Under Review to Resolved
  • % Done changed from 0 to 100
Actions #17

Updated by Gerald Rintisch over 2 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 :-/

Actions #18

Updated by Benni Mack over 1 year ago

  • Status changed from Resolved to Closed
Actions

Also available in: Atom PDF