Closed Bug 1676240 Opened 5 years ago Closed 5 years ago

Downloaded files get the extension dms

Categories

(Firefox :: File Handling, defect)

defect

Tracking

()

RESOLVED DUPLICATE of bug 1652520
Tracking Status
firefox-esr78 --- unaffected
firefox82 --- unaffected
firefox83 --- unaffected
firefox84 --- fixed

People

(Reporter: mossop, Unassigned)

References

(Regression)

Details

(Keywords: regression)

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=968296265399-compute@developer.gserviceaccount.com&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.

Flags: needinfo?(gijskruitbosch+bugs)

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.

Flags: needinfo?(gijskruitbosch+bugs) → needinfo?(dtownsend)

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.

Depends on: 1652520
Flags: needinfo?(dtownsend)

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.

(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.

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

Can we close this bug?

Flags: needinfo?(dtownsend)

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.

Flags: needinfo?(dtownsend) → needinfo?(gijskruitbosch+bugs)

(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

Status: NEW → RESOLVED
Closed: 5 years ago
Flags: needinfo?(gijskruitbosch+bugs)
Resolution: --- → DUPLICATE
Has Regression Range: --- → yes
You need to log in before you can comment on or make changes to this bug.