#18005 closed defect (bug) (fixed)
mixed http/https installation and add_custom_background / body_class
Reported by: | rfc1437 | Owned by: | ryan |
---|---|---|---|
Milestone: | 3.5 | Priority: | normal |
Severity: | minor | Version: | 3.2 |
Component: | Themes | Keywords: | 2nd-opinion has-patch needs-testing |
Focuses: | Cc: |
Description
The styling added to the body-tag that references the custom background image doesn't respect https/http protocol usage - it is referenced over http even when the site is accessed over https and so creates security complaints by the browser.
A case where you can see it: https://hugo.rfc1437.de/ will show it. This has been the case with 3.1, too (and probably longer, as it is referenced in this support forum post: http://wordpress.org/support/topic/twenty-ten-header-image-and-https
FORCE_SSL_ADMIN and FORCE_SSL_LOGIN are both set.
Attachments (3)
Change History (20)
#1
@
13 years ago
- Cc joachim.kudish@… added
- Keywords 2nd-opinion added
- Resolution set to invalid
- Severity changed from normal to trivial
- Status changed from new to closed
- Version 3.2 deleted
#2
@
13 years ago
To clarify: in all other situations wordpress properly honors http/https usage - links are kept local and items are referenced with the proper protocol. The custom background is the only part so far where wordpress doesn't honor it. I know that I would have to switch the site url to enable forced SSL all over the place but this is about mixed http/https installations. That's why I wrote it in the title of the ticket.
That my SSL certificate is not valid is correct observation but irrelevant to the problem.
#3
@
13 years ago
- Resolution invalid deleted
- Status changed from closed to reopened
Yup I agree that it's an irrelevant problem :)
I decided to look into a little further and noticed that the header_image() function does check for ssl and should be returning the url with the proper protocol. not sure why it isn't in your case, perhaps something to do with your particular installation, perhaps not.
http://core.trac.wordpress.org/browser/trunk/wp-includes/theme.php#L1430
#4
@
13 years ago
@rfc1437 does the problem persist if you revert to twenty ten as the theme rather than using your child theme?
#5
@
13 years ago
Well, I did some more digging and the problem is in the theme_mod background_image handling - when setting a custom background image in twentyten (or actually any other theme that supports custom backgrounds, since that part of the admin interface comes from WP core), that custom background image is stored in the theme_mods_twentyten option (or in my case in theme_mods_rfc1437) value as an URI and that URI directly includes the protocol. That value is put into the body styling directly without any change. On option storage the value of course uses the site URI to construct the background image URI and so stores it with http: (and the fully qualified URI for images in url() CSS specs is a requirement).
header_image does show the same value structure, but it is handled by get_header_image that actually contains code that checks is_ssl and replaces http:// with https:// (and vice versa). get_background_image is missing that code to handle is_ssl situations. I changed get_background_image to the following code in wp-includes/theme.php and the problem was fixed:
function get_background_image() { $default = defined('BACKGROUND_IMAGE') ? BACKGROUND_IMAGE : ''; $url = get_theme_mod('background_image', $default); if ( is_ssl() ) $url = str_replace( 'http://', 'https://', $url ); else $url = str_replace( 'https://', 'http://', $url ); return $url; }
my theme_mods_rfc1437 option value:
a:8:{s:12:"header_image";s:80:"http://hugo.rfc1437.de/wp-content/themes/rfc1437/images/headers/enigma-dark.jpeg";s:16:"background_color";s:3:"fff";s:16:"background_image";s:68:"http://hugo.rfc1437.de/wp-content/uploads/2011/02/steam-invers5.jpeg";s:22:"background_image_thumb";s:75:"http://hugo.rfc1437.de/wp-content/uploads/2011/02/steam-invers5-150x150.jpg";s:17:"background_repeat";s:9:"no-repeat";s:21:"background_position_x";s:4:"left";s:21:"background_attachment";s:6:"scroll";s:18:"nav_menu_locations";a:1:{s:7:"primary";i:78;}}
#6
@
13 years ago
- Component changed from Bundled Theme to Themes
- Keywords has-patch added
- Resolution set to fixed
- Severity changed from trivial to minor
- Status changed from reopened to closed
- Version set to 3.2
Well, sorry for having been so harsh at the beginning :)
I've uploaded a patch with your changes so that the core devs can apply it.
This made me think of a suggestion for a new WP function:
wp_ssl_url($url) or something like that, which would automatically do:
if ( is_ssl() ) $url = str_replace( 'http://', 'https://', $url ); else $url = str_replace( 'https://', 'http://', $url ); return $url;
#7
follow-up:
↓ 8
@
13 years ago
- Resolution fixed deleted
- Status changed from closed to reopened
Please don't close tickets as fixed until the patch is committed in trunk.
#8
in reply to:
↑ 7
@
13 years ago
Replying to kawauso:
Please don't close tickets as fixed until the patch is committed in trunk.
fair enough, still new-ish to WordPress' Trac best practices. Learning and patching as I go :)
#10
@
13 years ago
- Keywords needs-testing added
Rewrote the function to use set_url_scheme(), Related: #18017
#14
@
12 years ago
Related: #19037
See wp-hacker's thread titled: "Fixing some SSL cases for 3.4" to follow the evolving discussion.
This is not a bug.
FORCE_SSL_ADMIN and FORCE_SSL_LOGIN are for the admin only. If you want the front of your site to be SSL enabled, then you will need to change your site's url under settings > general to https://
Unrelated but your SSL certificate isn't valid either.