#21077 closed enhancement (fixed)
Add support for custom ports in multisite site addresses
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 6.6 | Priority: | normal |
Severity: | normal | Version: | 3.4 |
Component: | Bootstrap/Load | Keywords: | needs-patch |
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 (6)
Change History (79)
#2
@
13 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.
#5
@
12 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.
#8
@
12 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()
andPHP_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.
#9
@
12 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
@
12 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.
5 years ago
#12
@
5 years 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
#13
@
5 years 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:
- Is it enough to just support port numbers in the multisite bootstrap or should the UI fully support them as well? Things may seem a little strange at first.
- Should all port numbers be allowed or should there be a filtered list?
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.
5 years ago
This ticket was mentioned in Slack in #core-multisite by jeremyfelt. View the logs.
5 years ago
This ticket was mentioned in Slack in #core-multisite by spacedmonkey. View the logs.
5 years ago
This ticket was mentioned in Slack in #core-multisite by spacedmonkey. View the logs.
5 years ago
#18
follow-up:
↓ 19
@
4 years 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
@
4 years 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
@
4 years 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
This ticket was mentioned in PR #1963 on WordPress/wordpress-develop by soderlind.
3 years ago
#21
- Keywords has-patch added; needs-patch removed
Fix bug in patch 15936.5.diff, re-add the port number in ms-site.php
Trac ticket: https://core.trac.wordpress.org/ticket/21077
#22
@
3 years ago
Please be nice, the pull request is my first patch :)
You can test it locally using VS Code devcontainer available as part of my plugin. A how-to is also available.
This ticket was mentioned in Slack in #core-multisite by soderlind. View the logs.
3 years ago
soderlind commented on PR #1963:
3 years ago
#24
@costdev I'm looking into why the unit tests failed.
soderlind commented on PR #1963:
3 years ago
#25
@costdev New unit test added.
This ticket was mentioned in Slack in #core-committers by spacedmonkey. View the logs.
3 years ago
#27
@
3 years ago
I'll add a note that multisite with custom ports appear to work out of the box, but requires you to edit the wp_sites
database entry, and add the :<port>
your self. This is because the sanitize on save function in multisite strips out :
, leaving you with a url<port>
instead of url:<port>
db entry if using the interface.
I'm not sure if there's any other issue besides this one, if there's any concerns beyond back-compat then this is the place to outline them, but from some preliminary testing it would appear that updating the filters to allow a format of :[0-9]+
to pass through the sanitizer on save shouldn't break anything, and open things up for custom ports to the users hearts desire 🤔
#28
@
3 years ago
I spent another couple of hours hacking on this problem, still finding issues. The issue, as I see, it where to store the custom port. If you store it as part of a domain, then that makes issues all over core, when domain is no longer domain but domain + port. I had to make lots of changes to get multisite to even bootstrap. The best place to store port number is in site ( blog ) meta. This keep the port number close to the site in meta but does not break anything.
I prototyped how this might work in this sunrise.php dropin.
What made it hard to test, was the lack of this filter.
$allowed_ports = apply_filters( 'allowed_multisite_ports', array( ':80', ':443' ) );
To help third parties be able to work out their own solution for this, I recommend a new ticket to add this filter. That way this functionality is not blocked while we work on another solution.
This ticket was mentioned in Slack in #core-test by ironprogrammer. View the logs.
2 years ago
#32
@
17 months ago
My multisite network is running on localhost:8082
. Everything seems to be working fine for now with this code:
<?php add_filter( 'wp_normalize_site_data', function( $data ) { $data['domain'] = str_replace('8082', ':8082', $data['domain']); return $data; }, 50, 1 );
I've added this in mu-plugins, but I guess it can go in your theme's functions.php
or a regular plugin as well.
I can add a new site trough wp-admin or via wpmu_create_blog()
without any problems.
Note that if your domain is called domain8082.com or something similar, this code will not work :)
This ticket was mentioned in PR #5675 on WordPress/wordpress-develop by @obliviousharmony.
15 months ago
#33
- Keywords has-unit-tests added; needs-unit-tests removed
This _should_ cover all of the areas that break when using multisite with a port.
Trac ticket: https://core.trac.wordpress.org/ticket/21077
This ticket was mentioned in PR #5675 on WordPress/wordpress-develop by @obliviousharmony.
15 months ago
#34
This _should_ cover all of the areas that break when using multisite with a port.
Trac ticket: https://core.trac.wordpress.org/ticket/21077
#35
@
15 months ago
I ran into this problem adding multisite to our development environment and took a pass at fixing it. I looked through all of the multisite code to find any instances where the presence of a port would cause problems and fixed them. Let's get this across the finish line!
This ticket was mentioned in Slack in #core by obliviousharmony. View the logs.
13 months ago
@obliviousharmony commented on PR #5675:
13 months ago
#38
Thanks @JJJ, any suggestions on moving this forward? I didn't get a reply to anyone in #core on Slack and I can't seem to get any replies on the Trac ticket.
This ticket was mentioned in Slack in #core by obliviousharmony. View the logs.
12 months ago
12 months ago
#40
👋 @ObliviousHarmony thanks for working on this!
Just wanted to acknowledge that I have seen this, but I can't promise that I'll have the capacity to take a look for the next few weeks. I know it's frustrating, but it's not uncommon for it to take months or years for a given change to get the needed feedback and testing. Especially since a few of the contributors previously active on the ticket have no sponsored contribution time at the moment.
One thing I recommend for clarity's sake. If you could go through the existing concerns and just briefly document your thinking around how you solved them, that would help someone dive in looking at the ticket history.
This ticket was mentioned in Slack in #core by joemcgill. View the logs.
12 months ago
#42
@
10 months ago
- Keywords early added
- Milestone changed from Future Release to 6.6
- Owner set to johnbillion
- Status changed from reopened to reviewing
@johnbillion commented on PR #5675:
10 months ago
#43
@ObliviousHarmony I pushed a couple of changes to get the PHPUnit tests working on this PR. There's one failing job which is the newly introduced job that tests a single site installation with a domain name that includes a port number. The failure is only caused by the wp-api-generated.js
fixture getting updated. Not sure on the best fix for that.
@obliviousharmony commented on PR #5675:
10 months ago
#44
Thanks @johnbillion! Yeah, that's definitely tricky. I wouldn't want to do anything as broad as removing the port before writing the file but I don't see any real alternative. Modifying the test fixture this generates feels like a bad idea anyway. How would you feel about expanding the action's step to allow for certain differences in certain cases? The matrix is pretty extensive and I don't know that an exception in this one case would be that bad.
I added this test because (as this PR shows) there are a _ton_ of tests that break with a port that needed updating. It seems wise to have a test that uses a port to make sure everything works correctly. In the interest of moving this forward, however, it might make sense to pull that test out. The tests pass and so I think that's a good indicator for this PR at least. The problem
#45
@
9 months ago
I am going to commit PR 5675 to introduce support for port numbers to Multisite. Before I do, some notes:
- A concern in the discussion above is where in the database to store the port number, and the changes that would be required throughout the core codebase if it was stored in the
wp_blogs.domain
field. If you take a look at PR 5675 you'll see that actually there are very few places in core that need to be changed to support port numbers on Multisite, and most of the changes are small adjustments. Certainly nothing fundamental. Storing the port number elsewhere, for example in site meta, wouldn't negate the need for these changes and in fact would add additional complexity. That's why I support including the port number inwp_blogs.domain
. - The test suite has been updated to allow all the tests to run on a site with a port in its URL (via the
LOCAL_WP_TESTS_DOMAIN
environment variable) both on Multisite and single-site. A few bugs with port handling were found and fixed as a result. Don't forget to always use theWP_TESTS_DOMAIN
constant in tests instead of a hardcodedexample.org
. - This fixes #52088 too which allows the local development environment to run a Multisite installation. Set the
LOCAL_MULTISITE
environment variable totrue
during provisioning to enable this.
@obliviousharmony commented on PR #5675:
9 months ago
#46
Thanks for the review and helping get this across the finish line @johnbillion, I've been pretty swamped this week :smile:
- https://github.com/WordPress/wordpress-develop/commit/0665ca4a7b66536c2d60c61bd43ac53b5d5076ca: Checking for
example.org
like this seems like a hack at first but given the purpose of this test's output I don't think there's a problem. - https://github.com/WordPress/wordpress-develop/commit/49365cb8356e067a9807fd45187e1225982bed77: Nice catch!
It'll be nice to get this across the finish line. I'm planning to use this to run E2E test shards in WooCommerce without needing to spin up a bunch of identical environments. Faster tests both locally and in CI :smile:
@johnbillion commented on PR #5675:
9 months ago
#49
#50
@
9 months ago
@johnbillion Thanks for getting this committed.
I reset my local docker env and reinstalled the database. I setup the define( 'WP_ALLOW_MULTISITE', true );
in wp-config.php. I installed multisite in the CMS with the network setup screen. After multisite was setup, I was kicked out wp-admin. Since then I have been unable to login into the admin with my username and password.
Am I missing something or did something wrong?
#51
@
9 months ago
After clearing cookies I am seeing this message
Error: Cookies are blocked or not supported by your browser. You must enable cookies to use WordPress.
Checking the database, the site and blog tables are as expected.
This ticket was mentioned in Slack in #core by spacedmonkey. View the logs.
9 months ago
#53
@
9 months ago
- Keywords has-patch has-unit-tests removed
- Resolution fixed deleted
- Status changed from closed to reopened
Reopening to investigate.
Is this on a site with or without a port number in its address?
#54
@
9 months ago
I have discovered, I am able to look login when I set the cookie domain like this.
define( 'COOKIE_DOMAIN', '.localhost' );
The value of the cookie domain in my local without setting it, was .localhost:8889
.
I believe having the port number in here is an issue.
#55
@
9 months ago
Here is quick fix.
private function _set_cookie_domain() {
if ( ! empty( $this->cookie_domain ) ) {
return;
}
$this->cookie_domain = parse_url( $this->domain, PHP_URL_HOST );
if ( str_starts_with( $this->cookie_domain, 'www.' ) ) {
$this->cookie_domain = substr( $this->cookie_domain, 4 );
}
}
In the WP_Network class, when setting cookie domain, use parse_url
to get the domain and strip the port number.
#56
@
9 months ago
- Keywords needs-testing added; early removed
I've been testing this change and I haven't run into any problems. I've tested:
- Installing single site manually then using Tools -> Network Setup to install Multisite
- Installing single site automatically via
npm run env:install
then using Tools -> Network Setup to install Multisite - Installing Multisite directly by setting
LOCAL_MULTISITE
to true in.env
and runningnpm run env:install
I've tested with Chrome and Firefox and I can log in successfully in each case.
@spacedmonkey Can you confirm the exact steps you take to reset and install the environment?
If anyone else can help test this it would be appreciated.
#57
@
9 months ago
@johnbillion No, I didn't install multisite using LOCAL_MULTISITE
.
I installed a version of WordPress as single site. When added
/* Multisite */
define( 'WP_ALLOW_MULTISITE', true );
To wp-config.php. I setup using network setup screen - Sub-domains.
Sub-domains like site1.localhost:8889 and site2.localhost:8889
Using these settings
define( 'MULTISITE', true );
define( 'SUBDOMAIN_INSTALL', true );
define( 'DOMAIN_CURRENT_SITE', 'localhost:8889' );
define( 'PATH_CURRENT_SITE', '/' );
define( 'SITE_ID_CURRENT_SITE', 1 );
define( 'BLOG_ID_CURRENT_SITE', 1 );
After following these setups, I managed to replicate the issue.
In my testing the code fix https://core.trac.wordpress.org/ticket/21077#comment:55 made login work.
#58
@
9 months ago
According the RFC for cookies rfc6265
Cookies do not provide isolation by port.
So the cookie domain should be localhost, not localhost:8889.
It does raise the question, if you can multisite with a number of sites like this.
localhost:8889,
localhost:8888,
localhost:8887
Would WordPress considered you logged in, if logged into one, as the domain is the same for all of them?
#59
follow-up:
↓ 60
@
9 months ago
I'm pretty sure that localhost
doesn't support subdomains. Needs some more investigation.
#60
in reply to:
↑ 59
@
9 months ago
Replying to johnbillion:
I'm pretty sure that
localhost
doesn't support subdomains. Needs some more investigation.
Setting
define( 'SUBDOMAIN_INSTALL', false );
on my existing multisite, I am still unable to login.
#61
@
9 months ago
- Keywords needs-patch added; needs-testing removed
Have you done a fresh setup of a subdirectory installation?
It looks like the allow_subdomain_install()
function also needs to be updated to handle ports, in order to prevent setting up a subdomain installation on localhost:8889
. It prevents a subdomain installation on subdirectories, localhost, and IP addresses, but it doesn't take into account the port.
#62
@
9 months ago
@obliviousharmony Do you fancy taking a look at the above? The logic around $domain
in allow_subdomain_install()
needs to be updated to account for a port in addition to its current handling of a path and IP address.
#63
@
9 months ago
@johnbillion @obliviousharmony Worth noting that login is also broken in subdirectory networks as well. Just tested it and seeing the same issue.
This ticket was mentioned in PR #6521 on WordPress/wordpress-develop by @spacedmonkey.
9 months ago
#64
- Keywords has-patch added; needs-patch removed
Use parse_url
and PHP_URL_HOST
to ensure that domain is a 'localhost' and the port is stripped.
Trac ticket: https://core.trac.wordpress.org/ticket/21077
#65
@
9 months ago
- Keywords needs-patch added; has-patch removed
I have put together a simple fix in PR #6521.
This fixes the issues in my local dev.
This ticket was mentioned in Slack in #core-multisite by spacedmonkey. View the logs.
9 months ago
#68
@
9 months ago
@johnbillion Thanks for testing and committing.
I have updated the PR with fix for the cookie domain issue. The issue came about because in some case parse_url can return null or false. So just check to see the return value before assigning it to cookie_domain property.
Multisite custom port patch