Closed Bug 1382323 Opened 7 years ago Closed 7 years ago

Firefox 54 on Fedora 26 doesn't launch custom protocol handler

Categories

(Core :: Security: Process Sandboxing, defect, P1)

Product:
Core
Component:
Security: Process Sandboxing
Version:
54 Branch
Platform:
x86
Linux
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla58
Tracking Flags:
Tracking Status
firefox-esr52 --- unaffected
firefox55 --- wontfix
firefox56 --- wontfix
firefox57 --- wontfix
firefox58 --- fixed

People

(Reporter: firsov.f5, Assigned: gcp)

References

Details

(Keywords: regression, Whiteboard: sb+)

Attachments

(3 files)

1382323 console output.txt
6.50 KB, text/plain
Details
Sandboxing Log from Nightly
55.67 KB, text/plain
Details
Bug 1382323 - Remote OSProtocolHandlerExists.
59 bytes, text/x-review-board-request
jld
: review+
ritu
: approval-mozilla-beta-
Details 
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_11_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/59.0.3071.115 Safari/537.36

Steps to reproduce:

>navigator.userAgent
"Mozilla/5.0 (X11; Linux x86_64; rv:54.0) Gecko/20100101 Firefox/54.0"

Firefox 54 doesn't launch custom protocol handler if Firefox is run second+ time (i.e. firefox profile already exists) on Fedora 25/26.

This is a regression as Firefox 53 works fine with same repro steps.
Ubuntu 17.04 is NOT affected.

Repro steps #1:
0. a. Take clean Fedora 25 or 26 machine.
0. b. Ensure that Firefox 54.0 or 54.0.1 is installed.
0. c. Launch and close Firefox.
1. Install any protocol handler (e.g. Telegram Messesnger https://telegram.org/)
2. Open this repro page in Firefox 54:
[vmware@localhost ~]$ cat test.html 
<body>
<a href="tg://addstickers?set=LosSimpsonitos">tg://addstickers?set=LosSimpsonitos</a>
</body>
3. Click on the link

Repro steps #2:
0. Execute steps 0-1 from the previous block
1. Open JS Console (F12)
2. type:
window.location.href = "tg://something"


If user cleans up the profile:
rm -rf ~/.mozilla/
Protocol handlers start to work until Firefox is closed and launched again.



Actual results:

Telegram Client is NOT launched in both cases.

#1:
Firefox says:

The address wasn’t understood
Firefox doesn’t know how to open this address, because one of the following protocols (tg) isn’t associated with any program or is not allowed in this context.

    You might need to install other software to open this address.

#2:

[Exception... "The URI scheme corresponds to an unknown protocol handler"  nsresult: "0x804b0012 (NS_ERROR_UNKNOWN_PROTOCOL)"  location: "JS frame :: debugger eval code :: <TOP_LEVEL> :: line 1"  data: no]



Expected results:

Telegram client is launched.

Firefox 53 works fine on Fedora 25/26. Ubuntu 17.04 with Firefox 54.0 also works fine.
OS: Unspecified → Linux
Hardware: Unspecified → x86
This is reproducible with ANY non-default protocol handler.
Component: Untriaged → File Handling
Summary: Firefox 54 on Fedora doesn't launch custom protocol handler → Firefox 54 on Fedora 26 doesn't launch custom protocol handler
I'd have thought of bug 1287658, but this landed in Firefox 55 so it should not have affected Firefox 54.

The next step here is to get a defined regression range.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: regression, regressionwindow-wanted
Priority: -- → P5
Hmm. I just tried this on today's nightly on Linux and OSX 10.12, and it seems to be working fine with those STR. I made my own dummy "burger" protocol to my own web server:

    <body>
    <script>
    function register() {
    navigator.registerProtocolHandler("burger",
                                      "http://my.fqdn.com/%s",
                                      "Burger handler");
    }
    </script>
    <a href="#" onclick="register()">Register</a><br/><br/>
    <a href="burger://test">Test</a>
    </body>

I can click the link in that file, and I can also F12 and run the redirect-change with:

    window.location.href = "burger://something"

I also tried on 54.0.1, with no problem. I wonder if it's related to Telegram specifically somehow?
(In reply to Thomas Wisniewski from comment #3)
> Hmm. I just tried this on today's nightly on Linux and OSX 10.12, and it
> seems to be working fine with those STR. I made my own dummy "burger"
> protocol to my own web server:
> 
>     <body>
>     <script>
>     function register() {
>     navigator.registerProtocolHandler("burger",
>                                       "http://my.fqdn.com/%s",
>                                       "Burger handler");
>     }
>     </script>
>     <a href="#" onclick="register()">Register</a><br/><br/>
>     <a href="burger://test">Test</a>
>     </body>
> 
> I can click the link in that file, and I can also F12 and run the
> redirect-change with:
> 
>     window.location.href = "burger://something"
> 
> I also tried on 54.0.1, with no problem. I wonder if it's related to
> Telegram specifically somehow?

This is Fedora 26 specific (however, sometimes it also reproducible on Fedora 25) issue. On mac os x it works fine for me as well.
In fact, it doesn't work with ANY custom protocol handler. The Telegram was just an example as commonly used and known.
> This is Fedora 26 specific (however, sometimes it also reproducible on
> Fedora 25) issue. On mac os x it works fine for me as well.
> In fact, it doesn't work with ANY custom protocol handler. The Telegram was
> just an example as commonly used and known.

And please note that work fine on first Firefox launch. An only subsequent launch of Firefox has the issue. Please take a look at the steps.
May be removing a profile and re-running Firefox multiple times will expose the issue for you as well.
User Agent 	Mozilla/5.0 (X11; Linux x86_64; rv:57.0) Gecko/20100101 Firefox/57.0
Firefox: 57.0a1, Build ID 	20170821100350

I have tested this issue on latest Firefox release (55.0.2) and latest Nightly (57.0a1) build. I have managed to reproduce it, but only if I followed the STR #2 from the description, and only on the Nightly channel. Considering this, using the Mozregression tool, I managed to find a regression window. 

Last good revision: 3e9a2031152fa07b088d0cb5e168eb53a2c882c0
First bad revision: c838d2546cadd65bf8d5579db20a268c8b6e4b87
Pushlog:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=3e9a2031152fa07b088d0cb5e168eb53a2c882c0&tochange=c838d2546cadd65bf8d5579db20a268c8b6e4b87

From the pushlog, it seems that the bug 1289718 has caused this.

Gian-Carlo, can you please take a look at this?
Blocks: 1289718
Component: File Handling → Security: Process Sandboxing
Flags: needinfo?(gpascutto)
Keywords: regressionwindow-wanted
Product: Firefox → Core
Possibly related to bug 1292249.
Flags: needinfo?(gpascutto)
See Also: → 1292249
Assignee: nobody → gpascutto
Flags: needinfo?(gpascutto)
Priority: P5 → P1
I'm not able to reproduce this. I set up a fresh Fedora Core 26 VM, with the bundled Firefox 55.0.2, and reproduction steps similar to comment 3 work fine, as do some random add-on based protocol handlers.

I'm not able to test Telegram as the Linux version of that app doesn't seem to want to install itself without giving it a valid phone number.

You might try launching Firefox with:
MOZ_SANDBOX_LOGGING=1 firefox

The resulting log may reveal what is going wrong.

If there are STR that don't require sending phone numbers to third parties I can try that as well.
Flags: needinfo?(gpascutto) → needinfo?(marius.coman)
Marius, can you with that logging setting?

Did you use Fedora Core 26 for testing, or was the issue distro-independent?
(In reply to Gian-Carlo Pascutto [:gcp] from comment #9)
> Marius, can you with that logging setting?
> 
> Did you use Fedora Core 26 for testing, or was the issue distro-independent?

I still can reproduce this on Fedora 26, Firefox 55.0.2.

You don't need to have a phone number to repro this, just run telegram once, after that xdg-open properly opens 'tg://test'.

My repro:
# xdg-open works fine:
[vmware@localhost ~]$ xdg-open tg://test
[vmware@localhost ~]$ Fontconfig error: "/etc/fonts/conf.d/10-scale-bitmap-fonts.conf", line 72: non-double matrix element
Fontconfig error: "/etc/fonts/conf.d/10-scale-bitmap-fonts.conf", line 72: non-double matrix element
Fontconfig warning: "/etc/fonts/conf.d/10-scale-bitmap-fonts.conf", line 80: saw unknown, expected number
Fontconfig warning: "/etc/fonts/conf.d/65-0-lohit-bengali.conf", line 32: unknown element "langset"
Fontconfig warning: "/etc/fonts/conf.d/69-gnu-free-sans.conf", line 24: unknown element "langset"
Fontconfig warning: "/etc/fonts/conf.d/69-gnu-free-serif.conf", line 24: unknown element "langset"
Fontconfig error: "/etc/fonts/conf.d/10-scale-bitmap-fonts.conf", line 72: non-double matrix element
Fontconfig error: "/etc/fonts/conf.d/10-scale-bitmap-fonts.conf", line 72: non-double matrix element
Fontconfig warning: "/etc/fonts/conf.d/10-scale-bitmap-fonts.conf", line 80: saw unknown, expected number
Fontconfig warning: "/etc/fonts/conf.d/65-0-lohit-bengali.conf", line 32: unknown element "langset"
Fontconfig warning: "/etc/fonts/conf.d/69-gnu-free-sans.conf", line 24: unknown element "langset"
Fontconfig warning: "/etc/fonts/conf.d/69-gnu-free-serif.conf", line 24: unknown element "langset"
Fontconfig error: "/etc/fonts/conf.d/10-scale-bitmap-fonts.conf", line 72: non-double matrix element
Fontconfig error: "/etc/fonts/conf.d/10-scale-bitmap-fonts.conf", line 72: non-double matrix element
Fontconfig warning: "/etc/fonts/conf.d/10-scale-bitmap-fonts.conf", line 80: saw unknown, expected number
Fontconfig warning: "/etc/fonts/conf.d/65-0-lohit-bengali.conf", line 32: unknown element "langset"
Fontconfig warning: "/etc/fonts/conf.d/69-gnu-free-sans.conf", line 24: unknown element "langset"
Fontconfig warning: "/etc/fonts/conf.d/69-gnu-free-serif.conf", line 24: unknown element "langset"
Fontconfig error: "/etc/fonts/conf.d/10-scale-bitmap-fonts.conf", line 72: non-double matrix element
Fontconfig error: "/etc/fonts/conf.d/10-scale-bitmap-fonts.conf", line 72: non-double matrix element
Fontconfig warning: "/etc/fonts/conf.d/10-scale-bitmap-fonts.conf", line 80: saw unknown, expected number
Fontconfig warning: "/etc/fonts/conf.d/65-0-lohit-bengali.conf", line 32: unknown element "langset"
Fontconfig warning: "/etc/fonts/conf.d/69-gnu-free-sans.conf", line 24: unknown element "langset"
Fontconfig warning: "/etc/fonts/conf.d/69-gnu-free-serif.conf", line 24: unknown element "langset"
^C
[vmware@localhost ~]$ ^C

# try with firefox:
[vmware@localhost ~]$ firefox -v
Mozilla Firefox 55.0.2
# let's start firefox and type in console 'window.location.href = tg://'
[vmware@localhost ~]$ MOZ_SANDBOX_LOGGING=1 firefox &> firefox.tx
[vmware@localhost ~]$ tail firefox.txt 
Sandbox: SandboxBroker: denied op=1 rflags=5 perms=3 path=/usr for pid=17799 error="No such file or directory"
Sandbox: Rejected errno -13 op 1 flags 05 path /usr
Sandbox: SandboxBroker: denied op=1 rflags=5 perms=35 path=/ for pid=17799 error="No such file or directory"
Sandbox: Rejected errno -13 op 1 flags 05 path /
Sandbox: Rejected errno -2 op 0 flags 00 path /usr/share/applications/defaults.list
Sandbox: SandboxBroker: denied op=1 rflags=1 perms=3 path=/home/vmware/Downloads/tsetup.1.1.10/Telegram/Telegram for pid=17799 error="No such file or directory"
Sandbox: Rejected errno -13 op 1 flags 01 path /home/vmware/Downloads/tsetup.1.1.10/Telegram/Telegram
Sandbox: SandboxBroker: denied op=1 rflags=1 perms=3 path=/home/vmware/Downloads/tsetup.1.1.10/Telegram/Telegram for pid=17799 error="No such file or directory"
Sandbox: Rejected errno -13 op 1 flags 01 path /home/vmware/Downloads/tsetup.1.1.10/Telegram/Telegram
Sandbox: EOF from pid 17799
[vmware@localhost ~]$
# file not found, let's check if it exists
[vmware@localhost ~]$ file /home/vmware/Downloads/tsetup.1.1.10/Telegram/Telegram
/home/vmware/Downloads/tsetup.1.1.10/Telegram/Telegram: ELF 64-bit LSB executable, x86-64, version 1 (SYSV), dynamically linked, interpreter /lib64/ld-linux-x86-64.so.2, for GNU/Linux 2.6.24, BuildID[sha1]=d884b6d7d4636044fbcdaf04bba6dc2489e414d0, stripped

So, for some reason, firefox can't find the file.

Gian-Carlo, Please let me know if you need something else.

--
Ivan
Full log is here https://pastebin.com/FRfLcspY
Ok, thanks for the log. It looks like this may be a duplicate of bug 1386279.
FYI, the `error="No such file or directory"` isn't meaningful; this was fixed in 56 (bug 1308400).
(In reply to Jed Davis [:jld] (⏰UTC-6) from comment #13)
> FYI, the `error="No such file or directory"` isn't meaningful; this was
> fixed in 56 (bug 1308400).

Reading that again, I should maybe clarify: the thing that was fixed in bug 1308400 was just the misleading “error=…” on the end of the verbose log message.  With respect to the actual bug here, I agree with comment #12.
Hi Gian-Carlo

The issue is only reproducible with STR #2. I've successfully reproduced it on Fedora 23, because I don't have access to Fedora 26.
Regarding the need for a telephone number, you don't need a telephone number to use Telegram Messenger, just open the program and then close it.
I've attached the console output right after I've entered <window.location.href = "tg://something"> in the browser console.
Flags: needinfo?(marius.coman)
Thanks. Indeed I did not realize Telegram immediately installs the protocol handler.

I failed to reproduce this initially but after wiping my profile it worked (i.e. failed). I managed to reproduce with Nightly as well, which means it's not a dupe of bug 1386279.
Sandbox: SandboxBroker: denied op=access rflags=1 perms=0 path=/home/morbo/Downloads/Telegram/Telegram for pid=5034
Sandbox: Failed errno -13 op 1 flags 01 path /home/morbo/Downloads/Telegram/Telegram
Sandbox: SandboxBroker: denied op=access rflags=1 perms=0 path=/home/morbo/Downloads/Telegram/Telegram for pid=5034
Sandbox: Failed errno -13 op 1 flags 01 path /home/morbo/Downloads/Telegram/Telegram

This is consistent with comment 10. The reason it's not bug 1386279 is that /home/user/Downloads or /home/vmware/Downloads/ is not a whitelisted path for the content process, so we're not allowing access to the Telegram binary to it. Whatever is doing that check will need to be remoted.
I managed to get a stack for what's probing Telegram here:

#04: mozilla::SandboxBrokerClient::DoCall(mozilla::SandboxBrokerCommon::Request const*, char const*, char const*, void*, bool) (/home/morbo/hg/firefox/security/sandbox/linux/SandboxBrokerClient.cpp:68)
#05: mozilla::SandboxBrokerClient::Access(char const*, int) (crtstuff.c:?)
#06: mozilla::ContentSandboxPolicy::AccessTrap(sandbox::arch_seccomp_data const&, void*) (/home/morbo/hg/firefox/security/sandbox/linux/SandboxFilter.cpp:386)
#07: sandbox::Trap::SigSys(int, siginfo_t*, ucontext*) (/home/morbo/hg/firefox/security/sandbox/chromium/sandbox/linux/seccomp-bpf/trap.cc:244)
#08: sandbox::Trap::SigSysAction(int, siginfo_t*, void*) (crtstuff.c:?)
#09: mozilla::SigSysHandler(int, siginfo_t*, void*) (/home/morbo/hg/firefox/security/sandbox/linux/Sandbox.cpp:140)
#10: ??? (/lib64/libpthread.so.0)
#11: __access (/usr/src/debug/glibc-2.25-49-gbc5ace67fe/io/../sysdeps/unix/sysv/linux/access.c:42)
#12: g_file_test (/usr/src/debug/glib-2.52.3/glib/gfileutils.c:418 (discriminator 1))
#13: g_find_program_in_path (/usr/src/debug/glib-2.52.3/glib/gutils.c:357)
#14: g_desktop_app_info_load_from_keyfile (/usr/src/debug/glib-2.52.3/gio/gdesktopappinfo.c:1690)
#15: g_desktop_app_info_new_from_filename (/usr/src/debug/glib-2.52.3/gio/gdesktopappinfo.c:1831)
#16: g_app_info_get_default_for_type (/usr/src/debug/glib-2.52.3/gio/gdesktopappinfo.c:4079)
#17: g_app_info_get_default_for_uri_scheme (/usr/src/debug/glib-2.52.3/gio/gdesktopappinfo.c:4126)
#18: nsGIOService::GetAppForURIScheme(nsACString const&, nsIGIOMimeApp**) (/home/morbo/hg/firefox/toolkit/system/gnome/nsGIOService.cpp:275)
#19: nsGNOMERegistry::HandlerExists(char const*) (/home/morbo/hg/firefox/uriloader/exthandler/unix/nsGNOMERegistry.cpp:22)
#20: nsOSHelperAppService::OSProtocolHandlerExists(char const*, bool*) (/home/morbo/hg/firefox/uriloader/exthandler/unix/nsOSHelperAppService.cpp:1150)
#21: nsOSHelperAppService::GetProtocolHandlerInfoFromOS(nsACString const&, bool*, nsIHandlerInfo**) (crtstuff.c:?)
#22: nsExternalHelperAppService::GetProtocolHandlerInfo(nsACString const&, nsIHandlerInfo**) (crtstuff.c:?)
#23: nsExternalHelperAppService::ExternalProtocolHandlerExists(char const*, bool*) (crtstuff.c:?)
#24: nsDefaultURIFixup::GetFixupURIInfo(nsACString const&, unsigned int, nsIInputStream**, nsIURIFixupInfo**) (/home/morbo/hg/firefox/docshell/base/nsDefaultURIFixup.cpp:314)
#25: nsDocShell::LoadURIWithOptions(char16_t const*, unsigned int, nsIURI*, unsigned int, nsIInputStream*, nsIInputStream*, nsIURI*, nsIPrincipal*) (crtstuff.c:?)
#26: NS_InvokeByIndex (/home/morbo/hg/firefox/xpcom/reflect/xptcall/md/unix/xptcinvoke_asm_x86_64_unix.S:110)
#27: CallMethodHelper::Invoke() (/home/morbo/hg/firefox/js/xpconnect/src/XPCWrappedNative.cpp:1997)
#28: CallMethodHelper::Call() (crtstuff.c:?)
#29: XPCWrappedNative::CallMethod(XPCCallContext&, XPCWrappedNative::CallMode) (crtstuff.c:?)
#30: XPC_WN_CallMethod(JSContext*, unsigned int, JS::Value*) (crtstuff.c:?)
#31: js::CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) (/home/morbo/hg/firefox/js/src/jscntxtinlines.h:293)
#32: js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) (crtstuff.c:?)
#33: InternalCall(JSContext*, js::AnyInvokeArgs const&) (Interpreter.cpp:?)
#34: js::CallFromStack(JSContext*, JS::CallArgs const&) (crtstuff.c:?)
#35: Interpret(JSContext*, js::RunState&) (Interpreter.cpp:?)
#36: js::RunScript(JSContext*, js::RunState&) (crtstuff.c:?)
#37: js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) (crtstuff.c:?)
#38: InternalCall(JSContext*, js::AnyInvokeArgs const&) (Interpreter.cpp:?)
#39: js::CallFromStack(JSContext*, JS::CallArgs const&) (crtstuff.c:?)
#40: Interpret(JSContext*, js::RunState&) (Interpreter.cpp:?)
#41: js::RunScript(JSContext*, js::RunState&) (crtstuff.c:?)
#42: js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) (crtstuff.c:?)
#43: InternalCall(JSContext*, js::AnyInvokeArgs const&) (Interpreter.cpp:?)
#44: js::CallFromStack(JSContext*, JS::CallArgs const&) (crtstuff.c:?)
#45: Interpret(JSContext*, js::RunState&) (Interpreter.cpp:?)
#46: js::RunScript(JSContext*, js::RunState&) (crtstuff.c:?)
#47: js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) (crtstuff.c:?)
#48: InternalCall(JSContext*, js::AnyInvokeArgs const&) (Interpreter.cpp:?)
#49: js::CallFromStack(JSContext*, JS::CallArgs const&) (crtstuff.c:?)
#50: Interpret(JSContext*, js::RunState&) (Interpreter.cpp:?)
#51: js::RunScript(JSContext*, js::RunState&) (crtstuff.c:?)
#52: js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) (crtstuff.c:?)
#53: InternalCall(JSContext*, js::AnyInvokeArgs const&) (Interpreter.cpp:?)
#54: js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) (crtstuff.c:?)
#55: JS_CallFunctionValue(JSContext*, JS::Handle<JSObject*>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) (/home/morbo/hg/firefox/js/src/jsapi.cpp:2878)
#56: nsFrameMessageManager::ReceiveMessage(nsISupports*, nsIFrameLoader*, bool, nsAString const&, bool, mozilla::dom::ipc::StructuredCloneData*, mozilla::jsipc::CpowHolder*, nsIPrincipal*, nsTArray<mozilla::dom::ipc::StructuredCloneData>*) (/home/morbo/hg/firefox/dom/base/nsFrameMessageManager.cpp:1100)
#57: nsFrameMessageManager::ReceiveMessage(nsISupports*, nsIFrameLoader*, nsAString const&, bool, mozilla::dom::ipc::StructuredCloneData*, mozilla::jsipc::CpowHolder*, nsIPrincipal*, nsTArray<mozilla::dom::ipc::StructuredCloneData>*) (crtstuff.c:?)
#58: mozilla::dom::TabChild::RecvAsyncMessage(nsString const&, nsTArray<mozilla::jsipc::CpowEntry>&&, IPC::Principal const&, mozilla::dom::ClonedMessageData const&) (/home/morbo/hg/firefox/dom/ipc/TabChild.cpp:2337)
#59: mozilla::dom::PBrowserChild::OnMessageReceived(IPC::Message const&) (/home/morbo/hg/firefox/obj-x86_64-pc-linux-gnu/ipc/ipdl/PBrowserChild.cpp:2681)
#60: mozilla::dom::PContentChild::OnMessageReceived(IPC::Message const&) (/home/morbo/hg/firefox/obj-x86_64-pc-linux-gnu/ipc/ipdl/PContentChild.cpp:5162)
#61: mozilla::dom::ContentChild::OnMessageReceived(IPC::Message const&) (/home/morbo/hg/firefox/dom/ipc/ContentChild.cpp:3663)
#62: mozilla::ipc::MessageChannel::DispatchAsyncMessage(IPC::Message const&) (/home/morbo/hg/firefox/ipc/glue/MessageChannel.cpp:2110)
#63: mozilla::ipc::MessageChannel::DispatchMessage(IPC::Message&&) (crtstuff.c:?)
#64: mozilla::ipc::MessageChannel::RunMessage(mozilla::ipc::MessageChannel::MessageTask&) (crtstuff.c:?)
#65: mozilla::ipc::MessageChannel::MessageTask::Run() (crtstuff.c:?)
#66: mozilla::SchedulerGroup::Runnable::Run() (/home/morbo/hg/firefox/xpcom/threads/SchedulerGroup.cpp:396)
#67: nsThread::ProcessNextEvent(bool, bool*) (/home/morbo/hg/firefox/xpcom/threads/nsThread.cpp:1035)
#68: NS_ProcessNextEvent(nsIThread*, bool) (crtstuff.c:?)
#69: mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) (crtstuff.c:?)
#70: mozilla::ipc::MessagePumpForChildProcess::Run(base::MessagePump::Delegate*) (crtstuff.c:?)
#71: MessageLoop::RunInternal() (/home/morbo/hg/firefox/ipc/chromium/src/base/message_loop.cc:327)
#72: MessageLoop::RunHandler() (crtstuff.c:?)
#73: MessageLoop::Run() (crtstuff.c:?)
#74: nsBaseAppShell::Run() (/home/morbo/hg/firefox/widget/nsBaseAppShell.cpp:160)
#75: XRE_RunAppShell() (/home/morbo/hg/firefox/toolkit/xre/nsEmbedFunctions.cpp:866)
#76: mozilla::ipc::MessagePumpForChildProcess::Run(base::MessagePump::Delegate*) (crtstuff.c:?)
#77: MessageLoop::RunInternal() (/home/morbo/hg/firefox/ipc/chromium/src/base/message_loop.cc:327)
#78: MessageLoop::RunHandler() (crtstuff.c:?)
#79: MessageLoop::Run() (crtstuff.c:?)
#80: XRE_InitChildProcess(int, char**, XREChildData const*) (crtstuff.c:?)
#81: mozilla::BootstrapImpl::XRE_InitChildProcess(int, char**, XREChildData const*) (/home/morbo/hg/firefox/toolkit/xre/Bootstrap.cpp:66)
#82: content_process_main(mozilla::Bootstrap*, int, char**) (/home/morbo/hg/firefox/browser/app/../../ipc/contentproc/plugin-container.cpp:63)
#83: main (crtstuff.c:?)
#84: __libc_start_main (/usr/src/debug/glibc-2.25-49-gbc5ace67fe/csu/../csu/libc-start.c:329)
#85: _start (/home/morbo/hg/firefox/obj-x86_64-pc-linux-gnu/dist/bin/firefox)
#86: ??? (???:???)
So, this was supposedly fixed by bug 579388, but the above stacks shows that https://bugzilla.mozilla.org/show_bug.cgi?id=579388#c12 wasn't entirely accurate.

The chain:
nsGIOService::GetAppForURIScheme
nsGNOMERegistry::HandlerExists
nsOSHelperAppService::OSProtocolHandlerExists
calls into glib-2.52.3/gio which in turn is trying to find things on the filesystem. The fact that these are GNOME specific explains the distro dependence.

I need to see what nsDefaultURIFixup::GetFixupURIInfo does and why it needs to resolve this in content.
So, we could remote
NS_IMETHODIMP nsExternalHelperAppService::ExternalProtocolHandlerExists(const char * aProtocolScheme,
                                                                        bool * aHandlerExists)
but:

As far as I can see this GetFixupURIInfo tries to figure out what the URL that is being loaded is supposed to be. It wants to know whether the handler for a certain protocol exists to figure out if christmas:humbug is a call to the christmas protocol or an attempt to use a keyword search with the christmas search engine.

Given that such fixups are presumably for user-entered URLs, this could well be done in content. Unfortunately when I produced the stack I did enter the URL directly and the query still happened from content, so bummer.
Comment on attachment 8904334 [details]
Bug 1382323 - Remote OSProtocolHandlerExists.

https://reviewboard.mozilla.org/r/176118/#review181416

::: uriloader/exthandler/HandlerServiceParent.cpp:272
(Diff revision 1)
> +                                            bool* aHandlerExists)
> +{
> +#ifdef MOZ_WIDGET_GTK
> +  // Check the GNOME registry for a protocol handler
> +  *aHandlerExists = nsGNOMERegistry::HandlerExists(aProtocolScheme.get());
> +#endif

Nit: there should probably be a `#else` `*aHandlerExists = false;` here.
Whiteboard: sb+
The tricky thing here is that:

a) OSProtocolHandlerExists is called from both content and chrome side, sometimes in quick succession.
b) nsIHandlerService has 2 distinct implementations. One in C++, one in JS. Which one you get depends on whether you're instantiating from content or chrome (see bug 1190018).

I had some problems figuring out why the remoting calls added in OSProtocolHandler randomly seemed to fail or never reach the intended code. Of course, it was talking to the wrong nsIHandlerService...
Comment on attachment 8904334 [details]
Bug 1382323 - Remote OSProtocolHandlerExists.

https://reviewboard.mozilla.org/r/176118/#review189980

This seems reasonable to me, but I'm not an expert on the exthandler code.  You might want to get a docshell peer to look at this as well.

::: uriloader/exthandler/nsExternalHelperAppService.cpp:906
(Diff revision 5)
>                                                                          bool * aHandlerExists)
>  {
>    nsCOMPtr<nsIHandlerInfo> handlerInfo;
>    nsresult rv = GetProtocolHandlerInfo(nsDependentCString(aProtocolScheme), 
>                                         getter_AddRefs(handlerInfo));
> -  NS_ENSURE_SUCCESS(rv, rv);
> +  if (NS_SUCCEEDED(rv)) {

I'm confused about this part of the change — when would GetProtocolHandlerInfo fail but OSProtocolHandlerExists succeed?
Attachment #8904334 - Flags: review?(jld) → review+
Comment on attachment 8904334 [details]
Bug 1382323 - Remote OSProtocolHandlerExists.

https://reviewboard.mozilla.org/r/176118/#review190650

::: uriloader/exthandler/nsExternalHelperAppService.cpp:906
(Diff revision 5)
>                                                                          bool * aHandlerExists)
>  {
>    nsCOMPtr<nsIHandlerInfo> handlerInfo;
>    nsresult rv = GetProtocolHandlerInfo(nsDependentCString(aProtocolScheme), 
>                                         getter_AddRefs(handlerInfo));
> -  NS_ENSURE_SUCCESS(rv, rv);
> +  if (NS_SUCCEEDED(rv)) {

They don't quite do the same thing, i.e.: http://searchfox.org/mozilla-central/source/uriloader/exthandler/nsExternalHelperAppService.cpp#1094

versus: http://searchfox.org/mozilla-central/source/uriloader/exthandler/unix/nsOSHelperAppService.cpp#1150

It's extremely unobvious to me that failure in one might necessarily imply failure in the other.
Attachment #8904334 - Flags: review?(bugs)
Comment on attachment 8904334 [details]
Bug 1382323 - Remote OSProtocolHandlerExists.

https://reviewboard.mozilla.org/r/176118/#review190682

r+, but you need ipc peer review too, and it isn't clear to me whether r+ will be given. By default sync IPC should be r-.

::: ipc/ipdl/sync-messages.ini:1056
(Diff revision 5)
>  description =
>  [PPrinting::SavePrintSettings]
>  description =
>  [PHandlerService::FillHandlerInfo]
>  description =
> +[PHandlerService::ExistsForProtocol]

I can't review this. You need an ipc peer.
And _good_ explanation why adding a new sync IPC is ok. We should be getting rid of them all.

::: uriloader/exthandler/ContentHandlerService.cpp:157
(Diff revision 5)
>  NS_IMETHODIMP ContentHandlerService::Remove(nsIHandlerInfo *aHandlerInfo)
>  {
>    return NS_ERROR_NOT_IMPLEMENTED;
>  }
>  
> +NS_IMETHODIMP ContentHandlerService::ExistsForProtocol(const nsACString & aProtocolScheme, bool *_retval)

I'd prefer to use Mozilla coding style for new code, even if the surrounding code uses some random style like here.
Attachment #8904334 - Flags: review?(bugs) → review+
ah, perhaps jld's review is enough for the sync IPC.
Pushed by gpascutto@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/8129c4382245
Remote OSProtocolHandlerExists. r=jld,smaug
https://hg.mozilla.org/mozilla-central/rev/8129c4382245
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox58: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
Is this worth backporting to Beta or can it ride the 58 train?
status-firefox56: fix-optional → wontfix
status-firefox-esr52: --- → wontfix
Flags: needinfo?(gpascutto)
It's worth backporting, but it wasn't a trivial fix so it's not exactly risk-free either.
Flags: needinfo?(gpascutto)
Comment on attachment 8904334 [details]
Bug 1382323 - Remote OSProtocolHandlerExists.

Approval Request Comment
[Feature/Bug causing the regression]: bug 1289718
[User impact if declined]: External protocol handlers broken in some Linux environments
[Is this code covered by automated tests?]: If it was, it wouldn't have broken :)
[Has the fix been verified in Nightly?]: Landed yesterday
[Needs manual test from QE? If yes, steps to reproduce]: No, QA doesn't have the required setup
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: Somewhat
[Why is the change risky/not risky?]: Non-trivial change in the code dealing with external helper apps/protocol handlers
[String changes made/needed]: N/A
Attachment #8904334 - Flags: approval-mozilla-beta?
Comment on attachment 8904334 [details]
Bug 1382323 - Remote OSProtocolHandlerExists.

This was triaged as a fix-optional. Beta57 is going through a lot of uplifts and code churn. I don't think this issue, which isn't a new regression, is a must fix for 57. We should let this fix ride the 58 train.
Attachment #8904334 - Flags: approval-mozilla-beta? → approval-mozilla-beta-
Comment on attachment 8904334 [details]
Bug 1382323 - Remote OSProtocolHandlerExists.

[Approval Request Comment]
User impact if declined: This is an important issue preventing the use of external protocols, for example "apt:" links to install software on Linux, and a few duplicates have been filed already. This was not considered for Firefox 57 to reduce risk, but is actually an important fix for Linux users and should likely be included in ESR.
Fix Landed on Version: Firefox 58
Risk to taking this patch (and alternatives if risky): Limited to protocol handling functinality, this is more likely to fix things than further break them.
String or UUID changes made by this patch: None
Attachment #8904334 - Flags: approval-mozilla-esr52?
Comment on attachment 8904334 [details]
Bug 1382323 - Remote OSProtocolHandlerExists.

There is no evidence Firefox 52 is affected. Sandboxing for Linux shipped in Firefox 54, and that's consistent with the reports filed.
Attachment #8904334 - Flags: approval-mozilla-esr52?
Ah, what confused me is that Ubuntu LTS is affected as reported in bug 1400803, but probably it ships regular Firefox and not the ESR release. Sorry for the noise!
(In reply to :Paolo Amadini from comment #45)
> Ah, what confused me is that Ubuntu LTS is affected as reported in bug
> 1400803, but probably it ships regular Firefox and not the ESR release.
> Sorry for the noise!

Yes, Ubuntu ships release. Note that bug also has Firefox 55.0.2.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: