rockiger / reactpress

Plugin that lets you easily create, build and deploy React apps into your existing WordPress sites.
https://rockiger.com/en/reactpress/
GNU General Public License v2.0
51 stars 7 forks source link

`app_path` check ineffective on some hosting platforms because of differing directory structure #46

Closed jahtomini closed 8 months ago

jahtomini commented 8 months ago

So it took me a few hours to debug this but the app_path() function can break on some hosting platforms and throw a critical error if the DOCUMENT_ROOT and app_path are very dissimilar. For my company's marketing site for example, $_SERVER["DOCUMENT_ROOT"] returns something starting with /home/users/web/b1480/... while the app_path variable (which is generated in ReactPress' code) starts with /hermes/bosnacweb02/bosnacweb02an/b1480.... Everything from that point on is the same.

Old code for reference

public static function app_path(string $appname, $relative_to_home_path = false): string {
    $apppath = escapeshellcmd(FULC_APPS_PATH . "/{$appname}");
    $document_root = $_SERVER['DOCUMENT_ROOT'] ?? '';
    if ($relative_to_home_path) {
      return explode($document_root, $apppath)[1];
    } else {
      return $apppath;
    }
}

Since document root is not a ~superset~ subset of app path in this case, it returns null for the [1] index.

Modified

public static function app_path(string $appname, $relative_to_home_path = false): string {
    $apppath = escapeshellcmd(REPR_APPS_PATH . "/{$appname}");
    if ($relative_to_home_path) {
      $pos = strpos($apppath, '/wp-content');
      if ($pos !== false) {
       return substr($apppath, $pos);
      }
    } else {
      return $apppath;
    }
}

The fix was to modify the plugin code and write something like this for app_path(), checking for "/wp-content" instead of trimming off the part of the document root that isn't in the app path, considering there is little parity between them. I believe this should work for all cases

I would have made a PR but didn't understand the repo structure. Do I push to develop or reactpress-plugin? The repo seems to be changing. I'm seeing something about fulcrum everywhere

rockiger commented 8 months ago

Thanks for submitting the issue. Unfortunately, /wp-content can be changed in the WordPress configuration. So this solution could be unstable for some users.

There is a StackOverflow issue that discusses the problem: https://stackoverflow.com/questions/2354633/retrieve-wordpress-root-directory-path

It seems to me that ~get_home_path();~ ABSPATH is the best bet. (get_home_poth seems to be undefined in certain server configurations)

Does this version of app_path work for you?

public static function app_path(string $appname, $relative_to_home_path = false): string {
    $apppath = escapeshellcmd(REPR_APPS_PATH . "/{$appname}");
    $document_root = rtrim(ABSPATH,  '/') ?? '';
    if ($relative_to_home_path) {
      return explode($document_root, $apppath)[1];
    } else {
      return $apppath;
    }
}
jahtomini commented 8 months ago

That works for me, thank you

rockiger commented 7 months ago

I created a pull request with changes regarding the loading of the css- and js-assets. Do you care to give it a try if it still works with your WordPress hosting? The last two commits are interesting in this case.

https://github.com/rockiger/reactpress/pull/51

jahtomini commented 7 months ago

Okay, I'll try it out