Remove dead branch on ControllerMap::lookupDefaultController()
authorTim Düsterhus <duesterhus@woltlab.com>
Mon, 13 Jun 2022 13:28:28 +0000 (15:28 +0200)
committerTim Düsterhus <duesterhus@woltlab.com>
Mon, 13 Jun 2022 13:50:20 +0000 (15:50 +0200)
commit416dafddb18a3e82750c6e8ca089d4f9132b39dc
treef065c792cfeb726df5e9bfd5a1f9d5cba386dc34
parent21f7dbb61ed84bc4e02517011a297cf05b6ef30b
Remove dead branch on ControllerMap::lookupDefaultController()

It is impossible for this branch to ever be taken:

To even reach this branch it is a necessary precondition that the landing page
of the accessed application is a non-system CMS page.

A non-system CMS page can either be user-created (originIsSystem = 0) or
installed by a package (e.g. the Dashboard). These two have differently with
regard to application overrides: Only package-installed CMS pages can ever have
an application override. For a user-created CMS page instead of creating an
override the page's packageID will simply be updated.

1)

Let's first look into user-created CMS pages: For the reasons outlined above
the call to `->getApplicationOverride()` will simply pass through the
`$application`, because no override can exist. `$application !== $application`
will never be true, thus this branch will never be taken.

2)

The same is true for a package-installed CMS page without an override: The
second half of the condition will not be true.

3)

Thus the remaining case to look into is a package-installed CMS page with an
override.

Let us consider the situation with 3 installed apps, `wcf`, `forum` and `blog`.
The CMS page was installed by `blog` and virtually reassigned to `forum`. The
application overrides will look like the following:

    array (size=2)
      'lookup' =>
        array (size=1)
          'wbb' =>
            array (size=1)
              'testing' => string 'blog' (length=4)
      'reverse' =>
        array (size=1)
          'blog' =>
            array (size=1)
              'testing' => string 'wbb' (length=3)

And only the `reverse` case is relevant here.

Now when accessing the `blog` the first half of the condition will be false:
The accessed application matches the actual application.

When accessing any other app, the second half of the condition will be false,
as the `reverse` lookup maps from `blog` to `wbb` which was already ruled out
in the previous paragraph. As the lookup will not match, the `$application`
will be passed through unmodified.

In none of the cases can both parts of the conditional branch ever be true at
the same time, making this a dead branch.

The logic was last modified in commit 000c0c8a26491708b2af995bdd6f0e627cc75161.
Peeking at the code as of that commit does not indicate that the behavior of
`getApplicationOverride()` was different back then, indicating that that commit
broke the `redirect` return case.

It is likely that it was intended that `$cmsPageData['application']` was to be
passed as a parameter instead of `$application`. However this code being broken
for that long of a time indicates that it wasn't important after all.
wcfsetup/install/files/lib/system/request/ControllerMap.class.php