1606300 - When saving a video with a URL Encoded filename, it's saved with the encoded (rather than decoded) file name

Status: RESOLVED FIXED

(Toolkit :: Downloads API, defect, P1)

Description

[strannick]

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:71.0) Gecko/20100101 Firefox/71.0

Steps to reproduce:

1. go to URL https://files.rokiroki.ru/ff/Nemnogo%20Nervno%20-%20%D0%98%D1%81%D1%82%D0%B8%D0%BD%D0%BD%D0%BE%D0%B5%20%D0%B2%D0%BE%D0%BB%D1%88%D0%B5%D0%B1%D1%81%D1%82%D0%B2%D0%BE.mp4

2. press "Save video as…"

Actual results:

the filename suggested by Firefox looks like:

Nemnogo%20Nervno%20-%20%D0%98%D1%81%D1%82%D0%B8%D0%BD%D0%BD%D0%BE%D0%B5%20%D0%B2%D0%BE%D0%BB%D1%88%D0%B5%D0%B1%D1%81%D1%82%D0%B2%D0%BE.mp4

Expected results:

the filename suggested by Firefox looks like:

Nemnogo Nervno - Истинное волшебство.mp4

Comment 1

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

Bugbug thinks this bug should belong to this component, but please revert this change in case of error.

Comment 2

[strannick]

Example:

1. URL https://files.rokiroki.ru/Nemnogo%20Nervno%20-%20%D0%98%D1%81%D1%82%D0%B8%D0%BD%D0%BD%D0%BE%D0%B5%20%D0%B2%D0%BE%D0%BB%D1%88%D0%B5%D0%B1%D1%81%D1%82%D0%B2%D0%BE.mp4 — all right

2. URL https://files.rokiroki.ru/ff/Nemnogo%20Nervno%20-%20%D0%98%D1%81%D1%82%D0%B8%D0%BD%D0%BD%D0%BE%D0%B5%20%D0%B2%D0%BE%D0%BB%D1%88%D0%B5%D0%B1%D1%81%D1%82%D0%B2%D0%BE.mp4 — trouble.

What the difference?
in the second URL web-server sends HTTP-header:
Content-Disposition 'inline; filename="Nemnogo%20Nervno%20-%20%D0%98%D1%81%D1%82%D0%B8%D0%BD%D0%BD%D0%BE%D0%B5%20%D0%B2%D0%BE%D0%BB%D1%88%D0%B5%D0%B1%D1%81%D1%82%D0%B2%D0%BE.mp4"';

I think Firefox must decode URL Encoded filename from Content-Disposition header.

Comment 3

[Bryce Seager van Dyk [:bryce] (he/him) - Not reading bugmail]

I don't think this is a media issue, moving to Firefox in the hope it can find the correct home.

Comment 4

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

The priority flag is not set for this bug.
:Dolske, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 5

[:Gijs (he/him)]

Valentin, do you know what the current spec is for this, and is URL encoding normal for the contents of this header (see comment #2) ? (I believe we rely on https://searchfox.org/mozilla-central/rev/ad6234148892bc29bf08d736604ac71925138040/netwerk/base/nsNetUtil.cpp#2656 so moving to networking...)

Comment 6

[Valentin Gosu [:valentin] (he/him)]

(In reply to :Gijs (he/him) from comment #5)

Valentin, do you know what the current spec is for this, and is URL encoding normal for the contents of this header (see comment #2) ? (I believe we rely on https://searchfox.org/mozilla-central/rev/ad6234148892bc29bf08d736604ac71925138040/netwerk/base/nsNetUtil.cpp#2656 so moving to networking...)

I haven't actually found where we actually get the filename from but returning an arbitrary string from HttpBaseChannel::GetContentDispositionFilename or NS_GetFilenameFromDisposition doesn't change the suggested filename.

Gijs, can you happen to know where that happens?

Comment 7

[:Gijs (he/him)]

(In reply to Valentin Gosu [:valentin] (he/him) from comment #6)

(In reply to :Gijs (he/him) from comment #5)

Valentin, do you know what the current spec is for this, and is URL encoding normal for the contents of this header (see comment #2) ? (I believe we rely on https://searchfox.org/mozilla-central/rev/ad6234148892bc29bf08d736604ac71925138040/netwerk/base/nsNetUtil.cpp#2656 so moving to networking...)

I haven't actually found where we actually get the filename from but returning an arbitrary string from HttpBaseChannel::GetContentDispositionFilename or NS_GetFilenameFromDisposition doesn't change the suggested filename.

Gijs, can you happen to know where that happens?

Err, embarrassingly, no, I'm not sure off-hand - I thought it was from https://searchfox.org/mozilla-central/rev/2e355fa82aaa87e8424a9927c8136be184eeb6c7/uriloader/exthandler/nsExternalHelperAppService.cpp#714 ?

I also just realized I cannot reproduce this in nightly right now. Perhaps I could at the time, and something has changed in the website? Reporter, are you still seeing this problem? Can you try with a build from https://nightly.mozilla.org/ and confirm whether it shows the same behaviour?

Comment 8

[strannick]

I'm download nightly from here:
https://download.mozilla.org/?product=firefox-nightly-latest-l10n-ssl&os=linux64&lang=ru

It is a firefox-74.0a1.ru.linux-x86_64.tar.bz2

Next, I going to links and check it.

1. All right, like ordinary FF.
2. Trouble, like ordinary FF.

And I see somethung, what may help you to see whats wrong. Looks here:

P.S. if images not shows:

1. All right. Video saves with right filename: https://files.rokiroki.ru/ff-all-right.png
2. Wrong. Video saves with bad filename: https://files.rokiroki.ru/ff/ff-bad-luck.png

Hope, it helps.

Comment 9

[Valentin Gosu [:valentin] (he/him)]

It's still happening for me with both STR in comment 2 and comment 8.

Gijs, I've found the code that suggests the filename for saveAs - it's contentAreaUtils.js::getDefaultFileName

The codepath is either internalSave > initFileInfo > getDefaultFileName or DownloadURL > initFileInfo > getDefaultFileName
We seem to be unescaping it in some cases but not others

Comment 10

[:Gijs (he/him)]

OK, it seems there are two codepaths and they're both broken.

If you open https://files.rokiroki.ru/ff/Nemnogo%20Nervno%20-%20%D0%98%D1%81%D1%82%D0%B8%D0%BD%D0%BD%D0%BE%D0%B5%20%D0%B2%D0%BE%D0%BB%D1%88%D0%B5%D0%B1%D1%81%D1%82%D0%B2%D0%BE.mp4

and:

1. hit accel-s / save page as shortcut.

This hits getDefaultFileName but actually takes the passed aContentDisposition, reads the header parameter using the mime header param service ( https://searchfox.org/mozilla-central/rev/2e355fa82aaa87e8424a9927c8136be184eeb6c7/toolkit/content/contentAreaUtils.js#1179-1201 ) and returns that, without URL escaping.

2. right click the video, click "save video as"

This goes via saveMedia in nsContextMenu.js and uses the webbrowserpersist code, which hits the nsNetUtil code. If I return garbage in there, both the tab title and the offered filename in here use that garbage.

Comment 11

[:Gijs (he/him)]

Attachment: Bug 1606300 - escape filenames in tab titles, save as... dialog and as used by webbrowserpersist, r?valentin

The getDefaultFileName helper function does its own filename parsing using header info
it has previously 'manually' retrieved, so it needs to manually unescape its input.

Meanwhile, NS_GetFilenameFromDisposition is used by webbrowserpersist and for
the title of documents, and so updating that helps ensure better UI as well as
correct filename suggestions when saving from the context menu.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ab6076376754
escape filenames in tab titles, save as... dialog and as used by webbrowserpersist, r=valentin

Comment 13

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/ab6076376754