Make WordPress Core

Opened 5 years ago

Last modified 15 months ago

#46561 new enhancement

Make wp_normalize_path() on Windows resolve drive letter for drive–relative paths

Reported by: rarst's profile Rarst Owned by:
Milestone: Awaiting Review Priority: normal
Severity: minor Version: 3.9
Component: Filesystem API Keywords: needs-patch dev-feedback
Focuses: Cc:

Description

Though rarely used, Windows allows to omit drive letter in file path to treat is as drive–relative.

This causes inconsistency where paths pointing to the same dir are not normalized to the same representation by wp_normalize_path():

<?php
var_dump( wp_normalize_path( 'C:\server\www\dev' ) );
// "C:/server/www/dev

var_dump( wp_normalize_path( '\server\www\dev' ) );
// /server/www/dev << same path, but mismatch after normalize

var_dump( wp_normalize_path( realpath( '\server\www\dev' ) ) );
// C:/server/www/dev << resolved drive letter before normalize

I think drive letter should be explicitly resolved as part of normalization for this case.

Change History (5)

#1 @Rarst
5 years ago

  • Summary changed from Make wp_nomralize_path() on Windows resolve drive letter for drive–relative paths to Make wp_normalize_path() on Windows resolve drive letter for drive–relative paths

#2 @Rarst
5 years ago

One problem is that wp_normalize_path() doesn’t require path to be real, it normalizes path strings really. Introducing file_exists() check here feels somewhat unwanted.

Maybe we should introduce a new function, something like wp_normalize_real_path() that will both validate path as valid filesystem path and normalize it?

#3 @SergeyBiryukov
5 years ago

  • Component changed from General to Filesystem API

#4 @costdev
15 months ago

  • Keywords dev-feedback added
  • Version set to 3.9

Rather than introduce a new function, or use file_exists() for all set ups, what about only running realpath() when the OS is windows, and the path begins with '/'?

Something like this:

<?php

// function wp_normalize_path...

// ...

/*
 * On Windows, if the path starts with '/' after normalizing,
 * try detecting the real path to apply the correct drive letter.
 */
if ( 'WIN' === strtoupper( substr( PHP_OS, 0, 3 ) ) && str_starts_with( $path, '/' ) ) {
    $realpath = realpath( $path );
    
    if ( is_string( $realpath ) ) {
        $path = $realpath;
    }
}

return $wrapper . $path;

This would mean having the correct result for existing paths, and local testing seems to show this working properly:

<?php

var_dump( wp_normalize_path( '\Users\costdev\test_normalize.php' ) );
// C:\Users\costdev\test_normalize.php

  • As this would be new functionality, setting Version to 3.9 when wp_normalize_path() was introduced.
  • Adding dev-feedback to gather thoughts.

#5 @Rarst
15 months ago

I now think making this check for a real file is too major of a backwards compatibility break. It's simply not what the function currently does and it's so generic, there is no way to tell what could that break in the wild.

By the way disk checks can be very expensive for performance. Hitting disk is much slower than memory and on Windows file exists checks are not as heavily cached by file system as on Linux, so they can cause massive slowdowns there, if called often. I would probably only consider introducing an implicit disk lookup if one is functionally unavoidable.

Note: See TracTickets for help on using tickets.