Partial downloads do not properly take the length into account

Yes, I see what you mean. You are correct. Although $seek_end is calculated properly in the code, the loop ALWAYS tries to read until end of file, instead of stopping at $seek_end. Here's the part of the code that needs to be fixed:

  // Start buffered download
  while (!feof($fp)) {

    // Reset time limit for large files
    set_time_limit(0);

    // Push the data to the client.
    print(fread($fp, UC_FILE_BYTE_SIZE));
    flush();
    ob_flush();
  }

Also note that set_time_limit() should be taken out of that loop - we only need to set it once, not keep setting it every 8kbytes. Any patch to this section of code should be checked against #193383: set_time_limit: Centralize calls and prevent warnings and errors, which changes the way set_time_limit() works.

I'm not currently set up to test partial content transfers, so I don't want to roll a patch I can't test. Bumping the priority up to normal.

Wow, good find! I wonder if this issue would help some of our users who are getting incomplete downloads.

Subscribe! (I can test this if you want to post modified code.)

EDIT to add: Seems like we'll need to set a flag in the event of a partial content being downloaded (like a resumed download). If I start a new download and it reads the entire file, then $seek_end is the same as $size (filesize) and the $length ends up being 1, which results in a < 1KB-file being downloaded.

That's the math that I used, and I had several downloads fail. Can you post your modified code? Maybe I am doing something wrong. Thanks!

Changing component.

I am encountering an "Undefined offset: 1" error on line 349 of uc_file.pages.inc, in _uc_file_download_transfer(), when a user is downloading a purchased file.

The offending line:

list($range, $extra_ranges) = explode(',', substr($_SERVER['HTTP_RANGE'], 6), 2);

My error logger noted the "Range" header in this request was bytes=4915200-. Note that there are no commas in that range header (which is a valid request).

Since the $extra ranges variable is not used elsewhere in the code, I propose we simplify this line to the following which will gracefully handle situations where there is only a single range:

list($range) = explode(',', substr($_SERVER['HTTP_RANGE'], 6), 1);

The attached patch accomplishes this for D7.

That doesn't fix #1 and #7 ... and we still need a test to demonstrate that any fix does the right thing ...