Based on the discussion around file_transfer(): #1463656: Add a Drupal kernel; leverage HttpFoundation and HttpKernel.

8.x: file_transfer() is used by image_style_deliver() and file_download().

The kernel patch introduces StreamedResponses there to transfer files, inlining the code for streaming the files.

Proposed resolution

  • Make file_transfer() a helper function that returns a StreamedResponse.
  • Revert changes to core/includes/file.inc:
    diff --git a/core/includes/file.inc b/core/includes/file.inc
    index f204989..91e7eca 100644
    --- a/core/includes/file.inc
    +++ b/core/includes/file.incundefined
    @@ -5,6 +5,9 @@
      * API for handling file uploads and server file management.
      */
    +use Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException;
    +use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;
    +use Symfony\Component\HttpFoundation\StreamedResponse;
    use Drupal\Core\StreamWrapper\LocalStream;
    /**
    @@ -2026,18 +2029,27 @@ function file_download() {
           $function = $module . '_file_download';
           $result = $function($uri);
           if ($result == -1) {
    -        return drupal_access_denied();
    +        throw new AccessDeniedHttpException();
           }
           if (isset($result) && is_array($result)) {
             $headers = array_merge($headers, $result);
           }
         }
         if (count($headers)) {
    -      file_transfer($uri, $headers);
    +      return new StreamedResponse(function() use ($uri) {
    +        $scheme = file_uri_scheme($uri);
    +        // Transfer file in 1024 byte chunks to save memory usage.
    +        if ($scheme && file_stream_wrapper_valid_scheme($scheme) && $fd = fopen($uri, 'rb')) {
    +          while (!feof($fd)) {
    +            print fread($fd, 1024);
    +          }
    +          fclose($fd);
    +        }
    +      }, 200, $headers);
         }
    -    return drupal_access_denied();
    +    throw new AccessDeniedHttpException();
       }
    -  return drupal_not_found();
    +  throw new NotFoundHttpException();
    }
  • Use the new file_transfer() in core/modules/update/tests/modules/update_test/update_test.module, instead of creating a custom StreamedResponse. Or is it for managed files only?
    -  readfile("$path/$project_name.$availability_scenario.xml");
    +  $file = "$path/$project_name.$availability_scenario.xml";
    +  if (!is_file($file)) {
    +    // Return an empty response.
    +    return new Response('', 200, array('Content-Type' =>  'text/xml; charset=utf-8'));
    +  }
    +  return new StreamedResponse(function() use ($file) {
    +    // Transfer file in 1024 byte chunks to save memory usage.
    +    if ($fd = fopen($file, 'rb')) {
    +      while (!feof($fd)) {
    +        print fread($fd, 1024);
    +      }
    +      fclose($fd);
    +    }
    +  }, 200, array('Content-Type' =>  'text/xml; charset=utf-8'));

Also note #1561362: Change file_transfer() to use BinaryFileResponse that is about making a StreamedFileResponse or another replacement, or something in Symfony upstream.

Comments

Title:Use file_transfer() as a helper functions instead of inlining file transfersUse file_transfer() as a helper function instead of inlining file transfers

Issue summary:View changes

Updated issue summary.

I don't understand why we can't simplify the kernel branch right now with this change? We're making more work and the kernel patch harder to review, only to undo it later.

Status:Active» Fixed

Resolved with http://drupalcode.org/sandbox/Crell/1260830.git/commit/6340061 on the wscci kernel branch.

Status:Fixed» Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

Issue summary:View changes

.