download_url function should return the body in response on error

[nextendweb]

Our non-wordpress.org plugin uses custom url to fetch the update file from our server which needs authentication. So, when the downloading of the update file fails in WordPress we would like to get the response as it is a REST message which contains the exact error message what we should present to the user.

download_url function use wp_safe_remote_get with filename argument which stores the body of the response.

<?php
function download_url( $url, $timeout = 300 ) {
        //WARNING: The file is not automatically deleted, The script must unlink() the file.
        if ( ! $url )
                return new WP_Error('http_no_url', __('Invalid URL Provided.'));

        $url_filename = basename( parse_url( $url, PHP_URL_PATH ) );

        $tmpfname = wp_tempnam( $url_filename );
        if ( ! $tmpfname )
                return new WP_Error('http_no_file', __('Could not create Temporary file.'));

        $response = wp_safe_remote_get( $url, array( 'timeout' => $timeout, 'stream' => true, 'filename' => $tmpfname ) );

        if ( is_wp_error( $response ) ) {
                unlink( $tmpfname );
                return $response;
        }

        if ( 200 != wp_remote_retrieve_response_code( $response ) ){
                unlink( $tmpfname );
                return new WP_Error( 'http_404', trim( wp_remote_retrieve_response_message( $response ) ) );
        }

        $content_md5 = wp_remote_retrieve_header( $response, 'content-md5' );
        if ( $content_md5 ) {
                $md5_check = verify_file_md5( $tmpfname, $content_md5 );
                if ( is_wp_error( $md5_check ) ) {
                        unlink( $tmpfname );
                        return $md5_check;
                }
        }

        return $tmpfname;
}

If the wp_safe_remote_get fails we are unable to access to the response body as the $tmpfname file is unlinked and does not read into the response object.

<?php
        if ( is_wp_error( $response ) ) {
                unlink( $tmpfname );
                return $response;
        }

        if ( 200 != wp_remote_retrieve_response_code( $response ) ){
                unlink( $tmpfname );
                return new WP_Error( 'http_404', trim( wp_remote_retrieve_response_message( $response ) ) );
        }

It would be better if WordPress could read the content of the temporary file and parse it as a normal response body in the previous two if statement.

[soulseekah]

with tests

[soulseekah]

This patch tucks up to 1kb (altered by filter) into the data of the error, so data['code'] and data['body'] will contain the response code and up to 1km of the body.

Useful for debugging purposes.

Special thanks to @campusboy1987 @mihdan @SergeyBiryukov and others in the live coding stream of this ticket.

Godspeed.

[nextendweb]

I was able to find the solution for my issue without this modification, so this feature is not required for me. But I'm think it makes the HTTP api better.

My solution was:

<?php
    add_filter('upgrader_pre_download', 'n_upgrader_pre_download', 10, 3);

    n_upgrader_pre_download($reply, $package, $upgrader) {
        $needle = my_url_to_match();
        if (substr($package, 0, strlen($needle)) == $needle) {
            add_filter('http_response', 'n_http_response', 10, 3);
        }

        return $reply;
    }

    function n_http_response($response, $r, $url) {

        $needle = my_url_to_match();
        if (substr($url, 0, strlen($needle)) == $needle && (200 != wp_remote_retrieve_response_code($response) || is_wp_error($response))) {

            if (isset($response['filename']) && file_exists($response['filename'])) {

                $body = @file_get_contents($response['filename']);
                // Here I have the body of the error response

            }
        }

        return $response;
    }

[soulseekah]

typo in tests filter cleanup section

[johnbillion]

This looks like a good idea. What's the purpose of trimming the response (to 1kb by default)?

[soulseekah]

We don't want to risk loading up a potentially large file into memory. 1kb was chosen as it would offer enough hints as to what is going wrong (if anything).

Imagine a server spewing out 100MB or random data with a 418 I'm a teapot code. Downloading such a URL will result in an memory limit being hit in PHP probably.

Thus we just read as much as is useful into the error body.

Any other ideas?

Ideally, we'd want the file to stick around just in case and then be unlinked. But the only solution that seems viable is hooking into shutdown, which I think is over engineering.

[SergeyBiryukov]

In 42773:

Filesystem API: Allow download_url() to return the response code and body on error as an additional WP_Error object data.

The error response body size is limited to 1 KB by default to avoid taking up too much memory. The size can be increased using download_url_error_max_body_size filter.

Props soulseekah, campusboy1987, mihdan, SergeyBiryukov.
Fixes #43329.