From a5eb1f867a03e814651baa0fea0d94bc145a5531 Mon Sep 17 00:00:00 2001 From: tilllt Date: Sat, 11 Jul 2026 15:33:18 +0000 Subject: [PATCH] fix: address AsamK's review comments - HttpServerHandler.getManagerFromQuery(): return null when account not found instead of falling back to all managers (AsamK: 'should stay return null here') - MultiAccountManagerImpl.getManager(): use UuidUtil.isUuid() to branch early on ACI vs phone number, eliminating the try-number-then-fallback pattern (AsamK: 'check if identifier is a uuid first') --- .../internal/MultiAccountManagerImpl.java | 91 +++++++++---------- .../asamk/signal/http/HttpServerHandler.java | 2 +- 2 files changed, 44 insertions(+), 49 deletions(-) diff --git a/lib/src/main/java/org/asamk/signal/manager/internal/MultiAccountManagerImpl.java b/lib/src/main/java/org/asamk/signal/manager/internal/MultiAccountManagerImpl.java index b943a007..3adefbef 100644 --- a/lib/src/main/java/org/asamk/signal/manager/internal/MultiAccountManagerImpl.java +++ b/lib/src/main/java/org/asamk/signal/manager/internal/MultiAccountManagerImpl.java @@ -8,6 +8,7 @@ import org.asamk.signal.manager.SignalAccountFiles; import org.asamk.signal.manager.api.AccountCheckException; import org.asamk.signal.manager.api.NotRegisteredException; import org.slf4j.Logger; +import org.signal.core.util.UuidUtil; import org.slf4j.LoggerFactory; import java.io.IOException; @@ -97,57 +98,51 @@ public class MultiAccountManagerImpl implements MultiAccountManager { @Override public Manager getManager(final String identifier) { synchronized (managers) { - // Try to find already loaded manager by phone number - var manager = managers.stream() - .filter(m -> m.getSelfNumber().equals(identifier)) - .findFirst() - .orElse(null); - if (manager != null) { - return manager; - } - - // Try to load by phone number first - try { - final var newManager = signalAccountFiles.initManager(identifier); - managers.add(newManager); - return newManager; - } catch (NotRegisteredException e) { - // Not a valid phone number or not registered yet, try ACI - logger.debug("Manager not found by number, trying ACI: {}", identifier); - } catch (IOException | IllegalArgumentException | AccountCheckException e) { - logger.warn("Failed to load new manager by number: {}", identifier, e); - return null; - } - - // Before trying to initByAci (which would try to re-open an already-locked file), - // check if this UUID corresponds to an already-loaded manager - try { - final var phoneNumber = signalAccountFiles.getAccountNumberByAci(identifier); - if (phoneNumber != null) { - final var existingManager = managers.stream() - .filter(m -> m.getSelfNumber().equals(phoneNumber)) - .findFirst() - .orElse(null); - if (existingManager != null) { - logger.debug("Found already loaded manager for ACI: {}", identifier); - return existingManager; + if (UuidUtil.INSTANCE.isUuid(identifier)) { + // Check if UUID corresponds to an already-loaded manager + try { + final var phoneNumber = signalAccountFiles.getAccountNumberByAci(identifier); + if (phoneNumber != null) { + final var existing = managers.stream() + .filter(m -> m.getSelfNumber().equals(phoneNumber)) + .findFirst() + .orElse(null); + if (existing != null) { + logger.debug("Found already loaded manager for ACI: {}", identifier); + return existing; + } } + } catch (IOException e) { + logger.warn("Failed to lookup ACI in accounts: {}", identifier, e); + } + // Load by ACI + try { + final var newManager = signalAccountFiles.initManagerByAci(identifier); + managers.add(newManager); + return newManager; + } catch (NotRegisteredException e) { + logger.debug("Manager not found by ACI: {}", identifier); + } catch (IOException | IllegalArgumentException | AccountCheckException e) { + logger.warn("Failed to load new manager by ACI: {}", identifier, e); + } + } else { + // Phone number — check already loaded managers + var existing = managers.stream() + .filter(m -> m.getSelfNumber().equals(identifier)) + .findFirst() + .orElse(null); + if (existing != null) { + return existing; + } + // Load by phone number + try { + final var newManager = signalAccountFiles.initManager(identifier); + managers.add(newManager); + return newManager; + } catch (NotRegisteredException | IOException | IllegalArgumentException | AccountCheckException e) { + logger.warn("Failed to load manager by number: {}", identifier, e); } - } catch (IOException e) { - logger.warn("Failed to lookup ACI in accounts: {}", identifier, e); } - - // Try to load by ACI (useful for SSE endpoint with ?account=) - try { - final var newManager = signalAccountFiles.initManagerByAci(identifier); - managers.add(newManager); - return newManager; - } catch (NotRegisteredException e) { - logger.debug("Manager not found by ACI: {}", identifier); - } catch (IOException | IllegalArgumentException | AccountCheckException e) { - logger.warn("Failed to load new manager by ACI: {}", identifier, e); - } - return null; } } diff --git a/src/main/java/org/asamk/signal/http/HttpServerHandler.java b/src/main/java/org/asamk/signal/http/HttpServerHandler.java index 1d2c48f8..c797e22a 100644 --- a/src/main/java/org/asamk/signal/http/HttpServerHandler.java +++ b/src/main/java/org/asamk/signal/http/HttpServerHandler.java @@ -265,7 +265,7 @@ public class HttpServerHandler implements AutoCloseable { // Account not found by the given identifier (number or ACI/UUID) // Log the available accounts to help debug logger.warn("Account not found for identifier: {}", account); - return c.getManagers(); + return null; } return List.of(manager); }