fix: GitHub updater requests never follow redirects, whatever other plugins set

HTTP Requests Manager (blocking modes) sets every request's redirection to 1 on http_request_args. The updater's no-redirect requests then followed GitHub's redirect: signed_address() lost the Location, so private updates failed (and the token went to the download host), and fetch_web()'s HEAD on releases/latest got 200 instead of a redirect. remote_no_redirect() holds redirection at 0 for that one URL at PHP_INT_MAX. Updater copy 1.0.1 (the GH#261 change had not raised it).

changelog.txt

@@ -2,6 +2,10 @@
 
 Every change to WP Plugin Starter. readme.txt lists the latest version in short.
 
+= Unreleased =
+* Fix: the shared GitHub updater (copy 1.0.1) could not update from private repositories on sites where another plugin raises every request's redirection count (HTTP Requests Manager in a blocking mode sets it to 1): GitHub's redirect was followed, so the signed download address was lost and the token went along to the download host. Its requests to GitHub now never follow redirects, whatever other plugins set. Public repositories' release checks had the same problem.
+* Fix: the shared GitHub updater's private downloads survive updaters that reset upgrader_pre_download (GPLVault Updater).
+
 = 1.0.0 =
 * A fresh start, made from the parts every wpallstars plugin needs, taken from plugins in daily use: the settings screen with search, the Read Me tab, features as classes, replaced plugins, the shared GitHub updater, the release and check scripts, CI and the shared rules (STANDARDS.md, DEVELOPMENT.md, RELEASING.md). Earlier versions of this starter are retired; start again from this one.
 * New: scripts/rename-plugin.sh gives a copy of the starter its own names, and scripts/sync-core.sh keeps a plugin's core files the same as the starter's.

includes/github-updater/class-wpallstars-github-updater.php

@@ -82,6 +82,7 @@ final class WPAllStars_GitHub_Updater {
         // Update checks run in the admin, in cron and in WP-CLI.
         add_filter('pre_set_site_transient_update_plugins', array(__CLASS__, 'add_updates'));
         add_filter('plugins_api', array(__CLASS__, 'plugin_information'), 20, 3);
+        add_filter('upgrader_package_options', array(__CLASS__, 'private_package'));
         add_filter('upgrader_pre_download', array(__CLASS__, 'private_download'), 10, 3);
         add_filter('upgrader_source_selection', array(__CLASS__, 'fix_folder'), 10, 4);
     }
@@ -211,13 +212,40 @@ final class WPAllStars_GitHub_Updater {
             $headers['Authorization'] = 'Bearer ' . $token;
         }
         // No redirects: the token must never travel to another address.
-        return wp_safe_remote_get('https://api.github.com/repos/' . $repo . $path, array(
-            'timeout'     => 10,
-            'redirection' => 0,
-            'headers'     => $headers,
+        return self::remote_no_redirect('GET', 'https://api.github.com/repos/' . $repo . $path, array(
+            'timeout' => 10,
+            'headers' => $headers,
         ));
     }
 
+    /**
+     * A request that never follows a redirect, so the answer's Location can
+     * be read and a token never travels to another address.
+     *
+     * Some plugins raise every request's redirection count (HTTP Requests
+     * Manager sets it to 1), which would follow GitHub's redirect: the token
+     * would go along and the Location would be lost. Only this request is
+     * held at 0, after every other filter.
+     *
+     * @param string $method GET or HEAD.
+     * @param string $url    Address.
+     * @param array  $args   Request arguments.
+     * @return array|WP_Error
+     */
+    private static function remote_no_redirect($method, $url, array $args) {
+        $args['redirection'] = 0;
+        $hold                = function ($parsed_args, $request_url) use ($url) {
+            if ($request_url === $url && is_array($parsed_args)) {
+                $parsed_args['redirection'] = 0;
+            }
+            return $parsed_args;
+        };
+        add_filter('http_request_args', $hold, PHP_INT_MAX, 2);
+        $response = 'HEAD' === $method ? wp_safe_remote_head($url, $args) : wp_safe_remote_get($url, $args);
+        remove_filter('http_request_args', $hold, PHP_INT_MAX);
+        return $response;
+    }
+
     /**
      * Stored release answers.
      *
@@ -315,10 +343,7 @@ final class WPAllStars_GitHub_Updater {
      */
     private static function fetch_web($repo, $folder, $main) {
         // Redirects to the newest release that is not a draft or pre-release.
-        $response = wp_safe_remote_head('https://github.com/' . $repo . '/releases/latest', array(
-            'timeout'     => 10,
-            'redirection' => 0,
-        ));
+        $response = self::remote_no_redirect('HEAD', 'https://github.com/' . $repo . '/releases/latest', array('timeout' => 10));
         if (is_wp_error($response)) {
             return $response;
         }
@@ -345,7 +370,7 @@ final class WPAllStars_GitHub_Updater {
         // The release zip, by its usual name: {folder}-{version}.zip.
         $asset    = null;
         $download = 'https://github.com/' . $repo . '/releases/download/' . rawurlencode($tag) . '/' . rawurlencode($folder . '-' . $version . '.zip');
-        $head     = wp_safe_remote_head($download, array('timeout' => 10, 'redirection' => 0));
+        $head     = self::remote_no_redirect('HEAD', $download, array('timeout' => 10));
         if (is_wp_error($head)) {
             return $head;
         }
@@ -474,7 +499,7 @@ final class WPAllStars_GitHub_Updater {
     private static function package($repo, array $release, $asset_only) {
         $private = '' !== self::token($repo);
         if (!empty($release['asset'])) {
-            // Assets of private repositories download through the API (see private_download()).
+            // Assets of private repositories download through the API (see private_package()).
             return $private && $release['asset']['api'] ? $release['asset']['api'] : $release['asset']['public'];
         }
         return $asset_only ? '' : (string) $release['zipball'];
@@ -678,8 +703,34 @@ final class WPAllStars_GitHub_Updater {
     }
 
     /**
-     * Downloads from private repositories: ask the API with the token, then
-     * fetch the file from where GitHub sends us without it.
+     * Downloads from private repositories: before WordPress downloads, swap
+     * the API address for the signed, short-lived address GitHub sends the
+     * token holder to, so WordPress downloads it itself without the token.
+     *
+     * This runs before upgrader_pre_download on purpose: some plugins' own
+     * updaters return false there for every package (GPLVault Updater does,
+     * at priority 999999999), which throws away a file already downloaded
+     * and leaves WordPress fetching the API address without the token
+     * (GitHub answers 404).
+     *
+     * @param array $options Upgrader options; 'package' is the address.
+     * @return array
+     */
+    public static function private_package($options) {
+        if (!is_array($options) || !isset($options['package'])) {
+            return $options;
+        }
+        $address = self::signed_address($options['package']);
+        if (is_string($address)) {
+            $options['package'] = $address;
+        }
+        return $options;
+    }
+
+    /**
+     * Fallback for downloads that skip upgrader_package_options: ask the API
+     * with the token, then fetch the file from where GitHub sends us without
+     * it.
      *
      * @param mixed       $reply    False to let WordPress download.
      * @param string      $package  Download address.
@@ -687,9 +738,29 @@ final class WPAllStars_GitHub_Updater {
      * @return mixed File path, WP_Error or $reply.
      */
     public static function private_download($reply, $package, $upgrader) {
-        if (false !== $reply || !is_string($package) || !preg_match('#^https://api\.github\.com/repos/([^/]+/[^/]+)/(?:releases/assets/[0-9]+|zipball/[^/?]+)$#', $package, $match)) {
+        if (false !== $reply) {
             return $reply;
         }
+        $address = self::signed_address($package);
+        if (null === $address) {
+            return $reply;
+        }
+        return is_wp_error($address) ? $address : download_url($address, 300);
+    }
+
+    /**
+     * The signed, short-lived download address of a private repository's
+     * release asset or source zip, asked for with the token. No token is
+     * needed (or wanted) there.
+     *
+     * @param mixed $package Download address.
+     * @return string|WP_Error|null Address; error; null when the address is
+     *                              not one of ours or there is no token.
+     */
+    private static function signed_address($package) {
+        if (!is_string($package) || !preg_match('#^https://api\.github\.com/repos/([^/]+/[^/]+)/(?:releases/assets/[0-9]+|zipball/[^/?]+)$#', $package, $match)) {
+            return null;
+        }
         $repo  = $match[1];
         $known = false;
         foreach (self::plugins() as $plugin) {
@@ -697,13 +768,12 @@ final class WPAllStars_GitHub_Updater {
         }
         $token = self::token($repo);
         if (!$known || '' === $token) {
-            return $reply;
+            return null;
         }
 
-        $response = wp_safe_remote_get($package, array(
-            'timeout'     => 30,
-            'redirection' => 0,
-            'headers'     => array(
+        $response = self::remote_no_redirect('GET', $package, array(
+            'timeout' => 30,
+            'headers' => array(
                 'Accept'        => false !== strpos($package, '/releases/assets/') ? 'application/octet-stream' : 'application/vnd.github+json',
                 'Authorization' => 'Bearer ' . $token,
             ),
@@ -715,8 +785,7 @@ final class WPAllStars_GitHub_Updater {
         if ('' === $location || !in_array((int) wp_remote_retrieve_response_code($response), array(301, 302, 303, 307, 308), true)) {
             return new WP_Error('wpallstars_github_download', __('GitHub did not give a download address. Check the token’s access to the repository.', 'wp-plugin-starter-template'));
         }
-        // A signed, short-lived address: no token needed (or wanted) there.
-        return download_url($location, 300);
+        return $location;
     }
 
     /**

includes/github-updater/load.php

@@ -27,7 +27,7 @@ if (!isset($GLOBALS['wpallstars_github_updater']) || !is_array($GLOBALS['wpallst
     $GLOBALS['wpallstars_github_updater'] = array();
 }
 // Version of this copy => its class file.
-$GLOBALS['wpallstars_github_updater']['1.0.0'] = __DIR__ . '/class-wpallstars-github-updater.php';
+$GLOBALS['wpallstars_github_updater']['1.0.1'] = __DIR__ . '/class-wpallstars-github-updater.php';
 
 if (!function_exists('wpallstars_github_updater_load')) {
     /**