dev/joomla#26 - Fix path derivation when CMS is rooted in a subdir
authorTim Otten <totten@civicrm.org>
Tue, 24 Mar 2020 05:56:36 +0000 (22:56 -0700)
committerTim Otten <totten@civicrm.org>
Tue, 24 Mar 2020 05:56:36 +0000 (22:56 -0700)
Overview
--------

CiviCRM is deployed inside a CMS. The CMS is usually deployed at the HTTP root (`http://example.org`),
but it is sometimes deployed in a subdirectory (`http://example.org/my-cms`).

Some asset URLs are computed using the variables `[civicrm.bower]`, `[civicrm.packages]`, and `[civicrm.vendor]`, which
are derived from the value of `[civicrm.root]`.  However, if the site is deployed in a subdirectory, and if using v5.23,
then the computation of `[civicrm.bower]` (etc) can misbehave.

Before
------

When the URL for `[civicrm.bower]` (or similar) is derived, it goes through multiple filters - first, from absolute to
relative, and then later from relative back to absolute.  In the process, the base is inadvertently changed.

After
-----

When the URL is derived, it is computed in absolute format - and simply kept that way.

Comments
--------

Regarding test coverage, there are two relevant unit-tests. This PR only updates one.

* `E2E\Core\PathUrlTest`: This is a more concrete smoke test which demonstrates functional problems with variables like
  `[civicrm.bower]`.  It should already catch problems like dev/joomla#26...  but only if you run the E2E suite on a system
  where the CMS was deployed to a subdirectory.  `civibuild` doesn't currently include such a build-type.
* `Civi\Core\PathsTest`: This is a more abstract, headless test to examine edge-cases in `Civi\Core\Paths`. It does not
  specifically check for `[civicrm.bower]` or similar variables (b/c the URL routing is ill-defined in a headless context).
  However, I've updated it to compare/contrast the working and non-working ways to derive variables.

Civi/Core/Paths.php
tests/phpunit/Civi/Core/PathsTest.php

index 9099c3cff95a80758fde123d023a79d2958aa5dc..d0d15814b3671b2cf202d741ae920c083b611b05 100644 (file)
@@ -38,19 +38,19 @@ class Paths {
       ->register('civicrm.packages', function () {
         return [
           'path' => \Civi::paths()->getPath('[civicrm.root]/packages/'),
-          'url' => \Civi::paths()->getUrl('[civicrm.root]/packages/'),
+          'url' => \Civi::paths()->getUrl('[civicrm.root]/packages/', 'absolute'),
         ];
       })
       ->register('civicrm.vendor', function () {
         return [
           'path' => \Civi::paths()->getPath('[civicrm.root]/vendor/'),
-          'url' => \Civi::paths()->getUrl('[civicrm.root]/vendor/'),
+          'url' => \Civi::paths()->getUrl('[civicrm.root]/vendor/', 'absolute'),
         ];
       })
       ->register('civicrm.bower', function () {
         return [
           'path' => \Civi::paths()->getPath('[civicrm.root]/bower_components/'),
-          'url' => \Civi::paths()->getUrl('[civicrm.root]/bower_components/'),
+          'url' => \Civi::paths()->getUrl('[civicrm.root]/bower_components/', 'absolute'),
         ];
       })
       ->register('civicrm.files', function () {
index ee56ca85d885f6270d9c7f12fd17d71010dae467..7df04dbaa483f5b08ef26f144a61eeb2965d1606 100644 (file)
@@ -101,4 +101,52 @@ class PathsTest extends \CiviUnitTestCase {
     $this->assertEquals("$cmsRoot/foo", $p->getUrl('foo'));
   }
 
+  /**
+   * This test demonstrates how to (and how not to) compute a derivative path variable.
+   */
+  public function testAbsoluteRelativeConversions() {
+    $gstack = \CRM_Utils_GlobalStack::singleton();
+    $gstack->push(['_SERVER' => ['HTTP_HOST' => 'example.com']]);
+    $cleanup = \CRM_Utils_AutoClean::with([$gstack, 'pop']);
+
+    $paths = new Paths();
+    $paths->register('test.base', function () {
+      return [
+        'path' => '/var/foo/',
+        'url' => 'http://example.com/foo/',
+      ];
+    });
+    $paths->register('test.goodsub', function () use ($paths) {
+      return [
+        'path' => $paths->getPath('[test.base]/good/'),
+        'url' => $paths->getUrl('[test.base]/good/', 'absolute'),
+      ];
+    });
+    $paths->register('test.badsub', function () use ($paths) {
+      return [
+        'path' => $paths->getPath('[test.base]/bad/'),
+        // This *looks* OK, but `getUrl()` by default uses `$preferFormat==relative`.
+        // The problem is that `$civicrm_paths['...']['url']` is interpreted as relative to CMS root,
+        // and `getUrl(..., 'relative')` is interpreted as relative to HTTP root.
+        // So this definition misbehaves on sites where the CMS root and HTTP root are different.
+        'url' => $paths->getUrl('[test.base]/bad/'),
+      ];
+    });
+
+    // The test.base works as explicitly defined...
+    $this->assertEquals('/var/foo', $paths->getPath('[test.base]/.'));
+    $this->assertEquals('http://example.com/foo', $paths->getUrl('[test.base]/.', 'absolute'));
+    $this->assertEquals('/foo', $paths->getUrl('[test.base]/.', 'relative'));
+
+    // The test.goodsub works as expected...
+    $this->assertEquals('/var/foo/good', $paths->getPath('[test.goodsub]/.'));
+    $this->assertEquals('http://example.com/foo/good', $paths->getUrl('[test.goodsub]/.', 'absolute'));
+    $this->assertEquals('/foo/good', $paths->getUrl('[test.goodsub]/.', 'relative'));
+
+    // The test.badsub doesn't work as expected.
+    $this->assertEquals('/var/foo/bad', $paths->getPath('[test.badsub]/.'));
+    $this->assertNotEquals('http://example.com/foo/bad', $paths->getUrl('[test.badsub]/.', 'absolute'));
+    $this->assertNotEquals('/foo/bad', $paths->getUrl('[test.badsub]/.', 'relative'));
+  }
+
 }