Make WordPress Core

Opened 11 years ago

Closed 11 years ago

#23689 closed defect (bug) (fixed)

get_blogaddress_by_name() fails to return address for sites whose names begin with a number

Reported by: dllh's profile dllh Owned by: westi's profile westi
Milestone: 3.6 Priority: normal
Severity: normal Version: 3.0
Component: Multisite Keywords: has-patch
Focuses: Cc:

Description

In get_blogaddress_by_name(), we use a backreference to insert the passed $blogname into the return from network_home_url() when the site is a subdomain install. When the subdomain begins with a digit, the first digit is clipped off of the name because of the way preg_replace() handles backreferences. From php docs:

When working with a replacement pattern where a backreference is immediately followed by another number (i.e.: placing a literal number immediately after a matched pattern), you cannot use the familiar \\1 notation for your backreference. \\11, for example, would confuse preg_replace() since it does not know whether you want the \\1 backreference followed by a literal 1, or the \\11 backreference followed by nothing. In this case the solution is to use \${1}1. This creates an isolated $1 backreference, leaving the 1 as a literal.

Basically, the leading digit in the passed $blogname is being interpreted at present as an additional digit in the backreference name, so that the backreference becomes invalid. This also seems to cause the first digit to be stripped off of $blogname.

This code landed three years ago in [14703].

The attached patch uses php's recommended syntax for such cases. I tested the fix in a multisite install by writing the following plugin and testing it both with and without the patch:

function dllh_blog_address() {
        print '<div class="updated"><p>';
        print '<p>Passing the items to the left of the colon to get_blogaddress_by_name() results in the return on the right.</p>';
        print '123foo : ' . get_blogaddress_by_name( '123foo' ) . '<br />';
        print 'foobar : ' . get_blogaddress_by_name( 'foobar' ) . '<br />';
        print '</p></div>';
}

// Now we set that function up to execute when the admin_notices action is called
add_action( 'admin_notices', 'dllh_blog_address' );

Screen shots of the output from both are attached.

Attachments (3)

get_blogaddress_by_name.png (27.1 KB) - added by dllh 11 years ago.
Output demonstrating the bug
get_blogaddress_by_name-patched.png (28.9 KB) - added by dllh 11 years ago.
Output with the patch applied
get_blogaddress_by_name.patch (514 bytes) - added by dllh 11 years ago.

Download all attachments as: .zip

Change History (7)

@dllh
11 years ago

Output demonstrating the bug

@dllh
11 years ago

Output with the patch applied

#1 @SergeyBiryukov
11 years ago

  • Milestone changed from Awaiting Review to 3.6
  • Version changed from trunk to 3.0

#2 @jkudish
11 years ago

  • Cc jkudish added

#3 @westi
11 years ago

The patch looks good to me and it would be nice to have unit-tests for it.

The problem is the unit-tests currently run as subdirectory not subdomain so I don't think they would be simple to write.

So, I'm going to commit this without tests and rely on the manual testing and code review.

#4 @westi
11 years ago

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

In 23686:

Multisite: Ensure that get_blogaddress_by_name does not mangle blognames with leading digits.

Correctly specify the backreference in the regular expression so that it can not become ambiguous when there is a leading digit on the blogname.

Fixes #23689 props dllh.

Note: See TracTickets for help on using tickets.