1676240 - Downloaded files get the extension dms

Status: RESOLVED DUPLICATE of bug 1652520

(Firefox :: File Handling, defect)

Description

[Dave Townsend [:mossop]]

Recently when I've been downloading files from sites that are delivered as application/octet-stream with no Content-Disposition the file has been getting named with a dms extension which is not correct.

I saw this downloading Snapcamera (https://snapcamera.snapchat.com/download/). The url downloaded (it expires so probably doesn't work now) is https://storage.googleapis.com/prod-studio3d-binaries/binaries/582bdb3b-f1f8-4b79-b8dc-be446f0f4741/Snap%20Camera%201.10.0.pkg?GoogleAccessId=[e-mail]&Expires=1604948882&Signature=clipped and Nightly chose to save the file as Snap Camera 1.10.0.dms. Firefox release gives it the correct filename: Snap Camera 1.10.0.pkg.

Mozregression points to bug 1667787 as the culprit.

Comment 1

[:Gijs (away/busy until Mar 10)]

Would you mind confirming if the changes in https://phabricator.services.mozilla.com/D96406 ( bug 1652520 ) fix this for you? I would expect them to, but it'd be good to be sure.

Comment 2

[Dave Townsend [:mossop]]

Yeah that looks like it fixes it. Do we have a test for this case? It wasn't clear to me from looking over the patch.

Comment 3

[Dave Townsend [:mossop]]

I did notice one oddity, when downloading a test file the following got logged:

JavaScript error: resource://pdf.js/PdfStreamConverter.jsm, line 1086: NS_ERROR_FAILURE: Ignore PDF.js for this download.

Comment 4

[:Gijs (away/busy until Mar 10)]

(In reply to Dave Townsend [:mossop] from comment #2)

Yeah that looks like it fixes it. Do we have a test for this case? It wasn't clear to me from looking over the patch.

There's one in the patch for application/x-gobbledygook which is functionally sort of the same in terms of how the current code treats it, but it's probably worth adding an explicit one for application/octet-stream and application/binary-stream and application/x-download, which are all relatively common. I'll do that before landing.

(In reply to Dave Townsend [:mossop] from comment #3)

I did notice one oddity, when downloading a test file the following got logged:

JavaScript error: resource://pdf.js/PdfStreamConverter.jsm, line 1086: NS_ERROR_FAILURE: Ignore PDF.js for this download.

Yeah, this gets logged because we changed pdf.js's stream converter (in bug 1633270) to also run for application/octet-stream, because lots of sites use that for their PDFs. That error message is from the stream converter to leave a note if we ended up not running PDF.js . It should probably be quieter by default, but IIRC two things are true here: (1) as a stream converter provider you cannot indicate you don't want the stream without returning failure (ie throwing an exception, in JS!) (2) any exceptions get logged by the consumer and/or xpconnect. So it'd need some massaging to make it not do that - I tried to make the message clearer than just an NS_ERROR_FAILURE without any text, but I guess atm it's still confusing, you're not the first to remark on it. I filed bug 1676374.

Comment 5

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1667787

Comment 6

[Brendan Dahl [:bdahl]]

Can we close this bug?

Comment 7

[Dave Townsend [:mossop]]

I'm not spotting an added test for application/octet-stream, did that get added? If so we can close this. If not we should add one, it is probably the most common not-known content type on the web.

Comment 8

[:Gijs (away/busy until Mar 10)]

(In reply to Dave Townsend [:mossop] from comment #7)

I'm not spotting an added test for application/octet-stream, did that get added? If so we can close this. If not we should add one, it is probably the most common not-known content type on the web.

Line 47 at https://hg.mozilla.org/mozilla-central/file/0a93a065053d33501d5272f5388f6b7919879343/uriloader/exthandler/tests/mochitest/browser_extension_correction.js#l47 checks that we don't force-add an extension for application/octet-stream