WordPress.org

Make WordPress Core

Opened 3 years ago

Last modified 3 months ago

#15936 new defect (bug)

IPv6 literal support in multisite broken

Reported by: jmdh Owned by:
Milestone: Future Release Priority: normal
Severity: major Version: 3.0.3
Component: Permalinks Keywords: has-patch dev-feedback
Focuses: multisite Cc:

Description

The logic for handling explicit port numbers in wp-includes/ms-settings.php is confused by IPv6 literal addresses in URLs as defined by RFC 2732.
It tries to handle the URL as it as if there were a port appended, but then fails to strip it off. Incidentally the error message here: 'Multisite only works without the port number in the URL.' is untrue, since ports are handled (but for only two particular cases, port 80 and 443).

The attached patch, against Wordpress 3.0.3, fixes both these issues, and allows ports other than 80 and 443 to be used with Wordpress, by just stripping off the trailing port rather than special-casing the two well-known ports, and not incorrectly detecting IPv6 literals as URLs with ports in. It also has the advantage of being much more compact.

It may be worth someone thinking through whether the substitution is strictly correct with reference to the URL standards, but I'm pretty sure that this is an improvement on the current code.

Thanks,
Dominic.

Attachments (2)

fix_ipv6_literals.patch (1.0 KB) - added by jmdh 3 years ago.
Fix IPv6 literals in URLs for Wordpress 3.0.3
ms-settings_201301082151.patch (1.2 KB) - added by F J Kaiser 16 months ago.
Takes $_SERVER['SERVER_NAME'] also into account. Works with current (3.5) version. Fixes the global $domain.

Download all attachments as: .zip

Change History (17)

comment:1 jmdh3 years ago

My original patch mistakenly removes the munging of $domain. I've updated the patch to address this issue.

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

jmdh3 years ago

Fix IPv6 literals in URLs for Wordpress 3.0.3

comment:2 nacin3 years ago

  • Milestone changed from Awaiting Review to Future Release

comment:3 ofutur3 years ago

I can confirm that this fixes the problem.

comment:4 jmdh3 years ago

  • Keywords has-patch added

comment:5 jmdh3 years ago

Patch applies cleanly to SVN trunk r17268 too (but not tested there).

comment:6 follow-up: mitchoyoshitaka3 years ago

  • Cc mitcho@… added

Looks like this patch also helps with situations where the site is indeed running on a nonstandard port.

I'm concerned, however, that $_SERVER['HTTP_HOST'] is actually used in a number of places in the code, including situations where it is used in building a URL for printing and redirecting. Suppose we have WP on server:8080. We go into server:8080 and, with this patch, WP will no longer balk, but some functions such as auth_redirect will end up using the server:80 equivalent of the URL. Just munging $_SERVER['HTTP_HOST'] doesn't seem to be the right approach here.

comment:7 in reply to: ↑ 6 ; follow-up: F J Kaiser16 months ago

Replying to mitchoyoshitaka:

Looks like this patch also helps with situations where the site is indeed running on a nonstandard port.

I'm concerned, however, that $_SERVER['HTTP_HOST'] is actually used in a number of places in the code, including situations where it is used in building a URL for printing and redirecting. Suppose we have WP on server:8080. We go into server:8080 and, with this patch, WP will no longer balk, but some functions such as auth_redirect will end up using the server:80 equivalent of the URL. Just munging $_SERVER['HTTP_HOST'] doesn't seem to be the right approach here.

WordPress already replaces the value of the global $_SERVER['HTTP_HOST'] to one without :PORT_NR attached, so we know this one is working. Just take a look at WordPress@GitHub ms.settings.php. I also worked in the related $_SERVER['SERVER_NAME'] (as we can't rely on either) in #21077 (which I now closed in favor of this ticket).

F J Kaiser16 months ago

Takes $_SERVER['SERVER_NAME'] also into account. Works with current (3.5) version. Fixes the global $domain.

comment:8 F J Kaiser16 months ago

  • Keywords dev-feedback added
  • Severity changed from normal to major

comment:9 F J Kaiser16 months ago

  • Component changed from General to Multisite

comment:10 toscho16 months ago

  • Cc info@… added

comment:11 SergeyBiryukov16 months ago

#21077 was marked as a duplicate.

comment:12 micahwave15 months ago

  • Cc micahwave added

comment:13 in reply to: ↑ 7 nacin15 months ago

Replying to F J Kaiser:

Replying to mitchoyoshitaka:

Looks like this patch also helps with situations where the site is indeed running on a nonstandard port.

I'm concerned, however, that $_SERVER['HTTP_HOST'] is actually used in a number of places in the code, including situations where it is used in building a URL for printing and redirecting. Suppose we have WP on server:8080. We go into server:8080 and, with this patch, WP will no longer balk, but some functions such as auth_redirect will end up using the server:80 equivalent of the URL. Just munging $_SERVER['HTTP_HOST'] doesn't seem to be the right approach here.

WordPress already replaces the value of the global $_SERVER['HTTP_HOST'] to one without :PORT_NR attached, so we know this one is working.

Only for port 80 (which is the default for http) and port 443 (https). Thus, as long as the right protocols are used, the port numbers can be omitted. For other port numbers, not so good.

Has anyone tracked down why MU had these limitations to begin with?

comment:15 jeremyfelt3 months ago

  • Component changed from Multisite to Permalinks
  • Focuses multisite added
Note: See TracTickets for help on using tickets.