1624757 - Implement DownloadItem.finalUrl in the downloads API and let url be the URL before redirects

Status: NEW

(WebExtensions :: Compatibility, defect)

Description

[Masatoshi Kimura [:emk]]

... to align with Chrome 54+.

Currently Firefox sets the post-redirect URL to the url property and it does not implement the finalUrl property.

I had to use webRequest to develop my extension because I can't tell the pre-redirect URL without using webRequest.

Comment 1

[Tomislav Jovanovic :zombie]

(In reply to Masatoshi Kimura [:emk] from comment #0)

Currently Firefox sets the post-redirect URL to the url property

Are you saying this used to work differently?

Adding a finalUrl would be easy, but to be meaningful, we would also need to change the value of url to "original url", which might be surprising for existing extensions if any depend on it.

Nils or Mig, do you perhaps have an opinion on proposed changes here?

Comment 2

[Michel Gutierrez]

I checked that neither DownloadItem.url nor DownloadItem.originalUrl was used in DownloadHelper so i wouldn't be affected anyway.

But i think it is not a good idea to change the name of the property 'url' to 'originalUrl' because:

• it is the URL you requested the download from so it makes perfect sense to keep it named 'url'
• changing the property name would break extensions using it and you could be cursed by the developers
• it would add one more incompatibility with Chrome/Edge, and we, multi-browser extension developers, hate that

Comment 3

[Tomislav Jovanovic :zombie]

Sorry for the confusion, what I meant to say was that we would change the meaning of the url field to be the value of original url (before all redirects).

So the final state would be compatible with Chrome, and we would have url-after-redirects in the finalUrl field, and original-url-before-redirects in the url field -- which would still be a potentially backward incompatible, or at least surprising change for extensions which assume url is the url-after-redirects now.

Comment 4

[Masatoshi Kimura [:emk]]

(In reply to Tomislav Jovanovic :zombie from comment #1)

Are you saying this used to work differently?

No, url property had url-after-redirects from the day one in our implementation. It is incompatible with Chrome.

Adding a finalUrl would be easy, but to be meaningful, we would also need to change the value of url to "original url", which might be surprising for existing extensions if any depend on it.

Sure. I changed the summary to clarify that.

Comment 5

[Rob Wu [:robwu]]

FYI this was implemented in Chrome in https://bugs.chromium.org/p/chromium/issues/detail?id=620630

Contrary to comment 4, url is the pre-redirect URL, not the post-redirect one in Firefox.

Test case:

chrome.downloads.onChanged.addListener(d=>{
  console.log(d);
  chrome.downloads.search({id: d.id}, console.info)
});
chrome.downloads.download({url: "http://httpbingo.org/redirect-to?url=http%3A%2F%2Fexample.com%2F"});

In Firefox:

• url: "http://httpbingo.org/redirect-to?url=http%3A%2F%2Fexample.com%2F"

In Chrome:

• finalUrl: "http://example.com/"
• url: "http://httpbingo.org/redirect-to?url=http%3A%2F%2Fexample.com%2F"

Comment 6

[Masatoshi Kimura [:emk]]

(In reply to Rob Wu [:robwu] from comment #5)

Contrary to comment 4, url is the pre-redirect URL, not the post-redirect one in Firefox.

Oh, really? I filed this bug because I thought I could not get pre-redirect urls via downloads api in Firefox when I was implementing this addon. So I had to use a different way to get pre-redirect urls.

Comment 7

[Rob Wu [:robwu]]

Could you provide the exact reproduction steps? I'm seeing the behavior that I described in Firefox 84.0.1 with the STR from comment 5.
If you're seeing something different, then I'd like to know what had happened.

Comment 8

[Masatoshi Kimura [:emk]]

Ah, I got it. If addons initiated the download, url is correct as you said. If the user initiated the download by clicking a link, url is a post-redirect url.

Steps to reproduce:

1. Install your extension written in comment #5.
2. Navigate to https://emk.name/test/302.cgi.
3. Click the first link (?download) to download a file.
4. Open Multiprocess Browser Console or about:debugging > your addon > [Inspect] to inspect the download info.

Actual result:
url: "https://emk.name/symlink-1.07-src.zip"

Expected result:
url: "https://emk.name/test/302.cgi?download"

Comment 9

[Masatoshi Kimura [:emk]]

Since our current behavior is inconsistent, we could/should fix the bug without worrying about the compat risks.

Comment 10

[Masatoshi Kimura [:emk]]

Attachment: Bug 1624757 - Pass pre-redirect URL to nsITransfer::Init(WithBrowsingContext). r?robwu

Comment 11

[Rob Wu [:robwu]]

(In reply to Masatoshi Kimura [:emk] from comment #8)

Ah, I got it. If addons initiated the download, url is correct as you said. If the user initiated the download by clicking a link, url is a post-redirect url.

Confirmed, with the following STR:

chrome.downloads.onChanged.addListener(d=>{
  console.log(d);
  chrome.downloads.search({id: d.id}, console.info)
});
chrome.tabs.create({url: "http://httpbingo.org/redirect-to?url=http%3A%2F%2Fhttpbingo.org%2Fresponse-headers?Content-Disposition%3Dattachment"});

• expected: url: "http://httpbingo.org/redirect-to?url=http%3A%2F%2Fhttpbingo.org%2Fresponse-headers?Content-Disposition%3Dattachment"
• actual: url: "http://httpbingo.org/response-headers?Content-Disposition=attachment"

Comment 12

[Rob Wu [:robwu]]

There was an attempt to resolve the bug by changing the internals of the legacy downloader, to force "url" to be the pre-redirect URL instead of the final URL, in D102081. That approach is not ideal for its undesired side effects.

I provided some details on how this bug can be resolved in https://phabricator.services.mozilla.com/D102081#3309938

Comment 14

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

Clear a needinfo that is pending on an inactive user.

Inactive users most likely will not respond; if the missing information is essential and cannot be collected another way, the bug maybe should be closed as INCOMPLETE.

For more information, please visit auto_nag documentation.

Comment 15

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

Clear a needinfo that is pending on an inactive user.

Inactive users most likely will not respond; if the missing information is essential and cannot be collected another way, the bug maybe should be closed as INCOMPLETE.

For more information, please visit BugBot documentation.