D5911: Make sure that SNI service name is unique and allowed by default in flatpak
jgrulich added a comment. In https://phabricator.kde.org/D5911#110687, @davidedmundson wrote: > I explained badly, let me try again with code. > https://paste.kde.org/pvzge2wq8 > > It will work in flatpak, and the SNI test we have in p-w still works fine. That indeed seems to work, I'll discard this review and you can revert my previous patch and fix it your way. REPOSITORY R289 KNotifications REVISION DETAIL https://phabricator.kde.org/D5911 To: jgrulich, apol Cc: davidedmundson, #frameworks
D5911: Make sure that SNI service name is unique and allowed by default in flatpak
jgrulich abandoned this revision. REPOSITORY R289 KNotifications REVISION DETAIL https://phabricator.kde.org/D5911 To: jgrulich, apol Cc: davidedmundson, #frameworks
D5911: Make sure that SNI service name is unique and allowed by default in flatpak
davidedmundson added a comment. I explained badly, let me try again with code. https://paste.kde.org/pvzge2wq8 It will work in flatpak, and the SNI test we have in p-w still works fine. REPOSITORY R289 KNotifications REVISION DETAIL https://phabricator.kde.org/D5911 To: jgrulich, apol Cc: davidedmundson, #frameworks
D5911: Make sure that SNI service name is unique and allowed by default in flatpak
jgrulich added a comment. In https://phabricator.kde.org/D5911#110683, @davidedmundson wrote: > This is a big solution for a problem that I don't think exists. There are actually two problems: 1. You need to guarantee that the SNI dbus service name will be unique, you cannot use pid, because in flatpak the pid is usually 2 as it's isolated in the sandbox 2. To let the app have access to create SNI you have to either allow it in manifest, which you can do using i.e --own-name=org.kde.StatusNotifierItem.*, but if you use wildcard, then it means the first instance of the app will own all dbus services starting with org.kde.StatusNotifier. To solve this problem we decided to use org.kde.appName.StatusNotifier.* as this is enabled in flatpak by default without assumption that it will own all possible variants of this name. > There's no reason to register a new service, we pass the service name to the SNI watcher; it could be just the baseService (line 174 in the current code) Not sure what do you mean by registering a new service, I still register one with just a different name in case we are running in flatpak. REPOSITORY R289 KNotifications REVISION DETAIL https://phabricator.kde.org/D5911 To: jgrulich, apol Cc: davidedmundson, #frameworks
D5911: Make sure that SNI service name is unique and allowed by default in flatpak
davidedmundson added a comment. This is a big solution for a problem that I don't think exists. There's no reason to register a new service, we pass the service name to the SNI watcher; it could be just the baseService (line 174 in the current code) If you do do that, use m_dbus.baseName() not QDbusConnection::baseName() as each SNI is in a different DBus connection. REPOSITORY R289 KNotifications REVISION DETAIL https://phabricator.kde.org/D5911 To: jgrulich, apol Cc: davidedmundson, #frameworks
D5911: Make sure that SNI service name is unique and allowed by default in flatpak
jgrulich added a reviewer: apol. REPOSITORY R289 KNotifications REVISION DETAIL https://phabricator.kde.org/D5911 To: jgrulich, apol Cc: #frameworks
D5911: Make sure that SNI service name is unique and allowed by default in flatpak
jgrulich created this revision. Restricted Application added a project: Frameworks. Restricted Application added a subscriber: Frameworks. REVISION SUMMARY In my previous change I made sure that the dbus service name for SNI is predictable so we can allow it in advance, but it introduced problem for multiple instances of the same app. This change doesn't require predictable name as org.kde.appName.* is allowed in flatpak by default if we use same id in flatpak manifest (which we do for all KDE apps) and also makes sure that the name will be unique, because we cannot rely on pid for apps running in flatpak, pid will be always same for multiple instances. Btw. this is using same approach as in this review https://phabricator.kde.org/D5775. REPOSITORY R289 KNotifications BRANCH master REVISION DETAIL https://phabricator.kde.org/D5911 AFFECTED FILES src/kstatusnotifieritemdbus_p.cpp To: jgrulich Cc: #frameworks
