#9008 closed task (blessed) (fixed)
add a home_url() function to enforce http/https scheme
Reported by: | Denis-de-Bernardy | Owned by: | |
---|---|---|---|
Milestone: | 3.0 | Priority: | high |
Severity: | major | Version: | 2.7 |
Component: | Template | Keywords: | has-patch |
Focuses: | Cc: |
Description
bloginfo() seems to ignore the site's requested scheme for things like feed url, pingback url, edit uri, wlwmanifest.
Attachments (9)
Change History (54)
#2
@
16 years ago
Because your site url is set to http://yoursite.com in your options..?
#3
@
16 years ago
indeed, but it's still inconsistent:
If you look at the source, you'll notice that the permalinks to posts go to the https version, and that most of the includes (scripts, css) use https. (Those that don't are from my own plugins, which I'm in the process of fixing to use site_url().)
I might be getting this wrong, but it seems to me that on the https version of the site, some kind of site_home() function should be used in a manner that is identical to the site_url() function for the bloginfo links and like.
#4
@
16 years ago
Ah.. I see what you mean now.
I generally use site_url('')
or site_url('/')
over bloginfo now..
#5
@
16 years ago
- Keywords dev-feedback added
- Summary changed from bloginfo() ignores scheme to inconsistent http/https scheme
Right. The complicated one to change is get_feed_url(), because there is not equivalent (best I know anyway) to site_url() for the home url.
A separate candidate "fix" would be to enforce http for sites that are declared to use it. In other words, in site_url():
$url = str_replace( 'http://', "{$scheme}://", get_option('siteurl') );
Should really be something like:
$url = str_replace( array('http://', 'https://'), "{$scheme}://", get_option('siteurl') );
And then, when a url is browsed as https:// where http:// should be used, it would redirect to the http:// versions of that url.
#10
@
16 years ago
- Keywords needs-patch early added; dev-feedback removed
- Milestone changed from 2.8 to 2.9
- Priority changed from normal to high
- Summary changed from inconsistent http/https scheme to add a home_url() function to enforce http/https scheme
#11
@
15 years ago
- Keywords has-patch added; needs-patch removed
attached patch sticks to defining the function, enforcing the ssl setting of the page if relevant (no define forces the SSL), and replaces occurrences of get_option(home) with the new function.
#12
@
15 years ago
I suggest we worry about introducing a FORCE_SSL define once this gets checked in.
#13
@
15 years ago
adding to this, the links in wp-admin are kept verbatim, in case ADMIN_SSL is turned on.
#18
@
15 years ago
- Type changed from defect (bug) to enhancement
Marking this as an enhancement as we are talking about adding functionality by adding a home_url() function rather than fixing a bug.
#19
@
15 years ago
Refreshed patch. I changed it so if you're in the admin and is_ssl() the links are not forced to be https since most people wouldn't want that. We can add defines for those who want that.
#20
@
15 years ago
Reviewed, updated ryans patch a bit to follow code guidelines better for the new function.
#22
@
15 years ago
- Keywords 2nd-opinion added; has-patch needs-review early removed
I suggest we leave this one open pending extra discussion and maybe an extra patch. I had originally opened this one because I was testing the following two use-cases:
- browsing https when it really should be http
- browsing http when it really should be https
2nd opinion welcome on how we ought to better manage those two use cases. Maybe we should 301 redirect. Maybe we should introduce a FORCE_SSL that forces the SSL pref on the front end. All options are available.
#24
follow-up:
↓ 25
@
15 years ago
We need to go through and update bloginfo('url') references to home_url(), no? (29 instances in core.)
#25
in reply to:
↑ 24
@
15 years ago
Replying to nacin:
We need to go through and update bloginfo('url') references to home_url(), no? (29 instances in core.)
Unless I'm missing something -- I think we should do that, then mark bloginfo('url') as deprecated (see #11652) and change bloginfo() to use home_url() instead of get_option('home').
I wonder if we can also mark get_option('home') as deprecated in some way as well. (We would have to prevent the deprecated warning inside home_url()).
#26
follow-up:
↓ 27
@
15 years ago
for backwards compat, I think we should stick to making get_bloginfo('url') return home_url(). I'm sure a lot of themes and plugins use that function.
#27
in reply to:
↑ 26
@
15 years ago
Replying to Denis-de-Bernardy:
for backwards compat, I think we should stick to making get_bloginfo('url') return home_url(). I'm sure a lot of themes and plugins use that function.
Right but at the same time we should suggest that they switch over from get_bloginfo('url') as well. home_url() is much cleaner.
#29
@
15 years ago
Not perfect, but I have attached a quick patch to deprecate get_option('home') and *bloginfo*('url'). We'd still need to replace all bloginfo('url') references in core -- there's 41 instances when you include *bloginfo_rss().
There are ways we can do this to avoid the backtrace, but it's only running when WP_DEBUG is on so it isn't a big footprint. (This also means that it won't trigger the deprecated_argument_run hook, which would normally be triggered even if WP_DEBUG is off.)
And I guess instead of did_action('init') we could just replace the get_option('home') instance in wp-settings.php with home_url(). Ideas welcome.
#30
@
15 years ago
@nacin: is the additional logic in get_option() really needed? that function is called so many times all over the place that we ought to keep it as simple as possible.
#31
@
15 years ago
We can't put that logic into get_option().
We could put something on a pre_option filter hook to do the check there so it only runs for the home option.
I wonder if this is one place where putting the message in the face of the developer is a step too far due to the cost outweighing the benifit and we should just hilight it in the phpDoc.
#32
@
15 years ago
I wonder if this is one place where putting the message in the face of the developer is a step too far due to the cost outweighing the benifit and we should just hilight it in the phpDoc.
Thats my thoughts too.
Personally, I think we should just make it known that if a plugin wants to retrieve a correct url (http/https) that the api should be used instead of relying on the underlying options.
#33
@
15 years ago
ryan and I discussed this briefly on IRC today.
We can put logic into get_bloginfo(), for sure. (We would need to change a few dozen references.) If we are to do it for get_option(), the best idea so far I think is this:
- Add a simple do_action() to the top of the *_url() family (or maybe come up with a filter that is actually useful).
- On WP_DEBUG, attach a filter to pre_option_$option. In the callback, call _deprecated_argument(), but first check $wp_current_filter to make sure we're not coming from a *_url() function.
That avoids the backtrace and I feel the benefit would outweigh what I think would be minimal cost.
#34
@
15 years ago
We should deprecate bloginfo('url') for home_url(), and bloginfo('wpurl') for site_url(), we'll just need to cleanup the bloginfo calls first so there aren't lots of deprecated warnings in trunk.
#35
@
15 years ago
- Cc junsuijin@… added
- Keywords has-patches added
home_url() works well but I don't follow the logic on trunk wp-includes/link-template.php, li 1773
[...] && !is_admin() [...]
If I don't want to force SSL logins/admin (due to self-signed certificate or whatever other reason), but I want to use them personally for security purposes, any link that takes me back to the frontend returns me to the non-SSL scheme, which means any links from the frontend to the backend (at least those using the home_url api), don't work. This seems like a good enough reason to ensure that the current scheme is preserved on admin page links by default, and a define (as ryan suggested), could then be added to alter that behavior if desired. Basically, I think it's safe to assume that the user will want to maintain whatever scheme they've used to login with, since their cookies won't be valid for the other scheme.
Additionally, I think nacin's patch to deprecate bloginfo('url') is good and have submitted an additional patch to use home_url() appropriately on admin pages. I've also changed the offending line in link-template.php just to make it clear what I mean, and added comments where there is not yet an api function to handle the task. Thanks to nacin for his help finding the other locations that need updating :)
#36
@
15 years ago
- Keywords needs-patch added; 2nd-opinion has-patches removed
(In [13474]) get_bloginfo('wpurl') should use site_url(), not get_option('siteurl'). see #9008
Because of the filters in get_bloginfo(), get_bloginfo_rss(), and bloginfo_rss(), and their wide use, I think it would be okay to leave as-is and simply transition to home_url() and site_url() inside get_bloginfo().
We could still use a solution to route direct calls from get_option() to home_url() and site_url().
What's left: There are still calls to get_option('siteurl') in core. They need to become site_url(). Many of them then have a path then appended, those can be changed to use the first parameter of site_url().
#40
in reply to:
↑ 39
@
15 years ago
Replying to ryan:
Is that dropped parse_url() intentional?
Nope. I mentioned it to Pete, he didn't get around to refreshing the patch. It's good to go otherwise.
#42
@
15 years ago
- Keywords has-patch added; needs-patch removed
9008_script-loader.patch: Added home_url() to ignore
Prevents a fatal error:
Fatal error: Call to undefined function home_url() in \wp\wp-includes\script-loader.phpon line 399.
that being, if you browse https://yoursite.com, the relevant links will still point to http://yoursite.com