Bug #88973

ext:seo does not work for complex extbase routeenhancers

Added by Pascal Querner 3 months ago. Updated 3 months ago.

Status:
Rejected
Priority:
Won't have this time
Assignee:
-
Category:
SEO
Target version:
-
Start date:
2019-08-18
Due date:
% Done:

0%

TYPO3 Version:
9
PHP Version:
7.2
Tags:
Complexity:
Is Regression:
Sprint Focus:

Description

This issue is partially related to https://forge.typo3.org/issues/87016
However, I am creating a new issue because its missing that the comlex routeEnhancers do not work, when you use a field (like year) which results from a data row you can not simple fetch from the database (instead is fetched from crdate field, which must be converted from timestamp to year only)

My proposed change was https://review.typo3.org/c/Packages/TYPO3.CMS/+/61505
However, as Andreas Fernandez said in the code review, this opens the application to execute any functions which are programmed into the typoscript snippet.

Question now is, since its security related, how do we handle this?
I could make the PR read out an additional (GLOBALS) array which contains valid function names to be used, but that would only shift the issue to a new place imho. An attacker can modify that array and still use everything he wanted.
However, an attacker can do anything with typoscript, access, and php anyway.

Why is this needed anyway?
The easiest example I can think of, is the usage of ext:news. In this, you can routeEnhancer your URLs to contain the year/month/date values. You must set "hrdate" to "1" in the ts of ext:news for the extensions viewhelper to produce such URLs in the frontend (to include the GET parameters for month, year and date).
If you have that, ext:seo typoscript is unable to parse, or add that to the GET parameters for typoLink to build the URL properly.

Why?
Because month/date/year is build not from a column found in the database row. Instead its build from the "crdate" (timestamp) and formatted to only contain the month. Because /seo/Classes/XmlSitemap/RecordsXmlSitemapDataProvider.php is only able to fetch column data, but not perform any logic on it, I proposed the change previously mentioned.

Snippets:
RouteEnhancer config which will enhance ext:news URI to be year/month/date/newstitle

  News:
    type: Extbase
    extension: News
    plugin: Pi1
    routes:
      -
        routePath: '/{tag-name}'
        _controller: 'Tag::show'
        _arguments:
          tag-name: tag
        requirements:
          tag-name: '^[a-zA-Z0-9].*$'
      -
        routePath: '/page-{page}'
        _controller: 'News::list'
        _arguments:
          page: '@widget_0/currentPage'
        requirements:
          page: \d+
      -
        routePath: '/{date-year}/{date-month}/{date-day}/{news-title}'
        _controller: 'News::detail'
        _arguments:
          news-title: news
          date-month: month
          date-year: year
          date-day: day
      -
        routePath: '/{date-year}'
        _controller: 'News::list'
        _arguments:
          date-month: overwriteDemand/month
          date-year: overwriteDemand/year
          page: '@widget_0/currentPage'
        requirements:
          date-year: \d+
      -
        routePath: '/{date-year}/page-{page}'
        _controller: 'News::list'
        _arguments:
          date-year: overwriteDemand/year
          page: '@widget_0/currentPage'
        requirements:
          date-year: \d+
          page: \d+
      -
        routePath: '/{date-year}/{date-month}'
        _controller: 'News::list'
        _arguments:
          date-month: overwriteDemand/month
          date-year: overwriteDemand/year
          page: '@widget_0/currentPage'
        requirements:
          date-month: \d+
          date-year: \d+
      -
        routePath: '/{date-year}/{date-month}/page-{page}'
        _controller: 'News::list'
        _arguments:
          date-month: overwriteDemand/month
          date-year: overwriteDemand/year
          page: '@widget_0/currentPage'
        requirements:
          date-month: \d+
          date-year: \d+
          page: \d+
    defaultController: 'News::list'
    defaults:
      page: '0'
      date-month: ''
      date-year: ''
    aspects:
      news-title:
        type: PersistedAliasMapper
        tableName: tx_news_domain_model_news
        routeFieldName: path_segment
      tag-name:
        type: PersistedAliasMapper
        tableName: tx_news_domain_model_tag
        routeFieldName: slug
      page:
        type: StaticRangeMapper
        start: '1'
        end: '25'
      date-month:
        type: StaticValueMapper
        map:
          januar: '1'
          februar: '2'
          maerz: '3'
          april: '4'
          mai: '5'
          juni: '6'
          juli: '7'
          august: '8'
          september: '9'
          oktober: '10'
          november: '11'
          dezember: '12'
      date-year:
        type: StaticRangeMapper
        start: '2019'
        end: '2030'
      date-day:
        type: StaticRangeMapper
        start: '01'
        end: '31'

Proposed TS code for ext:seo to make the proposed PR work:

plugin.tx_seo.config {
    xmlSitemap {
        sitemaps {
            news {
                provider = TYPO3\CMS\Seo\XmlSitemap\RecordsXmlSitemapDataProvider
                config {
                    table = tx_news_domain_model_news
                    additionalWhere =
                    sortField = sorting
                    lastModifiedField = tstamp
                    pid = 26
                    recursive = 2
                    url {
                        pageId = 25
                        fieldToParameterMap {
                            uid = tx_news_pi1[news]
                        }

                        fieldToParameterMapDynamic {
                            day {
                                field = crdate
                                func = strftime
                                param = %e
                                as = tx_news_pi1[day]
                            }
                            month {
                                field = crdate
                                func = strftime
                                param = %-m
                                as = tx_news_pi1[month]
                            }
                            year {
                                field = crdate
                                func = strftime
                                param = %Y
                                as = tx_news_pi1[year]
                            }
                        }

                        additionalGetParameters {
                            tx_news_pi1.controller = News
                            tx_news_pi1.action = detail
                        }

                        useCacheHash = 1
                    }
                }
            }
        }
    }
}

History

#1 Updated by Georg Ringer 3 months ago

  • Status changed from New to Needs Feedback

Thanks for creating this issue. IMO there are 2 things which are important:

1st: It is not about the route enhancers but about providing the proper arguments to the link building
2nd: Why not just create your own sitemap provider? I don't like that the core must have a configurable solution for everything as this just increases the complexity while it can be very easily solveable with a custom provider. EXT:news will also ship a custom provider to support the custom configuration.

so what you think?

#2 Updated by Pascal Querner 3 months ago

  • Priority changed from Should have to Won't have this time

1) You are correct
2) After talking to you on Slack you made me realize and accept that custom providers are the best use for this. As you pointed out in Slack the recordxmlfetcher isnt part of public API.

Georg in the meantime provided his custom record provider for ext:news with the wanted functionality. https://github.com/georgringer/news/commit/f5fd8def68505c65236fe4703f6b7d255f970a55

Therefore I vote closing this issue. I will also abandon (=close) the proposed PR mentioned in the first post.

Thanks all who helped here specially Georg and Andreas. :)

~Pascal

#3 Updated by Georg Ringer 3 months ago

  • Status changed from Needs Feedback to Rejected

Also available in: Atom PDF