From e8dd00607fc9c5e2a6c5ca569e41379d29fd7fd0 Mon Sep 17 00:00:00 2001 From: Gabriele Svelto Date: Tue, 12 Apr 2022 12:31:27 +0000 Subject: [PATCH] Bug 1758468 - Use stable, anonymous IDs for Web MIDI ports r=padenot Differential Revision: https://phabricator.services.mozilla.com/D142550 --- dom/midi/MIDIAccess.cpp | 44 ++++++++-------- dom/midi/MIDIInputMap.h | 8 +++ dom/midi/MIDIOutputMap.h | 8 +++ dom/midi/MIDIPort.cpp | 14 ++++- dom/midi/MIDIPort.h | 1 + dom/midi/MIDIPortChild.cpp | 12 +++++ dom/midi/MIDIPortChild.h | 5 +- dom/midi/moz.build | 1 + dom/midi/tests/MIDITestUtils.js | 43 +++++++++++++-- dom/midi/tests/browser.ini | 12 +++++ .../tests/browser_stable_midi_port_ids.js | 52 +++++++++++++++++++ dom/midi/tests/port_ids_page_1.html | 17 ++++++ dom/midi/tests/port_ids_page_2.html | 17 ++++++ dom/midi/tests/test_midi_send_messages.html | 2 - 14 files changed, 206 insertions(+), 30 deletions(-) create mode 100644 dom/midi/tests/browser.ini create mode 100644 dom/midi/tests/browser_stable_midi_port_ids.js create mode 100644 dom/midi/tests/port_ids_page_1.html create mode 100644 dom/midi/tests/port_ids_page_2.html diff --git a/dom/midi/MIDIAccess.cpp b/dom/midi/MIDIAccess.cpp index ecaed62f523b..ed139b3c7c91 100644 --- a/dom/midi/MIDIAccess.cpp +++ b/dom/midi/MIDIAccess.cpp @@ -89,12 +89,14 @@ void MIDIAccess::FireConnectionEvent(MIDIPort* aPort) { aPort->GetId(id); ErrorResult rv; if (aPort->State() == MIDIPortDeviceState::Disconnected) { - if (aPort->Type() == MIDIPortType::Input && - MIDIInputMap_Binding::MaplikeHelpers::Has(mInputMap, id, rv)) { - MIDIInputMap_Binding::MaplikeHelpers::Delete(mInputMap, id, rv); - } else if (aPort->Type() == MIDIPortType::Output && - MIDIOutputMap_Binding::MaplikeHelpers::Has(mOutputMap, id, rv)) { - MIDIOutputMap_Binding::MaplikeHelpers::Delete(mOutputMap, id, rv); + if (aPort->Type() == MIDIPortType::Input && mInputMap->Has(id)) { + MIDIInputMap_Binding::MaplikeHelpers::Delete(mInputMap, aPort->StableId(), + rv); + mInputMap->Remove(id); + } else if (aPort->Type() == MIDIPortType::Output && mOutputMap->Has(id)) { + MIDIOutputMap_Binding::MaplikeHelpers::Delete(mOutputMap, + aPort->StableId(), rv); + mOutputMap->Remove(id); } // Check to make sure Has()/Delete() calls haven't failed. if (NS_WARN_IF(rv.Failed())) { @@ -106,31 +108,31 @@ void MIDIAccess::FireConnectionEvent(MIDIPort* aPort) { // this means a port that was disconnected has been reconnected, with the // port owner holding the object during that time, and we should add that // port object to our maps again. - if (aPort->Type() == MIDIPortType::Input && - !MIDIInputMap_Binding::MaplikeHelpers::Has(mInputMap, id, rv)) { + if (aPort->Type() == MIDIPortType::Input && !mInputMap->Has(id)) { if (NS_WARN_IF(rv.Failed())) { LOG("Input port not found"); return; } MIDIInputMap_Binding::MaplikeHelpers::Set( - mInputMap, id, *(static_cast(aPort)), rv); + mInputMap, aPort->StableId(), *(static_cast(aPort)), rv); if (NS_WARN_IF(rv.Failed())) { LOG("Map Set failed for input port"); return; } - } else if (aPort->Type() == MIDIPortType::Output && - !MIDIOutputMap_Binding::MaplikeHelpers::Has(mOutputMap, id, - rv)) { + mInputMap->Insert(id, aPort); + } else if (aPort->Type() == MIDIPortType::Output && mOutputMap->Has(id)) { if (NS_WARN_IF(rv.Failed())) { LOG("Output port not found"); return; } MIDIOutputMap_Binding::MaplikeHelpers::Set( - mOutputMap, id, *(static_cast(aPort)), rv); + mOutputMap, aPort->StableId(), *(static_cast(aPort)), + rv); if (NS_WARN_IF(rv.Failed())) { LOG("Map set failed for output port"); return; } + mOutputMap->Insert(id, aPort); } } RefPtr event = @@ -144,9 +146,7 @@ void MIDIAccess::MaybeCreateMIDIPort(const MIDIPortInfo& aInfo, MIDIPortType type = static_cast(aInfo.type()); RefPtr port; if (type == MIDIPortType::Input) { - bool hasPort = - MIDIInputMap_Binding::MaplikeHelpers::Has(mInputMap, id, aRv); - if (hasPort || NS_WARN_IF(aRv.Failed())) { + if (mInputMap->Has(id) || NS_WARN_IF(aRv.Failed())) { // We already have the port in our map. return; } @@ -157,15 +157,15 @@ void MIDIAccess::MaybeCreateMIDIPort(const MIDIPortInfo& aInfo, return; } MIDIInputMap_Binding::MaplikeHelpers::Set( - mInputMap, id, *(static_cast(port.get())), aRv); + mInputMap, port->StableId(), *(static_cast(port.get())), + aRv); if (NS_WARN_IF(aRv.Failed())) { LOG("Coudld't set input port in map"); return; } + mInputMap->Insert(id, port); } else if (type == MIDIPortType::Output) { - bool hasPort = - MIDIOutputMap_Binding::MaplikeHelpers::Has(mOutputMap, id, aRv); - if (hasPort || NS_WARN_IF(aRv.Failed())) { + if (mOutputMap->Has(id) || NS_WARN_IF(aRv.Failed())) { // We already have the port in our map. return; } @@ -176,11 +176,13 @@ void MIDIAccess::MaybeCreateMIDIPort(const MIDIPortInfo& aInfo, return; } MIDIOutputMap_Binding::MaplikeHelpers::Set( - mOutputMap, id, *(static_cast(port.get())), aRv); + mOutputMap, port->StableId(), *(static_cast(port.get())), + aRv); if (NS_WARN_IF(aRv.Failed())) { LOG("Coudld't set output port in map"); return; } + mOutputMap->Insert(id, port); } else { // If we hit this, then we have some port that is neither input nor output. // That is bad. diff --git a/dom/midi/MIDIInputMap.h b/dom/midi/MIDIInputMap.h index 481ac2988090..9a146871f1d7 100644 --- a/dom/midi/MIDIInputMap.h +++ b/dom/midi/MIDIInputMap.h @@ -7,7 +7,9 @@ #ifndef mozilla_dom_MIDIInputMap_h #define mozilla_dom_MIDIInputMap_h +#include "mozilla/dom/MIDIPort.h" #include "nsCOMPtr.h" +#include "nsTHashMap.h" #include "nsWrapperCache.h" class nsPIDOMWindowInner; @@ -27,9 +29,15 @@ class MIDIInputMap final : public nsISupports, public nsWrapperCache { explicit MIDIInputMap(nsPIDOMWindowInner* aParent); JSObject* WrapObject(JSContext* aCx, JS::Handle aGivenProto) override; + bool Has(nsAString& aId) { return mPorts.Get(aId) != nullptr; } + void Insert(nsAString& aId, RefPtr aPort) { + mPorts.InsertOrUpdate(aId, aPort); + } + void Remove(nsAString& aId) { mPorts.Remove(aId); } private: ~MIDIInputMap() = default; + nsTHashMap> mPorts; nsCOMPtr mParent; }; diff --git a/dom/midi/MIDIOutputMap.h b/dom/midi/MIDIOutputMap.h index 4941f83b6234..280cf97713c2 100644 --- a/dom/midi/MIDIOutputMap.h +++ b/dom/midi/MIDIOutputMap.h @@ -7,7 +7,9 @@ #ifndef mozilla_dom_MIDIOutputMap_h #define mozilla_dom_MIDIOutputMap_h +#include "mozilla/dom/MIDIPort.h" #include "nsCOMPtr.h" +#include "nsTHashMap.h" #include "nsWrapperCache.h" class nsPIDOMWindowInner; @@ -30,9 +32,15 @@ class MIDIOutputMap final : public nsISupports, public nsWrapperCache { JSObject* WrapObject(JSContext* aCx, JS::Handle aGivenProto) override; + bool Has(nsAString& aId) { return mPorts.Get(aId) != nullptr; } + void Insert(nsAString& aId, RefPtr aPort) { + mPorts.InsertOrUpdate(aId, aPort); + } + void Remove(nsAString& aId) { mPorts.Remove(aId); } private: ~MIDIOutputMap() = default; + nsTHashMap> mPorts; nsCOMPtr mParent; }; diff --git a/dom/midi/MIDIPort.cpp b/dom/midi/MIDIPort.cpp index d8ae4a48ff6d..7bf5e1f48963 100644 --- a/dom/midi/MIDIPort.cpp +++ b/dom/midi/MIDIPort.cpp @@ -14,6 +14,7 @@ #include "mozilla/dom/Document.h" #include "mozilla/dom/Promise.h" #include "mozilla/Unused.h" +#include "nsContentUtils.h" #include "nsISupportsImpl.h" // for MOZ_COUNT_CTOR, MOZ_COUNT_DTOR #include "MIDILog.h" @@ -63,8 +64,17 @@ MIDIPort::~MIDIPort() { } bool MIDIPort::Initialize(const MIDIPortInfo& aPortInfo, bool aSysexEnabled) { + nsIURI* uri = GetDocumentIfCurrent()->GetDocumentURI(); + nsAutoCString origin; + nsresult rv = nsContentUtils::GetASCIIOrigin(uri, origin); + if (NS_FAILED(rv)) { + return false; + } RefPtr port = new MIDIPortChild(aPortInfo, aSysexEnabled, this); + if (NS_FAILED(port->GenerateStableId(origin))) { + return false; + } PBackgroundChild* b = BackgroundChild::GetForCurrentThread(); MOZ_ASSERT(b, "Should always have a valid BackgroundChild when creating a port " @@ -91,7 +101,7 @@ void MIDIPort::UnsetIPCPort() { void MIDIPort::GetId(nsString& aRetVal) const { MOZ_ASSERT(mPort); - aRetVal = mPort->MIDIPortInterface::Id(); + aRetVal = mPort->StableId(); } void MIDIPort::GetManufacturer(nsString& aRetVal) const { @@ -252,4 +262,6 @@ void MIDIPort::DontKeepAliveOnStatechange() { } } +const nsString& MIDIPort::StableId() { return mPort->StableId(); } + } // namespace mozilla::dom diff --git a/dom/midi/MIDIPort.h b/dom/midi/MIDIPort.h index cd898b8fe265..a391eeb4f247 100644 --- a/dom/midi/MIDIPort.h +++ b/dom/midi/MIDIPort.h @@ -73,6 +73,7 @@ class MIDIPort : public DOMEventTargetHelper, IMPL_EVENT_HANDLER(statechange) void DisconnectFromOwner() override; + const nsString& StableId(); protected: // IPC Actor corresponding to this class diff --git a/dom/midi/MIDIPortChild.cpp b/dom/midi/MIDIPortChild.cpp index 1540c8cae4a9..f0323bf98bf8 100644 --- a/dom/midi/MIDIPortChild.cpp +++ b/dom/midi/MIDIPortChild.cpp @@ -7,6 +7,7 @@ #include "mozilla/dom/MIDIPortChild.h" #include "mozilla/dom/MIDIPort.h" #include "mozilla/dom/MIDIPortInterface.h" +#include "nsContentUtils.h" using namespace mozilla; using namespace mozilla::dom; @@ -55,3 +56,14 @@ void MIDIPortChild::SetActorAlive() { mActorWasAlive = true; AddRef(); } + +nsresult MIDIPortChild::GenerateStableId(const nsACString& aOrigin) { + const size_t kIdLength = 64; + mStableId.SetCapacity(kIdLength); + mStableId.Append(Name()); + mStableId.Append(Manufacturer()); + mStableId.Append(Version()); + nsContentUtils::AnonymizeId(mStableId, aOrigin, + nsContentUtils::OriginFormat::Plain); + return NS_OK; +} diff --git a/dom/midi/MIDIPortChild.h b/dom/midi/MIDIPortChild.h index d67896e73a1e..8eee47731e5f 100644 --- a/dom/midi/MIDIPortChild.h +++ b/dom/midi/MIDIPortChild.h @@ -7,8 +7,8 @@ #ifndef mozilla_dom_MIDIPortChild_h #define mozilla_dom_MIDIPortChild_h -#include "mozilla/dom/PMIDIPortChild.h" #include "mozilla/dom/MIDIPortInterface.h" +#include "mozilla/dom/PMIDIPortChild.h" namespace mozilla::dom { @@ -33,6 +33,8 @@ class MIDIPortChild final : public PMIDIPortChild, public MIDIPortInterface { MIDIPortChild(const MIDIPortInfo& aPortInfo, bool aSysexEnabled, MIDIPort* aPort); + nsresult GenerateStableId(const nsACString& aOrigin); + const nsString& StableId() { return mStableId; }; // virtual void Shutdown() override; void SetActorAlive(); @@ -43,6 +45,7 @@ class MIDIPortChild final : public PMIDIPortChild, public MIDIPortInterface { // Pointer to the DOM object this actor represents. The actor cannot outlive // the DOM object. MIDIPort* mDOMPort; + nsString mStableId; bool mActorWasAlive; }; } // namespace mozilla::dom diff --git a/dom/midi/moz.build b/dom/midi/moz.build index a63a87d55d61..81c484d2906f 100644 --- a/dom/midi/moz.build +++ b/dom/midi/moz.build @@ -73,3 +73,4 @@ LOCAL_INCLUDES += [ ] MOCHITEST_MANIFESTS += ["tests/mochitest.ini"] +BROWSER_CHROME_MANIFESTS += ["tests/browser.ini"] diff --git a/dom/midi/tests/MIDITestUtils.js b/dom/midi/tests/MIDITestUtils.js index 4d516fc2e40c..728cabd45b06 100644 --- a/dom/midi/tests/MIDITestUtils.js +++ b/dom/midi/tests/MIDITestUtils.js @@ -23,31 +23,41 @@ var MIDITestUtils = { // This list needs to stay synced with the ports in // dom/midi/TestMIDIPlatformService. inputInfo: { - id: "b744eebe-f7d8-499b-872b-958f63c8f522", + get id() { + return MIDITestUtils.stableId(this); + }, name: "Test Control MIDI Device Input Port", manufacturer: "Test Manufacturer", version: "1.0.0", }, outputInfo: { - id: "ab8e7fe8-c4de-436a-a960-30898a7c9a3d", + get id() { + return MIDITestUtils.stableId(this); + }, name: "Test Control MIDI Device Output Port", manufacturer: "Test Manufacturer", version: "1.0.0", }, stateTestInputInfo: { - id: "a9329677-8588-4460-a091-9d4a7f629a48", + get id() { + return MIDITestUtils.stableId(this); + }, name: "Test State MIDI Device Input Port", manufacturer: "Test Manufacturer", version: "1.0.0", }, stateTestOutputInfo: { - id: "478fa225-b5fc-4fa6-a543-d32d9cb651e7", + get id() { + return MIDITestUtils.stableId(this); + }, name: "Test State MIDI Device Output Port", manufacturer: "Test Manufacturer", version: "1.0.0", }, alwaysClosedTestOutputInfo: { - id: "f87d0c76-3c68-49a9-a44f-700f1125c07a", + get id() { + return MIDITestUtils.stableId(this); + }, name: "Always Closed MIDI Device Output Port", manufacturer: "Test Manufacturer", version: "1.0.0", @@ -60,4 +70,27 @@ var MIDITestUtils = { is(expected[i], actual[i], "Packet value " + expected[i] + " matches."); } }, + stableId: info => { + // This computes the stable ID of a MIDI port according to the logic we + // use in the Web MIDI implementation. See MIDIPortChild::GenerateStableId() + // and nsContentUtils::AnonymizeId(). + const Cc = SpecialPowers.Cc; + const Ci = SpecialPowers.Ci; + const id = info.name + info.manufacturer + info.version; + const encoder = new TextEncoder(); + const data = encoder.encode(id); + let key = self.origin; + + var digest; + let keyObject = Cc["@mozilla.org/security/keyobjectfactory;1"] + .getService(Ci.nsIKeyObjectFactory) + .keyFromString(Ci.nsIKeyObject.HMAC, key); + let cryptoHMAC = Cc["@mozilla.org/security/hmac;1"].createInstance( + Ci.nsICryptoHMAC + ); + + cryptoHMAC.init(Ci.nsICryptoHMAC.SHA256, keyObject); + cryptoHMAC.update(data, data.length); + return cryptoHMAC.finish(true); + }, }; diff --git a/dom/midi/tests/browser.ini b/dom/midi/tests/browser.ini new file mode 100644 index 000000000000..daac24c6eb31 --- /dev/null +++ b/dom/midi/tests/browser.ini @@ -0,0 +1,12 @@ +[DEFAULT] +prefs = + dom.webmidi.enabled=true + midi.testing=true + midi.prompt.testing=true + media.navigator.permission.disabled=true + +[browser_stable_midi_port_ids.js] +run-if = (os != 'android') +support-files = + port_ids_page_1.html + port_ids_page_2.html diff --git a/dom/midi/tests/browser_stable_midi_port_ids.js b/dom/midi/tests/browser_stable_midi_port_ids.js new file mode 100644 index 000000000000..e2f7f3a7ebbe --- /dev/null +++ b/dom/midi/tests/browser_stable_midi_port_ids.js @@ -0,0 +1,52 @@ +/* Any copyright is dedicated to the Public Domain. + * http://creativecommons.org/publicdomain/zero/1.0/ */ + +const EXAMPLE_COM_URL = "https://example.com/browser/dom/midi/tests/"; +const EXAMPLE_ORG_URL = "https://example.org/browser/dom/midi/tests/"; +const PAGE1 = "port_ids_page_1.html"; +const PAGE2 = "port_ids_page_2.html"; + +// Return the MIDI port id of the first input port for the given URL and page +function id_for_tab(url, page) { + return BrowserTestUtils.withNewTab( + { + gBrowser, + url: url + page, + waitForLoad: true, + }, + async function(browser) { + return SpecialPowers.spawn(browser, [""], function() { + return content.wrappedJSObject.get_first_input_id(); + }); + } + ); +} + +add_task(async function() { + let com_page1; + let com_page1_reload; + let org_page1; + let org_page2; + + [com_page1, com_page1_reload, org_page1, org_page2] = await Promise.all([ + id_for_tab(EXAMPLE_COM_URL, PAGE1), + id_for_tab(EXAMPLE_COM_URL, PAGE1), + id_for_tab(EXAMPLE_ORG_URL, PAGE1), + id_for_tab(EXAMPLE_ORG_URL, PAGE2), + ]); + Assert.equal( + com_page1, + com_page1_reload, + "MIDI port ids should be the same when reloading the same page" + ); + Assert.notEqual( + com_page1, + org_page1, + "MIDI port ids should be different in different origins" + ); + Assert.equal( + org_page1, + org_page2, + "MIDI port ids should be the same in the same origin" + ); +}); diff --git a/dom/midi/tests/port_ids_page_1.html b/dom/midi/tests/port_ids_page_1.html new file mode 100644 index 000000000000..8c313b04da90 --- /dev/null +++ b/dom/midi/tests/port_ids_page_1.html @@ -0,0 +1,17 @@ + + + +Stable MIDI port id test + + + + + + diff --git a/dom/midi/tests/port_ids_page_2.html b/dom/midi/tests/port_ids_page_2.html new file mode 100644 index 000000000000..8c313b04da90 --- /dev/null +++ b/dom/midi/tests/port_ids_page_2.html @@ -0,0 +1,17 @@ + + + +Stable MIDI port id test + + + + + + diff --git a/dom/midi/tests/test_midi_send_messages.html b/dom/midi/tests/test_midi_send_messages.html index b4204b230096..b7bb9232a0b9 100644 --- a/dom/midi/tests/test_midi_send_messages.html +++ b/dom/midi/tests/test_midi_send_messages.html @@ -17,7 +17,6 @@ const access = await navigator.requestMIDIAccess({ sysex: true }); const output = access.outputs.get(MIDITestUtils.stateTestOutputInfo.id); - dump("output = " + JSON.stringify(output, null, " ") + "\n"); // Note on(off). output.send([0xff, 0x90, 0x00, 0x00, 0x90, 0x07, 0x00]); @@ -25,7 +24,6 @@ try { output.send([0x00, 0x01]) } catch (ex) { - dump("Caught exception"); ok(true, "Caught exception"); }