Make WordPress Core

Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#10080 closed defect (bug) (fixed)

Insecure Content Warning appears when the Manage Themes page is loaded via HTTPS

Reported by: peaceablewhale Owned by:
Milestone: 2.8 Priority: normal
Severity: major Version: 2.8
Component: Administration Keywords: has-patch tested
Focuses: Cc:


When the Manage Themes page is loaded via HTTPS, the themes' screenshots and preview links may still be HTTP, causing broswer warnings.

Attachments (4)

10080.patch (1.5 KB) - added by peaceablewhale 5 years ago.
10080.diff (1.6 KB) - added by Denis-de-Bernardy 5 years ago.
10080.2.patch (1.6 KB) - added by peaceablewhale 5 years ago.
10080.3.patch (1.6 KB) - added by peaceablewhale 5 years ago.

Download all attachments as: .zip

Change History (20)

peaceablewhale5 years ago

comment:1 peaceablewhale5 years ago

  • Keywords tested added

Tested with PHP 5.2.9-2 NTS on IIS 7.5 via FastCGI.

comment:2 Denis-de-Bernardy5 years ago

  • Keywords needs-patch added; has-patch tested removed

get_option(home) != site_url() on sites that have the WP files in a separate folder.

comment:3 Denis-de-Bernardy5 years ago

that link can probably remain non-https, btw. what really counts are the screenshots, I suspect.

comment:4 peaceablewhale5 years ago

An insecure content warning will appear when the preview is loaded via the JavaScript

Denis-de-Bernardy5 years ago

comment:5 Denis-de-Bernardy5 years ago

  • Keywords has-patch tested added; needs-patch removed

comment:6 Denis-de-Bernardy5 years ago

  • Keywords needs-patch added; has-patch tested removed

oh, ok. in this case, we'd want to str_replace() the thingy if ssl is enabled.

comment:7 peaceablewhale5 years ago

Do we need to define a "home_url" function in link-template.php?

comment:8 Denis-de-Bernardy5 years ago

  • Severity changed from normal to major

That would be ideal imo, but I'm not sure such a function would make it into 2.8 -- since it's on the 2.9 todo list.

peaceablewhale5 years ago

comment:9 peaceablewhale5 years ago

  • Keywords has-patch added; needs-patch removed

I have uploaded a new patch that uses get_option(home) with the protocal removed.

peaceablewhale5 years ago

comment:10 follow-up: peaceablewhale5 years ago

  • Keywords tested added

Updated regex to do case-insensive match. Also tested on the same platform.

comment:11 Denis-de-Bernardy5 years ago

let's hope the committers get this fixed in 2.8...

comment:12 in reply to: ↑ 10 azaozz5 years ago

Replying to peaceablewhale: This regex won't work for all browsers. Some would always prepend http when the protocol is missing so removing it won't make them add the right one. This is not the same as the baseurl that the browsers calculate internally including the right protocol.

comment:13 peaceablewhale5 years ago

I have tested that the regex works at least with IE7, IE8, Firefox 3, Opera 10 and Safari 4. The relative URI is also valid per http://tools.ietf.org/html/rfc3986#section-4.2.

comment:14 peaceablewhale5 years ago

Since the recent versions of all major browsers support the regex, I suggest checking in the patch.

comment:15 azaozz5 years ago

  • Resolution set to fixed
  • Status changed from new to closed

Fixed in [11539].

I think you see which browser is missing above :) With about 20% of the people still using IE6 don't think we should exclude it. Also there are several ways to do that.

comment:16 peaceablewhale5 years ago

Actually I didn't test my patch with IE6 simply because I don't have it :)

Thanks for checking in a better patch anyway!

Note: See TracTickets for help on using tickets.