WordPress.org

Make WordPress Core

Opened 4 years ago

Last modified 3 months ago

#13941 new enhancement

WP_CONTENT_URL should use site_url() to support HTTPS / SSL

Reported by: micropat Owned by: ryan
Milestone: Future Release Priority: normal
Severity: normal Version:
Component: Bootstrap/Load Keywords: has-patch
Focuses: Cc:

Description

On HTTPS pages, users sometimes get 'insecure content' warnings from their browser. This is commonly caused by plugins which use the WP_PLUGIN_URL constant to get the full plugin directory URL for the sake of loading static content.

The problem is that WP_PLUGIN_URL will always return the siteurl (as specified in Settings > General) and thus does not adjust from http:... to https:... when needed.

WP_PLUGIN_URL is dependent on WP_CONTENT_URL, and WP_CONTENT_URL is derived from get_option('siteurl'), which only returns the siteurl and does not adjust for HTTPS pages. However, the site_url() function does adjust for HTTPS pages.

So, to fully support HTTPS and directives such as FORCE_SSL_LOGIN and FORCE_SSL_ADMIN, WP_CONTENT_URL needs to use the site_url() function as proposed in my riveting one-liner patch.

Related #10198 #9008

Attachments (2)

default-contstants.patch (604 bytes) - added by micropat 4 years ago.
pd-fix-wp-upload-dir-ssl.php (715 bytes) - added by pdclark 3 years ago.
Temporary fix for wp_upload_dir() : Must put in mu-plugins

Download all attachments as: .zip

Change History (25)

comment:1 nacin4 years ago

We also have content_url() and plugins_url() that can be used.

comment:2 follow-up: bueltge4 years ago

@nacin: yes, but the functions dont work on older WPs
many plugins has now this problems, the codex say to use this constants

comment:3 in reply to: ↑ 2 micropat4 years ago

I think nacin is saying we should use content_url() and plugins_url() to define these URL constants in WP core, yes?

This is the doc bueltge is referring to: http://codex.wordpress.org/Determining_Plugin_and_Content_Directories

comment:4 dd324 years ago

I think nacin is saying we should use content_url() and plugins_url() to define these URL constants in WP core, yes?

No. Plugins should be using those functions instead of using the constants.

If a Plugin wants to support older versions, they can do so through code such as:

if ( function_exists('plugins_url') )
$url = plugins_url(...);
else
$url = WP_PLUGIN_URL . '......';

the codex say to use this constants

Then update the codex (Its a wiki afterall) to reflect the functions being available, drop a note on the Discussion page even if you cant write it yourself)

comment:5 dd324 years ago

  • Milestone set to Unassigned

comment:6 micropat4 years ago

Cool, we can update the codex, but how about fixing the constants that plugins currently use per the long-standing advice of that doc? Are there any known caveats to applying the patch?

comment:7 dd324 years ago

Are there any known caveats to applying the patch?

I'm honestly not sure, It seems too early to be reliably calling the function to me, It might be posible for plugins to affect the SSL status, I'm not sure, I'm not the one to answer that :)

comment:8 voyagerfan57614 years ago

  • Cc WordPress@… added

comment:9 nacin3 years ago

  • Milestone changed from Awaiting Review to 3.1

I don't think this will be too early, so it should work. This is a good idea, as it brings SSL to a lot of plugins that otherwise are usng the old constants.

comment:10 nacin3 years ago

  • Type changed from defect (bug) to enhancement

pdclark3 years ago

Temporary fix for wp_upload_dir() : Must put in mu-plugins

comment:11 pdclark3 years ago

FWIW, wp_upload_dir() uses get_option( 'siteurl' ) 2/3 of the time, but WP_CONTENT_URL 1/3 of the time. The first gives https:// correctly, but the second doesn't.

This leads to unreliable behavior where wp_upload_dir() is used. Specifically, bp_core_avatar_url() and bp_core_avatar_upload_path() in BuddyPress use it, so avatars for members and groups throw an error on pages that are SSL.

I've attached a plugin that works as a temporary fix for wp_upload_dir(), in case anyone else out there is using BuddyPress over SSL. It needs to be put in mu-plugins to attach early enough.

comment:12 nacin3 years ago

  • Keywords 3.2-early added
  • Milestone changed from 3.1 to Future Release

get_option( 'siteurl' ) does not return https. So wp_upload_dir() does not return https.

comment:14 ocean903 years ago

Duplicate: #17876

comment:15 barrycarlyon3 years ago

I'd like to see this in the next WordPress release.

I just came across this and was "seriously".

I've fixed this in my core. Just swap get_option for site_url() already.....

I won't switch this to a Bug but I believe it is a Bug.

comment:16 marfarma2 years ago

  • Cc pauli.price@… added

Related: #15928, #18017, #19023, #18005

Last edited 2 years ago by marfarma (previous) (diff)

comment:17 jeremyclarke2 years ago

  • Cc jer@… added
  • Keywords 3.2-early removed

+1 for fixing this. Plugins that used WP_CONTENT_URL were doing_it_right() at the time and despite the new functions being better the constants should have always being obeying the FORCE_SSL_ADMIN etc. directives. I'm butting up against this in nov 2011 and was hoping that updating to the beta would fix it, sad to see it's still on the table.

Removing the 3.2-early tag, hopefully early in 3.4?

Last edited 2 years ago by jeremyclarke (previous) (diff)

comment:18 jeremyclarke2 years ago

Ugh. Just to add salt in the wound the definition of WP_CONTENT_URL in wp_plugin_directory_constants() happens so early that it's impossible to attach a filter to the output of get_option('siteurl') before the constant is defined, even using an mu-plugin. Any chance that function could be run later so we could affect the output?

Having to resort to output buffering the whole admin or fork plugins would be a bummer.

comment:19 marfarma2 years ago

+1 for fixing this -- or at the very least providing the opportunity to override it in wp-config or a mu plugin -- or somehow. As @jeremyclarke notes, the only options to fix this is to buffer output or fork plugins. I've ended up forking plugins, locally, and I'm not happy about it.

Beyond this, we really need to create a compelling reason for plugin authors to fix their code.

Grep the plugin repository for deprecated constants like WP_CONTENT_URL, and notify their authors that their plugins require remediation, for plugins listed as compatible with versions >= 3.0, and last updated within the past two years. Or send an alert to all authors of registered plugins -- something.

At the very least, check new code for deprecated constants, and they be removed when found -- like with themes.

comment:20 marfarma2 years ago

Related: #19037

See wp-hacker's thread titled: "Fixing some SSL cases for 3.4" to follow the evolving discussion.

comment:21 creativeinfusion11 months ago

The patch (though sensible) doesn't cater for sites that use the recommended procedure for moving wp-content out of the wordpress directory as per http://codex.wordpress.org/Editing_wp-config.php#Moving_wp-content_folder (for example when using an SVN or git e.g. http://ottopress.com/2011/creating-a-wordpress-site-using-svn/).

These constants can be a right pain, yet so many plugins use them instead of the current doing_it_right() functions that setting them up correctly still matters. Some really great plugins still use them and alas even then do not not always honour overrides (e.g. by assuming plugins are in WP_CONTENT_DIR/plugins). I'd ideally like to see the constants properly deprecated to force use of the functions, but that seems unlikely.

wp-config.php is too early to set WP_CONTENT_URL for a subsite in a multisite subdirectory install - it's not possible to reliably set it to http://domain.com/subsite/custom-content and know subsite exists. We can use sunrise.php but that would involve a lot of code duplication and in any case it's not available for single site installations.

Any mileage in having a drop in config file that can be used before these constants are set but after most of the functions have been loaded?

comment:22 SergeyBiryukov4 months ago

#26412 was marked as a duplicate.

comment:23 nacin3 months ago

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