From 03eb35ab8ef1e8482cfd710da2025986dbafb5b0 Mon Sep 17 00:00:00 2001 From: Kenneth VanderLinde Date: Mon, 10 Feb 2025 13:02:32 -0800 Subject: [PATCH 1/7] Log stack traces when handling unexpected exceptions in WebRTCServer --- .../net/rptools/clientserver/simple/server/WebRTCServer.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clientserver/src/main/java/net/rptools/clientserver/simple/server/WebRTCServer.java b/clientserver/src/main/java/net/rptools/clientserver/simple/server/WebRTCServer.java index 32ad0b8c7b..a7a42cd72a 100644 --- a/clientserver/src/main/java/net/rptools/clientserver/simple/server/WebRTCServer.java +++ b/clientserver/src/main/java/net/rptools/clientserver/simple/server/WebRTCServer.java @@ -105,7 +105,7 @@ public void onClose(int code, String reason, boolean remote) { @Override public void onError(Exception ex) { lastError = "WebSocket error: " + ex.toString() + "\n"; - log.error("S " + lastError); + log.error("S " + lastError, ex); // onClose will be called after this method } }; @@ -147,7 +147,7 @@ public void onDataChannelOpened(WebRTCConnection connection) { try { fireClientConnect(connection); } catch (Exception e) { - log.error(e); + log.error("Unexpected error while handling new data channel", e); } } From 2f48f47e0309557132d378846934a1d0b1d1ada8 Mon Sep 17 00:00:00 2001 From: Kenneth VanderLinde Date: Mon, 10 Feb 2025 13:14:12 -0800 Subject: [PATCH 2/7] Fix Easy Connect handshake flow when public key is requested It is vital that the non-Easy Connect flow for public keys is not run when `!playerDatabase.getPublicKey(...)`. --- src/main/java/net/rptools/maptool/server/ServerHandshake.java | 1 + 1 file changed, 1 insertion(+) diff --git a/src/main/java/net/rptools/maptool/server/ServerHandshake.java b/src/main/java/net/rptools/maptool/server/ServerHandshake.java index 919339615a..c1e4b537a1 100644 --- a/src/main/java/net/rptools/maptool/server/ServerHandshake.java +++ b/src/main/java/net/rptools/maptool/server/ServerHandshake.java @@ -523,6 +523,7 @@ private void sendAsymmetricKeyAuthType() } else { sendErrorResponseAndNotify(HandshakeResponseCodeMsg.INVALID_PUBLIC_KEY); } + return; } CipherUtil.Key publicKey = playerDatabase.getPublicKey(player, playerPublicKeyMD5).get(); String password = new PasswordGenerator().getPassword(); From 44af5344085049487fe018f757cd4abc091ee222 Mon Sep 17 00:00:00 2001 From: Kenneth VanderLinde Date: Mon, 10 Feb 2025 13:03:23 -0800 Subject: [PATCH 3/7] Change the Easy Connect fix to use an else{} block This more clearly communicates the relationship between the `!playerDatabase.hasPublicKey(...)` and the latter half of the method. --- .../maptool/server/ServerHandshake.java | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/main/java/net/rptools/maptool/server/ServerHandshake.java b/src/main/java/net/rptools/maptool/server/ServerHandshake.java index c1e4b537a1..eed28a7c44 100644 --- a/src/main/java/net/rptools/maptool/server/ServerHandshake.java +++ b/src/main/java/net/rptools/maptool/server/ServerHandshake.java @@ -523,19 +523,19 @@ private void sendAsymmetricKeyAuthType() } else { sendErrorResponseAndNotify(HandshakeResponseCodeMsg.INVALID_PUBLIC_KEY); } - return; + } else { + CipherUtil.Key publicKey = playerDatabase.getPublicKey(player, playerPublicKeyMD5).get(); + String password = new PasswordGenerator().getPassword(); + handshakeChallenges[0] = + HandshakeChallenge.createAsymmetricChallenge(player.getName(), password, publicKey); + + var authTypeMsg = + UseAuthTypeMsg.newBuilder() + .setAuthType(AuthTypeEnum.ASYMMETRIC_KEY) + .addChallenge(ByteString.copyFrom(handshakeChallenges[0].getChallenge())); + var handshakeMsg = HandshakeMsg.newBuilder().setUseAuthTypeMsg(authTypeMsg).build(); + sendMessage(State.AwaitingClientPublicKeyAuth, handshakeMsg); } - CipherUtil.Key publicKey = playerDatabase.getPublicKey(player, playerPublicKeyMD5).get(); - String password = new PasswordGenerator().getPassword(); - handshakeChallenges[0] = - HandshakeChallenge.createAsymmetricChallenge(player.getName(), password, publicKey); - - var authTypeMsg = - UseAuthTypeMsg.newBuilder() - .setAuthType(AuthTypeEnum.ASYMMETRIC_KEY) - .addChallenge(ByteString.copyFrom(handshakeChallenges[0].getChallenge())); - var handshakeMsg = HandshakeMsg.newBuilder().setUseAuthTypeMsg(authTypeMsg).build(); - sendMessage(State.AwaitingClientPublicKeyAuth, handshakeMsg); } @Override From 0b6cdce4f57752d846d40d2aa92bf57601716a54 Mon Sep 17 00:00:00 2001 From: Kenneth VanderLinde Date: Sun, 26 May 2024 00:02:47 -0700 Subject: [PATCH 4/7] Cherry-pick 70556be6eaafc473b745e5724820908aec1169d1 This commit never got merged from 1.15 into develop, so is missing from 1.16 and mainline. Original message was: > Do not run the InfoTextSwingWorker on the EDT thread > > It's meant to be run off-thread, so use `.execute()` instead of `.run()`. --- src/main/java/net/rptools/maptool/client/ui/SysInfoDialog.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/net/rptools/maptool/client/ui/SysInfoDialog.java b/src/main/java/net/rptools/maptool/client/ui/SysInfoDialog.java index 235ffd18d4..ea98fc0d90 100644 --- a/src/main/java/net/rptools/maptool/client/ui/SysInfoDialog.java +++ b/src/main/java/net/rptools/maptool/client/ui/SysInfoDialog.java @@ -47,7 +47,7 @@ private Container createContentPane() { infoTextArea.setWrapStyleWord(true); infoTextArea.setFont(new Font("Monospaced", Font.PLAIN, 13)); infoTextArea.setText(I18N.getText("action.gatherDebugInfoWait")); - EventQueue.invokeLater(new InfoTextSwingWorker()); + new InfoTextSwingWorker().execute(); JScrollPane scrollPane = new JScrollPane(infoTextArea); scrollPane.setHorizontalScrollBarPolicy(31); From bb011965ecb139fb4eb8a08e23564eed857cc478 Mon Sep 17 00:00:00 2001 From: Kenneth VanderLinde Date: Tue, 11 Feb 2025 00:15:22 -0800 Subject: [PATCH 5/7] Remove unused ResourceLoader class --- .../maptool/client/swing/ResourceLoader.java | 45 ------------------- 1 file changed, 45 deletions(-) delete mode 100644 src/main/java/net/rptools/maptool/client/swing/ResourceLoader.java diff --git a/src/main/java/net/rptools/maptool/client/swing/ResourceLoader.java b/src/main/java/net/rptools/maptool/client/swing/ResourceLoader.java deleted file mode 100644 index 33f9590afa..0000000000 --- a/src/main/java/net/rptools/maptool/client/swing/ResourceLoader.java +++ /dev/null @@ -1,45 +0,0 @@ -/* - * This software Copyright by the RPTools.net development team, and - * licensed under the Affero GPL Version 3 or, at your option, any later - * version. - * - * MapTool Source Code is distributed in the hope that it will be - * useful, but WITHOUT ANY WARRANTY; without even the implied warranty - * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. - * - * You should have received a copy of the GNU Affero General Public - * License * along with this source Code. If not, please visit - * and specifically the Affero license - * text at . - */ -package net.rptools.maptool.client.swing; - -import java.awt.Rectangle; -import java.util.StringTokenizer; - -// This should really be in rplib -public class ResourceLoader { - - /** - * Rectangles are in the form x, y, width, height - * - * @throws IllegalArgumentException if rectString can't be parsed - * @param rectString string describing the rectangle - * @return a {@link Rectangle} for x, y, width, height - */ - public static Rectangle loadRectangle(String rectString) { - - StringTokenizer strtok = new StringTokenizer(rectString, ","); - if (strtok.countTokens() != 4) { - throw new IllegalArgumentException( - "Could not load rectangle: '" + rectString + "', must be in the form x, y, w, h"); - } - - int x = Integer.parseInt(strtok.nextToken().trim()); - int y = Integer.parseInt(strtok.nextToken().trim()); - int w = Integer.parseInt(strtok.nextToken().trim()); - int h = Integer.parseInt(strtok.nextToken().trim()); - - return new Rectangle(x, y, w, h); - } -} From a7938cc0c5d3fd41281ccabdd1192efde3faa7cb Mon Sep 17 00:00:00 2001 From: Kenneth VanderLinde Date: Tue, 11 Feb 2025 00:26:12 -0800 Subject: [PATCH 6/7] Make the initial trivial campaign truly empty This fixes #5219 by not loading `ZoneFactory` too early, as its static initializer can only succeed if the default resources have already been installed. On MT's first launch, this is not true, so the asset is never made available to new zones. An added perk is that this trivial campaign is even more lightweight now, since there is no need to read, hash, and load an image. --- src/main/java/net/rptools/maptool/client/MapTool.java | 2 +- .../java/net/rptools/maptool/model/CampaignFactory.java | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/src/main/java/net/rptools/maptool/client/MapTool.java b/src/main/java/net/rptools/maptool/client/MapTool.java index 68e3cd98f3..de37477fde 100644 --- a/src/main/java/net/rptools/maptool/client/MapTool.java +++ b/src/main/java/net/rptools/maptool/client/MapTool.java @@ -182,7 +182,7 @@ public class MapTool { try { var connections = DirectConnection.create("local"); var playerDB = new PersonalServerPlayerDatabase(new LocalPlayer()); - var campaign = CampaignFactory.createBasicCampaign(); + var campaign = CampaignFactory.createEmptyCampaign(); var policy = new ServerPolicy(); server = new MapToolServer("", new Campaign(campaign), null, false, policy, playerDB); diff --git a/src/main/java/net/rptools/maptool/model/CampaignFactory.java b/src/main/java/net/rptools/maptool/model/CampaignFactory.java index 59decf9156..ef5523806c 100644 --- a/src/main/java/net/rptools/maptool/model/CampaignFactory.java +++ b/src/main/java/net/rptools/maptool/model/CampaignFactory.java @@ -15,6 +15,13 @@ package net.rptools.maptool.model; public class CampaignFactory { + /** + * @return A new campaign with no zones or properties. + */ + public static Campaign createEmptyCampaign() { + return new Campaign(); + } + public static Campaign createBasicCampaign() { Campaign campaign = new Campaign(); campaign.initDefault(); From 73aea8acf776ff65f08d00fbf75dc3f8897fa9b1 Mon Sep 17 00:00:00 2001 From: Kenneth VanderLinde Date: Tue, 11 Feb 2025 00:37:52 -0800 Subject: [PATCH 7/7] ZoneFactory to cache default grass texture only on success This is a second complete fix for #5219. If the `ZoneFactory` fails to get the default texture, it will not cache that result but will try again for the next zone that gets created. Additionally, if the texture was not loaded, `ZoneFactory` will fall back to a solid color so that the user is not left with a screenfull of red X's. This means that if the texture file is found but there is an error loading it, then `ZoneFactory` will try to read it again the next time a zone is created. For a corrupt file, this could result in repeated failures and extra work repeatedly trying to load the asset. However, zone creation is not a high-frequency event, so this is acceptable. --- .../rptools/maptool/model/ZoneFactory.java | 53 +++++++++++++------ 1 file changed, 38 insertions(+), 15 deletions(-) diff --git a/src/main/java/net/rptools/maptool/model/ZoneFactory.java b/src/main/java/net/rptools/maptool/model/ZoneFactory.java index 7a18412c0f..04e4e913e0 100644 --- a/src/main/java/net/rptools/maptool/model/ZoneFactory.java +++ b/src/main/java/net/rptools/maptool/model/ZoneFactory.java @@ -17,44 +17,67 @@ import java.awt.Color; import java.io.File; import java.io.IOException; +import javax.annotation.Nullable; import net.rptools.lib.MD5Key; import net.rptools.maptool.client.AppPreferences; import net.rptools.maptool.client.AppUtil; import net.rptools.maptool.model.drawing.DrawableColorPaint; +import net.rptools.maptool.model.drawing.DrawablePaint; import net.rptools.maptool.model.drawing.DrawableTexturePaint; import net.rptools.maptool.util.ImageManager; import org.apache.commons.io.FileUtils; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; public class ZoneFactory { + private static final Logger log = LogManager.getLogger(ZoneFactory.class); public static final String DEFAULT_MAP_NAME = "Grasslands"; - public static MD5Key defaultImageId; + private static final DrawableColorPaint fallbackBackgroundPaint = + new DrawableColorPaint(Color.LIGHT_GRAY); + private static @Nullable MD5Key defaultImageId; + + private static DrawablePaint getDefaultBackgroundPaint() { + if (defaultImageId != null) { + return new DrawableTexturePaint(defaultImageId); + } - static { // TODO: I really don't like this being hard wired this way, need to make it a preference or - // something + // something File grassImage = new File(AppUtil.getAppHome("resource/Default/Textures").getAbsolutePath() + "/Grass.png"); - if (grassImage.exists()) { - try { - Asset asset = - Asset.createImageAsset(DEFAULT_MAP_NAME, FileUtils.readFileToByteArray(grassImage)); - defaultImageId = asset.getMD5Key(); + if (!grassImage.exists()) { + log.warn( + "Unable to load the default background texture: file {} does not exist", + grassImage.getPath()); + return fallbackBackgroundPaint; + } + + MD5Key imageId; + try { + Asset asset = + Asset.createImageAsset(DEFAULT_MAP_NAME, FileUtils.readFileToByteArray(grassImage)); + imageId = asset.getMD5Key(); - // Make sure the image is loaded to avoid a flash screen when it becomes visible - ImageManager.getImageAndWait(asset.getMD5Key()); - } catch (IOException ioe) { - ioe.printStackTrace(); - } + // Make sure the image is loaded to avoid a flash screen when it becomes visible + ImageManager.getImageAndWait(asset.getMD5Key()); + } catch (IOException ioe) { + log.error("Error reading default background image", ioe); + return fallbackBackgroundPaint; } + + defaultImageId = imageId; + return new DrawableTexturePaint(defaultImageId); } public static Zone createZone() { - Zone zone = new Zone(); zone.setName(DEFAULT_MAP_NAME); - zone.setBackgroundPaint(new DrawableTexturePaint(defaultImageId)); + + var backgroundPaint = getDefaultBackgroundPaint(); + zone.setBackgroundPaint(backgroundPaint); + zone.setFogPaint(new DrawableColorPaint(Color.black)); zone.setVisible(AppPreferences.newMapsVisible.get());