Add a startup configuration for the built-in PHP server that supports clean URLs

This is a fairly simple patch, but if we include this file, I'd like to see it documented. For instance, the command that you listed in the issue body would be a good thing to include (example usage), as well as some documentation about what it's actually doing and why.

Also,

+++ b/.htrouter.phpundefined
@@ -0,0 +1,8 @@
+if (file_exists('.' . $url['path'])) {
+  return FALSE; // Serve the requested resource as-is.
+}
+// Remove opener slash.
+$_GET['q'] = substr($url['path'], 1);
+include ("index.php");

No inline comments on the same line as code, please.

Works well for Drupal 7

I would suggest updating also $_REQUEST array, as some modules might be using it.

+++ b/.htrouter.php
@@ -0,0 +1,18 @@
+ * The router.php for clean-urls when use PHP 5.4.0 built in webserver.

This file comment doesn't make sense to me. I suggest "Startup configuration for the built-in PHP web server."

I also think it is important to prevent remote execution of this file in the .htaccess and anywhere else it is appropriate.

Also there should be a @see comment explaining what this is for:

@see http://php.net/manual/en/features.commandline.webserver.php

@pp Thank you. I see that you are correct about files beginning with .ht.

http://svn.apache.org/viewvc/httpd/httpd/tags/2.4.10/docs/conf/httpd.con...

170	# The following lines prevent .htaccess and .htpasswd files from being 
171	# viewed by Web clients. 
172	#
173	<Files ".ht*">
174	    Require all denied
175	</Files>

I have done basic manual testing and this works.

It seems like #2 in the issue summary usage instructions should read "You must use 'admin/config/search/clean-urls' path with ?q= "...

Tagging novice because a novice could test this and update the issue summary.

Did a couple of updates to the patch and to the issue-description

1. Removed the point about being dependent on #1542186: PHP 5.4 "Illegal string offset" warning when install - its been comitted and i did installs on both D7 and D8 without any problems.
2. I have added a number of comments to the file.
3. Generally referred to the script as a router-script as it is what php.net calls it
4. Renamed the file to .ht.router.php to follow the convention used by the sqlite file generated at install for D8

Added a D7 patch with the additional remark about what to do if clean-urls are disabled. This is not an issue on D8 as clean-urls cannot be disabled there.

@danquah +1 for increasing the comments.

+++ b/.ht.router.php
@@ -0,0 +1,34 @@
+ * @file
+ * Routing-script for the built-in PHP web server to enable basic support for
+ * clean URLs by faking the "q" query parameter as the main .htaccess would do.

The first line of any docblock comment must be a single line.

https://www.drupal.org/coding-standards/docs#file

There is a problem with D8:

The asset links on e.g. admin/structure/types/manage/article/fields/node.article.body are wrong (they are calculated relatively, e.g.: /admin/structure/types/manage/article/fields/core/assets/vendor/normalize-css/normalize.css which results in a 404).

This is probably because D8 tries to guess if clean URLs are supported (admin/config/search/clean-urls is removed: https://www.drupal.org/node/1659580).

Going to index.php/admin/structure/types/manage/article/fields/node.article.body however does output correct asset URLs.

So maybe it should be stated in the file that it is best to go to index.php (as each link from now on will include it).

Don't know how I missed the first line of the dockblock :)

Updated patch attached, now with a better conforming docblock. It now also sets base_url, it was what kept the stream-wrapper from constructing the correct url for assets.

While I'd personally would really like to see a router-file in core, but we should also be aware that drush have pretty solid support for getting a server up and running via its runserver command. Drush takes a slightly different approach and fixes the lack of a .htaccess with a prepend file: https://github.com/drush-ops/drush/blob/master/commands/runserver/runser.... The main two extra features drush has (as far as I can tell) is the ability to redirect watchdog log-statements into error_log() (php's default error-logging), and to override $conf[] settings from the commandline.

@cilefen saw you removed the backport tag, would be nice with a reasoning? (I personally started following this issue because I needed to use the build-in server with D7).

Patch tested and works. (+1)

On the summary page, suggest change of usage to resolve the following error on web browser running localhost:8888

Fatal error: Unknown: Failed opening required '.htrouter.php'

Should change from

php -S localhost:8888 .htrouter.php

to

php -S localhost:8888 .ht.router.php

Thanks for this patch. I added it to Drush 7, shortly after the 7.0.0-alpha9 release. `drush runserver` now works correctly with Drupal 8.

I also tested this patch with Drupal 8.0-beta7, without Drush, and it works great.

@danquah I can't remember now why I removed 'needs backport'. It seems like a good idea.

+1 for backport :)

If this has a chance of going into 8.0, a beta phase evaluation will need to be added to the issue summary. See this page for details.

Even though this is a feature request, it is non-breaking for any existing funcitonality (and could be argued as a bug since it fixes a known issue with PHP's built in server).

Added the Beta eval.

I don't see this issue as a typical feature request - it's more a of a task - ie. make Drupal 8 work with php's inbuilt webserver.

+++ b/.ht.router.php
@@ -0,0 +1,39 @@
+// Build up the base-url.
+$base_url = 'http://' . $_SERVER['SERVER_NAME'] . ':' . $_SERVER['SERVER_PORT'];

I don't think that this is required for D8.

@alexpott: I agree.

Manually tested and the new patch still works as expected. Back to RTBC.

Why is the file called .ht.router.php? I don't get the ht bit.

So I guess this is the same ht as in htaccess and htdocs - so hyper text. But does it make sense?

@alexpott, my understanding (mainly from #13 but also from manual testing) is that files beginning with .ht* are by default denied access by Apache.

Although, actually it appears all . files are denied access. I can't find that in the default config, so that might be Drupal's doing:

#
# The following lines prevent .htaccess and .htpasswd files from being
# viewed by Web clients.
#
<Files ".ht*">
    Require all denied
</Files>

I tested the patch from #30 against D7 & it works fine.

Many thanks!

I think this is back to RTBC unless somebody has a better name than .ht.router.php.

+++ b/.ht.router.php
@@ -0,0 +1,36 @@
+ *   - The webserver does not enforce any of the settings in .htacccess in

small nit.. too many 'c' in .htacccess :)

So this does actually seem kinda like a feature request to me, and even as a normal task it'd need a beta evaluation to determine if it were acceptable during the beta phase as per https://www.drupal.org/core/beta-changes. It doesn't really seem in scope for the beta to me. I'm not completely opposed to it going in because it's about as non-disruptive as it gets -- the only downside is that we're adding another piece of functionality we need to support, and of course that it's sort of "jumping the line" when we disallow new features during the beta as a rule.

However, it sounds like @alexpott thought this change was okay to go in per #28. So assigning to him for review/feedback.

So I don't see why this "needs backport to D7" - it's not as if D7 is broken without it. So reasoning

This issue is prioritized because it is going to be back-ported.

seems a little off.

We can wait for 8.1 for this since it is an addition and breaks nothing. People who want to experiment with PHP's built in webserver and Drupal 8 can just add this file to their Drupal root - that's not hacking core.

Drush runserver also works, and uses this exact technique with the PHP built-in webserver.

@greg.1.anderson Then do we need this issue?

Drush runserver performs some additional services for the user, like automatically logging you in, and opening a web browser to the specified URL, so Drush users typically will not use the file added by this issue.

It is useful for non-Drush users, though.

I tried this but no assets (js, css, images) loaded, when running directly php -S localhost:8888 .ht.router.php or using drush drush rs

@jmolivas Both #30 and drush rs work for me. Did you drush cr?

I did cache-rebuild using drush cr all and tried both php -S localhost:8888 and drush rs.

Front page works and other pages looks fine, but once I tried to edit a field at admin/structure/types/manage/article/fields/node.article.body assest are not loaded.

I am attaching an image.

+1 Back to "Needs work".

With patch #30 in I see the same as #48 because assets are being addressed at, for example:

http://localhost:8888/admin/structure/types/manage/article/fields/sites/default/files/css/css_tH28lQhvdF7J5DNKBprPug3qggSPzLar_jigc6ws4P4.css?nyzypi

Without #30, and php -S localhost:8888 I get page not found for admin/structure/types/manage/article/fields/node.article.body

The file has been tested as per last status at #37.

This file works for both Drupal 7 & 8 and it's required in order to handle callback paths with dots (which doesn't work by default in PHP built-in server).

The problem is explained in this drush bug: https://github.com/drush-ops/drush/issues/1641

Drush provides similar file (d8-rs-router.php) which works for D7 as well as per this pull-request.

See also: How to deal with callbacks having dot in the URL with built-in PHP server? at stackoverflow

I just fix that on DrupalConsole, now when running the server command:

drupal server --learning

I passed --learning option to make output more verbose and you can see from where the router file is loaded.

This is the CLI output:

After running that command open browser and load the same url http://127.0.0.1:8088/admin/structure/types/manage/article/fields/node.article.body using dots, this time, it works as expected.

This is the browser screenshot:

Fixed by adding this file:
https://github.com/hechoendrupal/DrupalConsole/blob/master/config/dist/r...

Original file is from Symfony Framework:
https://github.com/symfony/framework-bundle/blob/master/Resources/config...

With this patch applied, I can reproduce the problem mentioned in #50 - moving back to "needs work" for that.

Also:

+// Populate the "q" query key with the path, skip the leading slash.
+$_GET['q'] = $_REQUEST['q'] = substr($url['path'], 1);

This does not seem relevant for Drupal 8. Didn't Drupal 8 lose $_GET['q'] a long time ago (https://www.drupal.org/node/1659562)?

+ * This script provides basic support for clean URLs by faking the "q" query
+ * parameter as the main .htaccess would do.

Same as above, plus I don't think this comment is correct for Drupal 7 either? I thought Drupal 6 was the last version of Drupal that dealt with that in the .htaccess file.

+ * The built-in webserver should _only_ be used for development and test as it

The "_only_" looks like a non-standard way to highlight that word. I think highlighting isn't necessary here anyway and it could just be "only".

Also, "test" => "testing".

+ *   - The webserver does not enforce any of the settings in .htacccess in

".htaccess" still misspelled (as noted in #39)

@jmolivas, I saw your tweet. I had come across this issue with php's built web server just the other day.

When I run drupal server --learning I get the following:

Is router.php not in Console version 0.10.5?

Here's a better screenshot. The first one's too fuzzy.

Taking a clue from

• https://cloud.google.com/appengine/docs/php/config/mod_rewrite
• https://github.com/hechoendrupal/DrupalConsole/blob/master/config/dist/r...
• https://bugs.php.net/bug.php?id=64951

It appears that SCRIPT_NAME is not as simple as the name suggests. According to the php bug-report linked above SCRIPT_NAME won't necessarily point to index.php. Any url that looks like a request for a directory seems to be translated into a request for index.php as expected. But for anything that looks like a file-request (last element containing a dot?) SCRIPT_NAME will instead contain the non-existing requested path. I have reproduced this behaviour with the router-script, but as expected it cannot be reproduced apache running mod_rewrite. So, the argument could be made that whatever php does to set SCRIPT_NAME it is not as compatible with mod_rewrite as the person in the bug-report seems to indicate.

As core uses SCRIPT_NAME to build paths (http://cgit.drupalcode.org/drupal/tree/core/lib/Drupal/Core/DrupalKernel...) it seems that modifying SCRIPT_NAME in the router-script makes sense.

I've updated the patch with the comments from #53, and added a section to the script that modifies SCRIPT_NAME, SCRIPT_FILENAME and PHP_SELF and attempts to explain the reasoning behind the modifications.

@esod Yes is on 0.10.5, try using drupal init --override first to copy the router file to your home directory, it must read from project directory if not copied I will take a look.

Just realised that including the index-script is probably to weak. If index.php for some reason is missing the router should fail with a fatal error, so I have switch it to using require.

Also a quick comment on the script used by Symfony and Drupal Console: For some reason it merges _ENV into _SERVER, overwriting in favor of _ENV if there are collisions. The line where added to the Symfony codebase by this commit https://github.com/symfony/framework-bundle/commit/5dd7cd94f79f8427a4b15... as a part of release 2.3.19 (http://symfony.com/blog/symfony-2-3-19-released) but apart from the commit message "added an analyze of environment parameters for built-in server" I cannot find any explanation as to why this is done. It probably makes good sense in the context of Symfony, but I doubt there is any need for it in Drupal.

Thanks @jmolivas. I ran drupal init --override which copied seven files to my ~/.console directory.

Now the server that is launched with drupal server --learning successfully reaches admin pages, such as http://127.0.0.1:8088/admin/structure/types/manage/article/fields/node.article.body

So Console's good to go for this issue, although this may be a side issue to the primary discussion.

I've issue with .ht.router.php on 7.x when editing panels panes of panel pages as per #1524668-47: An AJAX HTTP error occurred. HTTP Result Code: 403 (Panels), not sure if it's related.

Possibly failing URL: /panels/ajax/editor/pane-css/panel_context%3Apage-homepage%3A%3Apage_homepage__panel%3A%3A%3A%3A/new-80d3d68a-da4b-4f7b-8cd8-0482e79040d8. Could be URL encodings? Can anybody test that?

it works out of box without of patch, just field edit broken somehow

In related issue SCRIPT_NAME could become useless #2753591: Fix mismatch of $base_url with Symfony request object

Was having an issue with the PHP built-in server and Drupal 7 where any first level subdirectory was returning a 404 (i.e. example.com/first_level). For some reason it was only the first level, but that's likely irrelevant.

The patch in #58 did the trick. Thanks @danquah!

According to #64 things are working without this. Before we rtbc for Drupal 8 based on Drupal 7 testing we need to confirm whether this is required for Drupal 8.

I can confirm that D8 is working without this, but do we have an issue about the broken field edit (#64)? I can confirm that too.

Confirming what @mikran mentioned in #69. When using PHP built-in server to test D8, D8 appears to function normally, until trying to edit any fields on any entities.

Using the Drupal Console built-in server resolves the issue. But this patch should probably be applied to still allow for PHP built-in server testing.

The same applies to drush8 https://github.com/drush-ops/drush/blob/master/commands/runserver/d8-rs-...

Sp probably it's good idea to standardize on that in core and clean-up drush & console

@alexpott - I have seen this recently (field UI paths breaking) on Drupal 8 using php -S, but otherwise it works.

Re: #68 comment (@alexpott)

I believe this is required for D8 (or any other version), because by default all paths with full stop character are not parsed by Drupal (they're directly redirected to the files it-self which doesn't exist). With the router file, URLs like ?something=content.xml will work fine (normally they won't).

For more examples, see: Drush runserver HTTP server doesn't support callbacks with dot in the URL. This relates to both Drupal 7 and 8.

(removed)

A really interesting side-effect (good one) is that with the patch in #58 image styles started working when using the php built-in server.

Without the patch I was getting a in the server log like:
[Wed Jul 26 12:44:19 2017] ::1:60958 [404]: /sites/default/files/styles/large/public/2017-07/ndzsndyz7mss3pbx2gjl.jpg?itok=tdHJ8OrQ - No such file or directory
without any other hints. Permissions were correct.
After applying the patch this was solved.

What more, paths like:
http://localhost/admin/structure/paragraphs_type/text_with_image/fields/paragraph.text_with_image.field_image
started working just fine, while without the patch they produce a 404 error.

I suggest that this gets applied. It solves several problems that occur under very specific scenarios, while it doesn't affect the project otherwise.
So, in contrast to #64 it seems like things aren't working as expected without the patch, and the patch solves several issues at once.

I am bumping this to RTBC once again.

Marking for 8.4.x since this is not a disruptive change.

Url like "/fields/entity.bundle.field_status/storage"

with dots inside return 404.

Could we expose this as a composer script so its for example a little bit more visible?

Re: #78. As per https://www.drupal.org/core/d8-allowed-changes#minor feature and API additions go against the next minor release, as is required by semantic versioning. Committers have moved it to the minor branch several times; please do not move it back again.

I still haven't seen a strong case for why this needs to be in core; in fact

+++ b/.ht.router.php
@@ -0,0 +1,45 @@
+ * The built-in webserver should only be used for development and testing as it
+ * has a number of limitations that makes running Drupal on it highly insecure
+ * and somewhat limited.

This does not sound like something that should be in core.

Is there something in core we'd want to use this for? If so, please update the summary with that information. Thanks!

I expanded the issue summary with my personal view of the world. Feel free to disagree :)

Re. #81

I still haven't seen a strong case for why this needs to be in core; in fact

Yes, the PHP-Server has its problems.

But I have (at least for me) a strong usecase for this: Teaching Drupal.
I am teaching Content Management Systems at a university.
On all Macs and our lab machines is PHP installed, so the students can just download Drupal, extract it, start the PHP-Server, install it, use sqlite at a database und learn Drupal 8.

With the PHP-Server and SQlite there is no need to fiddle with Apache or Ngnix. Which can go a long time and is very problematic on multiuser lab machines, as you need admin rights to do it.

And due to the SQLite database, the whole install can be stored on their network share or carried around with an usb-stick. Which is great.

Using drush for that is another difficult setup to get working, before you can start working. (Not for me, but for first time users)

I still haven't seen a strong case for why this needs to be in core; in fact

+1 to the justification in #84

Making Drupal easy to test and evaluate should be important to core!

Symfony has a startup script, with similar "don't use this in production" warnings that ships with Symfony standard.

I'm sure here 2 different issues:
- add "kickstart" script
- fix bug in routing - without this .ht.router.php we can't edit fields, filed #2908377: Allow edit fields using internal PHP-server

I think the IS is updated, so removing tag. However, the current patch has fails for some reason, so setting to NW.

@dawehner could you clarify #80? I'm not sure which parts would be moved?

I think the IS is updated, so removing tag. However, the current patch has fails for some reason, so setting to NW.

Does it really? It fails against 8.1.x but seems to work against 8.5.x and 8.4.x

I've added a similar justification like #84 into the issue summary in #81/#82. Just imagine together with #2910136: Experiment: package PHP libraries in a single Phar file we could download Drupal and directly boot up.

I added a core idea to provide an entire start script. This issue will be a part of it #2911319: Provide a single command to install & run Drupal

I love the idea of people being able to try out Drupal installs locally with a single command. This will be great for distributions and potentially for on-boarding new contributors as well.

It looks like @dawehner has updated the issue summary with the information asked for by @xjm. I've changed some things around.

It needs to be mentioned in README or maybe CR

@andypost good point. In core we have core/INSTALL.txt and core/INSTALL.sqlite.txt

I think that adding some documentation to core/INSTALL.txt about using this to quickly try out Drupal is a good idea. Also maybe a CR is a good idea so we can spread the idea from the Contenta distribution to other distributions.

It needs to be mentioned in README or maybe CR

This is a good idea! I decided for myself to use a CR and postpone the readme update once we have all in one command.

I've added the documentation requirement to #2911319: Provide a single command to install & run Drupal

Another use case scenario is plain linux systems, personal systems, where options like acquia-dev-desktop or XAMPP are not an option.
I know that one can install a complete LAMP/LEMP configuration on its linux laptop, but it is sooo much faster to do development work with the php-dev server (no vhosts, no strange configs, just run php -S and you are good to go).

As for the security concers: php dev server is exactly that, a development server. I believe it is tedious discussing or even mentioning security in such a scenario. No cheap web-hoster allow using the dev server anyway, so things are safe for most newcomers that want to try drupal.

The "single command" to install and run drupal, while good as an idea, does not really have something to do with this issue.

Like I said before, I agree with most people above, this should at some point be included in the project.

❤️

This is nifty, good to see it RTBC. I only have nits:

1. +++ b/.ht.router.php
   @@ -0,0 +1,45 @@
   + *   - The webserver does not enforce any of the settings in .htaccess in
   

   'in particular' should be preceded by a comma or semicolon.

2. +++ b/.ht.router.php
   @@ -0,0 +1,45 @@
   +// The use of a router-script means that a number of $_SERVER variables has to
   

   s/has to/have to/

3. +++ b/.ht.router.php
   @@ -0,0 +1,45 @@
   +// SCRIPT_FILENAME will point to the router-script itself, it should point to
   

   Is it a routing-script as it says in the @file docs or a router-script? We should standardize on one.

Fixed 1,2 and 3. Went for "router script" because that's what http://php.net/manual/en/features.commandline.webserver.php calls it. Also noted that "webserver" is not one word but two. And the list format was not as per documentation standards.

+++ b/.ht.router.php
@@ -0,0 +1,46 @@
+ * has a number of limitations that makes running Drupal on it highly insecure
+ * and somewhat limited.

I'm sure it needs to point to issue https://bugs.php.net/bug.php?id=61286 that PHP team won't fix as said in #2908377-2: Allow edit fields using internal PHP-server

Because it's only reason why we need this script

Yes let's add that in too.

+++ b/.ht.router.php
@@ -15,6 +15,9 @@
+ * We need thsi file, because there is a bug with dots in PHP itself, see

I hate to do this, but here we have a typo which I think could be fixed on commit:
thsi -> this

Should we also use the see tag - @see?

@hchonov nice spot.

Thank you @hchonov
I think its save to set this back to RTBC

Just some nits:

1. +++ b/.ht.router.php
   @@ -0,0 +1,49 @@
   +// be updated to point to the index-file.
   ...
   +// the full path to index.php.
   ...
   +// SCRIPT_NAME and PHP_SELF will either point to /index.php or contain the full
   ...
   +// Require the main index-file and let core take over.
   

   index-file vs index.php vs /index.php

2. +++ b/.ht.router.php
   @@ -0,0 +1,49 @@
   +// virtual path being requested depending on the url being requested. They
   

   Nit: s/url/URL/

Done

Thank you. I think it looks good now. There is only one minor nit, but I hope it could be fixed on commit.

+++ b/.ht.router.php
@@ -0,0 +1,49 @@
+// SCRIPT_FILENAME will point to the router script itself, it should point to
+// the full path to index.php.

"point to the full path to index.php" => "point to the full path of index.php"

Asking committers to make minor changes on commit can result in committers having to concentrate on extra things and make it more difficult to review a patch and do all the other tasks around committing a patch, for example, assigning issue credit (given there are 108 comments and over 30 commenters on this issue will take some time).

Here's a fixed patch - I can't commit this one because I've contributed to it.

Lesson learned. Thank you @alexpott for updating the patch!

Committed b912d77 and pushed to 8.5.x. Thanks!

I think it still needs a CR, doesn't it ? Can't set the status for that, though.

Cr already exists but not ublished, & status somehow strange

I just published the change record. Thank you @fgm for noticing!

That's https://www.drupal.org/node/2913019 for anyone looking for the CR. Thanks @dawehner.

@fgm FYI: you can find the CR on the right-hand side of this issue, at the top, just below the issue metadata!

@wim leers: Ooops... my bad. Maybe we should add a d.o. issue to have the right metadata block stay in view since the comments don't extend into the right column anyway, then: it would avoid this type of mistake.

It's really hard to find CR on mobile - it's between comments & comment form

#120 & #121: I agree!

Can we port this into Drupal 7.x, please? Basically the same file can be used and it works fine. It just needs to be committed.

@kenorb please file separate issue, that makes sense!

Yes, this is still tagged for backport and has Drupal 7 patches above. It can be moved back to "Fixed" if/when someone creates a separate backport issue.

People are welcome to open the 7.x issue, but please don't re-open 8.x issues just for that - see "The 8.x issue can be closed normally once it has been fixed" from https://www.drupal.org/core/backport-policy.

I opened this because I saw it had been updated and thought it was being proposed for backport to 8.4.x.

@catch, that paragraph of documentation is assuming the 7.x issue already exists, but see https://www.drupal.org/core/maintainers/review-commit :

After commit, the issue can normally be marked fixed. However, if the issue is also relevant for an older version of Drupal core (for example, if it has the needs backport to D7 tag) then either create a child issue for the backport (and remove the "needs backport to D7" tag) or leave the issue open until someone else does the same.

The goal is to keep backportable issues from getting lost.

But in any case, I'm just going to go ahead and create a backport issue now.

Created #2923830: [D7] Add a startup configuration for the built-in PHP server that supports clean URLs for the backport.

Used to test https://bugs.php.net/bug.php?id=61286 on PHP 8 and looks we can remove the file when core require PHP 8.0 or 8.1

PS via https://github.com/php/php-src/pull/3215

Filed follow-up for PHP 8.0 #3228972: Remove startup configuration for the built-in PHP server when core require PHP 8.0