Make WordPress Core

Opened 7 years ago

Last modified 3 years ago

#42080 new defect (bug)

get_site_url() does not always return the appropriate protocol

Reported by: ravanh's profile RavanH Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 4.9
Component: Administration Keywords: needs-testing
Focuses: administration, multisite Cc:

Description

Consider this case: a Multisite where the main site is running on https with a Let's Encrypt certificate while a sub-site is set to use http because, for example, it is a sub-domain install and the LE certificate does not support the subdomain wildcard yet or the sub-site is using another domain (mapped without plugin) not included in the certificate.

The FORCE_ADMIN_SSL is not set because that would cause the browser to panic and throw an "insecure" warning when trying to access the sub-site admin.

Both sites work well on both front and admin.

However, there is a problem that is visible in three places:

  1. on /wp-admin/my-sites.php on the main site (on https)
  2. on /wp-admin/network/sites.php
  3. in the "My sites" list in the Admin Bar on the main site (on https)

On these locations, while the (non-ssl) sub-site's Visit link URL scheme is showing the correct protocol, the admin Dashboard link URL scheme is forced to https. This will make a sub-site owner that follows this link, to get caught in the browser panic about insecure connection, which makes him/her then go running to the network admin in fear of his/her site being hacked or being unable to connect at all...

Not a very good promo for WordPress even if there is no real problem.

Now before people start saying things like "every site should be on https", there are reasons for site owners to stick with (or even prefer) http. Let's not go into that discussion.

The cause of this URL scheme mishap, is the is_ssl() in set_url_scheme() that returns true when on the main site (being on https) in spite of the fact that the URL in question might be from a site that is not on https.

To fix this, I see several ways to go. Not sure which is better though...

A. Adapt function get_site_url() by moving the set_url_scheme() up into the first if statement, effectively disabling scheme treatment when switch_to_blog() is used. This is a simple fix but seems too crude and does not address the admin bar links.

B. Adapt function set_url_scheme() either (1) so that it does not use is_ssl() anymore when $scheme is 'admin', and use parse_url( $url, PHP_URL_SCHEME ) when force_ssl_admin is false (so not forcing it to 'http' but just keep the scheme that is set in the site options) or (2) so that it can accept another $scheme value like 'network-admin' for example, dedicated for these cases, where is_ssl() is not used.

C. Adapt my-sites.php, network.sites.php and admin-bar.php to approach the retrieval of these URLs differently, not using get_admin_url() and get_home_url() at all.

Any better ideas?

Change History (3)

#1 @RavanH
7 years ago

  • Component changed from General to Administration

This ticket was mentioned in Slack in #core by peterwilsoncc. View the logs.


3 years ago

#3 @costdev
3 years ago

  • Keywords needs-testing added

This ticket was discussed in a triage session.

After looking through /wp-admin/my-sites.php, /wp-admin/network/sites.php and /wp-includes/admin-bar.php, it appears that there are a total of three calls to get_admin_url() and get_home_url() - all located within /wp-includes/admin-bar.php.

Some calls may have been removed since this ticket was created. As we were unable to set up testing environments during the triage session, we are moving this forward with a needs-testing keyword so that testers can check if the issue still exists.

Note: See TracTickets for help on using tickets.