Changeset 19651 in josm

Timestamp:

2026-10-22T16:46:16+02:00 (4 hours ago)

Author:

stoecker

Message:

fix #24907 - patch by wangi - Trailing slash in API breaks OAuth and links - better normalization

Location:

trunk

Files:

• . (modified) (1 prop)
• src/org/openstreetmap/josm/data/oauth/OAuthParameters.java (modified) (2 diffs)
• src/org/openstreetmap/josm/gui/oauth/TestAccessTokenTask.java (modified) (3 diffs)
• src/org/openstreetmap/josm/gui/preferences/server/ApiUrlTestTask.java (modified) (8 diffs)
• src/org/openstreetmap/josm/gui/preferences/server/OsmApiUrlInputPanel.java (modified) (5 diffs)
• src/org/openstreetmap/josm/io/OsmApi.java (modified) (2 diffs)
• test/unit/org/openstreetmap/josm/data/preferences/JosmUrlsTest.java (modified) (1 diff)
• test/unit/org/openstreetmap/josm/io/OsmApiTest.java (modified) (3 diffs)

trunk

@@ -25 +25 @@
 target
 failed.png
+dependency-reduced-pom.xml

@@ -25 +25 @@
 target
 failed.png
+dependency-reduced-pom.xml

trunk/src/org/openstreetmap/josm/data/oauth/OAuthParameters.java

@@ -148 +148 @@
         final String redirectUri = "http://127.0.0.1:8111/oauth_authorization";
         final String baseUrl;
-        apiUrl = apiUrl == null ? OsmApi.getOsmApi().getServerUrl() : apiUrl;
+        apiUrl = OsmApi.normalizeApiUrl(apiUrl == null ? OsmApi.getOsmApi().getServerUrl() : apiUrl);
         switch (apiUrl) {
             case OSM_API_DEV:
@@ -239 +239 @@
      */
     public static IOAuthParameters createFromApiUrl(String apiUrl, OAuthVersion oAuthVersion) {
+        apiUrl = OsmApi.normalizeApiUrl(apiUrl);
         final String originalApiUrl = apiUrl;
         // We actually need the host

trunk/src/org/openstreetmap/josm/gui/oauth/TestAccessTokenTask.java

@@ -19 +19 @@
 import org.openstreetmap.josm.gui.PleaseWaitRunnable;
 import org.openstreetmap.josm.gui.help.HelpUtil;
+import org.openstreetmap.josm.io.OsmApi;
 import org.openstreetmap.josm.io.OsmApiException;
 import org.openstreetmap.josm.io.OsmServerUserInfoReader;
@@ -60 +61 @@
         this.tokenOAuth2 = accessToken;
         this.parent = parent;
-        this.apiUrl = apiUrl;
+        this.apiUrl = OsmApi.normalizeApiUrl(apiUrl);
     }
 
@@ -82 +83 @@
     }
 
-    protected String normalizeApiUrl(String url) {
-        // remove leading and trailing white space
-        url = url.trim();
-
-        // remove trailing slashes
-        while (url.endsWith("/")) {
-            url = url.substring(0, url.lastIndexOf('/'));
-        }
-        return url;
-    }
-
     protected UserInfo getUserDetails() throws OsmOAuthAuthorizationException, XmlParsingException, OsmTransferException {
         boolean authenticatorEnabled = true;
         try {
-            URL url = new URL(normalizeApiUrl(apiUrl) + "/0.6/user/details");
+            URL url = new URL(apiUrl + "/0.6/user/details");
             authenticatorEnabled = DefaultAuthenticator.getInstance().isEnabled();
             DefaultAuthenticator.getInstance().setEnabled(false);

trunk/src/org/openstreetmap/josm/gui/preferences/server/ApiUrlTestTask.java

@@ -17 +17 @@
 import org.openstreetmap.josm.gui.help.HelpUtil;
 import org.openstreetmap.josm.io.Capabilities;
+import org.openstreetmap.josm.io.OsmApi;
 import org.openstreetmap.josm.io.OsmTransferException;
 import org.openstreetmap.josm.tools.CheckParameterUtil;
@@ -49 +50 @@
         CheckParameterUtil.ensureParameterNotNull(url, "url");
         this.parent = parent;
-        this.url = url;
+        this.url = OsmApi.normalizeApiUrl(url);
     }
 
@@ -67 +68 @@
     }
 
-    protected void alertInvalidCapabilitiesUrl(String url) {
+    protected void alertInvalidCapabilitiesUrl(String capabilitiesUrl) {
         HelpAwareOptionPane.showMessageDialogInEDT(
                 parent,
@@ -74 +75 @@
                         + "Please check the spelling of ''{1}'' and validate again."
                         +"</html>",
-                        url,
-                        getNormalizedApiUrl()
+                        capabilitiesUrl,
+                        url
                 ),
                 tr("Invalid API URL"),
@@ -91 +92 @@
                         +"</html>",
                         url,
-                        getNormalizedApiUrl()
+                        url
                 ),
                 tr("Connection to API failed"),
@@ -108 +109 @@
                         + "</html>",
                         retCode,
-                        getNormalizedApiUrl()
+                        url
                 ),
                 tr("Connection to API failed"),
@@ -124 +125 @@
                         + "Please check the spelling of ''{0}'' and validate again."
                         + "</html>",
-                        getNormalizedApiUrl()
+                        url
                 ),
                 tr("Connection to API failed"),
@@ -147 +148 @@
     }
 
-    /**
-     * Removes leading and trailing whitespace from the API URL and removes trailing '/'.
-     *
-     * @return the normalized API URL
-     */
-    protected String getNormalizedApiUrl() {
-        String apiUrl = url.trim();
-        while (apiUrl.endsWith("/")) {
-            apiUrl = apiUrl.substring(0, apiUrl.lastIndexOf('/'));
-        }
-        return apiUrl;
-    }
-
     @Override
     protected void realRun() throws SAXException, IOException, OsmTransferException {
         try {
             try {
-                new URL(getNormalizedApiUrl());
+                new URL(url);
             } catch (MalformedURLException e) {
-                alertInvalidUrl(getNormalizedApiUrl());
+                alertInvalidUrl(url);
                 return;
             }
             URL capabilitiesUrl;
-            String getCapabilitiesUrl = getNormalizedApiUrl() + "/0.6/capabilities";
+            String getCapabilitiesUrl = url + "/0.6/capabilities";
             try {
                 capabilitiesUrl = new URL(getCapabilitiesUrl);

trunk/src/org/openstreetmap/josm/gui/preferences/server/OsmApiUrlInputPanel.java

@@ -126 +126 @@
     public void saveToPreferences() {
         String oldUrl = OsmApi.getOsmApi().getServerUrl();
-        String hmiUrl = getStrippedApiUrl();
+        String hmiUrl = getApiUrl();
         if (cbUseDefaultServerUrl.isSelected() || Config.getUrls().getDefaultOsmApiUrl().equals(hmiUrl)) {
             Config.getPref().put("osm-server.url", null);
@@ -147 +147 @@
 
     /**
-     * Returns the entered API URL, stripped of leading and trailing white characters.
-     * @return the entered API URL, stripped of leading and trailing white characters. May be an empty string
-     *         if nothing has been entered. In this case, it means the user wants to use {@link IUrls#getDefaultOsmApiUrl}.
-     * @see Utils#strip(String)
-     * @since 6602
-     */
-    public final String getStrippedApiUrl() {
-        return Utils.strip(tfOsmServerUrl.getText());
+     * Returns the entered API URL, normalized with {@link OsmApi#normalizeApiUrl(String)}, that is stripped of
+     * leading and trailing white characters and of trailing '/'.
+     * @return the entered API URL. May be an empty string if nothing has been entered. In this case, it means
+     *         the user wants to use {@link IUrls#getDefaultOsmApiUrl}.
+     * @see OsmApi#normalizeApiUrl(String)
+     * @since 19651 (renamed from {@code getStrippedApiUrl}, which existed since 6602)
+     */
+    public final String getApiUrl() {
+        return OsmApi.normalizeApiUrl(tfOsmServerUrl.getText());
     }
 
@@ -168 +169 @@
         @Override
         public void actionPerformed(ActionEvent arg0) {
-            final String url = getStrippedApiUrl();
+            final String url = getApiUrl();
             final ApiUrlTestTask task = new ApiUrlTestTask(OsmApiUrlInputPanel.this, url);
             MainApplication.worker.submit(task);
@@ -191 +192 @@
 
         protected final void updateEnabledState() {
-            String url = getStrippedApiUrl();
+            String url = getApiUrl();
             boolean enabled = !url.isEmpty() && !url.equals(lastTestedUrl);
             if (enabled) {
@@ -276 +277 @@
     class ApiUrlPropagator extends FocusAdapter implements ActionListener {
         protected void propagate() {
-            propagate(getStrippedApiUrl());
+            propagate(getApiUrl());
         }
 

trunk/src/org/openstreetmap/josm/io/OsmApi.java

@@ -134 +134 @@
      */
     public static OsmApi getOsmApi(String serverUrl) {
-        OsmApi api = instances.get(serverUrl);
+        String url = normalizeApiUrl(serverUrl);
+        OsmApi api = instances.get(url);
         if (api == null) {
-            api = new OsmApi(serverUrl);
+            api = new OsmApi(url);
             cacheInstance(api);
         }
         return api;
+    }
+
+    /**
+     * Normalizes an OSM API URL: removes leading and trailing white space and any trailing {@code '/'}.
+     * <p>
+     * The URL may be entered with a trailing slash, but code which compares it to a known API URL or derives
+     * another URL from it does not expect one: OAuth would not find the client id of the server, and the links
+     * to the web site of the server (browse, history, user) would be built from the API path.
+     * @param serverUrl the server URL, may be null
+     * @return the normalized URL, or null if {@code serverUrl} is null
+     * @since 19651
+     */
+    public static String normalizeApiUrl(String serverUrl) {
+        if (serverUrl == null) {
+            return null;
+        }
+        String url = Utils.strip(serverUrl);
+        while (url.endsWith("/") && !url.endsWith("://")) {
+            url = url.substring(0, url.length() - 1);
+        }
+        return url;
     }
 
@@ -182 +204 @@
     protected OsmApi(String serverUrl) {
         CheckParameterUtil.ensureParameterNotNull(serverUrl, "serverUrl");
-        this.serverUrl = serverUrl;
+        this.serverUrl = normalizeApiUrl(serverUrl);
     }
 

trunk/test/unit/org/openstreetmap/josm/data/preferences/JosmUrlsTest.java

@@ -20 +20 @@
         assertEquals("https://api06.dev.openstreetmap.org/user", Config.getUrls().getBaseUserUrl());
     }
+
+    /**
+     * An API URL entered with a trailing slash must give the same web site URLs as one without, so that
+     * "History (web)" and the other links to the server are built from the web site and not from the API path.
+     */
+    @Test
+    void testApiUrlWithTrailingSlash() {
+        Config.getPref().put("osm-server.url", "https://api06.dev.openstreetmap.org/api/");
+        assertEquals("https://api06.dev.openstreetmap.org", Config.getUrls().getOSMWebsiteDependingOnSelectedApi());
+        assertEquals("https://api06.dev.openstreetmap.org", Config.getUrls().getBaseBrowseUrl());
+        assertEquals("https://api06.dev.openstreetmap.org/user", Config.getUrls().getBaseUserUrl());
+    }
+
+    /**
+     * The default API URL is recognized even when it is entered with a trailing slash, so the links point to
+     * the OSM web site rather than to the API host.
+     */
+    @Test
+    void testDefaultApiUrlWithTrailingSlash() {
+        Config.getPref().put("osm-server.url", Config.getUrls().getDefaultOsmApiUrl() + '/');
+        assertEquals(Config.getUrls().getOSMWebsite(), Config.getUrls().getOSMWebsiteDependingOnSelectedApi());
+    }
 }

trunk/test/unit/org/openstreetmap/josm/io/OsmApiTest.java

@@ -3 +3 @@
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertSame;
 
 import java.io.ByteArrayInputStream;
@@ -11 +13 @@
 import org.openstreetmap.josm.data.osm.User;
 import org.openstreetmap.josm.gui.progress.NullProgressMonitor;
+import org.openstreetmap.josm.spi.preferences.Config;
+import org.openstreetmap.josm.testutils.annotations.BasicPreferences;
 
 /**
  * Unit tests of {@link OsmApi} class.
  */
+@BasicPreferences
+@org.openstreetmap.josm.testutils.annotations.OsmApi
 class OsmApiTest {
     /**
@@ -37 +43 @@
         assertEquals(User.getAnonymous(), cs2.getUser());
     }
+
+    /**
+     * Unit test of {@link OsmApi#normalizeApiUrl}: white space and trailing slashes are removed.
+     */
+    @Test
+    void testNormalizeApiUrl() {
+        assertEquals("https://example.com/api", OsmApi.normalizeApiUrl("https://example.com/api"));
+        assertEquals("https://example.com/api", OsmApi.normalizeApiUrl("https://example.com/api/"));
+        assertEquals("https://example.com/api", OsmApi.normalizeApiUrl("https://example.com/api///"));
+        assertEquals("https://example.com/api", OsmApi.normalizeApiUrl("  https://example.com/api/\t"));
+        assertEquals("https://example.com", OsmApi.normalizeApiUrl("https://example.com/"));
+        assertEquals("", OsmApi.normalizeApiUrl("   "));
+        assertNull(OsmApi.normalizeApiUrl(null));
+        // the slashes of the scheme must not be eaten
+        assertEquals("https://", OsmApi.normalizeApiUrl("https://"));
+    }
+
+    /**
+     * A URL which only differs by a trailing slash is the same API, and is stored without the slash.
+     */
+    @Test
+    void testGetOsmApiIgnoresTrailingSlash() {
+        OsmApi api = OsmApi.getOsmApi("https://example.com/api/");
+        assertEquals("https://example.com/api", api.getServerUrl());
+        assertSame(api, OsmApi.getOsmApi("https://example.com/api"));
+        assertSame(api, OsmApi.getOsmApi(" https://example.com/api// "));
+    }
+
+    /**
+     * An API URL stored with a trailing slash is used without it, so that code comparing it to a known URL or
+     * deriving another URL from it works. Non-regression test for the OAuth and "History (web)" breakage.
+     */
+    @Test
+    void testTrailingSlashInPreference() {
+        Config.getPref().put("osm-server.url", "https://example.com/api/");
+        assertEquals("https://example.com/api", OsmApi.getOsmApi().getServerUrl());
+        assertEquals("https://example.com/api/", OsmApi.getOsmApi().getBaseUrl());
+    }
 }

@@ -25 +25 @@
 target
 failed.png
+dependency-reduced-pom.xml