Make WordPress Core

Opened 12 years ago

Closed 10 years ago

#20534 closed defect (bug) (duplicate)

Featured Image (Post Thumbnail) SSL Issue

Reported by: justindgivens's profile justindgivens Owned by: johnbillion's profile johnbillion
Milestone: Priority: normal
Severity: major Version: 2.9
Component: Post Thumbnails Keywords: needs-testing has-patch
Focuses: Cc:

Description

If the wp-config FORCE_SSL_ADMIN constant has been set to true then the Featured Image within the Edit Post page will still load without HTTPS. This isn't a huge deal but it breaks the SSL on the page. Patch fixes this.

Attachments (5)

20534.diff (696 bytes) - added by justindgivens 12 years ago.
Patch for 20534
20534.2.diff (896 bytes) - added by justindgivens 12 years ago.
Better fix instead of just for the post thumbnail. This fixes the Media and Media Edit pages as well.
20534.3.diff (913 bytes) - added by justindgivens 12 years ago.
Updated with the new function.
20534.4.diff (1006 bytes) - added by justindgivens 11 years ago.
Updated from last comments.
20534.5.diff (1.0 KB) - added by janpeters 10 years ago.
Patch for WordPress 3.8.1

Download all attachments as: .zip

Change History (21)

@justindgivens
12 years ago

Patch for 20534

#1 @justindgivens
12 years ago

  • Severity changed from minor to normal
  • Version changed from 3.3.1 to 3.4

Can we get this included in 3.4?

@justindgivens
12 years ago

Better fix instead of just for the post thumbnail. This fixes the Media and Media Edit pages as well.

#2 @nacin
12 years ago

  • Version changed from 3.4 to 2.9

We now have set_url_scheme() we can use here.

@justindgivens
12 years ago

Updated with the new function.

#3 @xyzzy
12 years ago

  • Cc dennen@… added

#4 @xyzzy
12 years ago

Might this be considered for 3.5 or beyond?

#5 @theandystratton
12 years ago

Ran into this today – code works great, thanks! Sad I couldn't have contributed, ya'll are too fast ;]

#6 @AaronAsh
11 years ago

Hi,
I ran into this problem with wordpress 3.4.2 today and manually patched it using the attached fix.
I also noticed that there's a bug in the latest fix: http://core.trac.wordpress.org/attachment/ticket/20534/20534.3.diff

The following if statement should have brackets:

        if ( $src && $width && $height ) 
                        if( is_admin() ) 
                                $src = set_url_scheme( $src, 'admin' ); 
                        else 
                                $src = set_url_scheme( $src ); 
                        return array( $src, $width, $height ); 
                return false; 
        } 

Like so:

                if ( $src && $width && $height ) {
                        if( is_admin() ) 
                                $src = set_url_scheme( $src, 'admin' ); 
                        else 
                                $src = set_url_scheme( $src ); 
                        return array( $src, $width, $height ); 
                }
                return false; 
        } 

@justindgivens
11 years ago

Updated from last comments.

#7 @here
11 years ago

  • Cc mike@… added

#8 @janpeters
11 years ago

Hi,

this does not only effect the Admin Area.
Right now I am using a Theme (Elegant Themes - Foxy) which uses the Featured Image for a Slider on the home page.

Independent of is_admin or not, our page is delivered HTTPS only. The images are, sadly, always delivered via HTTP. HTTPS is broken on any page that uses the Featured Images thereby.

So I think checking for is_admin() is not enough to fix this.
More or less using protocol relevant URL Include (omit the "http:" at the http:// include) should work for any case dependent of the real protocol used.

Hope this information are not too late to get into any Bugfix release.

Brgds

Jan

Last edited 11 years ago by janpeters (previous) (diff)

#9 @joostdevalk
11 years ago

  • Cc joost@… added

Just got bitten by this as well, would be cool to get a fix...

I should add: I got bitten by it on the frontend, on an SSL checkout page where the image returned was not using SSL, thus breaking it...

Last edited 11 years ago by joostdevalk (previous) (diff)

#10 @janpeters
10 years ago

  • Cc janpeters added
  • Severity changed from normal to major

The last patch doesn't work anymore with WordPress 3.7.X
Please find the fixed diff against the wp-include/media.php of WP 3.7.1 below.

Due to the security implications I increased the severity to major. As this issue opens a potential vector for causing Man in the Middle attacks on https sites due to http included content.

Brgds

Jan

512c512,518
< 	if ( $image = image_downsize($attachment_id, $size) )
---
> 	if ( $image = image_downsize($attachment_id, $size) ) {
> 		if( is_admin() ) {
> 			$image[0] = set_url_scheme( $image[0] , 'admin' );
> 		}
> 		else {
> 			$image[0] = set_url_scheme( $image[0] );
> 		}
513a520
> 	}
522c529,535
< 	if ( $src && $width && $height )
---
> 	if ( $src && $width && $height ) {
> 		if( is_admin() ) {
> 			$src = set_url_scheme( $src , 'admin' );
> 		}
> 		else {
> 			$src = set_url_scheme( $src );
> 		}
523a537
> 	}

@janpeters
10 years ago

Patch for WordPress 3.8.1

#11 @janpeters
10 years ago

  • Keywords dev-feedback added

#12 @johnbillion
10 years ago

  • Milestone changed from Awaiting Review to 4.0
  • Owner set to johnbillion
  • Status changed from new to accepted

This is indirectly a duplicate of #15928, but I'll keep it open for now. Moving to 4.0 for investigation.

Note also that this doesn't only affect the featured image meta box, but the media manager as a whole.

This ticket was mentioned in IRC in #wordpress-dev by DrewAPicture. View the logs.


10 years ago

This ticket was mentioned in IRC in #wordpress-dev by johnbillion. View the logs.


10 years ago

This ticket was mentioned in IRC in #wordpress-dev by DrewAPicture. View the logs.


10 years ago

#16 @DrewAPicture
10 years ago

  • Keywords dev-feedback removed
  • Milestone 4.0 deleted
  • Resolution set to duplicate
  • Status changed from accepted to closed

Closing this as a duplicate of #15928, just to consolidate the effort. We'll pick that up again in 4.1-early.

Note: See TracTickets for help on using tickets.