Wrong guess in wp_guess_url() for symlinked install, or unit tests

[bpetty]

The changes made in r25396 add additional checks, but could use a couple more fixes.

1. In the case of symlinked installations (checkout of develop.svn, and symlink public_html to 'src' for example), this fails to properly detect the URL, and ends up appending a full path disclosure to the siteurl option.
2. In the case of unit tests, this also fails to properly strip the absolute file path off (since SCRIPT_FILENAME points to tests/phpunit/includes/install.php instead), causing most canonical unit tests to fail.

For (A), a confirmed solution is using realpath() on SCRIPT_FILENAME first.

In the case of (B), we should probably just set WP_SITEURL in bootstrap.php most likely - it would help keep wp_guess_url() simple and specific to real installations.

If no-one else jumps on it, I can easily take care of (B), but still need a patch on (A).

[bpetty]

Related: #16884, #24173, and #24480

[SergeyBiryukov]

[25396] also doesn't work on Windows.

In my case, the values for unit tests were:

$_SERVER['SCRIPT_FILENAME']: S:\\usr\\local\\php5\phpunit
ABSPATH: S:\home\wordpress\develop/src/
Resulting URL: http://example.orgS:\home\wordpress\develop/src

When trying to set up a new install:

$_SERVER['SCRIPT_FILENAME']: S:/home/wordpress/trunk/index.php
ABSPATH: S:\home\wordpress\trunk/
Resulting URL: http://trunk.wordpressS:\home\wordpress\trunk

Visiting http://trunk.wordpress results in a failed redirect to http://trunk.wordpressS:homewordpresstrunk/wp-admin/install.php.

25317.patch​ fixes both the unit tests and the installation for me by normalizing slashes and providing a fallback if none of the strpos() conditions are met.

[SergeyBiryukov]

IRC discussion: ​https://irclogs.wordpress.org/chanlog.php?channel=wordpress-dev&day=2013-09-13&sort=asc#m687765

[dd32]

attachment:25317.patch​​

That last fallback case might as well be $path = $_SERVER['REQUEST_URI'], since wp-admin and wp-login are caught in the first block.

I would love it if we could simply fix ABSPATH once-and-for-all, sanitizing the slashes at define time..

What we need to get is a set of unit tests here, except, the fact we're dealing with constants kind of makes that almost impossible with the current testing methodology. thoughts?

Perhaps those with scenario's that break can post a set of $_SERVER data that could be used in a test? (SCRIPT_FILENAME, PHP_SELF, REQUEST_URI, ABSPATH, etc.)

[SergeyBiryukov]

Replying to dd32:

That last fallback case might as well be $path = $_SERVER['REQUEST_URI'], since wp-admin and wp-login are caught in the first block.

Thanks, updated.

Perhaps those with scenario's that break can post a set of $_SERVER data that could be used in a test? (SCRIPT_FILENAME, PHP_SELF, REQUEST_URI, ABSPATH, etc.)

My data, when running unit tests:

$_SERVER['SCRIPT_FILENAME']: S:\\usr\\local\\php5\\phpunit
$_SERVER['PHP_SELF']: /index.php
$_SERVER['REQUEST_URI']:
ABSPATH: S:\home\wordpress\develop/src/

When setting up a new install:

$_SERVER['SCRIPT_FILENAME']: S:/home/wordpress/trunk/index.php
$_SERVER['PHP_SELF']: /index.php
$_SERVER['REQUEST_URI']: /
ABSPATH: S:\home\wordpress\trunk/

[dd32]

In 25436:

Account for Windows and CLI instances in wp_guess_url(). Props SergeyBiryukov. See #25317

[atimmer]

I can confirm trunk now works on my installation.

When running tests:

$_SERVER['SCRIPT_FILENAME']: /home/anton/sites/wordpress-trunk/repo/tests/phpunit/includes/install.php
$_SERVER['PHP_SELF']: /index.php
$_SERVER['REQUEST_URI']:
ABSPATH: /home/anton/sites/wordpress-trunk/repo/src/

In the browser:

$_SERVER['SCRIPT_FILENAME']: /home/anton/sites/wordpress-trunk/www/index.php
$_SERVER['PHP_SELF']: /index.php
$_SERVER['REQUEST_URI']: /
ABSPATH: /home/anton/sites/wordpress-trunk/repo/src/

[dd32]

So after thinking about it, the only way we could really unit test this would be to change
$abspath_fix = str_replace( '\\', '/', ABSPATH );
to something like this:
$abspath_fix = str_replace( '\\', '/', ( isset( $_SERVER['ABSPATH'] ) ? $_SERVER['ABSPATH'] : ABSPATH ) );
or to put a filter around ABSPATH.

It'd be nice to have a consistent way to unit test things that use ABSPATH, for example, for WP_Filesystem I had to override the abspath() method in the Mock FS handler to accept an optional path for ABSPATH for use during the tests.
Passing an parameter in works for Unit-test-only functions, but not for core functions, leading me to the $_SERVER hack above.

[nacin]

Replying to dd32:

It'd be nice to have a consistent way to unit test things that use ABSPATH, for example, for WP_Filesystem I had to override the abspath() method in the Mock FS handler to accept an optional path for ABSPATH for use during the tests.

Usually mocking the class is exactly how testing would occur.

Would $_ENV make more sense than $_SERVER? I'm not sure; just throwing it out there.

I think we can safely close this as fixed, leaving needs-unit-tests of course. Am I correct that this is handled?

[dd32]

Replying to nacin:

Would $_ENV make more sense than $_SERVER? I'm not sure; just throwing it out there.

$_ENV would probably make sense here I agree. (I forgot about that super global)

I think we can safely close this as fixed, leaving needs-unit-tests of course. Am I correct that this is handled?

Correct (AFAIK)

[phazei]

This is not actually repaired in v4
It still definitely still has the issue "A" from OP.