WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 4 years ago

Last modified 4 years ago

#22378 closed enhancement (fixed)

Add ability to hijack plugin install/upgrade downloads

Reported by: rmccue Owned by: dd32
Milestone: 3.7 Priority: normal
Severity: normal Version: 3.4
Component: Upgrade/Install Keywords: has-patch commit
Focuses: Cc:

Description

Problem

Many plugins have their own updating system. For paid plugins, this can involve
sending authentication/authorization headers. At the moment, to implement this,
certain parts of the plugin upgrader system have to be duplicated to replace
parts of the system, such as the downloading method. In addition, the package
download URL has to be a valid URL. Some systems may wish to use alternate URLs
apart from HTTP/FTP-based URLs.

The code for this is shared between the single plugin upgrader and the bulk
plugin upgrader. While the upgrader class can be replaced fairly easily with
existing filters for single plugins, it's not possible to do this for the bulk
upgrader (especially when multiple systems want to do this), requiring falling
back to a low-level filter like http_request_args, ensuring that one only
hooks in after the admin_action_update-selected action has failed.

Solution

Add a filter to hijack the download system before it can act at all. This allows
replacing the entire downloading system, with the only requirement being to
return a valid package filename. This filter will work across both the single
and bulk upgraders, ensuring consistency in the approach.

Patch attached. Related: #22377, #22129

Attachments (3)

22378.diff (578 bytes) - added by rmccue 5 years ago.
Add filter to WP_Upgrader::download_package()
22378.2.diff (575 bytes) - added by rmccue 5 years ago.
More consistent filter name
22378.3.diff (888 bytes) - added by DrewAPicture 4 years ago.
+ docs

Download all attachments as: .zip

Change History (21)

#1 @scribu
5 years ago

  • Cc scribu added

#2 @scribu
5 years ago

  • Type changed from defect (bug) to enhancement

@rmccue
5 years ago

Add filter to WP_Upgrader::download_package()

#3 @rmccue
5 years ago

Trivial example of how to implement it:

<?php

function test_new_hook($return, $package, $upgrader) {
	if ($return !== false) {
		return $return;
	}

	$package = add_query_arg('key', 'token', $package);

	$upgrader->skin->feedback('downloading_package', $package);

	$download_file = download_url($package);

	if ( is_wp_error($download_file) )
		return new WP_Error('download_failed', $upgrader->strings['download_failed'], $download_file->get_error_message());

	return $download_file;
}

add_filter('wp_upgrader_pre_download', 'test_new_hook', 10, 3);

#4 follow-up: @scribu
5 years ago

We should try to keep some consistency between hook names.

First, there's general upgrader hooks:

'upgrader_pre_install'
'upgrader_post_install'
'upgrader_source_selection'
'upgrader_clear_destination'

And then there's plugin hooks:

'install_plugin_complete_actions'
'update_plugin_complete_actions'
'update_bulk_plugins_complete_actions'

And theme hooks:

'install_theme_complete_actions'
'update_theme_complete_actions'
'update_bulk_theme_complete_actions'

So, a better hook name would be 'upgrader_pre_download'.

@rmccue
5 years ago

More consistent filter name

#5 in reply to: ↑ 4 @rmccue
5 years ago

Replying to scribu:

We should try to keep some consistency between hook names.

[...]

So, a better hook name would be 'upgrader_pre_download'.

Whoops, missed that. Updated now.

#6 @johnbillion
5 years ago

  • Cc johnbillion added

#7 @nacin
5 years ago

  • Milestone changed from 3.5 to Awaiting Review

This is not 3.5 material.

#8 @toscho
5 years ago

  • Cc info@… added

#9 @SergeyBiryukov
5 years ago

  • Version changed from trunk to 3.4

#10 @dd32
4 years ago

  • Keywords needs-docs added
  • Milestone changed from Awaiting Review to 3.7

Needs filter documentation, moving for review

@DrewAPicture
4 years ago

+ docs

#11 @DrewAPicture
4 years ago

  • Keywords needs-docs removed

22378.3.diff adds a docblock for the upgrader_pre_download filter hook.

Side note: We'll need to do a functional docs pass through this whole class in a separate ticket.

#12 @rmccue
4 years ago

  • Keywords commit added

If I may be so bold...

#13 @dd32
4 years ago

  • Owner set to dd32
  • Resolution set to fixed
  • Status changed from new to closed

In 25427:

Upgrader: Allow plugins to short-circuit the package download. Props rmccue and DrewAPicture. Fixes #22378

#14 @farinspace
4 years ago

my apologies if I am not understanding this correctly ... doesn't the plugin still need to be activated in order to use the hook, if the plugin is deactivated, wordpress will always check the plugins repo and if a plugin of the same name exists will return that data?

I think wordpress would be more open if a plugin author could specify their own update URI, this would avoid namespace grabs just for the sole purpose of not having the repo interfere with and external updater .. see ticket #25314

Last edited 4 years ago by farinspace (previous) (diff)

#15 @dd32
4 years ago

doesn't the plugin still need to be activated in order to use the hook,

Yes, This is for cases where a plugin is offering a non-wordpress.org update location. This is not currently supported by WordPress in any way, except that plugins can, and do, bring such things to the table. So yes, that plugin WOULD have to be active for this to be of any use to the plugin.

#16 follow-up: @farinspace
4 years ago

Would the ability for wordpress to forgo an update check for a specific "installed" but deactivated plugin sound better .. something as simple as a plugin comment header parameter .. "UpdateURI" plugin authors provide a URI but it would be used as a boolean mostly, asking wordpress to skip an update check, which then would give the plugin a chance to hook in after activation .. I just want to make a reasonable request for such a feature and understand all implications.

#17 in reply to: ↑ 16 ; follow-up: @dd32
4 years ago

Replying to farinspace:

Would the ability for wordpress to forgo an update check for a specific "installed" but deactivated plugin sound better ..

This ticket is not really related to your request in the least, I can't find the ticket reference for it, but short story is that there are no current plans for what you're requesting.

#18 in reply to: ↑ 17 @SergeyBiryukov
4 years ago

Replying to dd32:

This ticket is not really related to your request in the least, I can't find the ticket reference for it, but short story is that there are no current plans for what you're requesting.

#22129 appears to be related (and its duplicate #25314).

Last edited 4 years ago by SergeyBiryukov (previous) (diff)
Note: See TracTickets for help on using tickets.