[Client] Make the link-connection gates port-aware and keyboard-safe

Second-pass review notes for the shared-deck link flow (Cockatrice#7244):

- FlowWidget arrow-key navigation is opt-in via addNavigableWidget, so
  combo/spin controls on the analytics flows keep their own arrow keys
- isConnectedTo and the open-deck/join-game preconditions compare the
  configured server port alongside the host, so a same-host/different-port
  link cannot resolve its share token or game id on the wrong instance
- the link sign-in dialog reuses an existing server entry's saved name
  instead of renaming it to the raw hostname
- skipStartupAutoConnect is cleared once the launch chain connects, so a
  later mid-session declined link cannot fire the startup fallback
- the plain-launch path of SingleInstanceManager no longer blocks on the
  primary's ACK
- link- and server-supplied text is html-escaped in the confirm prompts and
  shared-deck preview so markup cannot spoof the shown messages

cockatrice/src/interface/intents/intent_join_server_game.cpp

@@ -19,12 +19,17 @@ bool IntentJoinServerGame::checkPrecondition() const
     if (remoteClient->getStatus() != ClientStatus::StatusLoggedIn) {
         return false;
     }
-    // serverName() reflects the server the client was configured to connect to,
-    // which may differ from the actual TCP peer (e.g. when connecting through a
-    // proxy), so only the hostname is compared here.
+    // serverName()/serverPort() reflect the server the client was configured
+    // to connect to, which may differ from the actual TCP peer (e.g. when
+    // connecting through a proxy), so compare those configured values. A link
+    // naming the same host on another port is a different server and must not
+    // reuse the session there.
     if (remoteClient->serverName().compare(context->roomContext.serverContext.hostname, Qt::CaseInsensitive) != 0) {
         return false;
     }
+    if (QString::number(remoteClient->serverPort()) != context->roomContext.serverContext.port) {
+        return false;
+    }
 
     if (!tabSupervisor->getRoomTabs().contains(context->roomContext.roomId)) {
         return false;

cockatrice/src/interface/intents/intent_login.cpp

@@ -60,8 +60,19 @@ void IntentGetLoginCredentials::onPreconditionNotSatisfied()
 
     if (dialog.savePassword() && !context->username.isEmpty()) {
         ServersSettings &servers = SettingsCache::instance().servers();
-        servers.addNewServer(context->hostname, context->hostname, context->port, context->username, context->password,
-                             true);
+        // The host may already be saved under a friendly name (e.g. a public-server
+        // list entry) with no credentials; reuse that name instead of overwriting
+        // it with the raw hostname when addNewServer updates the entry in place.
+        QString saveName = context->hostname;
+        const int existingIndex = servers.findServerIndex(context->hostname, context->port);
+        if (existingIndex >= 0) {
+            saveName =
+                servers.getValue(QString("saveName%1").arg(existingIndex), "server", "server_details").toString();
+            if (saveName.isEmpty()) {
+                saveName = context->hostname;
+            }
+        }
+        servers.addNewServer(saveName, context->hostname, context->port, context->username, context->password, true);
     }
 
     emitFinished();

cockatrice/src/interface/intents/intent_open_shared_deck.cpp

@@ -34,10 +34,15 @@ bool IntentOpenSharedDeck::checkPrecondition() const
     if (remoteClient->getStatus() != ClientStatus::StatusLoggedIn) {
         return false;
     }
-    // serverName() reflects the server the client was configured to connect to,
-    // which may differ from the actual TCP peer (e.g. when connecting through a
-    // proxy), so only the hostname is compared here.
-    return remoteClient->serverName().compare(context->serverContext.hostname, Qt::CaseInsensitive) == 0;
+    // serverName()/serverPort() reflect the server the client was configured
+    // to connect to, which may differ from the actual TCP peer (e.g. when
+    // connecting through a proxy), so compare those configured values. The
+    // share token must be resolved against the host the link named — a link to
+    // the same host on another port is a different server.
+    if (remoteClient->serverName().compare(context->serverContext.hostname, Qt::CaseInsensitive) != 0) {
+        return false;
+    }
+    return QString::number(remoteClient->serverPort()) == context->serverContext.port;
 }
 
 void IntentOpenSharedDeck::onPreconditionSatisfied()

cockatrice/src/interface/intents/url_parser.cpp

@@ -208,9 +208,13 @@ Intent *IntentUrlParser::createOpenDeckIntent(const QUrlQuery &query, PendingInt
     // taking the session anywhere it isn't already, naming the host we would
     // connect to. Remember the link's target when it moves us away from a live
     // session so a failed or cancelled chain can restore the session it left.
+    // The hostname is link-supplied and percent-decoded, so escape it: QMessageBox
+    // renders AutoText, and markup in a hostname would otherwise flip the whole
+    // prompt to rich text and let a link pad the message the user is shown.
     const bool alreadyConnected = isConnectedTo(ctx->serverContext.hostname, ctx->serverContext.port);
     if (!alreadyConnected) {
-        const QString target = QStringLiteral("%1:%2").arg(ctx->serverContext.hostname, ctx->serverContext.port);
+        const QString target =
+            QStringLiteral("%1:%2").arg(ctx->serverContext.hostname.toHtmlEscaped(), ctx->serverContext.port);
 
         if (client->getStatus() == StatusLoggedIn) {
             const QString current =
@@ -273,12 +277,14 @@ Intent *IntentUrlParser::createOpenDeckIntent(const QUrlQuery &query, PendingInt
 
 bool IntentUrlParser::isConnectedTo(const QString &hostname, const QString &port) const
 {
-    Q_UNUSED(port);
-    // Deliberately hostname-only (no port): the intents' preconditions apply the
-    // same rule, so a link to the same host on another port still connects
-    // rather than silently reusing an existing session on a different server.
+    // serverName() reflects the server the client was configured to connect to,
+    // which may differ from the actual TCP peer (e.g. when connecting through a
+    // proxy), so compare the configured host and port — exactly what a link
+    // names. A link to the same host on another port is a different server and
+    // must not silently reuse an existing session there.
     RemoteClient *client = mainWindow->getRemoteClient();
-    return client->getStatus() == StatusLoggedIn && client->serverName().compare(hostname, Qt::CaseInsensitive) == 0;
+    return client->getStatus() == StatusLoggedIn && client->serverName().compare(hostname, Qt::CaseInsensitive) == 0 &&
+           QString::number(client->serverPort()) == port;
 }
 
 void IntentUrlParser::startNextChain()

cockatrice/src/interface/widgets/deck_share/shared_deck_preview_widget.cpp

@@ -33,7 +33,8 @@ SharedDeckPreviewWidget::SharedDeckPreviewWidget(QWidget *parent,
     colorIdentityWidget = new ColorIdentityWidget(this, colorIdentity);
     colorIdentityWidget->setVisible(!colorIdentity.isEmpty());
 
-    gameFormatLabel = new QLabel(gameFormat, this);
+    // gameFormat is server-supplied and the QLabel renders AutoText, so escape it.
+    gameFormatLabel = new QLabel(gameFormat.toHtmlEscaped(), this);
     gameFormatLabel->setAlignment(Qt::AlignCenter);
     gameFormatLabel->setVisible(!gameFormat.isEmpty());
 

cockatrice/src/interface/widgets/dialogs/dlg_shared_decks_preview.cpp

@@ -25,7 +25,10 @@ DlgSharedDecksPreview::DlgSharedDecksPreview(QWidget *parent,
 
     auto *mainLayout = new QVBoxLayout(this);
 
-    auto *titleLabel = new QLabel(tr("Share: %1").arg(shareName.isEmpty() ? tr("Untitled") : shareName), this);
+    // shareName and serverText come from the share server, so escape them: the
+    // QLabels render AutoText and markup would otherwise be shown as rich text.
+    auto *titleLabel =
+        new QLabel(tr("Share: %1").arg((shareName.isEmpty() ? tr("Untitled") : shareName).toHtmlEscaped()), this);
     QFont titleFont = titleLabel->font();
     titleFont.setBold(true);
     titleFont.setPointSize(titleFont.pointSize() + 2);
@@ -33,7 +36,7 @@ DlgSharedDecksPreview::DlgSharedDecksPreview(QWidget *parent,
     mainLayout->addWidget(titleLabel);
 
     if (!serverText.isEmpty()) {
-        mainLayout->addWidget(new QLabel(tr("From %1").arg(serverText), this));
+        mainLayout->addWidget(new QLabel(tr("From %1").arg(serverText.toHtmlEscaped()), this));
     }
 
     if (expiresAt > 0) {
@@ -57,7 +60,7 @@ DlgSharedDecksPreview::DlgSharedDecksPreview(QWidget *parent,
         auto *tile = new SharedDeckPreviewWidget(
             this, querier, QString::fromStdString(item.name()), QString::fromStdString(item.banner_card()),
             QString::fromStdString(item.color_identity()), QString::fromStdString(item.game_format()), tags.join(", "));
-        flowWidget->addWidget(tile);
+        flowWidget->addNavigableWidget(tile);
         tiles.append(tile);
         itemIds.append(item.id());
     }

cockatrice/src/interface/widgets/general/layout_containers/flow_widget.cpp

@@ -81,13 +81,30 @@ FlowWidget::FlowWidget(QWidget *parent,
 /**
  * @brief Adds a widget to the flow layout within the FlowWidget.
  *
- * The widget is filtered for arrow-key events so keyboard navigation between
- * the flow items keeps working even when the flow sits inside a QScrollArea,
- * which swallows arrow keys before they can reach FlowWidget::keyPressEvent.
+ * Plain widgets are not filtered for arrow keys: intercepting them would steal
+ * Up/Down/Left/Right from controls that use them (combo boxes, spin boxes
+ * etc.). Widgets that want keyboard navigation between flow items must be
+ * added via addNavigableWidget instead.
  *
  * @param widget_to_add The widget to add to the flow layout.
  */
 void FlowWidget::addWidget(QWidget *widget_to_add)
+{
+    flowLayout->addWidget(widget_to_add);
+}
+
+/**
+ * @brief Adds a widget and routes its arrow keys to FlowWidget focus navigation.
+ *
+ * The widget is filtered for arrow-key events so keyboard navigation between
+ * the flow items keeps working even when the flow sits inside a QScrollArea,
+ * which swallows arrow keys before they can reach FlowWidget::keyPressEvent.
+ * Only widgets added through this method are affected; anything that needs its
+ * own arrow keys should use plain addWidget.
+ *
+ * @param widget_to_add The widget to add to the flow layout.
+ */
+void FlowWidget::addNavigableWidget(QWidget *widget_to_add)
 {
     widget_to_add->installEventFilter(this);
     flowLayout->addWidget(widget_to_add);

cockatrice/src/interface/widgets/general/layout_containers/flow_widget.h

@@ -30,6 +30,7 @@ public:
                Qt::ScrollBarPolicy verticalPolicy);
 
     void addWidget(QWidget *widget_to_add);
+    void addNavigableWidget(QWidget *widget_to_add);
     void insertWidgetAtIndex(QWidget *toInsert, int index);
     void removeWidget(QWidget *widgetToRemove) const;
     void clearLayout();

cockatrice/src/interface/window_main.cpp

@@ -939,7 +939,16 @@ void MainWindow::onUrlChainFinished(bool connected)
     // chain ended without connecting (declined, invalid, offline), fall back to
     // the startup connection so the activation launch still behaves like a
     // normal launch.
-    if (connected || !skipStartupAutoConnect || getRemoteClient()->getStatus() != StatusDisconnected) {
+    if (connected) {
+        // The launch link connected, so the startup fallback has served its
+        // purpose: drop the skip so a later mid-session link that ends declined
+        // or offline cannot silently fire auto-connect or applyStartupDestination
+        // again.
+        skipStartupAutoConnect = false;
+        return;
+    }
+
+    if (!skipStartupAutoConnect || getRemoteClient()->getStatus() != StatusDisconnected) {
         return;
     }
 

cockatrice/src/single_instance_manager.cpp

@@ -92,6 +92,14 @@ SingleInstanceManager::ForwardResult SingleInstanceManager::forwardToPrimary(con
     socket.flush();
     socket.waitForBytesWritten(1000);
 
+    // A plain launch has nothing for the primary to act on, so there is nothing
+    // to acknowledge. Waiting here would block the new instance for seconds if
+    // the primary is busy in a modal dialog, so only the activation path (which
+    // needs the ACK to avoid stealing a live primary's socket) waits below.
+    if (filesToSend.isEmpty()) {
+        return ForwardResult::Delivered;
+    }
+
     // Only report a successful hand-off once the primary has acknowledged that
     // it actually read the payload. A socket that connects but is still working
     // on an earlier payload is alive but busy, not dead: give it more room