Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Firefox: Corrupt downloads ("padded" downloads) when there is no content length #3634

Merged
merged 1 commit into from Sep 3, 2013

Conversation

Projects
None yet
8 participants
Contributor

nmaier commented Sep 1, 2013

When the server does not provide a Content-Length with the initial PDF request, e.g. when chunked transfer-encoding is used - which is common for dynamically generated files, then the Download action of pdf.js will not save the data as retrieved, but instead the data will be padded by 0-bytes up to a certain bound.

What happens is the following:

  • PdfDataListener will get an initial buffer of 0x10000 (64kb) length when the size is unknown.
    this.data = new Uint8Array(length >= 0 ? length : 0x10000);
  • When that buffer would overflow during the data transfer, it is doubled again and again until the data would fit. (BTW: This seems a bit wasteful once you get into the multi-MB area...)
    if (this.length < 0 && this.data.length < willBeLoaded) {
    // data length is unknown and new chunk will not fit in the existing
    // buffer, resizing the buffer by doubling the its last length
    var newLength = this.data.length;
    for (; newLength < willBeLoaded; newLength *= 2) {}
    var newData = new Uint8Array(newLength);
    newData.set(this.data);
    this.data = newData;
    }
  • However, when the data transfer completes, neither is the buffer shrunk to the actual size again, nor is the actual size noted -> On download the full, over-sized buffer will be saved. Only a new .subarray() view is taken:
    data = data.subarray(0, this.loaded);
  • The blobURI created for the download will use the underlying buffer, which is still over-sized:
    var blob = PDFJS.createBlob(data.buffer, 'application/pdf');

STR:

Expected:
File size should be 5420 byte.

Actual:
File size is 65536 byte (0x10000, initial buffer size) and the file is padded by 0-bytes after the initial transfer.

cc @yurydelendik

Contributor

nmaier commented Sep 1, 2013

I missed some things in my initial analysis and edited the issue description to include the missing details on .subarray() and the blobURI

@nmaier nmaier Download: Use the typed array view instead of the buffer
The ArrayBuffer holding the data might be over-sized in case the
data length was not known during the transfer, e.g. when using a
Content-Encoding other than `identity` or when using a
Transfer-Encoding.
Only the view into the buffer has the correct length then, hence
always use the view directly when creating the blob URI for the
download, instead of the over-sized underlying buffer.

Closes GH-3627
Closes GH-3634
16a1c38
Contributor

nmaier commented Sep 1, 2013

I'm a pull-request now ;)

Member

Snuffleupagus commented Sep 1, 2013

/botio-linux preview

Collaborator

pdfjsbot commented Sep 1, 2013

From: Bot.io (Linux)


Received

Command cmd_preview from @Snuffleupagus received. Current queue size: 0

Live output at: http://107.21.233.14:8877/15df0eab97ff6e6/output.txt

@yurydelendik yurydelendik added a commit that referenced this pull request Sep 3, 2013

@yurydelendik yurydelendik Merge pull request #3634 from nmaier/download-buffer
Firefox: Corrupt downloads ("padded" downloads) when there is no content length
c55cb9d

@yurydelendik yurydelendik merged commit c55cb9d into mozilla:master Sep 3, 2013

1 check passed

default The Travis CI build passed
Details
Contributor

yurydelendik commented Sep 3, 2013

Looks good. Thanks for looking into this.

@nmaier nmaier deleted the nmaier:download-buffer branch Sep 4, 2013

durack1 commented Sep 19, 2013

Just wondering which version of Firefox this update will be released in - is it likely to make it's way into 24.x?

Contributor

yurydelendik commented Sep 19, 2013

durack1 commented Sep 19, 2013

Ok great, thanks for the heads up..

I see this thread supersedes #3627

The tech discussion is far beyond me. However, I notice that some PDF's download just fine through the viewer. And others get corrupted per the discussion above. While we are waiting for a fix in FF26, is there a way to modify our PDFs to avoid this issue?

Thanks!

Contributor

timvandermeij commented Oct 8, 2013

@josephbarbier Not that I'm aware of. This was a problem in PDF.js itself, not in the PDF files. The issue has been fixed in the meantime, but it will probably land in Firefox 26 like Yury commented. If it's for local use, you could use the development addon, but otherwise you'll have to wait until the patch lands in Firefox.

salvo1 commented Jan 27, 2014

Finally, has this fix really been deployed? I'm still getting some pdf files not been displayed with FF26 and FF27b8.
Using Acrobat Reader or installing PDF Viewer 0.8.944 is not an option for the average FF user...

Contributor

yurydelendik commented Jan 27, 2014

@salvo1, yes it was deployed in FF26, which was released. Are you sure you are experiencing the same issue? Please open other issue with details mentioned in FAQs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment