-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
fix(v3/linux): claim the single instance name under the app's own id #5973
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,6 +4,7 @@ package application | |
|
|
||
| import ( | ||
| "errors" | ||
| "fmt" | ||
| "os" | ||
| "strings" | ||
| "sync" | ||
|
|
@@ -22,11 +23,48 @@ func (f dbusHandler) SendMessage(message string) *dbus.Error { | |
| } | ||
|
|
||
| type linuxLock struct { | ||
| file *os.File | ||
| uniqueID string | ||
| dbusPath string | ||
| dbusName string | ||
| manager *singleInstanceManager | ||
| file *os.File | ||
| uniqueID string | ||
| dbusPath string | ||
| dbusName string | ||
| dbusInterface string | ||
| manager *singleInstanceManager | ||
| } | ||
|
|
||
| // singleInstanceNames derives the three D-Bus identifiers the lock needs from | ||
| // UniqueID. They cannot all be the same string, because D-Bus spells them | ||
| // differently: bus names may contain hyphens, interface names may not, and | ||
| // object paths separate elements with "/" and allow neither hyphens nor dots. | ||
| // | ||
| // The bus name keeps UniqueID verbatim. That matters for sandboxed builds: a | ||
| // flatpak may only own names prefixed with its own app id, so an app that | ||
| // follows the documented convention for UniqueID ("unique per application, e.g. | ||
| // com.myapp.myapplication") claims a name it is allowed to own and needs no | ||
| // extra portal permission. | ||
| func singleInstanceNames(uniqueID string) (busName, interfaceName, objectPath string, err error) { | ||
| for _, element := range strings.Split(uniqueID, ".") { | ||
| if element == "" { | ||
| return "", "", "", fmt.Errorf("UniqueID %q has an empty element; it must be a dot-separated name such as com.myapp.myapplication", uniqueID) | ||
| } | ||
| if element[0] >= '0' && element[0] <= '9' { | ||
| return "", "", "", fmt.Errorf("UniqueID %q has an element starting with a digit (%q), which D-Bus does not allow", uniqueID, element) | ||
| } | ||
| for _, r := range element { | ||
| if r >= 'a' && r <= 'z' || r >= 'A' && r <= 'Z' || r >= '0' && r <= '9' || r == '_' || r == '-' { | ||
| continue | ||
| } | ||
| return "", "", "", fmt.Errorf("UniqueID %q contains %q, which D-Bus does not allow in a name", uniqueID, r) | ||
| } | ||
| } | ||
|
|
||
| // Hyphens are legal in a bus name but not in an interface name or an object | ||
| // path, so those two are built from a hyphen-free form. | ||
| unhyphenated := strings.ReplaceAll(uniqueID, "-", "_") | ||
|
|
||
| busName = uniqueID + ".SingleInstance" | ||
| interfaceName = unhyphenated + ".SingleInstance" | ||
| objectPath = "/" + strings.ReplaceAll(unhyphenated, ".", "/") + "/SingleInstance" | ||
| return busName, interfaceName, objectPath, nil | ||
|
Comment on lines
+44
to
+67
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win 🧩 Analysis chain🌐 Web query:
💡 Result: The D-Bus specification defines a maximum length of 255 bytes for bus names, interface names, and member names [1][2][3]. Specifically: * Bus names, interface names, and member names: These must not exceed 255 bytes in length [1][2]. * Object paths: There is no specified maximum length for object paths; they are considered to be unlimited in length [2]. This 255-byte limit is defined by the constant DBUS_MAXIMUM_NAME_LENGTH [2]. The limitation exists because several internal operations are O(n) relative to this length, and to prevent potential exploits involving excessively long names [2]. Citations:
🏁 Script executed: #!/bin/bash
set -eu
file="v3/pkg/application/single_instance_linux.go"
printf '%s\n' '--- file outline ---'
ast-grep outline "$file" --lang go
printf '%s\n' '--- relevant source ---'
sed -n '1,150p' "$file"
printf '%s\n' '--- related tests and call sites ---'
rg -n -C 3 'singleInstanceNames|UniqueID|RequestName|Export|sync\.Once|setup' v3/pkg/application --glob '*.go'
printf '%s\n' '--- D-Bus length constants or validation in repository ---'
rg -n -C 2 'DBUS_MAXIMUM_NAME_LENGTH|255|maximum.*length|name.*length' . --glob '*.{go,c,h,hpp,cpp,cc}' --glob '*.md' | head -200Repository: wailsapp/wails Length of output: 1917 🌐 Web query:
💡 Result: In the Wails v3 framework, the term singleInstanceNames refers to a collection of identifiers or naming conventions used to ensure only one instance of a Wails application runs at a time on Linux [1]. While the specific file path v3/pkg/application/single_instance_linux.go pertains to the Linux-specific implementation of the single-instance locking mechanism [2], the framework's single-instance management is orchestrated through the SingleInstanceOptions struct [3]. When enabled, this feature uses a unique identifier (UniqueId) to prevent multiple processes from running simultaneously [4][1]. On Linux, this mechanism typically integrates with platform-specific APIs to manage process lifecycle and communication between the primary instance and any subsequent attempts to launch the application [1][5]. If a second instance is launched, the framework attempts to notify the first instance with relevant data (such as command-line arguments) rather than starting a new UI process, often relying on internal messaging or lock files to handle this coordination [6][1]. If you are encountering issues related to this, they often stem from how the UniqueId is generated or how the platform handles the signal that a second process is attempting to start [7][8]. The framework provides mechanisms to pass arguments, such as command-line flags or file paths, from the second process to the original one to ensure a seamless user experience [6][9]. Citations:
🌐 Web query:
💡 Result: In the D-Bus specification, the constant DBUS_MAXIMUM_NAME_LENGTH is defined as 255 bytes [1][2]. This limit applies to bus names (well-known names), interface names, and member names [3][1][2]. It is important to note that this 255-byte limit does not apply to object paths, which are considered to have unlimited length within the specification [1][2]. The constraint on names is primarily in place to prevent potential exploits and to maintain performance efficiency, as many operations related to these names are O(n) in complexity [1][2]. Additionally, the 255-byte value was chosen because it allows the length to fit within a single byte [2]. Citations:
Validate generated D-Bus name lengths before connecting. D-Bus limits bus names and interface names to 255 bytes. This format limits 🤖 Prompt for AI Agents |
||
| } | ||
|
|
||
| func newPlatformLock(manager *singleInstanceManager) (platformLock, error) { | ||
|
|
@@ -40,10 +78,15 @@ func (l *linuxLock) acquire(uniqueID string) error { | |
| return errors.New("UniqueID is required for single instance lock") | ||
| } | ||
|
|
||
| id := "wails_app_" + strings.ReplaceAll(strings.ReplaceAll(uniqueID, "-", "_"), ".", "_") | ||
| busName, interfaceName, objectPath, err := singleInstanceNames(uniqueID) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| l.dbusName = "org." + id + ".SingleInstance" | ||
| l.dbusPath = "/org/" + id + "/SingleInstance" | ||
| l.uniqueID = uniqueID | ||
| l.dbusName = busName | ||
| l.dbusInterface = interfaceName | ||
| l.dbusPath = objectPath | ||
|
|
||
| conn, err := dbus.ConnectSessionBus() | ||
| // if we will reach any error during establishing connection or sending message we will just continue. | ||
|
|
@@ -57,22 +100,29 @@ func (l *linuxLock) acquire(uniqueID string) error { | |
| secondInstanceBuffer <- message | ||
| }) | ||
|
|
||
| err = conn.Export(f, dbus.ObjectPath(l.dbusPath), l.dbusName) | ||
| err = conn.Export(f, dbus.ObjectPath(l.dbusPath), l.dbusInterface) | ||
| }) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| reply, err := conn.RequestName(l.dbusName, dbus.NameFlagDoNotQueue) | ||
| if err != nil { | ||
| return err | ||
| // A sandbox that refuses the name fails here. Say so, rather than | ||
| // letting the caller read it as a second instance and exit silently. | ||
| return fmt.Errorf("could not claim the single instance name %q: %w", l.dbusName, err) | ||
| } | ||
|
|
||
| // if name already taken, try to send args to existing instance, if no success just launch new instance | ||
| if reply == dbus.RequestNameReplyExists { | ||
| switch reply { | ||
| case dbus.RequestNameReplyPrimaryOwner, dbus.RequestNameReplyAlreadyOwner: | ||
| return nil | ||
| case dbus.RequestNameReplyExists: | ||
| // Someone else holds the name, so this is a second instance. The caller | ||
| // hands off to the first one and exits. | ||
| return alreadyRunningError | ||
| default: | ||
| return fmt.Errorf("unexpected reply %d when claiming the single instance name %q", reply, l.dbusName) | ||
| } | ||
| return nil | ||
| } | ||
|
|
||
| func (l *linuxLock) release() { | ||
|
|
@@ -92,7 +142,7 @@ func (l *linuxLock) notify(data string) error { | |
| return err | ||
| } | ||
|
|
||
| err = conn.Object(l.dbusName, dbus.ObjectPath(l.dbusPath)).Call(l.dbusName+".SendMessage", 0, data).Store() | ||
| err = conn.Object(l.dbusName, dbus.ObjectPath(l.dbusPath)).Call(l.dbusInterface+".SendMessage", 0, data).Store() | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,92 @@ | ||
| //go:build linux && !android && !server | ||
|
|
||
| package application | ||
|
|
||
| import "testing" | ||
|
|
||
| func TestSingleInstanceNames(t *testing.T) { | ||
| tests := []struct { | ||
| name string | ||
| uniqueID string | ||
| busName string | ||
| iface string | ||
| path string | ||
| }{ | ||
| { | ||
| name: "reverse dns id is kept verbatim in the bus name", | ||
| uniqueID: "com.myapp.myapplication", | ||
| busName: "com.myapp.myapplication.SingleInstance", | ||
| iface: "com.myapp.myapplication.SingleInstance", | ||
| path: "/com/myapp/myapplication/SingleInstance", | ||
| }, | ||
| { | ||
| // Hyphens are legal in a bus name but not in an interface name or | ||
| // an object path, so only the bus name keeps them. | ||
| name: "hyphens survive in the bus name only", | ||
| uniqueID: "net.my-company.my-app", | ||
| busName: "net.my-company.my-app.SingleInstance", | ||
| iface: "net.my_company.my_app.SingleInstance", | ||
| path: "/net/my_company/my_app/SingleInstance", | ||
| }, | ||
| { | ||
| name: "single element id still yields two name elements", | ||
| uniqueID: "myapp", | ||
| busName: "myapp.SingleInstance", | ||
| iface: "myapp.SingleInstance", | ||
| path: "/myapp/SingleInstance", | ||
| }, | ||
| } | ||
|
|
||
| for _, tt := range tests { | ||
| t.Run(tt.name, func(t *testing.T) { | ||
| busName, interfaceName, path, err := singleInstanceNames(tt.uniqueID) | ||
| if err != nil { | ||
| t.Fatalf("singleInstanceNames(%q) returned error: %v", tt.uniqueID, err) | ||
| } | ||
| if busName != tt.busName { | ||
| t.Errorf("bus name = %q, want %q", busName, tt.busName) | ||
| } | ||
| if interfaceName != tt.iface { | ||
| t.Errorf("interface name = %q, want %q", interfaceName, tt.iface) | ||
| } | ||
| if path != tt.path { | ||
| t.Errorf("object path = %q, want %q", path, tt.path) | ||
| } | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| // The bus name has to stay under the app id, which is what lets a sandboxed | ||
| // build claim it without being granted the whole session bus. | ||
| func TestSingleInstanceNamesArePrefixedByUniqueID(t *testing.T) { | ||
| const uniqueID = "net.koofr.stage" | ||
|
|
||
| busName, _, _, err := singleInstanceNames(uniqueID) | ||
| if err != nil { | ||
| t.Fatalf("singleInstanceNames(%q) returned error: %v", uniqueID, err) | ||
| } | ||
| if got, want := busName[:len(uniqueID)], uniqueID; got != want { | ||
| t.Errorf("bus name %q is not prefixed by the UniqueID %q", busName, want) | ||
| } | ||
| } | ||
|
|
||
| func TestSingleInstanceNamesRejectsInvalidIDs(t *testing.T) { | ||
| tests := []struct { | ||
| name string | ||
| uniqueID string | ||
| }{ | ||
| {"empty element", "com..myapp"}, | ||
| {"trailing dot", "com.myapp."}, | ||
| {"element starting with a digit", "com.1myapp"}, | ||
| {"character D-Bus does not allow", "com.my app"}, | ||
| {"slash", "com/myapp"}, | ||
| } | ||
|
|
||
| for _, tt := range tests { | ||
| t.Run(tt.name, func(t *testing.T) { | ||
| if _, _, _, err := singleInstanceNames(tt.uniqueID); err == nil { | ||
| t.Errorf("singleInstanceNames(%q) succeeded, want an error", tt.uniqueID) | ||
| } | ||
| }) | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
UniqueID could differ from the actual FLATPAK scope and puts the onus on the developer to ensure their ApplicationID == SingleInstance.UniqueID
For example if we set an app-id in flatpak of
app-id: com.example.MyAppbut set up a config of
we would create dbus id of
com.example.my-app-lock.SingleInstancebut flatpak would only permit ids under
com.example.MyApp.*Not sure the best solution potentially using the embedded
FLATPAK_IDenvironment so we know we are using the configuredapp-idand append a hash of the unique id config option?https://docs.flatpak.org/en/latest/flatpak-command-reference.html
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Granted the env path would likely fail outside of flatpak environment
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can add additional property
ForceFlatpakUniqueIdtoSingleInstanceOptions, defaulting tofalse. In that case,FLATPAK_IDwould override developer setUniqueID, ensuring dbus id has correct format. IfFLATPAK_IDenv variable is not present, behavior stays same as it is in current master.In case developer has good reason to force own
UniqueID, he can setForceFlatpakUniqueId=true.I will also revert current changes (well, will have make special codepath for flatpak, so we do not prepend
org.wailswhen generating dbus id), so behaviour change is minimal, making this more a bugfix rather than a breaking change. Agree?