Make WordPress Core

Opened 13 years ago

Closed 13 years ago

#18592 closed defect (bug) (fixed)

add trailingslashit to BLOGUPLOADDIR in ms-files.php

Reported by: wonderboymusic's profile wonderboymusic Owned by: nacin's profile nacin
Milestone: 3.3 Priority: normal
Severity: normal Version: 3.3
Component: Multisite Keywords: has-patch needs-unit-tests
Focuses: Cc:

Description

BLOGUPLOADDIR is trailingslashit'd inconsistently - since this value can be set by the user, adding trailingslashit to the constant value in ms-files.php will prevent broken images in sub-blogs when in Multisite mode

// WRONG
define( 'BLOGUPLOADDIR', $_SERVER['DOCUMENT_ROOT'] . "/blogs.dir/{$the_id}/files" );

// RIGHT
define( 'BLOGUPLOADDIR', $_SERVER['DOCUMENT_ROOT'] . "/blogs.dir/{$the_id}/files/" );

// CURRENT 
$file = BLOGUPLOADDIR . str_replace( '..', '', $_GET[ 'file' ] );

// PROPOSED
$file = trailingslashit( BLOGUPLOADDIR ) . str_replace( '..', '', $_GET[ 'file' ] );

It's subtle, and hard to debug

Attachments (1)

add-trailingslashit.diff (498 bytes) - added by wonderboymusic 13 years ago.
Patch for this ticket

Download all attachments as: .zip

Change History (8)

@wonderboymusic
13 years ago

Patch for this ticket

#1 @SergeyBiryukov
13 years ago

  • Milestone changed from Awaiting Review to 3.3

#2 @SergeyBiryukov
13 years ago

  • Component changed from Media to Multisite

#3 @nacin
13 years ago

In [18637]:

Use untrailingslashit in url_shorten() and recurse_dirsize(). Remove unnecessary camelCase variables. see #18592.

#4 @nacin
13 years ago

  • Keywords needs-unit-tests added

Noticed a few other things while looking at BLOGUPLOADDIR usage. We rather consistently slash/unslash the constant depending on what we want to do with it -- except for here.

We should probably have unit tests for a lot of these constants.

#5 @nacin
13 years ago

In [18638]:

Force a trailing slash on BLOGUPLOADDIR in ms-files.php. props wonderboymusic, see #18592.

#6 @davecpage
13 years ago

  • Cc davecpage added

This patch causes images in multisites not to display.

The issue is that the 'SHORTINIT' define at the top of wp-includes/ms-files.php causes wp-settings.php to not include wp-includes/formatting.php, which is where trailingslashit() exists.

#7 @nacin
13 years ago

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

In [18642]:

trailingslashit isn't normally available in ms-files. props davecpage, fixes #18592.

Note: See TracTickets for help on using tickets.