Fixed getThreadFromUrl essentially ignores $type argument when friendly urls is off

[Xon]

Affected version

2.2.3

Calling getThreadFromUrl from an admin app context with a non-friendly public thread URL will fail.

This is because getThreadFromUrl($url, $type) calls getRoutePathFromUrl($url) which calls getExtendedUrl($url). However getExtendedUrl work with the current request object to determine the base URL, not the actual $type argument.

public function getThreadFromUrl($url, $type = null, &$error = null)
{
   $routePath = $this->app()->request()->getRoutePathFromUrl($url);
...
public function getRoutePathFromUrl($url)
{
   $url = $this->convertToAbsoluteUri($url);
   $url = str_replace($this->getHostUrl(), '', $url);

   $routePath = ltrim($this->getExtendedUrl($url), '/');
...
public function getExtendedUrl($requestUri = null)
{
   $baseUrl = $this->getBaseUrl();
...
public function getBaseUrl()
{
   $baseUrl = $this->getServer('SCRIPT_NAME', '');
   $basePath = dirname($baseUrl);

   if (strlen($basePath) <= 1)
   {
      // Looks to be at the root, so trust that.
      return $baseUrl;
   }

   $requestUri = $this->getRequestUri();
...

In the admin context, getBaseUrl returns /admin.php, which causes /index.php?threads/1/ to not be resolved to the expected ?threads/1/ fir the rest of the getThreadFromUrl code.

 

[XF Bug Bot]

Thank you for reporting this issue, it has now been resolved. We are aiming to include any changes that have been made in a future XF release (2.2.8).

Change log:

Optionally normalize a URL passed into `getRoutePathFromUrl` to exclude the script part of the URL if it is passed in.

There may be a delay before changes are rolled out to the XenForo Community.

 

[Chris D]

Full of confidence on a Monday morning: I think that should workaround it

diff --git a/src/XF/Http/Request.php b/src/XF/Http/Request.php
index d85982c3cb..bbbaabb623 100644
--- a/src/XF/Http/Request.php
+++ b/src/XF/Http/Request.php
@@ -1539,11 +1539,16 @@ public function getRoutePathFromExtended($extended)
         return $this->getRoutePathInternal($routePath);
     }
 
-    public function getRoutePathFromUrl($url)
+    public function getRoutePathFromUrl($url, bool $stripScript = false)
     {
         $url = $this->convertToAbsoluteUri($url);
         $url = str_replace($this->getHostUrl(), '', $url);
 
+        if ($stripScript)
+        {
+            $url = preg_replace('#^/.*[a-z0-9-_]+\.php\?#i', '?', $url);
+        }
+
         $routePath = ltrim($this->getExtendedUrl($url), '/');
 
         return $this->getRoutePathInternal($routePath);
diff --git a/src/XF/Repository/Thread.php b/src/XF/Repository/Thread.php
index bcc5013590..ebc991d558 100644
--- a/src/XF/Repository/Thread.php
+++ b/src/XF/Repository/Thread.php
@@ -370,7 +370,7 @@ public function sendModeratorActionAlert(\XF\Entity\Thread $thread, $action, $re
      */
     public function getThreadFromUrl($url, $type = null, &$error = null)
     {
-        $routePath = $this->app()->request()->getRoutePathFromUrl($url);
+        $routePath = $this->app()->request()->getRoutePathFromUrl($url, true);
         $routeMatch = $this->app()->router($type)->routeToController($routePath);
         $params = $routeMatch->getParameterBag();

By the time we get to the if ($stripScript) path we should have normalized the URL portion to e.g.

community/index.php?threads/title.1

The stripScript path then pretty much does what getExtendedUrl intends to do but left it in. It may still be needed if $striipScript is left at false.