1773907 - installer download on java.com lacks file extension .exe

Status: VERIFIED FIXED

(Firefox :: File Handling, defect, P2)

Description

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Firefox 103.0a1 20220612095651 and 102.0b6 on Windows 8.1

Since bug 1770683 landed, the installer download lacks the file extension .exe.

Steps to reproduce:

1. Type java.com in the url bar and press enter.
2. Decline all cookies (other options might also reproduce the issue).
3. Click Download Java.
4. Click the new Download Java button.

Actual result:
File with name jre-8u333-windows-x64 downloaded.

Expected result::
File with name jre-8u333-windows-x64.exe downloaded.

Comment 1

[Neil Deakin]

The content type being sent is "application/x-sdlc" which some sources suggest can be associated with an executable on Windows. Firefox has no specific handling for this type, nor does my Windows machine at all, so it should be treated as a file of an unknown type.

For safety reasons, the code is probably now stripping off the .exe extension since it isn't known to be executable. Perhaps we shouldn't be doing this?

Tomorrow I can look in more detail exactly what is happening.

Comment 2

[Neil Deakin]

Several factors are involved here:

1. The filetype is application/x-sdlc which we don't recognize.
2. The request does not sent a content-disposition header so no filename is known.
3. Instead, we guess the filename from the url. However, since this is a POST request, we ignore the extension at https://searchfox.org/mozilla-central/rev/4519912ada795963299e4ba84c324adba762eff3/uriloader/exthandler/nsExternalHelperAppService.cpp#3228 We ignore the extension because it is likely to be a server script extension (cgi, php, etc)

The old code before bug 1770683 likely went though some other codepath.

Gijs, do you think we should modify this behaviour?

Comment 3

[David Cuthbert]

I suspect this is also affecting .dmg on MacOS files in certain cases, e.g.: https://www.docker.com/products/docker-desktop/
Clicking on a download button for "Mac with Apple Chip" sends the request to https://desktop.docker.com/mac/main/arm64/Docker.dmg?utm_source=docker&utm_medium=webreferral&utm_campaign=dd-smartbutton&utm_location=module

The file is saved as ~/Downloads/Docker instead of ~/Downloads/Docker.dmg.

This is Firefox 102.0b9 (64-bit) on MacOS 12.4, Apple M1 Max system (MacBookPro18,2).

Request and response headers:

GET /mac/main/arm64/Docker.dmg?utm_source=docker&utm_medium=webreferral&utm_campaign=dd-smartbutton&utm_location=module HTTP/2
Host: desktop.docker.com
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0) Gecko/20100101 Firefox/102.0
Accept: text/html,application/xhtml+xml,application/xml;q=0.9,image/avif,image/webp,*/*;q=0.8
Accept-Language: en-US,en;q=0.5
Accept-Encoding: gzip, deflate, br
DNT: 1
Connection: keep-alive
Referer: https://www.docker.com/
Upgrade-Insecure-Requests: 1
Sec-Fetch-Dest: document
Sec-Fetch-Mode: navigate
Sec-Fetch-Site: same-site
Sec-Fetch-User: ?1
Pragma: no-cache
Cache-Control: no-cache
TE: trailers

HTTP/2 200 OK
content-length: 581888600
last-modified: Thu, 16 Jun 2022 17:33:00 GMT
x-amz-version-id: vUQbkFSn_JGIhsJXa_M_NI63ZdkQFsag
server: AmazonS3
date: Fri, 17 Jun 2022 17:37:37 GMT
etag: "2698192956f427208f3438a8c166e0ab"
x-cache: Hit from cloudfront
via: 1.1 d8792dbd3191bbe722eba5b536b979c8.cloudfront.net (CloudFront)
x-amz-cf-pop: SEA19-C2
x-amz-cf-id: DUzyhObkD58ErGDwXg5aqdF2TUSlqN9tWxaIPiM2Iq4FxV_4hcQtfA==
age: 5803
X-Firefox-Spdy: h2

Comment 4

[:Gijs (he/him)]

(In reply to Neil Deakin from comment #2)

Several factors are involved here:

1. The filetype is application/x-sdlc which we don't recognize.
2. The request does not sent a content-disposition header so no filename is known.
3. Instead, we guess the filename from the url. However, since this is a POST request,

FWIW, I don't think this is a POST request, I'm only seeing GET requests.

we ignore the extension at https://searchfox.org/mozilla-central/rev/4519912ada795963299e4ba84c324adba762eff3/uriloader/exthandler/nsExternalHelperAppService.cpp#3228 We ignore the extension because it is likely to be a server script extension (cgi, php, etc)

I think we ignore it based on the fact that there's a query string.

The old code before bug 1770683 likely went though some other codepath.

Right, but this meant it "worked". ;-)

I dug into the behaviour of 101 using the mozilla-release branch for a bit.

It appears that the code here did this:

  if (url && aFileName.IsEmpty()) {
    if (aAllowURLExtension) {
      url->GetFileExtension(aExtension);
      UnescapeFragment(aExtension, url, aExtension);

      // Windows ignores terminating dots. So we have to as well, so
      // that our security checks do "the right thing"
      // In case the aExtension consisted only of the dot, the code below will
      // extract an aExtension from the filename
      aExtension.Trim(".", false);
    }

    // try to extract the file name from the url and use that as a first pass as
    // the leaf name of our temp file...
    nsAutoCString leafName;
    url->GetFileName(leafName);
    if (!leafName.IsEmpty()) {
      gotFileNameFromURI = true;
      rv = UnescapeFragment(leafName, url, aFileName);
      if (NS_FAILED(rv)) {
        CopyUTF8toUTF16(leafName, aFileName);  // use escaped name
      }
    }
  }

Specifically, it checks if we're "allowed" to use the extension from the URL before doing so. But it does not check that same variable to determine the leafname, and does not strip the extension from the leafname it determines like that. So we do still end up with the full filename from the URL in this case. We never do find a different extension (ie the extension variable here stays empty and attempting to find it using the mimetype application/x-sdlc also fails), so we just leave the one that the filename came with. The stack, fwiw, looks like this:

>	xul.dll!nsExternalHelperAppService::CreateListener(const nsTSubstring<char> & aMimeContentType, nsIRequest * aRequest, mozilla::dom::BrowsingContext * aContentContext, bool aForceSave, nsIInterfaceRequestor * aWindowContext, nsIStreamListener * * aStreamListener) Line 873	C++
 	xul.dll!mozilla::net::ParentProcessDocumentOpenInfo::TryExternalHelperApp(nsIExternalHelperAppService * aHelperAppService, nsIChannel * aChannel) Line 274	C++
 	xul.dll!nsDocumentOpenInfo::DispatchContent(nsIRequest * request) Line 461	C++
 	xul.dll!nsDocumentOpenInfo::OnStartRequest(nsIRequest * request) Line 155	C++
 	xul.dll!mozilla::net::ParentProcessDocumentOpenInfo::OnDocumentStartRequest(nsIRequest * request) Line 293	C++
 	xul.dll!mozilla::net::nsHttpChannel::CallOnStartRequest() Line 1642	C++
 	xul.dll!mozilla::net::nsHttpChannel::ContinueProcessNormal(nsresult rv) Line 2595	C++
 	xul.dll!mozilla::net::nsHttpChannel::ContinueProcessResponse3(nsresult rv) Line 2369	C++
 	xul.dll!mozilla::net::nsHttpChannel::ContinueProcessResponse2(nsresult rv) Line 2194	C++
 	xul.dll!mozilla::net::nsHttpChannel::ContinueProcessResponse1() Line 2167	C++
 	xul.dll!mozilla::net::nsHttpChannel::ProcessResponse() Line 2076	C++
 	xul.dll!mozilla::net::nsHttpChannel::OnStartRequest(nsIRequest * request) Line 6928	C++

(GetFilenameAndExtensionFromChannel gets inlined so that frame isn't there; the line numbers in nsExternalHelperAppService might be off a little because I added some printfs in places)

We no longer do this.

Gijs, do you think we should modify this behaviour?

I think we should keep the extension from the URL if we can't find a better one. I don't know what that looks like in practice. But I agree with comment #3 that it probably affects that case as well. This is unfortunate - though of course the websites could fix it by (a) using Content-Disposition: attachment headers with a filename, and (b) sending a more reasonable mimetype (for the dmg; I don't think there's a mimetype that Windows associates with exe files, unfortunately).

Neil, is this enough for you to go on?

Comment 5

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

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

Comment 6

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

[Tracking Requested - why for this release]: Many duplicates, see bug 1778322.

Comment 7

[Neil Deakin]

I have a fix here.

Comment 8

[Alice0775 White]

[Tracking Requested - why for this release]: file extension of the downloaded file is lost.

I can reproduce this issue with STR of comment#0 in Firefox102.0esr Windows10

Comment 9

[Arthur K. (he/him)]

With FF not running, what happens when you remove handlers.json from your profile and try STR again? Save your handlers.json file somewhere just in case. Just curious if it bears any remote resemblance to my issue in bug 1772140, but "The Upside Down" version.

Comment 10

[Ed Sabol]

I'm not sure if this is the correct bug or not. I think it is. If not, I apologize for the noise and would appreciate either a pointer to the correct bug or letting me know if I should open a new bug.

I have a web application that I've developed which does some scientific data analysis and generates a specialized file which the user can then download for further analysis by other software. My web application has an <a href="/relative/path/to/web/app?id=FOO"> tag. When a user clicks on that link, the web application at that path sends the following headers:

Content-Disposition: attachment; filename=some_prefix_FOO.pi
Content-Type: image/x-fits

and then the contents of the file in question. This web application has been working just fine like this for many years.

Since the Firefox 102.0 release, however, Firefox has been stripping the ".pi" file extension from these file downloads. Unfortunately, the local software that scientists use to do additional data analysis using the files downloaded from this web application requires the ".pi" file extension, so users have to rename the files after downloading them with Firefox 102+. Have received complaints about this. This doesn't happen with Chrome or Safari, and it didn't happen with Firefox prior to version 102.0, AFAIK.

I've tried changing the Content-Disposition: header to put double quotes around the filename, and I've tried adding a download="some_prefix_FOO.pi attribute to the <a> tag in the HTML. Neither change improved things. I hope this helps.

Comment 11

[Neil Deakin]

Attachment: Bug 1773907, don't clear the extension on the possible filename if we don't have one to replace it with, r=mhowell

The issue here is that ShouldModifyExtension returns Replace because the content type isn't known.

Comment 12

[ericd586]

Same issue here: https://www.onlyoffice.com/download-desktop.aspx?from=desktop in version 102.0.1. Link for the Windows installer downloads a blank file with no extension. Manually adding .exe to the file name when saving creates the correct full installation package.

Comment 13

[Ed Sabol]

My server-side web application is supplying a file name with a file extension using a Content-Disposition: header, Firefox 102+ is still stripping off the file extension. Is my situation a different bug? Not clear to me if this bug is only for downloads without a Content-Disposition: header.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ed5f1dbedf55
don't clear the extension on the possible filename if we don't have one to replace it with, r=mhowell

Comment 15

[Atila Butkovits]

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

Comment 16

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

The patch landed in nightly and beta is affected.
:enndeakin, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox103 to wontfix.

For more information, please visit auto_nag documentation.

Comment 17

[:Gijs (he/him)]

(In reply to Ed Sabol from comment #13)

My server-side web application is supplying a file name with a file extension using a Content-Disposition: header, Firefox 102+ is still stripping off the file extension. Is my situation a different bug? Not clear to me if this bug is only for downloads without a Content-Disposition: header.

Right now it'd be hard to tell if the issue you're seeing is the same or not - the easiest way would be for you to check to see if the issue still reproduces with today's nightly build - https://nightly.mozilla.org/ (check in about:support if the build id reflects today's date, July 12th). Can you report back with that?

Comment 18

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Verified fixed with Firefox 104.0a1 20220712093327 on Windows 8.1.

Comment 19

[Donal Meehan [:dmeehan]]

:Neil Denkin could you add a beta uplfit request for this?
This is the final beta week, it would be nice to get this into tonights beta build

Comment 20

[Neil Deakin]

Comment on attachment 9284802 [details]
Bug 1773907, don't clear the extension on the possible filename if we don't have one to replace it with, r=mhowell

Beta/Release Uplift Approval Request

• User impact if declined: The filename used when saving/dragging files is omitted. Has 7-8 other duplicates as well.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Only omits a step that incorrectly truncates the extension from the filename.
• String changes made/needed: None
• Is Android affected?: Yes

Comment 21

[Donal Meehan [:dmeehan]]

Comment on attachment 9284802 [details]
Bug 1773907, don't clear the extension on the possible filename if we don't have one to replace it with, r=mhowell

Approved for 103.0b8, thanks.

Comment 22

[Donal Meehan [:dmeehan]]

https://hg.mozilla.org/releases/mozilla-beta/rev/47cd23149e17

Comment 23

[Ed Sabol]

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

(In reply to Ed Sabol from comment #13)

My server-side web application is supplying a file name with a file extension using a Content-Disposition: header, Firefox 102+ is still stripping off the file extension. Is my situation a different bug? Not clear to me if this bug is only for downloads without a Content-Disposition: header.

Right now it'd be hard to tell if the issue you're seeing is the same or not - the easiest way would be for you to check to see if the issue still reproduces with today's nightly build - https://nightly.mozilla.org/ (check in about:support if the build id reflects today's date, July 12th). Can you report back with that?

I downloaded the latest nightly and tested my web application with it. The issue appears to be resolved in my testing with a Content-Disposition: header. Thanks, Firefox team! :thumbsup:

Comment 24

[Artem Russakovskii]

What about without Content-Disposition? Like https://bugzilla.mozilla.org/show_bug.cgi?id=1779135

Comment 26

[:Gijs (he/him)]

(In reply to Artem Russakovskii from comment #24)

What about without Content-Disposition? Like https://bugzilla.mozilla.org/show_bug.cgi?id=1779135

I think the same fix addressed your issue, so I duped it over - see my comment in that bug.

Comment 27

[Donal Meehan [:dmeehan]]

:Neil Denkin could you add an ESR uplift request for this?

Comment 28

[Monica Chiorean]

Reproduce the issue on Win10 using build 103.0a1(20220612214538).
Verified the issue on Win10 using builds Beta 103.0b8 (20220712185923) and Nightly 104.0a1(20220712215641). Verified same issue on Mac10.13 and Ubuntu20.4 but the corresponding .dmg and .tar.gz is being downloaded.

Comment 29

[Neil Deakin]

Comment on attachment 9284802 [details]
Bug 1773907, don't clear the extension on the possible filename if we don't have one to replace it with, r=mhowell

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration:
• User impact if declined: The filename used when saving/dragging files is omitted. Has 7-8 other duplicates as well.
• Fix Landed on Version: 103
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Only omits a step that incorrectly truncates the extension from the filename.

Comment 30

[Donal Meehan [:dmeehan]]

Comment on attachment 9284802 [details]
Bug 1773907, don't clear the extension on the possible filename if we don't have one to replace it with, r=mhowell

Approved for ESR102.1, thanks.

Comment 31

[Donal Meehan [:dmeehan]]

https://hg.mozilla.org/releases/mozilla-esr102/rev/878beab802e6

Comment 32

[Monica Chiorean]

Verified as fixed on Win7 x64 using build 102.1.0esr(20220718131551).