Index: trunk/src/org/openstreetmap/josm/data/oauth/OAuthParameters.java =================================================================== --- trunk/src/org/openstreetmap/josm/data/oauth/OAuthParameters.java (revision 19650) +++ trunk/src/org/openstreetmap/josm/data/oauth/OAuthParameters.java (revision 19651) @@ -148,5 +148,5 @@ 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,4 +239,5 @@ */ public static IOAuthParameters createFromApiUrl(String apiUrl, OAuthVersion oAuthVersion) { + apiUrl = OsmApi.normalizeApiUrl(apiUrl); final String originalApiUrl = apiUrl; // We actually need the host Index: trunk/src/org/openstreetmap/josm/gui/oauth/TestAccessTokenTask.java =================================================================== --- trunk/src/org/openstreetmap/josm/gui/oauth/TestAccessTokenTask.java (revision 19650) +++ trunk/src/org/openstreetmap/josm/gui/oauth/TestAccessTokenTask.java (revision 19651) @@ -19,4 +19,5 @@ 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,5 +61,5 @@ this.tokenOAuth2 = accessToken; this.parent = parent; - this.apiUrl = apiUrl; + this.apiUrl = OsmApi.normalizeApiUrl(apiUrl); } @@ -82,19 +83,8 @@ } - 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); Index: trunk/src/org/openstreetmap/josm/gui/preferences/server/ApiUrlTestTask.java =================================================================== --- trunk/src/org/openstreetmap/josm/gui/preferences/server/ApiUrlTestTask.java (revision 19650) +++ trunk/src/org/openstreetmap/josm/gui/preferences/server/ApiUrlTestTask.java (revision 19651) @@ -17,4 +17,5 @@ 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,5 +50,5 @@ CheckParameterUtil.ensureParameterNotNull(url, "url"); this.parent = parent; - this.url = url; + this.url = OsmApi.normalizeApiUrl(url); } @@ -67,5 +68,5 @@ } - protected void alertInvalidCapabilitiesUrl(String url) { + protected void alertInvalidCapabilitiesUrl(String capabilitiesUrl) { HelpAwareOptionPane.showMessageDialogInEDT( parent, @@ -74,6 +75,6 @@ + "Please check the spelling of ''{1}'' and validate again." +"", - url, - getNormalizedApiUrl() + capabilitiesUrl, + url ), tr("Invalid API URL"), @@ -91,5 +92,5 @@ +"", url, - getNormalizedApiUrl() + url ), tr("Connection to API failed"), @@ -108,5 +109,5 @@ + "", retCode, - getNormalizedApiUrl() + url ), tr("Connection to API failed"), @@ -124,5 +125,5 @@ + "Please check the spelling of ''{0}'' and validate again." + "", - getNormalizedApiUrl() + url ), tr("Connection to API failed"), @@ -147,28 +148,15 @@ } - /** - * 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); Index: trunk/src/org/openstreetmap/josm/gui/preferences/server/OsmApiUrlInputPanel.java =================================================================== --- trunk/src/org/openstreetmap/josm/gui/preferences/server/OsmApiUrlInputPanel.java (revision 19650) +++ trunk/src/org/openstreetmap/josm/gui/preferences/server/OsmApiUrlInputPanel.java (revision 19651) @@ -126,5 +126,5 @@ 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,12 +147,13 @@ /** - * 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,5 +169,5 @@ @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,5 +192,5 @@ protected final void updateEnabledState() { - String url = getStrippedApiUrl(); + String url = getApiUrl(); boolean enabled = !url.isEmpty() && !url.equals(lastTestedUrl); if (enabled) { @@ -276,5 +277,5 @@ class ApiUrlPropagator extends FocusAdapter implements ActionListener { protected void propagate() { - propagate(getStrippedApiUrl()); + propagate(getApiUrl()); } Index: trunk/src/org/openstreetmap/josm/io/OsmApi.java =================================================================== --- trunk/src/org/openstreetmap/josm/io/OsmApi.java (revision 19650) +++ trunk/src/org/openstreetmap/josm/io/OsmApi.java (revision 19651) @@ -134,10 +134,32 @@ */ 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 '/'}. + *

+ * 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,5 +204,5 @@ protected OsmApi(String serverUrl) { CheckParameterUtil.ensureParameterNotNull(serverUrl, "serverUrl"); - this.serverUrl = serverUrl; + this.serverUrl = normalizeApiUrl(serverUrl); } Index: trunk/test/unit/org/openstreetmap/josm/data/preferences/JosmUrlsTest.java =================================================================== --- trunk/test/unit/org/openstreetmap/josm/data/preferences/JosmUrlsTest.java (revision 19650) +++ trunk/test/unit/org/openstreetmap/josm/data/preferences/JosmUrlsTest.java (revision 19651) @@ -20,3 +20,25 @@ 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()); + } } Index: trunk/test/unit/org/openstreetmap/josm/io/OsmApiTest.java =================================================================== --- trunk/test/unit/org/openstreetmap/josm/io/OsmApiTest.java (revision 19650) +++ trunk/test/unit/org/openstreetmap/josm/io/OsmApiTest.java (revision 19651) @@ -3,4 +3,6 @@ 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,8 +13,12 @@ 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,3 +43,41 @@ 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()); + } }