1771276 - Uncaught exception on Download.launch for object URLs

Status: VERIFIED FIXED

(Firefox :: Enterprise Policies, defect)

Description

[Shane Hughes [:aminomancer]]

Here's an error I'm seeing in the browser console since bug 1722050:

TypeError: URL constructor: null is not a valid URL. EnterprisePoliciesParent.jsm:440:20
    isExemptExecutableExtension resource://gre/modules/EnterprisePoliciesParent.jsm:440
    launchDownload resource://gre/modules/DownloadIntegration.jsm:740
    launch resource://gre/modules/DownloadCore.jsm:820
    _succeed resource://gre/modules/DownloadCore.jsm:616
    InterpretGeneratorResume self-hosted:1469
    AsyncFunctionNext self-hosted:679
    (Async: async)
    D_start resource://gre/modules/DownloadCore.jsm:591
    InterpretGeneratorResume self-hosted:1469
    AsyncFunctionNext self-hosted:679
    (Async: async)
    D_start resource://gre/modules/DownloadCore.jsm:595
    _nsITransferInitInternal resource://gre/modules/DownloadLegacy.jsm:431
    (Async: promise callback)
    _nsITransferInitInternal resource://gre/modules/DownloadLegacy.jsm:424
    initWithBrowsingContext resource://gre/modules/DownloadLegacy.jsm:316
    onOK resource://gre/modules/HelperAppDlg.jsm:1101
    handleEvent resource://gre/modules/HelperAppDlg.jsm:1148
    _fireButtonEvent chrome://global/content/elements/dialog.js:495
    _doButtonCommand chrome://global/content/elements/dialog.js:474
    _handleButtonCommand chrome://global/content/elements/dialog.js:468
    _handleButtonCommand self-hosted:1162
    (Async: EventListener.handleEvent)
    _configureButtons chrome://global/content/elements/dialog.js:369
    connectedCallback chrome://global/content/elements/dialog.js:163
    <anonymous> chrome://global/content/customElements.js:217
    (Async: EventListener.handleEvent)
    <anonymous> chrome://global/content/customElements.js:209
    <anonymous> chrome://global/content/customElements.js:861
    observe resource://gre/modules/CustomElementsListener.jsm:26

So what seems to be happening is I'm downloading from a blob URL, for example, and DownloadIntegration.launchDownload is trying to create a new URL object from the source URL and pass url.origin to isExemptExecutableExtension.

The instantiation of the URL object and the invocation of isExemptExecutableExtension are wrapped in a try block, but the exception happens within isExemptExecutableExtension, when hostname is defined.

A simple fix is to check that origin exists above the hostname declaration, or even to check that url.origin exists before invoking isExemptExecutableExtension. But it would be safer to just call new URL(origin) in a try block, since it's conceivable that future code might pass origins that fail for other reasons. Currently I only know of cases where origin is null or undefined, but I guess it's possible that some caller might pass invalid characters or something. That pattern also seems pretty common in the Firefox code base, so that seems best.

STR

1. Try downloading a file from an object URL, e.g., an icon from here
2. Once it's finished, check the browser console

Expected

No errors logged in console

Actual

There's an unhandled exception, the stack trace is pasted above

Comment 1

[Shane Hughes [:aminomancer]]

Attachment: Bug 1771276 - Fix an error in EnterprisePoliciesManager. r=mkaply

Services.policies.isExemptExecutableExtension will throw if passed a
falsy origin, which causes errors to be logged in the console when the
user downloads a file with an object URL source. Technically, it can
also throw if the URL is invalid for a variety of other reasons, so I
just wrapped the URL construction in a try...catch statement.

Comment 2

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

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

Comment 3

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

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

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/cd98e6c5f7fc
Fix an error in EnterprisePoliciesManager. r=mkaply

Comment 5

[Marian-Vasile Laza]

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

Comment 6

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

The patch landed in nightly and beta is affected.
:aminomancer, is this bug important enough to require an uplift?
If not please set status_beta to wontfix.

For more information, please visit auto_nag documentation.

Comment 7

[Mike Kaply [:mkaply]]

Comment on attachment 9278308 [details]
Bug 1771276 - Fix an error in EnterprisePoliciesManager. r=mkaply

Beta/Release Uplift Approval Request

• User impact if declined: Some downloads produce an error
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: No
• 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): Just adds a try catch.
• String changes made/needed:
• Is Android affected?: No

Comment 8

[Pascal Chevrel:pascalc (PTO until April 26)]

Comment on attachment 9278308 [details]
Bug 1771276 - Fix an error in EnterprisePoliciesManager. r=mkaply

Approved for 102 beta 5, thanks.

Comment 9

[Pascal Chevrel:pascalc (PTO until April 26)]

https://hg.mozilla.org/releases/mozilla-beta/rev/635c2b619ed3

Comment 10

[Shane Hughes [:aminomancer]]

Thanks!

Comment 11

[Timea Zsoldos [:zstimi/tzsoldos], Desktop QA]

I have reproduced this issue using Firefox 102.0a1 (2022.05.26) on Win 10 x64, downloading an icon from this link in the Browser console caught an isExemptExecutableExtension error.
I can confirm this issue is fixed, I verified using Firefox 102.0b5 and 103.0a1 on Windows 10 x64, Ubuntu 18.04 x64 and macOS 10.15.