WordPress.org

Make WordPress Core

Opened 17 months ago

Last modified 13 months ago

#25449 new defect (bug)

wp_upload_dir() doesn't support https

Reported by: ryanhellyer Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 3.8
Component: Upload Keywords: has-patch
Focuses: Cc:

Description

The wp_upload_dir() function does not support https. I have added a simple is_ssl() check and a string replacement to serve the correct URL type.

Background behind what prompted me to write the patch:
I read a blog post by Kaspars Dambis in which he discussed fixing this problem via his own plugin, but it seems to me that since WordPress outputting an incorrect URL, that it would make most sense to fix it there.

http://kaspars.net/blog/wordpress/minit-plugin-ssl-https

Attachments (2)

add-https-wp_upload_dir.patch (438 bytes) - added by ryanhellyer 17 months ago.
Adds https support to wp_upload_dir()
add-https-wp_upload_dir2.patch (378 bytes) - added by ryanhellyer 17 months ago.
New patch for making wp_upload_dir() work with https.

Download all attachments as: .zip

Change History (10)

@ryanhellyer17 months ago

Adds https support to wp_upload_dir()

comment:1 follow-up: @kasparsd17 months ago

We should probably do the if_ssl() check for both $baseurl and $url here.

comment:2 follow-ups: @johnbillion17 months ago

Simpler patch: Just wrap $url in set_url_scheme().

comment:3 in reply to: ↑ 2 @ryanhellyer17 months ago

Replying to johnbillion:

Simpler patch: Just wrap $url in set_url_scheme().

I've never heard of that before. I'll look into it and put a new patch together.

@ryanhellyer17 months ago

New patch for making wp_upload_dir() work with https.

comment:4 in reply to: ↑ 1 @ryanhellyer17 months ago

Replying to kasparsd:

We should probably do the if_ssl() check for both $baseurl and $url here.

Oops, I didn't think of doing that. The new patch uses $baseurl = $url = set_url_scheme( $url ); so that it does them both at the same time.

comment:5 in reply to: ↑ 2 @ryanhellyer17 months ago

Replying to johnbillion:

Simpler patch: Just wrap $url in set_url_scheme().

That worked perfectly! I wish I knew about that before as it makes for much cleaner code.

comment:6 @nacin17 months ago

There are a lot of potential issues here. Just because the admin is in SSL, for example, doesn't mean the frontend is. See #15928 for where I think most of this conversation has taken place.

comment:7 @SergeyBiryukov17 months ago

Related: #19873 (closed as a duplicate of #15928), #13941.

comment:8 @nacin13 months ago

  • Component changed from General to Upload
Note: See TracTickets for help on using tickets.