From 927065b44f5fd87415d52f1f313968dd4a89e6c1 Mon Sep 17 00:00:00 2001 From: John Freed Date: Fri, 20 Aug 2021 12:29:26 +0200 Subject: [PATCH] Fixed daemon subcommand and App.java logic Formerly the daemon subcommand failed to create a SignalControl when a username was specified. (It worked correctly when no username was specified, implying that all local usernames should be exported.) Partly this was because of a logic flaw in App.java, whereby daemon called with a username was incorrectly classified as a LocalCommand instead of a MultiLocalCommand. Removed no-longer-needed globals in App.java and Main.java as a result. --- man/signal-cli-dbus.5.adoc | 3 + src/main/java/org/asamk/signal/App.java | 94 +++++++++++++------ src/main/java/org/asamk/signal/Main.java | 4 +- .../asamk/signal/commands/DaemonCommand.java | 26 +++-- .../signal/commands/MultiLocalCommand.java | 1 + .../signal/dbus/DbusSignalControlImpl.java | 12 ++- 6 files changed, 97 insertions(+), 43 deletions(-) diff --git a/man/signal-cli-dbus.5.adoc b/man/signal-cli-dbus.5.adoc index b3fa7493..7435ab62 100755 --- a/man/signal-cli-dbus.5.adoc +++ b/man/signal-cli-dbus.5.adoc @@ -547,6 +547,9 @@ dbus-send --session --print-reply --type=method_call --dest=org.asamk.Signal /or Print the group name corresponding to a groupId; the daemon runs on system bus, and was started without an explicit `-u USERNAME`:: dbus-send --system --print-reply --type=method_call --dest='org.asamk.Signal' /org/asamk/Signal/_1234567890 org.asamk.Signal.getGroupName array:byte:139,22,72,247,116,32,170,104,205,164,207,21,248,77,185 +Same as above, but daemon running on the session bus:: +dbus-send --session --print-reply --type=method_call --dest='org.asamk.Signal' /org/asamk/Signal/_1234567890 org.asamk.Signal.getGroupName array:byte:139,22,72,247,116,32,170,104,205,164,207,21,248,77,185 + == Authors Maintained by AsamK , who is assisted by other open source contributors. diff --git a/src/main/java/org/asamk/signal/App.java b/src/main/java/org/asamk/signal/App.java index 6225fa62..1e596eab 100644 --- a/src/main/java/org/asamk/signal/App.java +++ b/src/main/java/org/asamk/signal/App.java @@ -43,8 +43,6 @@ public class App { private final static Logger logger = LoggerFactory.getLogger(App.class); private final Namespace ns; - public static File dataPath; - public static ServiceEnvironment serviceEnvironment; static ArgumentParser buildArgumentParser() { var parser = ArgumentParsers.newFor("signal-cli", VERSION_0_9_0_DEFAULT_SETTINGS) @@ -88,13 +86,9 @@ public class App { } public App( - final Namespace ns, - File dataPath, - ServiceEnvironment serviceEnvironment + final Namespace ns ) { this.ns = ns; - this.dataPath = dataPath; - this.serviceEnvironment = serviceEnvironment; } public void init() throws CommandException { @@ -123,7 +117,7 @@ public class App { return; } -// final File dataPath; + final File dataPath; var config = ns.getString("config"); if (config != null) { dataPath = new File(config); @@ -132,7 +126,7 @@ public class App { } final var serviceEnvironmentCli = ns.get("service-environment"); - serviceEnvironment = serviceEnvironmentCli == ServiceEnvironmentCli.LIVE + ServiceEnvironment serviceEnvironment = serviceEnvironmentCli == ServiceEnvironmentCli.LIVE ? ServiceEnvironment.LIVE : ServiceEnvironment.SANDBOX; @@ -154,14 +148,19 @@ public class App { return; } + if (command instanceof MultiLocalCommand) { + List usernames = new ArrayList<>(); + if (username == null) { + usernames = Manager.getAllLocalUsernames(dataPath); + handleMultiLocalCommand((MultiLocalCommand) command, dataPath, serviceEnvironment, usernames); + } else { + handleMultiLocalCommand((MultiLocalCommand) command, dataPath, serviceEnvironment, username); + } + return; + } + if (username == null) { var usernames = Manager.getAllLocalUsernames(dataPath); - - if (command instanceof MultiLocalCommand) { - handleMultiLocalCommand((MultiLocalCommand) command, dataPath, serviceEnvironment, usernames); - return; - } - if (usernames.size() == 0) { throw new UserErrorException("No local users found, you first need to register or link an account"); } else if (usernames.size() > 1) { @@ -222,8 +221,10 @@ public class App { final File dataPath, final ServiceEnvironment serviceEnvironment ) throws CommandException { - try (var m = loadManager(username, dataPath, serviceEnvironment)) { - command.handleCommand(ns, m); + Manager m = loadManager(username, dataPath, serviceEnvironment); + command.handleCommand(ns, m); + try { + m.close();; } catch (IOException e) { logger.warn("Cleanup failed", e); } @@ -235,16 +236,8 @@ public class App { final ServiceEnvironment serviceEnvironment, final List usernames ) throws CommandException { - final var managers = new ArrayList(); - for (String u : usernames) { - try { - managers.add(loadManager(u, dataPath, serviceEnvironment)); - } catch (CommandException e) { - logger.warn("Ignoring {}: {}", u, e.getMessage()); - } - } + SignalCreator c = new SignalCreator() { - command.handleCommand(ns, managers, new SignalCreator() { @Override public ProvisioningManager getNewProvisioningManager() { return ProvisioningManager.init(dataPath, serviceEnvironment, BaseConfig.USER_AGENT); @@ -254,7 +247,18 @@ public class App { public RegistrationManager getNewRegistrationManager(String username) throws IOException { return RegistrationManager.init(username, dataPath, serviceEnvironment, BaseConfig.USER_AGENT); } - }); + }; + + final var managers = new ArrayList(); + for (String u : usernames) { + try { + managers.add(loadManager(u, dataPath, serviceEnvironment)); + } catch (CommandException e) { + logger.warn("Ignoring {}: {}", u, e.getMessage()); + } + } + + command.handleCommand(ns, managers, c); for (var m : managers) { try { @@ -265,6 +269,42 @@ public class App { } } + private void handleMultiLocalCommand( + final MultiLocalCommand command, + final File dataPath, + final ServiceEnvironment serviceEnvironment, + final String username + ) throws CommandException { + + SignalCreator c = new SignalCreator() { + + @Override + public ProvisioningManager getNewProvisioningManager() { + return ProvisioningManager.init(dataPath, serviceEnvironment, BaseConfig.USER_AGENT); + } + + @Override + public RegistrationManager getNewRegistrationManager(String username) throws IOException { + return RegistrationManager.init(username, dataPath, serviceEnvironment, BaseConfig.USER_AGENT); + } + }; + + Manager manager = null; + try { + manager = loadManager(username, dataPath, serviceEnvironment); + } catch (CommandException e) { + logger.warn("Ignoring {}: {}", username, e.getMessage()); + } + + command.handleCommand(ns, manager, c); + + try { + manager.close(); + } catch (IOException e) { + logger.warn("Cleanup failed", e); + } + } + private Manager loadManager( final String username, final File dataPath, final ServiceEnvironment serviceEnvironment ) throws CommandException { diff --git a/src/main/java/org/asamk/signal/Main.java b/src/main/java/org/asamk/signal/Main.java index 2a05d55e..4552f4d1 100644 --- a/src/main/java/org/asamk/signal/Main.java +++ b/src/main/java/org/asamk/signal/Main.java @@ -47,12 +47,10 @@ public class Main { var parser = App.buildArgumentParser(); var ns = parser.parseArgsOrFail(args); - File dataPath = null; - ServiceEnvironment serviceEnvironment = null; int status = 0; try { - new App(ns, dataPath, serviceEnvironment).init(); + new App(ns).init(); } catch (CommandException e) { System.err.println(e.getMessage()); status = getStatusForError(e); diff --git a/src/main/java/org/asamk/signal/commands/DaemonCommand.java b/src/main/java/org/asamk/signal/commands/DaemonCommand.java index 7b6f243d..38ee7411 100644 --- a/src/main/java/org/asamk/signal/commands/DaemonCommand.java +++ b/src/main/java/org/asamk/signal/commands/DaemonCommand.java @@ -4,6 +4,7 @@ import net.sourceforge.argparse4j.impl.Arguments; import net.sourceforge.argparse4j.inf.Namespace; import net.sourceforge.argparse4j.inf.Subparser; +import org.asamk.signal.App; import org.asamk.signal.DbusConfig; import org.asamk.signal.DbusReceiveMessageHandler; import org.asamk.signal.JsonDbusReceiveMessageHandler; @@ -11,6 +12,7 @@ import org.asamk.signal.JsonWriter; import org.asamk.signal.OutputType; import org.asamk.signal.OutputWriter; import org.asamk.signal.PlainTextWriter; +import org.asamk.signal.commands.SignalCreator; import org.asamk.signal.commands.exceptions.CommandException; import org.asamk.signal.commands.exceptions.UnexpectedErrorException; import org.asamk.signal.dbus.DbusSignalControlImpl; @@ -22,6 +24,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import java.io.IOException; +import java.util.ArrayList; import java.util.List; import java.util.Set; import java.util.concurrent.TimeUnit; @@ -51,9 +54,10 @@ public class DaemonCommand implements MultiLocalCommand { } @Override - public void handleCommand(final Namespace ns, final Manager m) throws CommandException { + public void handleCommand( + final Namespace ns, final Manager manager, SignalCreator c + ) throws CommandException { boolean ignoreAttachments = ns.getBoolean("ignore-attachments"); - DBusConnection.DBusBusType busType; if (ns.getBoolean("system")) { busType = DBusConnection.DBusBusType.SYSTEM; @@ -62,15 +66,21 @@ public class DaemonCommand implements MultiLocalCommand { } try (var conn = DBusConnection.getConnection(busType)) { - var objectPath = DbusConfig.getObjectPath(); - var t = run(conn, objectPath, m, ignoreAttachments); + final var signalControl = new DbusSignalControlImpl(c, m -> { + try { + final var objectPath = DbusConfig.getObjectPath(); + return run(conn, objectPath, m, ignoreAttachments); + } catch (DBusException e) { + logger.error("Failed to export object", e); + return null; + } + }, DbusConfig.getObjectPath()); + + signalControl.addManager(manager); conn.requestBusName(DbusConfig.getBusname()); - try { - t.join(); - } catch (InterruptedException ignored) { - } + signalControl.run(); } catch (DBusException | IOException e) { logger.error("Dbus command failed", e); throw new UnexpectedErrorException("Dbus command failed"); diff --git a/src/main/java/org/asamk/signal/commands/MultiLocalCommand.java b/src/main/java/org/asamk/signal/commands/MultiLocalCommand.java index 58416e50..587610c9 100644 --- a/src/main/java/org/asamk/signal/commands/MultiLocalCommand.java +++ b/src/main/java/org/asamk/signal/commands/MultiLocalCommand.java @@ -10,6 +10,7 @@ import java.util.List; public interface MultiLocalCommand extends LocalCommand { void handleCommand(Namespace ns, List m, final SignalCreator c) throws CommandException; + void handleCommand(Namespace ns, Manager m, final SignalCreator c) throws CommandException; @Override default void handleCommand(final Namespace ns, final Manager m) throws CommandException { diff --git a/src/main/java/org/asamk/signal/dbus/DbusSignalControlImpl.java b/src/main/java/org/asamk/signal/dbus/DbusSignalControlImpl.java index ac1921c0..5ca09dc2 100644 --- a/src/main/java/org/asamk/signal/dbus/DbusSignalControlImpl.java +++ b/src/main/java/org/asamk/signal/dbus/DbusSignalControlImpl.java @@ -118,9 +118,11 @@ public class DbusSignalControlImpl implements org.asamk.SignalControl { ) { try { try { - registrationManager = RegistrationManager.init(number, App.dataPath, App.serviceEnvironment, BaseConfig.USER_AGENT); + registrationManager = c.getNewRegistrationManager(number); } catch (IOException e) { e.printStackTrace(); + } catch (Exception e) { + throw new Error.Failure(e.getMessage()); } registrationManager.register(voiceVerification, captcha); } catch (CaptchaRequiredException e) { @@ -159,7 +161,7 @@ public class DbusSignalControlImpl implements org.asamk.SignalControl { public static String link(final String newDeviceName) { try { - provisioningManager = ProvisioningManager.init(App.dataPath, App.serviceEnvironment, BaseConfig.USER_AGENT); + provisioningManager = c.getNewProvisioningManager(); final URI deviceLinkUri = provisioningManager.getDeviceLinkUri(); new Thread(() -> { try { @@ -167,11 +169,11 @@ public class DbusSignalControlImpl implements org.asamk.SignalControl { logger.info("Linking of " + newDeviceName + " successful"); manager.close(); } catch (TimeoutException e) { - throw new SignalControl.Error.Failure(e.getClass().getSimpleName() + "Link request timed out, please try again."); + throw new SignalControl.Error.Failure(e.getClass().getSimpleName() + ": Link request timed out, please try again."); } catch (IOException e) { - throw new SignalControl.Error.Failure(e.getClass().getSimpleName() + "Link request error: " + e.getMessage()); + throw new SignalControl.Error.Failure(e.getClass().getSimpleName() + ": Link request error: " + e.getMessage()); } catch (UserAlreadyExists e) { - throw new SignalControl.Error.Failure(e.getClass().getSimpleName() + "The user " + throw new SignalControl.Error.Failure(e.getClass().getSimpleName() + ": The user " + e.getUsername() + " already exists\nDelete \"" + e.getFileName()