WordPress.org

Make WordPress Core

Opened 10 days ago

Last modified 9 days ago

#48289 new defect (bug)

wp_normalize_path() breaks path_is_absolute() in Windows.

Reported by: paultgoodchild Owned by:
Milestone: 5.4 Priority: normal
Severity: normal Version: 5.2.3
Component: Filesystem API Keywords: has-patch needs-unit-tests
Focuses: Cc:
PR Number:

Description

Apologies in advance if this is considered correct behaviour, but it's strange that this is the case.

On Windows, when you take an absolute file system path and use wp_normalize_path() and then pass this result to path_is_absolute(), you get false instead of true.

Super simple demo:

<?php
var_dump( path_is_absolute( __FILE__ ) );
var_dump( path_is_absolute( wp_normalize_path( __FILE__ ) ) );

*nix Result:

bool(true)
bool(true)

Windows Result:

bool(true)
bool(false)

Attachments (4)

48289.diff (872 bytes) - added by paultgoodchild 10 days ago.
a git diff to fix the problem
48289.1.diff (447 bytes) - added by itowhid06 9 days ago.
updated patch using DIRECTORY_SEPARATOR
48289.2.diff (449 bytes) - added by itowhid06 9 days ago.
48289.3.diff (457 bytes) - added by itowhid06 9 days ago.
updated the regex

Download all attachments as: .zip

Change History (10)

#1 @paultgoodchild
10 days ago

A potential fix for this is to adjust the regex in the function, so instead of:

preg_match( '#^[a-zA-Z]:\\\\#', $path )

you could have

preg_match( '#^[a-zA-Z]:(\\\\|/)#', $path )

@paultgoodchild
10 days ago

a git diff to fix the problem

#2 @SergeyBiryukov
10 days ago

  • Component changed from General to Filesystem API
  • Keywords has-patch needs-unit-tests added
  • Milestone changed from Awaiting Review to 5.4

#3 @jrf
9 days ago

Had a quick look at the patch.
Suggestion: look into using the PHP constant DIRECTORY_SEPARATOR.

@itowhid06
9 days ago

updated patch using DIRECTORY_SEPARATOR

#4 @itowhid06
9 days ago

Uploaded an updated patch with suggestion of @jrf. Works for me.

#5 @jrf
9 days ago

@itowhid06 Your patch doesn't address the problem as it only checks for the separator, but no longer checks whether something is an absolute path starting with C: (as in drive letter).

Last edited 9 days ago by jrf (previous) (diff)

@itowhid06
9 days ago

@itowhid06
9 days ago

updated the regex

#6 @paultgoodchild
9 days ago

I'd need to take a much closer look at the diffs, but how does such a complex IF solve this, unless I'm missing something.

I know the DIRECTORY SEPARATOR constant certainly cannot fix this problem, because of the way normalize works.

Note: See TracTickets for help on using tickets.