Skip to content
Snippets Groups Projects
Select Git revision
  • 12.4
  • main default
  • 11.5
  • 13.3
  • l10n_main
  • 13.1
  • 13.0
  • l10n_12.4
  • 10.4
  • 12.1
  • 9.5
  • 11.3
  • master
  • 11.1
  • TYPO3_8-7
  • 10.2
  • 9.3
  • 9.2
  • TYPO3_7-6
  • TYPO3_7-0
  • v13.3.1
  • v12.4.21
  • v11.5.40
  • v13.3.0
  • v12.4.20
  • v12.4.19
  • v12.4.18
  • v11.5.39
  • v12.4.17
  • v13.2.1
  • v13.2.0
  • v12.4.16
  • v11.5.38
  • v13.1.1
  • v12.4.15
  • v11.5.37
  • v13.1.0
  • v12.4.14
  • v12.4.13
  • v12.4.12
40 results
Search by author
  • Any Author
  • Daniel Hürtgen Daniel Hürtgen higidi
  • Jenkins Jenkins jenkins
  • **** **** project_53_bot
3 authors
  1. Sep 15, 2024
  3. Sep 14, 2024
  5. Sep 10, 2024
  7. Aug 28, 2024
  9. Aug 15, 2024
  11. Aug 14, 2024
  13. Jul 23, 2024
  15. Jun 26, 2024
  17. May 27, 2024
  19. Apr 12, 2024
  21. Apr 07, 2024
  23. Mar 19, 2024
    • Christian Kuhn's avatar
      [TASK] Add FrontendTypoScriptFactory · 5712a422
      Christian Kuhn authored 
      In version 12, the introduction of the new TypoScript parser
was accompanied by the implementation of factories for
PageTsConfig and UserTsConfig.
A factory for Frontend TypoScript has not been added,
though: Frontend TypoScript creation ended up in
TSFE->getFromCache(). At this point, establishing a proper
factory was unfeasible due to the numerous dependencies of
TypoScript creation to TSFE internals.

With recent refactorings around TSFE, coupled with lots of
state now being represented as request attributes, it's now
possible to decompose getFromCache() and establish a
FrontendTypoScriptFactory.

getFromCache() is a complex beast: It influences Frontend
rendering performance a lot, and tries to trigger the least
amount of calculations, especially in 'fully cached pages'
context. This results in high required complexity due to
lots of state with diverse cross dependencies.

The method composes of three main steps:
1. Bootstrap TypoScript setting ("constants") and calculate
   setup condition verdicts. This creates required TypoScript
   related state needed to calculate the page cache identifier.
2. Access page cache, lock page rendering if needed, and see
   if a possible page cache content contains uncached ("_INT")
   sections.
3. Calculate at least setup "config." depending on given
   type/typeNum, but create full TypoScript setup if a cached
   page contains uncached sections or could not be retrieved
   from cache.

The patch extracts parts 1 and 3 to FrontendTypoScriptFactory.
Part 2 is moved into PrepareTypoScriptFrontendRendering
middleware.

This approach allowed these related refactorings:
* The release of rendering locks is now consolidated within the
  PrepareTypoScriptFrontendRendering middleware. This guarantees
  locks are released, even in scenarios where lower middleware
  components encounter errors. This addresses an issue where
  locks retained during crashes, leading to deadlock situations
  in subsequent requests.
* Dependencies to TSFE within ext:redirects RedirectService
  are reduced, and it no longer locks page rendering.
* The Extbase BackendConfigurationManager utilizes the new
  factory, eliminating the need for its own implementation.

The patch unlocks further refactorings: It especially allows
removing the cache related properties from TSFE by representing
them as request attributes. Subsequent patches will address this
task accordingly.

Resolves: #103410
Related: #97816
Related: #98914
Related: #102932
Releases: main
Change-Id: I7fd158cffeebe6b2c64e0e3595284b8780fb73cf
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/83179


Tested-by: default avatarChristian Kuhn <lolli@schwarzbu.ch>
Reviewed-by: default avatarStefan Bürk <stefan@buerk.tech>
Reviewed-by: default avatarGarvin Hicking <gh@faktor-e.de>
Tested-by: default avatarStefan Bürk <stefan@buerk.tech>
Tested-by: default avatarcore-ci <typo3@b13.com>
Tested-by: default avatarGarvin Hicking <gh@faktor-e.de>
Reviewed-by: default avatarChristian Kuhn <lolli@schwarzbu.ch>
      5712a422
  25. Mar 08, 2024
  27. Mar 07, 2024
  29. Jan 29, 2024
  31. Jan 26, 2024
    • Christian Kuhn's avatar
      [!!!][FEATURE] Avoid TSFE->config['config'] · a085e885
      Christian Kuhn authored 
      Frontend TypoScript has two special details, next to
'configuration' ("constants") and 'setup'.

First, there is the determined "PAGE" object that depends
on type / typeNum. It allows to render multiple different
variants of a pages content. Historically, this has often
been a "print" view, nowadays, this is usually a "json"
variant, or some XML for sitemaps.

A frontend is not rendered without a proper PAGE object.
The FE rendering chain determines the given 'type' and maps
it to a configured PAGE with this 'typeNum', defaulting
to zero '0'.
The patch models the determined PAGE object to the Request
attribute 'FrontendTypoScript' now, which is used by FE
RequestHandler to manage rendering of this PAGE type.

Second special thing is 'config' TypoScript as top-level
'config' settings. Those can be overridden by the specific
PAGE object (often 'page.config'). A typical use case is
'json.config.disableAllHeaderCode = 1'.
This "merged" 'config' array has been available as
TSFE->config['config'] before. The patch makes the merged
config available in the 'FrontendTypoScript' object as
well, available in the Request object as
'frontend.typoscript' attribute.

The patch adapts all usages of TSFE->config['config'] to
the attribute.

There is a special quirk with 'config': This part of
TypoScript 'setup' is so important, that it needs to be
available in 'fully cached page' scenarios as well, to
for instance decide if a content-length header should be
added to the response.
This has been the reason 'config' has been cached along
with page cache content in 'page' cache before.
The patch changes this and adds dedicated cache entries
for 'merged config' in the (file based) 'typoscript' cache.
This reduces page cache size a bit and allows re-using
these 'config' cache accross different pages when they
are identical, which is determined by the cache identifiers.

All these changes are logical follow-ups to the TypoScript
parser that has been implemented with v12.
They pave the way to extract TypoScript calculation from
TypoScriptFrontendController with upcoming patches.

Next to this refactoring, the existing hooks in this area
had to fall, especially to get rid of their dependency to
"pObj" TypoScriptFrontendController. They are mostly
substituted by new events, details are documented with the
.rst files and class comments.

The patch reduces usages of $GLOBALS['TSFE'] and usages
of TypoScriptFrontendController in general.

$GLOBALS['TSFE']->config['config'] is still available as
b/w compat layer (a dedicated deprecation will follow),
but core usages have been removed. This became a bit
complicated with TimeTrackerInitialization and
WorkspacePreview middlewares: Both are run before the
final TypoScript 'config' is determined, so they do
not have that attribute attached to their incoming
Request. But they still need to know TS 'config'
details to decide on details *after* calling lower handle()
middlewares. They by now accessed $GLOBALS['TSFE'], which
is (still) set by a lower middleware. To avoid this
indirect state usage, these middlewares now register an event
listener that is dispatched after TypoScript 'config' has been
determined, and set local state to know if they should add
additional Response data.

Resolves: #102932
Releases: main
Change-Id: I8282f7a93fe9594e78865db63a3e656cc4d5f8da
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/82522


Tested-by: default avatarStefan Bürk <stefan@buerk.tech>
Tested-by: default avatarBenni Mack <benni@typo3.org>
Reviewed-by: default avatarBenni Mack <benni@typo3.org>
Reviewed-by: default avatarStefan Bürk <stefan@buerk.tech>
Tested-by: default avatarChristian Kuhn <lolli@schwarzbu.ch>
Tested-by: default avatarcore-ci <typo3@b13.com>
Reviewed-by: default avatarChristian Kuhn <lolli@schwarzbu.ch>
      a085e885
  33. Jan 19, 2024
  35. Jan 18, 2024
    • Christian Kuhn's avatar
      [TASK] Streamline TypoScriptFrontendItitialization · 6af4f29e
      Christian Kuhn authored 
      This patch is the result of a careful refactoring of
the TypoScriptFrontendInitialization 'body' methods.

The patch establishes the new PageInformationFactory
which does all the main lifting. It either returns
the verified PageInformation object, or throws an
early dedicated exception carrying a response from the
ErrorController when something went wrong (access
checks, language overlays ...), or a StatusException
when ErrorController itself error'ed.

When PageInformation could be determined, rendering
is dispatched to middlewares below. In case of error,
the code flow is streamlined to always *return* the
early response, so upper middlewares can kick in. This
was a mixed bag before, where such exceptions sometimes
bubbled up, skipping upper middlewares.

While most error scenarios are covered by the complex
'SiteHandling' related functional tests already, a set
of simple additional tests is set up to check for
casual things in a more obvious way.

Note the internal code flow within PageInformationFactory
can still be simplified and disentangled some more, but
the outer class communication is fine now and the
internal handling is already much more easy to follow.

As drive-by, @internal getUriToCurrentPageForRedirect()
is moved from TSFE to the consuming 'shortcut and
mountpoint redirect' middleware.

The patch also outlines flaws of the PageRepository
strategy, that becomes obvious after disentangling the
consuming code. It adds a todo on how PageRepository
could be improved. We should follow this path soon, since
the entire area with all the access and logic checks is
security relevant: We want to end up with code that is as
easy to grasp and follow as possible.

Resolves: #102856
Related: #102715
Releases: main
Change-Id: Icf65dd21ced16af521735f9af003b65ec94909c9
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/82452


Tested-by: default avatarcore-ci <typo3@b13.com>
Tested-by: default avatarChristian Kuhn <lolli@schwarzbu.ch>
Reviewed-by: default avatarBenni Mack <benni@typo3.org>
Reviewed-by: default avatarChristian Kuhn <lolli@schwarzbu.ch>
Tested-by: default avatarStefan Bürk <stefan@buerk.tech>
Reviewed-by: default avatarStefan Bürk <stefan@buerk.tech>
Tested-by: default avatarBenni Mack <benni@typo3.org>
      6af4f29e
    • linawolf's avatar
      [BUGFIX] Prevent type error on static route · 3dc75cf8
      linawolf authored 
      If you happen to have inconsistent data in the
config.yaml like:

```
routes:
  -
    route: robots.txt
    type: staticText
    source: 'EXT:my_sitepackage_mysite/Resources/Public/robots.txt'
```

Currently a PHP type error is thrown instead of a proper exception.

Resolves: #102773
Releases: main, 12.4
Change-Id: If16a93b851237c9d39893cd3e56fa9693986721d
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/82367


Reviewed-by: default avatarOliver Klee <typo3-coding@oliverklee.de>
Tested-by: default avatarcore-ci <typo3@b13.com>
Tested-by: default avatarBenjamin Franzke <ben@bnf.dev>
Reviewed-by: default avatarBenjamin Franzke <ben@bnf.dev>
      3dc75cf8
  37. Jan 12, 2024
  39. Jan 09, 2024
  41. Jan 08, 2024
  43. Jan 04, 2024
  45. Dec 30, 2023
  47. Dec 23, 2023
    • Christian Kuhn's avatar
      [!!!][FEATURE] Establish FE frontend.page.information attribute · a4cef9b3
      Christian Kuhn authored 
      The patch extracts TSFE->determineId() and its sub
methods from TSFE into TypoScriptFrontendInitialization
middleware.

The information created by these methods is modeled into
the new DTO PageInformation, which is added as
'frontend.page.information' request attribute before
handling other middlewares below. The non-internal old
properties within TSFE are set and kept as b/w compat
layer for now.

This is a powerful change: It allows us to reduce
dependencies to TSFE significantly, which we will
start to leverage with upcoming patches.

The patch is an intermediate change as such: Looking
at determineId() and its related methods makes clear
the code can benefit heavily from further refactoring.
The method could be ultimately extracted into
a service class that only returns the 'final'
PageInformation object, or throws exceptions for
'early' responses. To keep the patch reviewable at
this point, these refactorings will continue with
additional patches.

Detail notes:

* TSFE->sys_page PageRepository instance is for now
  modeled as (@internal) PageInformation->pageRepository.
  This can be refactored away later: Consumers should
  create instances of that class on demand.

* "original mount and shortcut page record" are also
  modeled as (@internal) PageInformation properties
  for now. They are internal handling since the redirects
  can only be created after TypoScript has been calculated
  later on, so the information has to be carried around
  for now. We *may* be able to change this later.

* The two middleware properties $pageNotFound and
  $pageAccessFailureHistory are a tribute to the current
  code flow in determineId(). They should be refactored
  away when the code is further shuffled around. The
  properties currently require a hack that needs to make
  the middleware "shared: false" to circumvent side
  effects in combination with error handling sub requests.

* The code flow around determineId() updates local state
  and state of PageInformation multiple times and is in
  general very hard to follow. The patch crafted this
  carefully and did not refactor the code flow heavily
  for now. Changing the early response creation strategy
  will make the code flow much more straightforward
  later.

* determineId() and with it the PageInformation object
  is now created *before* TSFE is instantiated, shifting
  the instantiation to a slightly later point.

* getPageAccessFailureReasons() is for now transferred
  to the middleware as well. It can be later dissolved
  when determineId() receives further refactoring. Two
  consumers (indirectly) by ShortcutAndMountPointRedirect
  middleware could be dissolved already, so the method could
  be removed from TSFE.

* TSFE->linkVars needs to be dissolved soon: The strategy
  should be to create "linkVars" on demand within the
  link handling code and eventually encapsulate it with
  a runtime cache entry.

* getRedirectUriForMountPoint() and getRedirectUriForShortcut()
  are relocated to ShortcutAndMountPointRedirect
  middleware. TSFE->getUriToCurrentPageForRedirect() had
  to be made (@internal) public to do that, but that method
  can be refactored away when TSFE->linkVars is dissolved.

* The hacks within RedirectService are extended to deal
  with the new situation for now. The entire thing needs
  to fall at a later point completely.

* Further patches should start avoiding the TSFE properties
  that are now set as b/w compat layer, and start using
  the PageInformation request attribute instead. This will
  reduce overall dependencies to TSFE significantly.

Resolves: #102715
Releases: main
Change-Id: I6470899cf65cbaaeb2177a8b20c0800f045a070c
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/82267


Reviewed-by: default avatarBenni Mack <benni@typo3.org>
Reviewed-by: default avatarStefan Bürk <stefan@buerk.tech>
Tested-by: default avatarBenni Mack <benni@typo3.org>
Tested-by: default avatarcore-ci <typo3@b13.com>
Tested-by: default avatarChristian Kuhn <lolli@schwarzbu.ch>
Tested-by: default avatarStefan Bürk <stefan@buerk.tech>
Reviewed-by: default avatarChristian Kuhn <lolli@schwarzbu.ch>
      a4cef9b3
  49. Dec 09, 2023
  51. Dec 05, 2023
  53. Dec 01, 2023
  55. Nov 23, 2023
  57. Oct 30, 2023
  59. Oct 26, 2023
  61. Oct 24, 2023
  63. Oct 23, 2023
  65. Oct 18, 2023
    • Benjamin Franzke's avatar
      [TASK] Add phpstan check for unneeded pseudo uncertain instanceof usage · 6c0b5518
      Benjamin Franzke authored 
      An `instanceof Type` on `Type|null` is unneeded and is to be replaced by
a null-check (or modern alternatives like optional chaning or the null
coalescing operator) in order to avoid narrowing code branches
unnecessarily. We call them "pseudo" uncertain checks there is no need
to express uncertainty regarding the type in a condition where native
type declarations define a specific type *or* null:

  It is `null` or `!null`.

Definition of a pseudo uncertain instanceof check:

  `$foo instanceof Bar` is fully equivalent to `$foo !== null`,
    when `$foo` is defined (via native PHP types) to be `Bar|null`.
   ⇒ `instanceof` expresses pseudo uncertainty regarding the type.

From what we have seen in previous gerrit discussions, there were two
reasons why instanceof was preferred over null checks although being
unneeded:

1) Cognitive load for an instanceof check is perceived to be lower in
   contrast to negated null (not null) conditions
2) Preparatory safe-guard against type of $foo being changed at sometime
   later

1) Cognitive load is a subjective term and the opinions actually differ
   a lot. Some developers prefer narrowing instanceof conditions because
   they claim the desired type for a certain code branch.
   Some others say that's a clear signal for code that needs refactoring
   and perceive a high cognitive load because they do not understand why
   the type is unnecessarily checked if it can only be null or not null.
   Lets call that: "reverse cognitive load".
   That means, this argument basically boils down to
   "congitive load" (for the good "then" case: inner code block) vs
   "reverse cognitive load" (for the bad "else" case: outer code block)
   ⇒ Due to being subjective "cognitive load" is not a good argument to
     base a decision upon.

2) The second argument is that an instanceof ensures a method that is to
   be called actually exists and doesn't lead to an error – that is a
   "preparatory safe-guard".
   This is true and works, but doesn't "answer" the question,
   what happens if the object is not an instance of the desired type
   (but not null).
   While preparatory safe-guards against the type of variable being
   changed sometime later was probably a pretty good idea for code that
   is not statically analyzed and had no native type declarations, but
   such checks effectively preclude that the type must/should never
   change (which might not be true!) and has no chance of actually
   detecting when that case (type change/extension) ever happens.
   All advantages offered by pseudo uncertain instanceof checks are
   accomplished with static code analysis as well, but with the added
   downside that an `instanceof` hardcodes our human static code
   analysis result, instead of letting the static analyzer do the job.

   To explain that:
   If the type of the variable under test is actually widened (like a
   union type, or change to a base class), it will never be
   automatically detected that there is an instanceof condition that
   restricts the type too narrowly. It will always be valid code from
   static code analysis perspective.

   In comparison to that, static analysis on null-checked variables will
   report invalid method calls or assignments not allowed by the
   (natively defined) types and will notify in case a type change
   requires the code to be adapted.
   We gain the advantage that the code will not be forgotten to
   be updated to a new type.

   That means !== null combined with static code analysis has the same
   level of being a safeguard against the bad cases, while instanceof
   silently transforms new "good"-cases into bugs, where !== null is a
   transparent and secure passthrough.

   Actually to make an uncertain instanceof robust, an elseif branch
   would be needed to be somehow notified about new good-cases without
   silently ignoring them:

   if ($foo instanceof Foo) {
       …
   } elseif ($foo !== null) {
       throw new MustNeverHappenException(…);
   }

In other words an unneeded pseudo uncertain instanceof check is
basically like a switch construct without a default case.
Just to be explicit: Of course, instanceof is fine to be used
when multiples types are to be expected and handled in different code
branches.

That means pseudo uncertain instanceof usage instead of null-checks is
an antipattern for the following reasons:

 * It narrow code branches for the sake of less cognitive load
   * The cognitive load appears to be lower, but actually future-bad
     cases are overseen and are never auto-detectable in future – while
     null-checks will resolve to static analysis errors in case the
     input type is *ever* widened (which uncertain `instanceof` checks
     try to prepare for, but actually introduce future-bugs because of
     missing `else` cases)
 * It embraces deep nesting instead of early returns via null-checks
 * It embraces conditions over newer and more elegant PHP techniques
   like optional chaing
 * Tries to "help" the developer by explicitly anotating the
   current type of the variable under test
   ⇒ This is a clear sign of code smell, that needs to refactored into
      smaller chunks and methods and type autocompletion/information
      can be provided by IDEs when using proper types (which this change
      is about) anyway
 * Has zero advantages over static code analysis

Resolves: #102140
Releases: main, 12.4
Change-Id: I10de41e9744a814c9e24255573b5a5eaf6fb8b0f
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/80859


Tested-by: default avatarAndreas Kienast <a.fernandez@scripting-base.de>
Tested-by: default avatarcore-ci <typo3@b13.com>
Tested-by: default avatarOliver Hader <oliver.hader@typo3.org>
Reviewed-by: default avatarOliver Hader <oliver.hader@typo3.org>
Tested-by: default avatarBenjamin Franzke <ben@bnf.dev>
Reviewed-by: default avatarBenjamin Franzke <ben@bnf.dev>
Reviewed-by: default avatarAndreas Kienast <a.fernandez@scripting-base.de>
Reviewed-by: default avatarNikita Hovratov <nikita.h@live.de>
      6c0b5518
  67. Sep 25, 2023
  69. Sep 09, 2023
  71. Jul 28, 2023