-
Notifications
You must be signed in to change notification settings - Fork 209
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Static files run as PHP when path includes directory like <some-dir>.php/
#1365
Comments
I wonder what's the path resolution algorithm Apache of Nginx uses and can we do the same here. One complication in the web version of Playground is that some static files aren't shipped in the VFS but are |
@adamziel I am currently trying to work through this. Initially, I was going to create a PR to quickly communicate my thoughts but kept finding edge cases. What is difficult is:
I think it is probably OK in most cases to treat files without an extension (i.e., without a "." in them) as directories. That is probably a reasonable first pass. In addition, maybe we can only do this for WP source directories. That way, plugins that include requests for extension-less files do not break when we incorrectly guess their static file path points to a directory. If we find we need to handle these decisions with more certainty within requests for WP core assets, perhaps the stripped-down WordPress builds could include an explicit listing of the static files that exist remotely. |
I was mistaken. We can't easily treat Playground-provided asset paths in a special way because some of Playground's remote assets are under As a first pass, we can handle a path with PHP when any of the following are true:
The line
will probably work for most cases, but it will break if custom permalink structures contain A couple options to solve this are:
|
@adamziel I got ahead of myself and forgot to answer directly. The nginx behavior is completely defined in the config, and we can use the WordPress.org nginx example for reference:
This is what makes the last rule Other paths are tried first as a file, then as a directory where index.php may be present, then delegated to WP's index.php hard to implement. The first thing to try is whether a file exists at that path, but we cannot know that without additional info. So we either need to take a different approach as mentioned above or add metadata to the stripped WP builds so we know what files exist remotely. After spelling out the above nginx config, I think we should just generate the metadata, use it, and be done with all the special cases. What do you think? |
Metadata sound great and could also help with building a PWA (unless we'll just use a non-minified build for that as it's easy to do and already enables fetch-less usage). Cc @bgrgicak |
Having a list of metadata would also help me in the case of this draft PR #1203. I wanted to add the list to it, but didn't have the time. Similarly for offline support. We will need to download all files when the PWA is installed and this list is a requirement. |
… them in wp-content/mu-plugins) (#1366) This PR moves the Playground mu-plugins from `/wordpress/wp-content/mu-plugins`, where they may interfere with the user-provided files and pollute the git repository, to `/internal/mu-plugins`, where they're invisible to the Playground consumer and don't show up in exports or OPFS mounts. Closes #1318 ## Changes made Technically, this PR uses the `auto_prepend_file` PHP options to pre-load the following script: ```php <?php foreach (glob('/internal/preload/*.php') as $file) { require_once $file; } ``` From here, preloading files is as simple as creating them in `/internal/preload`. This PR ships a few such preloaded files: * `consts.php`, that enables predefining PHP constants without affecting `wp-config.php`. * `env.php`, that preloads mu-plugins from `/internal/mu-plugins` using [the method described by](#1318 (comment)) @brandonpayton. * `phpinfo.php`, that just calls `phpinfo();` when the request URL is `/phpinfo.php` – this prevents creating an actual `phpinfo.php` file in the document root. This involves a slight change in the path resolution logic which will have to change even more soon to solve #1365. From there, the SQLite integration plugin and the Playground mu-plugins are placed in `/internal/mu-plugins` and do not affect the WordPress file structure. This PR also reduces code duplication by moving some parts of the WordPress setup to the `@wp-playground/wordpress` package where they can be imported by both the web and the CLI versions of Playground. We'll only see more of these moving forward. ## Follow-up work * Define clear use-cases and behaviors related to the location of Playground-provided files. E.g. Full site export, where we want to keep the SQLite integration plugin, or a GitHub repo, where we only want to keep track of our changes in wp-content. * Use `@wp-playground/cli` to build and setup WordPress in `packages/playground/wordpress-builds/build/Dockerfile` * Playground web: do not ship the SQLite integration plugin in the pre-built `wp-content` directory. Source it from the `/internal/mu-plugins` directory same way as in the CLI build. * Do not create the drop-in plugin `wp-content/db.php` * Do not install the WordPress importer plugin in `wp-content/plugins` * Figure out how to avoid touching `wp-config.php` – defining PHP constants may require rewriting it and it might be desirable to keep those changes on export ## Testing instructions * Confirm the tests pass * In the browser – export Playground as zip, import it, also refresh the page and import again, confirm it all worked.
Thanks! Ping me for a review when it's ready and then I can use it in #1203. |
Will do! I ended up spending most of today working on things related to the website migration but hope to work on this tomorrow. It looks like this where we can save a list of remote static files which can be added to the stripped-down WP zip and read later: wordpress-playground/packages/playground/wordpress-builds/build/Dockerfile Lines 48 to 58 in d72387c
|
I made a draft PR for listing remote static asset paths so we can make better decisions in request handling: #1417 We also need to make a change to this line as a separate PR so we don't try to handle a static file under a |
If a static file is under a directory tree where an ancestor directory has a name ending in ".php" (e.g.,
/root/some.php/assets/photo.jpeg
), Playground will say that the file seems like a PHP file:wordpress-playground/packages/php-wasm/universal/src/lib/php-request-handler.ts
Lines 595 to 597 in 7d62755
And IIUC Playground will try to run such non-PHP files as PHP:
wordpress-playground/packages/php-wasm/universal/src/lib/php-request-handler.ts
Lines 315 to 321 in 7d62755
Discovered when investigating #1332.
The text was updated successfully, but these errors were encountered: