WordPress.org

Make WordPress Core

Opened 9 years ago

Last modified 7 months ago

#21077 reopened enhancement

Add support for custom ports in multisite site addresses

Reported by: djzone Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version: 3.4
Component: Bootstrap/Load Keywords: needs-patch needs-unit-tests
Focuses: multisite Cc:

Description

This patch enables MultiSite to be used with a custom port, what must be defined as WP_CUSTOM_PORT in wp-config.php.

Attachments (4)

multisite-custom-port.patch (945 bytes) - added by djzone 9 years ago.
Multisite custom port patch
ms-settings_201308012128.php (1.4 KB) - added by F J Kaiser 9 years ago.
ms-settings_201201082128.patch (1.4 KB) - added by F J Kaiser 9 years ago.
Ignore previous patch - added .php extension per accident
15936.5.diff (4.6 KB) - added by jeremyfelt 17 months ago.
Prior work on custom port numbers from 15936

Download all attachments as: .zip

Change History (24)

@djzone
9 years ago

Multisite custom port patch

#1 @scribu
9 years ago

Couldn't we use parse_url() for these things?

#2 @nacin
9 years ago

Yeah. ms-settings.php could stand for a scrub.

I've never been sure why custom ports are blocked in multisite. During the merge in 3.0, we tried to clean things up, but we tried not to ask "why" too often as everything would have been a rabbit hole.

We should probably review the history in MU and then work to just allow custom ports to work, without a constant.

#3 @ipstenu
9 years ago

  • Cc ipstenu added

#4 @ipstenu
9 years ago

Looks like it was hard coded in to use 80.

http://mu.wordpress.org/forums/topic/14587

#5 @F J Kaiser
9 years ago

  • Cc 24-7@… added
  • Severity changed from normal to major

+1 Just encountered a situation where I - in a local MU setup - can't use the default port of :80/:443. Now I'm left with a core hack.

#7 @mindctrl
9 years ago

  • Cc mindctrl added

#8 @F J Kaiser
9 years ago

  • Keywords needs-testing dev-feedback added

Scribus idea with parse_url is likely the route to go. Patch following that will

  • also takes the absence of $_SERVER['HTTP_HOST'] into account and switch to $_SERVER['SERVER_NAME'] for reliability and those edge cases
  • takes the domain via parse_url() and PHP_URL_PATH
  • still falls back to the default wp_die()-message in case somehow couldn't get rid of the port

Patch needs more intense testing.

@F J Kaiser
9 years ago

Ignore previous patch - added .php extension per accident

#9 @F J Kaiser
9 years ago

  • Keywords close added; has-patch needs-testing dev-feedback removed

Closed in favor of #15936 as it seems to address IPv6 issues as well. Adding new patch there.

#10 @SergeyBiryukov
9 years ago

  • Keywords close removed
  • Milestone Awaiting Review deleted
  • Resolution set to duplicate
  • Status changed from new to closed

Duplicate of #15936.

This ticket was mentioned in Slack in #core-multisite by jeremyfelt. View the logs.


17 months ago

#12 @jeremyfelt
17 months ago

  • Component changed from Multisite to Bootstrap/Load
  • Focuses multisite added
  • Keywords needs-patch needs-unit-tests added
  • Milestone set to Future Release
  • Resolution duplicate deleted
  • Severity changed from major to normal
  • Status changed from closed to reopened
  • Summary changed from Custom port patch for MultiSite to Add support for custom ports in multisite site addresses

@jeremyfelt
17 months ago

Prior work on custom port numbers from 15936

#13 @jeremyfelt
17 months ago

I've reopened this ticket since it was one of the first closed as a duplicate of #15936, which we've now closed as a separate area of focus. See also #42993, which can be considered a duplicate of this ticket. I've also attached the latest patch from #15936, 15936.5.diff, which handles some port number changes and some unnecessary IPv6 changes.

A few things to watch for / decide on:

The best place to test this now may be in the default WordPress local development environment, which uses localhost:8889 as its address and requires a port number.

This ticket was mentioned in Slack in #core-multisite by johnbillion. View the logs.


17 months ago

This ticket was mentioned in Slack in #core-multisite by jeremyfelt. View the logs.


16 months ago

This ticket was mentioned in Slack in #core-multisite by spacedmonkey. View the logs.


16 months ago

This ticket was mentioned in Slack in #core-multisite by spacedmonkey. View the logs.


16 months ago

#18 follow-up: @johnjamesjacoby
10 months ago

Should all port numbers be allowed or should there be a filtered list?

I think all port numbers should be allowed.

Story time. Yesterday I had to switch back to using MAMP PRO for local stuff. It makes both Apache and Nginx web servers available, both with http and https schemes, all on different ports. Depending on my /etc/hosts setup, depending on WordPress constants and options being filtered, depending on what I'm working on or testing, it's pretty convenient to have WordPress available on so many different ports.

MAMP has its own default ports but you can easily use 80/443/81/7443, or whatever you want really. The ability to use any port numbers is useful if you also have Docker/Lando running, or have Local running, or Valet, etc... especially when transitioning between environments.

I assume it would also be really useful with running multiple instances of something like VVV.

Lastly, ports are valid URL segments, even if they are weird to see. IMO, that means there is some inherent obligation to support them.

#19 in reply to: ↑ 18 @jeremyfelt
7 months ago

Replying to johnjamesjacoby:

Should all port numbers be allowed or should there be a filtered list?

I think all port numbers should be allowed.

I think this makes sense, especially given the story. That said, if there was a way to just get #52088 out the door by supporting only the port that WordPress is using for its development environment, that would at least be a start.

I'm not actively involved with this ticket, but I want to clarify my last comment in case it's preventing further progress here. 15936.5.diff is some prior work from another ticket and I don't think it addresses what is actually needed for this. It does provide some prior patch art though.

What's missing for this current ticket (IMO) is:

  • Proposed solution to how a custom port should be recognized and managed in bootstrap/load and (maybe? maybe not) managed with networks/sites in the network admin UI.
  • How to test the solution as part of the standard WP build process. Unit tests, end to end tests?
  • Code to implement the above.
  • How to test the solution locally. (Maybe it's just that the dev environment works!) :)

I'm happy to help with testing in a local environment once it's there, but I don't have the bandwidth for solution creating right now. :)

#20 @spacedmonkey
7 months ago

I think the question is, do we officially support custom ports, add a input when you register a site and save the port number somewhere ( site meta ). Then change bootstrap to support any port.

Or do we just give enough filters and hooks to enable developers to add support themselves, similar to domain mapping.

I would recommend the second course and I would even write a drop in / plugin to allow developers to do this.

I think that the current patch 15936.5.diff does solve this problem and does allow developers to do what they want.

Currently to add a custom port, you would have to add a sunrise.php and then add a filter that looks like this.

add_filters('allowed_multisite_ports', function( $current ){
  $current[] = ':81';

  return $current;
});

I think we would also need something in the bootstrap to looks up the site via a port.

This is work of course, but is not exactly use friendly. Maybe instead of filter, we could do with a PHP const.

define( 'WP_ALLOWED_PORTED', '80, 443' );

Once noting that not really good idea to have an array value of a const. So we could just use wp_parse_list.

So when we use it, would looks like this.

 $allowed_ports = wp_parse_list( WP_ALLOWED_PORTED );

Thoughts @jeremyfelt @johnjamesjacoby

Note: See TracTickets for help on using tickets.