WordPress.org

Make WordPress Core

Opened 2 years ago

Last modified 3 weeks ago

#20534 new defect (bug)

Featured Image (Post Thumbnail) SSL Issue

Reported by: justindgivens Owned by:
Milestone: Awaiting Review Priority: normal
Severity: major Version: 2.9
Component: Post Thumbnails Keywords: needs-testing has-patch dev-feedback
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 2 years ago.
Patch for 20534
20534.2.diff (896 bytes) - added by justindgivens 2 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 23 months ago.
Updated with the new function.
20534.4.diff (1006 bytes) - added by justindgivens 14 months ago.
Updated from last comments.
20534.5.diff (1.0 KB) - added by janpeters 2 months ago.
Patch for WordPress 3.8.1

Download all attachments as: .zip

Change History (16)

justindgivens2 years ago

Patch for 20534

comment:1 justindgivens2 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?

justindgivens2 years ago

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

comment:2 nacin23 months ago

  • Version changed from 3.4 to 2.9

We now have set_url_scheme() we can use here.

justindgivens23 months ago

Updated with the new function.

comment:3 xyzzy21 months ago

  • Cc dennen@… added

comment:4 xyzzy21 months ago

Might this be considered for 3.5 or beyond?

comment:5 theandystratton20 months ago

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

comment:6 AaronAsh14 months 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; 
        } 

justindgivens14 months ago

Updated from last comments.

comment:7 here11 months ago

  • Cc mike@… added

comment:8 janpeters10 months 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 9 months ago by janpeters (previous) (diff)

comment:9 joostdevalk8 months 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 8 months ago by joostdevalk (previous) (diff)

comment:10 janpeters5 months 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
> 	}

janpeters2 months ago

Patch for WordPress 3.8.1

comment:11 janpeters3 weeks ago

  • Keywords dev-feedback added
Note: See TracTickets for help on using tickets.