Problem/Motivation

In #1543858: Add a startup configuration for the built-in PHP server that supports clean URLs we add configuration for PHP's inbuilt webserver to work with clean URLs. Unfortunately this configuration does not support update.php

Proposed resolution

Make it support it.

Steps to test/reproduce

  1. Install standard
  2. Execute $ php -S localhost:8888 .ht.router.php from the command line from DRUPAL_ROOT
  3. Visit http://localhost:8888/update.php
  4. Click on the continue button
  5. Ensure that admin/structure/types/manage/article/fields/node.article.body is not broken since this is why with the script in the first place - see https://bugs.php.net/bug.php?id=61286
  6. Ensure that http://localhost:8888/update.php/selection/blahd.blah

    7. falls back to an update screen since this is the behaviour on Apache.

  7. Go to node/add/article
  8. Create a node with an image uploaded
  9. Ensure the image appears

Remaining tasks

User interface changes

None

API changes

None

Data model changes

None

CommentFileSizeAuthor
#10 2929198-10.patch1.54 KBalexpott
#10 9-10-interdiff.txt1.36 KBalexpott
#9 8-9-interdiff.txt2.38 KBalexpott
#9 2929198-9.patch1.73 KBalexpott
#8 2929198-6.patch2.23 KBalexpott
#8 5-6-interdiff.txt456 bytesalexpott
#7 2929198-5.patch2.2 KBalexpott
#7 3-5-interdiff.txt426 bytesalexpott
#4 2929198-3.patch2.2 KBalexpott
#4 2-3-interdiff.txt2.31 KBalexpott
#2 2929198-2.patch836 bytesalexpott
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

alexpott created an issue. See original summary.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Thunder commented
Status: Active » Needs review
FileSize
2929198-2.patch836 bytes

Here's a patch that fixes it.

chr.fritsch’s picture

chr.fritsch
he/him
Primary language German
Location 🇩🇪🇪🇺🌍
CreditAttribution: chr.fritsch at Thunder commented

I was able reproduce the issue and can confirm that #2 fixes it.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Thunder commented
Issue summary: View changes
FileSize
2-3-interdiff.txt2.31 KB
2929198-3.patch2.2 KB

Here's a better fix with support for things other than update.php.

The last submitted patch, 2: 2929198-2.patch, failed testing. View results

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Thunder commented
Issue summary: View changes

Testing has shown that #3 breaks image styles :(

Here is a fix.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Thunder commented
Issue tags: +Needs manual testing
FileSize
3-5-interdiff.txt426 bytes
2929198-5.patch2.2 KB
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Thunder commented
FileSize
5-6-interdiff.txt456 bytes
2929198-6.patch2.23 KB

We can limit the amount of file system checking by just looking for paths that end in .php

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Thunder commented
FileSize
2929198-9.patch1.73 KB
8-9-interdiff.txt2.38 KB

Further testing of the equivalent Drush command has shown that this needs a small tweak.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Thunder commented
FileSize
9-10-interdiff.txt1.36 KB
2929198-10.patch1.54 KB

Worked out a nicer way with way less conditionals and probably quicker to boot too.

The last submitted patch, 7: 2929198-5.patch, failed testing. View results

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Thunder commented

Just to note that Thunder is now successfully using this patch to spin up a server using PHP in travis testing.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner at Chapter Three commented 
  1. +++ b/.ht.router.php
@@ -30,10 +30,26 @@
+// Work around the PHP bug.
...
+if (strpos($path, '.php') !== FALSE) {

    Is this a PHP bug we can link to? 

  2. +++ b/.ht.router.php
@@ -30,10 +30,26 @@
+    if (preg_match('/\.php$/', $path) && is_file('.' . $path)) {

    I'm curious, is there a reason why we use a regex instead of a strpos?

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Thunder commented

1. The PHP bug is already linked in the file. See https://github.com/drupal/drupal/blob/8.5.x/.ht.router.php
2. Yep because I wanted to ensure that it was a .php file so the $ is significant.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner at Chapter Three commented
Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs manual testing

2. Yep because I wanted to ensure that it was a .php file so the $ is significant.

I was trying to apply some microoptimization, sorry.

1. The PHP bug is already linked in the file. See https://github.com/drupal/drupal/blob/8.5.x/.ht.router.php

Oh right, thank you!

This no longer needs manual testing.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Version: 8.6.x-dev » 8.5.x-dev
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.6.x and cherry-picked to 8.5.x. Thanks!

  • catch committed 74d9b41 on 8.6.x 
    Issue #2929198 by alexpott: .ht.router.php does not support update.php

  • catch committed 0b2d458 on 8.5.x 
    Issue #2929198 by alexpott: .ht.router.php does not support update.php...
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Thunder commented
Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.