WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 2 years ago

Last modified 2 years ago

#23175 closed enhancement (fixed)

Make get_home_path() return consistent slashes

Reported by: SergeyBiryukov Owned by: SergeyBiryukov
Milestone: 3.6 Priority: normal
Severity: normal Version:
Component: Filesystem API Keywords: has-patch
Focuses: Cc:

Description (last modified by SergeyBiryukov)

As noted in ticket:23073:40, get_home_path() doesn't return consistent slashes.

If home and siteurl are identical, it falls back to ABSPATH:

C:\wamp\www\multitest/

If home and siteurl are different:

C:/wamp/www/wordpress/test_subd/ 

Attachments (3)

23175.patch (1.3 KB) - added by SergeyBiryukov 3 years ago.
23175.2.patch (1.2 KB) - added by SergeyBiryukov 3 years ago.
23175.3.patch (2.1 KB) - added by Marventus 2 years ago.
Fixes slash inconsistencies in ABSPATH constant

Download all attachments as: .zip

Change History (15)

@SergeyBiryukov3 years ago

comment:1 @SergeyBiryukov3 years ago

I've reviewed all the instances of get_home_path() in core to make sure both types of slashes are allowed there (since it currently returns both types anyway).

It's mostly used in file_exists() and win_is_writable() checks, which is fine.

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

comment:2 @SergeyBiryukov3 years ago

  • Description modified (diff)

@SergeyBiryukov3 years ago

comment:3 @SergeyBiryukov2 years ago

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

In 23669:

Make get_home_path() return consistent slashes. fixes #23175.

comment:4 follow-up: @bananastalktome2 years ago

  • Cc bananastalktome@… added
  • Keywords needs-unit-tests added
  • Resolution fixed deleted
  • Status changed from closed to reopened

Breaks unit test test_get_home_path in tests/admin/includesFile.php

1) Tests_Admin_includesFile::test_get_home_path
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'D:\root\vhosts\site\httpdocs/'
+'D:/root/vhosts/site/httpdocs/'

(I'm reopening the ticket because of this, which I think is the proper thing to do in these cases. Please let me know if this is not what I should do.)

comment:5 @SergeyBiryukov2 years ago

In 1244/tests:

Update get_home_path() test to reflect the change in [WP23669]. see #23175.

comment:6 in reply to: ↑ 4 @SergeyBiryukov2 years ago

  • Keywords needs-unit-tests removed
  • Resolution set to fixed
  • Status changed from reopened to closed

Replying to bananastalktome:

I'm reopening the ticket because of this, which I think is the proper thing to do in these cases.

It is, thanks.

@Marventus2 years ago

Fixes slash inconsistencies in ABSPATH constant

comment:7 @Marventus2 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Could this have come from the various

define('ABSPATH', dirname(__FILE__).'/');

commands scaretted accross multiple files? I took the liberty of creating a diff.

comment:8 @Marventus2 years ago

  • Keywords dev-feedback added

comment:9 @SergeyBiryukov2 years ago

  • Keywords dev-feedback removed
  • Resolution set to fixed
  • Status changed from reopened to closed

Yes, the trailing slash in ABSPATH is inconsistent in Windows, but it's out of scope for this ticket, and it's not really an issue, since Windows allows to use both types of slashes.

23175.3.patch would make the slashes inconsistent on non-Windows installs.

comment:10 @Marventus2 years ago

it's not really an issue, since Windows allows to use both types of slashes

Well, it could be if you are doing path string manipulations. What if we used DIRECTORY_SEPARATOR instead?

define('ABSPATH', dirname(__FILE__).DIRECTORY_SEPARATOR);

Would that address non-Windows OSs?

it's out of scope for this ticket

If you agree with the above, should I create a new ticket and update the diff?
Thanks!

Last edited 2 years ago by Marventus (previous) (diff)

comment:11 @SergeyBiryukov2 years ago

I'd suggest to normalize the slashes with str_replace( '\\', '/', ... ) before doing any string manipulations. This is what the core does in a few functions.

Using DIRECTORY_SEPARATOR was previously discussed in #15598, #17494, #20849, and #22267. The consensus in each ticket was that it's not needed.

Last edited 2 years ago by SergeyBiryukov (previous) (diff)

comment:12 @Marventus2 years ago

Ok, no worries, and thanks for the detailed answer!

Note: See TracTickets for help on using tickets.