Bug #88973
closedext:seo does not work for complex extbase routeenhancers
0%
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
}
}
}
}
}
}
Updated by Georg Ringer about 5 years 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?
Updated by Pascal Querner about 5 years 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
Updated by Georg Ringer about 5 years ago
- Status changed from Needs Feedback to Rejected