get_rest_url() not returning SSL version of the URL when the home_url it's a subdomain

[nicholas_io]

The code below doesn't set the https scheme for some cases, I have a client site using a subdomain (it's a multisite installation) with SSL and the ajax requests to all REST route is failing due to the fact that get_rest_url is not returning the URL with ​https://. Debugging this showed to me that $_SERVER["SERVER_NAME"] is not returning the full site address (e.g if sub.domain.com it returns only domain.com) and thus the if clause fails and https is not set. I believe we can't really trust on SERVER_NAME.

I'm not sure however if we really need this additional if check since we're already checking for is_ssl().

rest-api.php L226

if ( is_ssl() ) {
   if ( $_SERVER['SERVER_NAME'] === parse_url( get_home_url( $blog_id ),PHP_URL_HOST ) ) {
      $url = set_url_scheme( $url, 'https' );
   }
}

[rachelbaker]

Looks like the quoted block of code was introduced in r35351. See #34299

[ChopinBach]

Hi Everyone,

I spent some time testing this and I am fairly certain there is not an issue with WordPress around this. Instead there is most likely some sort of server config error going on.

I tested a subdomain multisite install on both nginx and apache using wildcard and single domain certs; everything worked as expected. I tried various other things to break it and it broke as expected and worked when I expected it to. SSL support for multisite and the API seems good.

(Note: this was tested against what currently is in trunk; 4.7 Beta 1.)

[jnylen0]

I don't see anything we should do here either - the mentioned block of code is a special case that only turns SSL *on* under fairly unique circumstances.

The real issue in the report is probably that get_home_url is not returning an https: URL; once that is fixed, the REST will fall into place.

If there were any special situations where that wouldn't work, then there's a rest_url filter that could be used instead.

[igmoweb]

I'm going to reopen this as is causing problems to me too and I see lots of situations where the function won't work properly.

The problem is in the line where the host is compared:

 if ( $_SERVER['SERVER_NAME'] === parse_url( get_home_url( $blog_id ),PHP_URL_HOST ) ) {

Please, read $_SERVER variables names in PHP manual: ​http://php.net/manual/en/reserved.variables.server.php

SERVER_NAME: The name of the server host under which the current script is executing. If the script is running on a virtual host, this will be the value defined for that virtual host.

HTTP_HOST: Contents of the Host: header from the current request, if there is one.

So, let's say we have a subdomain installation in a multisite with the main URL example.com and the user is trying to make a request to mysite.example.org. The $_SERVER variable values would be:

SERVER_NAME: example.com
HTTP_HOST: mysite.example.com

But get_rest_url() is actually comparing to example.com so the line above will never be true. The line should be actually

if ( $_SERVER['HTTP_HOST'] === parse_url( get_home_url( $blog_id ),PHP_URL_HOST ) ) {

Now, how to reproduce? You'll need:

• A multisite with subdomains
• FORCE_ADMIN_SSL set to true
• Do not force SSL in front
• Create a new site in the multisite
• Add the following code into a plugin or theme:

<?php
add_action( 'admin_init', function() {
        var_dump( get_rest_url() );
        wp_die();
});

And go to the subdomain/wp-admin

You should see that the REST URL has no https prefixed but http.

This is really bad when you try to make an AJAX call to any REST endpoint from admin.

You can test an example by installing Jetpack 4.4.2 (is the current last one) and go to the Jetpack connection page. You'll see that the button connection link is wrong and if you open the JS console... Tada!

Please, reconsider this ticket.

[mohanjith]

Attached patch will address cases where the Host header is not set (e.g. using IP) and when ServerName is not set, wrong or different.

[nicholas_io]

It's been a while that I debug this but it looks like get_rest_url calls get_home_url passing 'rest' scheme (I'm not sure what that means exactly) but that caused the url to be returned without https and the quoted block failed to set the https scheme due to @igmoweb explanation.

BTW, this is how I fixed this issue: forcing https using the rest_url filter

<?php
        add_filter( 'rest_url', function( $url ) {
                if ( is_ssl() ) {
                        $url = set_url_scheme( $url, 'https' );
                        return $url;
                }

                return $url;
        });

[rmccue]

Worth mentioning that the Host header is user input, so can't necessarily be trusted, but I don't think that generally applies in this case.

@johnbillion You wrote the original code for this in #34299, can you take a look at this ticket?

[joehoyle]

Pinging @johnbillion to see if he has any input to add here.

[siboulet]

Also running into this issue. @nicholas_io filter aboves works for me. Basically my Wordpress site has a CDN in front that does the SSL offload and talks to my backend Wordpress server over plain HTTP. Even though all links are properly returned as ​https:// from Wordpress, the link from rest_url() were returned non-https.

[jnylen0]

Is there any reason to ever send an http: URL for the REST API if is_ssl() is true? This is guaranteed to be broken due to mixed content restrictions, at least in a browser context.

See also #40771.

[johnbillion]

#40771 was marked as a duplicate.

[jnylen0]

Replying to jnylen0:

Is there any reason to ever send an http: URL for the REST API if is_ssl() is true?

Or perhaps more accurate/useful: if force_ssl_admin() is true, then any admin usage of the REST API should also be over SSL.

[westonruter]

I think it could be as easy as 36451.2.diff​:

src/wp-includes/rest-api.php

@@ -330 +330 @@
                 $url = add_query_arg( 'rest_route', $path, $url );
         }
 
-        if ( is_ssl() ) {
+        if ( is_ssl() || ( is_admin() && force_ssl_admin() ) ) {
                 // If the current host is the same as the REST URL host, force the REST URL scheme to HTTPS.
                 if ( $_SERVER['SERVER_NAME'] === parse_url( get_home_url( $blog_id ), PHP_URL_HOST ) ) {
                         $url = set_url_scheme( $url, 'https' );

This is similar to what set_url_scheme() is ​doing which is what is used to obtain URLs in the admin in the first place.

On the other hand, what if in the admin we opt to use get_site_url( $site_id, $prefix, 'admin' ) instead of get_home_url( … )? Then we'd know for sure that the domain would be the same and so HTTPS would be assured to work.

[jnylen0]

36451.2.diff​ is a lot like what I had in mind, though I think we should skip the hostname check for this case, as even if the hostnames are different, we still need to force the API to SSL.

Replying to westonruter:

On the other hand, what if in the admin we opt to use get_site_url( $site_id, $prefix, 'admin' ) instead of get_home_url( … )? Then we'd know for sure that the domain would be the same and so HTTPS would be assured to work.

I am hesitant to make this larger change, maybe it should have been that way from the beginning but it seems like we shouldn't assume that these domains are the same.

[westonruter]

Remove hostname check

[jnylen0]

@igmoweb @nicholas_io does 36451.3.diff​ fix the issue for you?

@johnbillion we definitely need to remove the hostname check for the is_admin() case. For the broader is_ssl() case it seems reasonable to me to remove it also. What do you think?

[johnbillion]

The original discussion linked in #34299 has some importance: ​https://github.com/WP-API/WP-API/issues/259

The REST API endpoint URL is not blindly forced to HTTPS if the current request is HTTPS because the domain name can differ and not be available over HTTPS, therefore breaking the endpoint.

That said, the current situation results in the REST API endpoint URL having an HTTP scheme when you're on an HTTPS URL, which means it's most likely broken due to cross-protocol restrictions in browsers anyway.

I think we could go ahead and force the REST API endpoint URL to HTTPS according to 36451.3.diff​, but I don't want to do that during beta 2. I think this needs to be punted to 4.9 early.

---

@nicholas_io @igmoweb @siboulet For your specific cases, the ​rest_url filter can be used to fix the URL.

[nicholas_io]

@jnylen0 yes the patch fixes the issue I was having and with the patch applied I don't need to force HTTPS with rest_url filter.

---

@johnbillion Yes I know, I provided the code to force that as reference in a previous comment. Thanks!

[jnylen0]

Replying to johnbillion:

The REST API endpoint URL is not blindly forced to HTTPS if the current request is HTTPS because the domain name can differ and not be available over HTTPS, therefore breaking the endpoint.

I now think this is less common than the current situation described in this ticket...

That said, the current situation results in the REST API endpoint URL having an HTTP scheme when you're on an HTTPS URL, which means it's most likely broken due to cross-protocol restrictions in browsers anyway.

... (this one).

It seems pretty likely to me that if a request that serves a rest_url is_ssl(), then everything is going to be SSL. If not, then the rest_url filter can be used as above.

I think we could go ahead and force the REST API endpoint URL to HTTPS according to 36451.3.diff​, but I don't want to do that during beta 2. I think this needs to be punted to 4.9 early.

IMO a pretty solid argument for getting this into 4.8 is that we now have usage of the REST API in the admin context (the new oembed proxy endpoint in the media modal), and we know details of cases where the current code is broken and 36451.3.diff​ fixes it.

We could also do a hybrid approach where we preserve the hostname check for the previous code, but remove it if is_admin() && force_ssl_admin(). I can get this done today, but let's discuss in Slack first.

[johnbillion]

We should specifically request that users who are running an HTTP site behind an HTTPS proxy (for example, Cloudflare Flexible SSL) test any changes we make here.

[johnbillion]

See #29708 for background behind the above comment.

[jnylen0]

Replying to jnylen0:

We could also do a hybrid approach where we preserve the hostname check for the previous code, but remove it if is_admin() && force_ssl_admin().

36451.4.diff​ does exactly this.

[johnbillion]

In 40843:

REST API: In the admin area, ensure the REST API endpoint URL is forced to https when necessary.

In this situation, a site which uses http on the front end but https in the admin area is more likely to have a working REST API endpoint URL when used in the admin area.

Props mohanjith, westonruter, jnylen0

Fixes #36451