WordPress.org

Make WordPress Core

Opened 10 years ago

Closed 10 years ago

Last modified 10 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:

Description

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 10 years ago.
10080.diff (1.6 KB) - added by Denis-de-Bernardy 10 years ago.
10080.2.patch (1.6 KB) - added by peaceablewhale 10 years ago.
10080.3.patch (1.6 KB) - added by peaceablewhale 10 years ago.

Download all attachments as: .zip

Change History (20)

#1 @peaceablewhale
10 years ago

  • Keywords tested added

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

#2 @Denis-de-Bernardy
10 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.

#3 @Denis-de-Bernardy
10 years ago

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

#4 @peaceablewhale
10 years ago

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

#5 @Denis-de-Bernardy
10 years ago

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

#6 @Denis-de-Bernardy
10 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.

#7 @peaceablewhale
10 years ago

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

#8 @Denis-de-Bernardy
10 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.

#9 @peaceablewhale
10 years ago

  • Keywords has-patch added; needs-patch removed

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

#10 follow-up: @peaceablewhale
10 years ago

  • Keywords tested added

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

#11 @Denis-de-Bernardy
10 years ago

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

#12 in reply to: ↑ 10 @azaozz
10 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.

#13 @peaceablewhale
10 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.

#14 @peaceablewhale
10 years ago

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

#15 @azaozz
10 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.

#16 @peaceablewhale
10 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.