🐛 Encode remote-only URLs. Resolves #433

ebullient · Apr 24, 2024 · d6e5a30 · d6e5a30
1 parent fee7583
commit d6e5a30
Show file tree

Hide file tree

Showing 3 changed files with 60 additions and 36 deletions.
diff --git a/src/main/java/dev/ebullient/convert/io/Tui.java b/src/main/java/dev/ebullient/convert/io/Tui.java
@@ -6,8 +6,6 @@
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.PrintWriter;
-import java.io.UnsupportedEncodingException;
-import java.net.MalformedURLException;
 import java.net.URI;
 import java.net.URL;
 import java.nio.channels.Channels;
@@ -438,28 +436,6 @@ private void copyImageResource(ImageRef image, Path targetPath) {
         }
     }
 
-    private final static String allowedCharacters = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789-._~/";
-
-    String escapeUrlImagePath(String url) throws MalformedURLException, UnsupportedEncodingException {
-        URL urlObject = new URL(url);
-        String path = urlObject.getPath();
-
-        StringBuilder encodedPath = new StringBuilder();
-        for (char ch : path.toCharArray()) {
-            if (allowedCharacters.indexOf(ch) == -1) {
-                byte[] bytes = String.valueOf(ch).getBytes("UTF-8");
-                for (byte b : bytes) {
-                    encodedPath.append(String.format("%%%02X", b));
-                }
-            } else {
-                encodedPath.append(ch);
-            }
-        }
-
-        return url.replace(path, encodedPath.toString())
-                .replace("/imgur.com", "/i.imgur.com");
-    }
-
     private void copyRemoteImage(ImageRef image, Path targetPath) {
         targetPath.getParent().toFile().mkdirs();
 
@@ -474,7 +450,6 @@ private void copyRemoteImage(ImageRef image, Path targetPath) {
         }
 
         try {
-            url = escapeUrlImagePath(url);
             Tui.instance().debugf("copy image %s", url);
 
             ReadableByteChannel readableByteChannel = Channels.newChannel(new URL(url).openStream());

diff --git a/src/main/java/dev/ebullient/convert/qute/ImageRef.java b/src/main/java/dev/ebullient/convert/qute/ImageRef.java
@@ -1,6 +1,8 @@
 package dev.ebullient.convert.qute;
 
+import java.io.IOException;
 import java.io.UnsupportedEncodingException;
+import java.net.URL;
 import java.nio.charset.StandardCharsets;
 import java.nio.file.Path;
 
@@ -220,7 +222,7 @@ public ImageRef build() {
                     .replace('\\', '/');
 
             try {
-                // Remove escaped characters here (local file paths won't want it)
+                // Remove escaped characters here (inconsistent escaping in the source URL)
                 sourceUrl = java.net.URLDecoder.decode(sourceUrl, StandardCharsets.UTF_8.name());
             } catch (UnsupportedEncodingException e) {
                 Tui.instance().errorf("Error decoding image URL: %s", e.getMessage());
@@ -244,14 +246,15 @@ public ImageRef build() {
 
                 // remote images to be copied into the vault
                 if (sourceUrl.startsWith("http") || sourceUrl.startsWith("file")) {
-                    return new ImageRef(sourceUrl, null, targetFilePath, title, vaultPath, width);
+                    return new ImageRef(escapeUrlImagePath(sourceUrl),
+                            null, targetFilePath, title, vaultPath, width);
                 }
                 // local image to be copied into the vault
                 return new ImageRef(null, Path.of(sourceUrl), targetFilePath, title, vaultPath, width);
             }
 
             // remote images that are not copied to the vault --> url image ref, no target
-            return new ImageRef(sourceUrl,
+            return new ImageRef(escapeUrlImagePath(sourceUrl),
                     null, null, title, null, width);
         }
 
@@ -267,5 +270,31 @@ public ImageRef build(ImageRef previous) {
                 return build();
             }
         }
+
+        private final static String allowedCharacters = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789-._~/";
+
+        public static String escapeUrlImagePath(String url) {
+            try {
+                URL urlObject = new URL(url);
+                String path = urlObject.getPath();
+
+                StringBuilder encodedPath = new StringBuilder();
+                for (char ch : path.toCharArray()) {
+                    if (allowedCharacters.indexOf(ch) == -1) {
+                        byte[] bytes = String.valueOf(ch).getBytes("UTF-8");
+                        for (byte b : bytes) {
+                            encodedPath.append(String.format("%%%02X", b));
+                        }
+                    } else {
+                        encodedPath.append(ch);
+                    }
+                }
+                return url.replace(path, encodedPath.toString())
+                        .replace("/imgur.com", "/i.imgur.com");
+            } catch (IOException e) {
+                Tui.instance().errorf(e, "Unable to escape URL path %s (%s)", url, e);
+                return url;
+            }
+        }
     }
 }
diff --git a/...ev/ebullient/convert/io/ImgurUrlTest.java → .../ebullient/convert/qute/ImageRefTest.java b/...ev/ebullient/convert/io/ImgurUrlTest.java → .../ebullient/convert/qute/ImageRefTest.java
@@ -1,4 +1,4 @@
-package dev.ebullient.convert.io;
+package dev.ebullient.convert.qute;
 
 import static org.assertj.core.api.Assertions.assertThat;
 
@@ -10,14 +10,14 @@
 import org.junit.jupiter.api.Test;
 
 import dev.ebullient.convert.config.TtrpgConfig;
-import dev.ebullient.convert.qute.ImageRef;
+import dev.ebullient.convert.io.Tui;
 import dev.ebullient.convert.tools.dnd5e.Tools5eIndex;
 import dev.ebullient.convert.tools.dnd5e.Tools5eSources;
 import io.quarkus.arc.Arc;
 import io.quarkus.test.junit.QuarkusTest;
 
 @QuarkusTest
-public class ImgurUrlTest {
+public class ImageRefTest {
     protected static Tui tui;
     protected static Tools5eIndex index;
 
@@ -32,17 +32,20 @@ public static void prepare() {
     public void testImgurUrl() throws MalformedURLException, UnsupportedEncodingException {
         String input = "https://imgur.com/lQfZ1dF.png";
 
-        assertThat(tui.escapeUrlImagePath(input))
+        assertThat(ImageRef.Builder.escapeUrlImagePath(input))
                 .isEqualTo("https://i.imgur.com/lQfZ1dF.png");
     }
 
     @Test
     public void testAccentedCharacters() throws MalformedURLException, UnsupportedEncodingException {
         String input = "https://whatever.com/áé.png?raw=true";
 
-        assertThat(tui.escapeUrlImagePath(input))
+        assertThat(ImageRef.Builder.escapeUrlImagePath(input))
                 .isEqualTo("https://whatever.com/%C3%A1%C3%A9.png?raw=true");
+    }
 
+    @Test
+    public void testEncodedRemoteUrl() throws Exception {
         Tools5eSources sources = Tools5eSources.findOrTemporary(
                 Tui.MAPPER.createObjectNode()
                         .put("name", "Critter")
@@ -53,7 +56,7 @@ public void testAccentedCharacters() throws MalformedURLException, UnsupportedEn
                 Path.of("something.png"),
                 false);
 
-        assertThat(tui.escapeUrlImagePath(ref.url()))
+        assertThat(ref.url())
                 .isEqualTo(
                         "https://raw.githubusercontent.com/TheGiddyLimit/homebrew/master/_img/MonsterManualExpanded3/creature/Hill%20Giant%20Warlock%20Of%20Ogr%C3%A9moch.jpg");
 
@@ -62,9 +65,26 @@ public void testAccentedCharacters() throws MalformedURLException, UnsupportedEn
                 Path.of("something.png"),
                 false);
 
-        assertThat(tui.escapeUrlImagePath(ref.url()))
+        assertThat(ref.url())
                 .isEqualTo(
                         "https://raw.githubusercontent.com/TheGiddyLimit/homebrew/master/_img/MonsterManualExpanded3/creature/token/Stone%20Giant%20Warlock%20Of%20Ogr%C3%A9moch%20%28Token%29.png");
-    }
 
+        // only Remote URL (not local)
+        ref = new ImageRef.Builder()
+                .setUrl("https://raw.githubusercontent.com/TheGiddyLimit/homebrew/master/_img/MonsterManualExpanded3/creature/token/Stone%20Giant%20Warlock%20Of%20Ogrémoch%20%28Token%29.png")
+                .build();
+
+        assertThat(ref.url())
+                .isEqualTo(
+                        "https://raw.githubusercontent.com/TheGiddyLimit/homebrew/master/_img/MonsterManualExpanded3/creature/token/Stone%20Giant%20Warlock%20Of%20Ogr%C3%A9moch%20%28Token%29.png");
+
+        // use Remote URL
+        ref = new ImageRef.Builder()
+                .setUrl("https://raw.githubusercontent.com/5etools-mirror-2/5etools-img/main/bestiary/tokens/MM/Giant Owl.webp#token")
+                .build();
+
+        assertThat(ref.url())
+                .isEqualTo(
+                        "https://raw.githubusercontent.com/5etools-mirror-2/5etools-img/main/bestiary/tokens/MM/Giant%20Owl.webp#token");
+    }
 }