#20759 closed enhancement (fixed)
Implement set_url_scheme() where appropriate
Reported by: | johnbillion | Owned by: | ryan |
---|---|---|---|
Milestone: | 3.5 | Priority: | normal |
Severity: | normal | Version: | 3.4 |
Component: | General | Keywords: | has-patch needs-testing |
Focuses: | Cc: |
Description
#18017 introduced set_url_scheme()
. We should now use this function in place of all the places where we do scheme logic.
Patch coming up.
Attachments (6)
Change History (23)
#11
@
12 years ago
- Owner set to ryan
- Resolution set to fixed
- Status changed from new to closed
In [21734]:
#12
@
12 years ago
- Resolution fixed deleted
- Status changed from closed to reopened
r21664 causes a fatal error on a fresh install.
Just checked out revision 21768, config'd, and went to the home page expecting to be automatically redirected to the wp-admin/install.php but was presented with
Fatal error: Call to undefined function set_url_scheme() in /path/to/trunk/wp-includes/functions.php on line 2987
If I bypass and go directly to wp-admin/install.php, setup the site details, and return to the homepage, no error.
#13
@
12 years ago
20759.2.diff fixes the fatal error in wp_guess_url()
(with a comment).
#15
follow-up:
↓ 16
@
12 years ago
- Cc mindctrl added
- Resolution fixed deleted
- Status changed from closed to reopened
- Type changed from enhancement to defect (bug)
Something in this set of patches broke the Visit Site links in the admin bar when the site is SSL only. It strips out the https and instead of linking https://domain.com it only links domain.com (no http* - only relative). I've tested this on three separate installs and found this to be true.
My test sites are all full-time SSL, with no option to access the sites via non-SSL means which results in the following error:
400 Bad Request
Your browser sent a request that this server could not understand.
Reason: You're speaking plain HTTP to an SSL-enabled server port.
Instead use the HTTPS scheme to access this URL, please.
Shouldn't this not be stripping off the protocol at the front of the link?
#16
in reply to:
↑ 15
;
follow-up:
↓ 17
@
12 years ago
- Type changed from defect (bug) to enhancement
The ticket itself is still an enhancement.
Replying to mindctrl:
It strips out the https and instead of linking https://domain.com it only links domain.com (no http* - only relative).
I could not reproduce stripping off the protocol, it's 'http'
on my install. Before [21734], however, it used to be 'https'
in this case.
In [21734], scheme calculation was removed from get_site_url()
, but not from get_home_url()
, which is used for "Visit Site" link in the toolbar.
20759.3.diff brings consistency and fixes the issue with "Visit Site" link in my testing.
#17
in reply to:
↑ 16
@
12 years ago
Replying to SergeyBiryukov:
The ticket itself is still an enhancement.
Sorry about that. Wasn't thinking when I changed that. Not enough sleep!
I could not reproduce stripping off the protocol, it's
'http'
on my install. Before [21734], however, it used to be'https'
in this case.
20759.3.diff brings consistency and fixes the issue with "Visit Site" link in my testing.
You're right. It wasn't stripping the protocol. My browser was just hiding it and I didn't inspect the code.
This patch fixes the problem for me. Thanks!
#18
@
12 years ago
20759.3.diff breaks Tests_URL::test_home_url_from_admin() and Tests_URL::test_network_home_url_from_admin().
#19
@
12 years ago
The Visit Site link is 'http' in 3.4 as well. We don't currently support forcing https for frontend. AFAICT the code currently in trunk behaves the same as 3.4.
#20
@
12 years ago
To get an https home link in 3.4 you would need to have https in the home option. Support for https in home is hit or miss. It looks like a fluke that get_home_url() in 3.4 allowed the raw home option through in this situation.
#21
@
12 years ago
We *could* implicitly support front SSL if https is the protocol used for the home option. But that seems a bit tenuous.
#24
@
12 years ago
Yes, on my sites I have the WordPress Address (URL) and Site Address (URL) configured with https and it has worked fine in 3.4.x.
I just updated to the latest trunk containing this latest patch and it works great.
@nacin, I'm not sure why WP wouldn't support SSL if https is in the home option.
Patch which replaces instances in core where we're doing things like
str_replace( 'http://', 'https://, $url )
withset_url_scheme()
.There are several other places in core where we're building the URL out based on the
$_SERVER
superglobal andis_ssl()
, but I've left these as-is and only replaced instances where we're actually replacing the scheme in a URL with another.