WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 2 years ago

#22138 closed defect (bug) (fixed)

wp_basename wrong on Windows Systems

Reported by: sebber Owned by: ryan
Milestone: 3.5 Priority: normal
Severity: normal Version: 3.4.2
Component: Formatting Keywords: has-patch commit
Focuses: Cc:

Description

On Windows System the Directory Seperator is \ instead of Unix /.

So the urlencode in wp_basename() encodes the \ and it is not removed in basename().

WP_basename is not working properly on Windows Systems.

Just take a look in the Support Forum:

http://wordpress.org/support/topic/function-wp_basename-is-wrong-in-windows?replies=3

Attachments (1)

22138.patch (500 bytes) - added by SergeyBiryukov 3 years ago.

Download all attachments as: .zip

Change History (11)

comment:1 @sebber3 years ago

Possible Resolution:

function wp_basename( $path, $suffix = '' ) {
$path = str_replace( '\', '/', $path );
$path = str_replace( '%2F', '/', urlencode( $path ) );
return urldecode( basename( $path ), $suffix );

}
Last edited 3 years ago by SergeyBiryukov (previous) (diff)

@SergeyBiryukov3 years ago

comment:2 @SergeyBiryukov3 years ago

  • Keywords has-patch needs-unit-tests added; needs-patch removed
  • Milestone changed from Awaiting Review to 3.5

Confirmed. wp_basename() is mostly used on attachment URLs in core, which is why, I guess, we didn't catch this earlier.

comment:3 @SergeyBiryukov3 years ago

  • Summary changed from wp_basname wrong on Windows Systems to wp_basename wrong on Windows Systems

comment:4 @sebber3 years ago

I checked your solution. It seems all right. Thank you

comment:5 @knutsp3 years ago

  • Cc knut@… added

comment:6 @bpetty2 years ago

  • Keywords needs-unit-tests removed

I added unit tests for wp_basename for this situation.

comment:7 @bpetty2 years ago

Obviously, I also tested, and [attachement:22138.patch] works.

Version 0, edited 2 years ago by bpetty (next)

comment:8 @bpetty2 years ago

  • Cc bpetty added

comment:9 @nacin2 years ago

  • Keywords commit added

comment:10 @ryan2 years ago

  • Owner set to ryan
  • Resolution set to fixed
  • Status changed from new to closed

In [22310]:

Fix wp_basename() for Windows by replacing %5C with /.

Props SergeyBiryukov
fixes #22138

Note: See TracTickets for help on using tickets.