D7: DrupalHTTPRequestTestCase breaks on php-fpm [testbot PHP 5.4 blocker]

Rather than removing, could we make the test a no-op if Apache PHP module isn't being used? That way non-testbot sites can still run this test?

Did some digging. The origin of this test was in #296310: TestingParty08: drupal_http_request redirects need a test. The title implies that it was something we came up with for the Awesome Testing Party With Hungarian Pancakes in Szeged, and the participants seem to imply this was later re-purposed as a GHOP task. In other words, to some extent this was introduced as a way to create "valuable busy work," rather than because we were trying to solve some sort of problem.

drupal_http_request() is removed in D8 in favour of Guzzle, so I'm not completely opposed to blitzing the test. OTOH, the test itself does add value and is one of a suite of tests for this function making sure it works properly. Since Alex Pott cited drupal_http_request() in his "Core Complexity" talk in Prague as being the most complex function in all of Drupal, I do lean more towards keeping it around and simply fixing it for non-Apache environments. But rather than a crazy detailed workaround as in the OP (I don't feel this test is of that much value given D8's direction, personally), something more along the lines of #1.

#670454: Support HTTP Authorization in CGI environment made it into D8 and D7 already, and my backport for D6 is still ready.

By the looks of it, we just need to change the code to access the $_SERVER['HTTP_AUTHORIZATION'] variable now.

Attached patch changes the system_test.module page callback to extract and rebuild the HTTP Basic Authentication parameters by leveraging the $_SERVER['HTTP_AUTHORIZATION'] variable that is guaranteed to exist in all environments now.

@rfay: Alrighty, this patch passed against HEAD, so now it's time to check whether it also works on a php-fpm SAPI. :)

Bumping this to critical, since Drupal 8 cannot bump its PHP version requirement before testbots are able to run on php-fpm.

+++ b/modules/simpletest/tests/system_test.module
@@ -114,8 +114,12 @@ function system_test_sleep($seconds) {
+  list($user, $pw) = explode(':', base64_decode(substr($_SERVER['HTTP_AUTHORIZATION'], 6)));
...
+  // @see http://www.php.net/manual/en/features.http-auth.php
...
+  // HTTP_AUTHORIZATION.
...
+  // Resemble PHP_AUTH_USER and PHP_AUTH_PW for a Basic authentication from

Maybe a reference to our .htaccess file would be great, as the .htacess file ensures that this variable is available.

Well, that seemed simple enough. Sigh. Testing this locally with fpm, the HTTP_AUTHORIZATION variable isn't available. It's hiding as REDIRECT_HTTP_AUTHORIZATION. This seems to be undocumented behavior in Apache.

There's also REDIRECT_protossl

See http://stackoverflow.com/questions/2267976/apache-setenv-prepends-redire...

http://stackoverflow.com/questions/3050444/when-setting-environment-vari...

Symfony does it like so:

https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Htt...

Thanks for digging that out. Attached patch should work correctly. Also addressed @dawehner's concern.

#12 solves dawehner's problems, and fixes the problems I've addressed.

This is RTBC.

I have to note, though, that I'm seeing notices from DrupalHTTPRequestTestCase::testDrupalHTTPRequestRedirect(). Specifically, the 307 testing.

I've tested 5.3.28 and 5.4.24 mod_php/fpm and both are showing the same issue. When trying to perform a 307 redirect from fpm, the reason phrase isn't showing up which triggers #205969: drupal_http_request() assumes presence of Reason-Phrase in response Status-Line.

Committed to 7.x - thanks!

I fixed a couple small code comment issues on commit (replaced inline @see with "See", and got rid of the "/en/" in the php.net URL, due to #692366: Stop forcing language/mirror for PHP manual links).

I also wonder what would happen if neither $_SERVER['HTTP_AUTHORIZATION'] nor $_SERVER['REDIRECT_HTTP_AUTHORIZATION'] is set (the Symfony code seems to handle this possibility) but I guess if it's a legit issue in practice we'll eventually hear about it.