Ticket #24907: api-strip-slash.patch

File api-strip-slash.patch, 17.5 KB (added by wangi, 2 days ago)

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

    diff --git src/org/openstreetmap/josm/data/oauth/OAuthParameters.java src/org/openstreetmap/josm/data/oauth/OAuthParameters.java
index 73ed6ce10c..f4905c04c1 100644
    public final class OAuthParameters {  
    147147        final String clientSecret;
    148148        final String redirectUri = "http://127.0.0.1:8111/oauth_authorization";
    149149        final String baseUrl;
    150         apiUrl = apiUrl == null ? OsmApi.getOsmApi().getServerUrl() : apiUrl;
     150        apiUrl = OsmApi.normalizeApiUrl(apiUrl == null ? OsmApi.getOsmApi().getServerUrl() : apiUrl);
    151151        switch (apiUrl) {
    152152            case OSM_API_DEV:
    153153            case OSM_API_MASTER:
    … … public final class OAuthParameters {  
    238238     * @since 18650
    239239     */
    240240    public static IOAuthParameters createFromApiUrl(String apiUrl, OAuthVersion oAuthVersion) {
     241        apiUrl = OsmApi.normalizeApiUrl(apiUrl);
    241242        final String originalApiUrl = apiUrl;
    242243        // We actually need the host
    243244        if (apiUrl.startsWith("https://") || apiUrl.startsWith("http://")) {

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

    diff --git src/org/openstreetmap/josm/gui/oauth/TestAccessTokenTask.java src/org/openstreetmap/josm/gui/oauth/TestAccessTokenTask.java
index 86fe3d9a31..ba6938d5c4 100644
    import org.openstreetmap.josm.data.osm.UserInfo;  
    1818import org.openstreetmap.josm.gui.HelpAwareOptionPane;
    1919import org.openstreetmap.josm.gui.PleaseWaitRunnable;
    2020import org.openstreetmap.josm.gui.help.HelpUtil;
     21import org.openstreetmap.josm.io.OsmApi;
    2122import org.openstreetmap.josm.io.OsmApiException;
    2223import org.openstreetmap.josm.io.OsmServerUserInfoReader;
    2324import org.openstreetmap.josm.io.OsmTransferException;
    … … public class TestAccessTokenTask extends PleaseWaitRunnable {  
    5960        CheckParameterUtil.ensureParameterNotNull(accessToken, "accessToken");
    6061        this.tokenOAuth2 = accessToken;
    6162        this.parent = parent;
    62         this.apiUrl = apiUrl;
     63        this.apiUrl = OsmApi.normalizeApiUrl(apiUrl);
    6364    }
    6465
    6566    @Override
    … … public class TestAccessTokenTask extends PleaseWaitRunnable {  
    8182        this.tokenOAuth2.sign(con);
    8283    }
    8384
    84     protected String normalizeApiUrl(String url) {
    85         // remove leading and trailing white space
    86         url = url.trim();
    87 
    88         // remove trailing slashes
    89         while (url.endsWith("/")) {
    90             url = url.substring(0, url.lastIndexOf('/'));
    91         }
    92         return url;
    93     }
    94 
    9585    protected UserInfo getUserDetails() throws OsmOAuthAuthorizationException, XmlParsingException, OsmTransferException {
    9686        boolean authenticatorEnabled = true;
    9787        try {
    98             URL url = new URL(normalizeApiUrl(apiUrl) + "/0.6/user/details");
     88            URL url = new URL(apiUrl + "/0.6/user/details");
    9989            authenticatorEnabled = DefaultAuthenticator.getInstance().isEnabled();
    10090            DefaultAuthenticator.getInstance().setEnabled(false);
    10191

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

    diff --git src/org/openstreetmap/josm/gui/preferences/server/ApiUrlTestTask.java src/org/openstreetmap/josm/gui/preferences/server/ApiUrlTestTask.java
index ad84e7e2f8..9681092738 100644
    import org.openstreetmap.josm.gui.HelpAwareOptionPane;  
    1616import org.openstreetmap.josm.gui.PleaseWaitRunnable;
    1717import org.openstreetmap.josm.gui.help.HelpUtil;
    1818import org.openstreetmap.josm.io.Capabilities;
     19import org.openstreetmap.josm.io.OsmApi;
    1920import org.openstreetmap.josm.io.OsmTransferException;
    2021import org.openstreetmap.josm.tools.CheckParameterUtil;
    2122import org.openstreetmap.josm.tools.HttpClient;
    … … public class ApiUrlTestTask extends PleaseWaitRunnable {  
    4849        super(parent, tr("Testing OSM API URL ''{0}''", url), false /* don't ignore exceptions */);
    4950        CheckParameterUtil.ensureParameterNotNull(url, "url");
    5051        this.parent = parent;
    51         this.url = url;
     52        this.url = OsmApi.normalizeApiUrl(url);
    5253    }
    5354
    5455    protected void alertInvalidUrl(String url) {
    … … public class ApiUrlTestTask extends PleaseWaitRunnable {  
    6667        );
    6768    }
    6869
    69     protected void alertInvalidCapabilitiesUrl(String url) {
     70    protected void alertInvalidCapabilitiesUrl(String capabilitiesUrl) {
    7071        HelpAwareOptionPane.showMessageDialogInEDT(
    7172                parent,
    7273                tr("<html>"
    7374                        + "Failed to build URL ''{0}'' for validating the OSM API server.<br>"
    7475                        + "Please check the spelling of ''{1}'' and validate again."
    7576                        +"</html>",
    76                         url,
    77                         getNormalizedApiUrl()
     77                        capabilitiesUrl,
     78                        url
    7879                ),
    7980                tr("Invalid API URL"),
    8081                JOptionPane.ERROR_MESSAGE,
    … … public class ApiUrlTestTask extends PleaseWaitRunnable {  
    9091                        + "Please check the spelling of ''{1}'' and your Internet connection and validate again."
    9192                        +"</html>",
    9293                        url,
    93                         getNormalizedApiUrl()
     94                        url
    9495                ),
    9596                tr("Connection to API failed"),
    9697                JOptionPane.ERROR_MESSAGE,
    … … public class ApiUrlTestTask extends PleaseWaitRunnable {  
    107108                        + "Please check the spelling of ''{1}'' and validate again."
    108109                        + "</html>",
    109110                        retCode,
    110                         getNormalizedApiUrl()
     111                        url
    111112                ),
    112113                tr("Connection to API failed"),
    113114                JOptionPane.ERROR_MESSAGE,
    … … public class ApiUrlTestTask extends PleaseWaitRunnable {  
    123124                        + "It is likely that ''{0}'' is not an OSM API server.<br>"
    124125                        + "Please check the spelling of ''{0}'' and validate again."
    125126                        + "</html>",
    126                         getNormalizedApiUrl()
     127                        url
    127128                ),
    128129                tr("Connection to API failed"),
    129130                JOptionPane.ERROR_MESSAGE,
    … … public class ApiUrlTestTask extends PleaseWaitRunnable {  
    146147        // Do nothing
    147148    }
    148149
    149     /**
    150      * Removes leading and trailing whitespace from the API URL and removes trailing '/'.
    151      *
    152      * @return the normalized API URL
    153      */
    154     protected String getNormalizedApiUrl() {
    155         String apiUrl = url.trim();
    156         while (apiUrl.endsWith("/")) {
    157             apiUrl = apiUrl.substring(0, apiUrl.lastIndexOf('/'));
    158         }
    159         return apiUrl;
    160     }
    161 
    162150    @Override
    163151    protected void realRun() throws SAXException, IOException, OsmTransferException {
    164152        try {
    165153            try {
    166                 new URL(getNormalizedApiUrl());
     154                new URL(url);
    167155            } catch (MalformedURLException e) {
    168                 alertInvalidUrl(getNormalizedApiUrl());
     156                alertInvalidUrl(url);
    169157                return;
    170158            }
    171159            URL capabilitiesUrl;
    172             String getCapabilitiesUrl = getNormalizedApiUrl() + "/0.6/capabilities";
     160            String getCapabilitiesUrl = url + "/0.6/capabilities";
    173161            try {
    174162                capabilitiesUrl = new URL(getCapabilitiesUrl);
    175163            } catch (MalformedURLException e) {

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

    diff --git src/org/openstreetmap/josm/gui/preferences/server/OsmApiUrlInputPanel.java src/org/openstreetmap/josm/gui/preferences/server/OsmApiUrlInputPanel.java
index ca7f38d466..16a6a66184 100644
    public class OsmApiUrlInputPanel extends JPanel {  
    125125     */
    126126    public void saveToPreferences() {
    127127        String oldUrl = OsmApi.getOsmApi().getServerUrl();
    128         String hmiUrl = getStrippedApiUrl();
     128        String hmiUrl = getApiUrl();
    129129        if (cbUseDefaultServerUrl.isSelected() || Config.getUrls().getDefaultOsmApiUrl().equals(hmiUrl)) {
    130130            Config.getPref().put("osm-server.url", null);
    131131        } else {
    … … public class OsmApiUrlInputPanel extends JPanel {  
    146146    }
    147147
    148148    /**
    149      * Returns the entered API URL, stripped of leading and trailing white characters.
    150      * @return the entered API URL, stripped of leading and trailing white characters. May be an empty string
    151      *         if nothing has been entered. In this case, it means the user wants to use {@link IUrls#getDefaultOsmApiUrl}.
    152      * @see Utils#strip(String)
    153      * @since 6602
     149     * Returns the entered API URL, normalized with {@link OsmApi#normalizeApiUrl(String)}, that is stripped of
     150     * leading and trailing white characters and of trailing '/'.
     151     * @return the entered API URL. May be an empty string if nothing has been entered. In this case, it means
     152     *         the user wants to use {@link IUrls#getDefaultOsmApiUrl}.
     153     * @see OsmApi#normalizeApiUrl(String)
     154     * @since xxx (renamed from {@code getStrippedApiUrl}, which existed since 6602)
    154155     */
    155     public final String getStrippedApiUrl() {
    156         return Utils.strip(tfOsmServerUrl.getText());
     156    public final String getApiUrl() {
     157        return OsmApi.normalizeApiUrl(tfOsmServerUrl.getText());
    157158    }
    158159
    159160    class ValidateApiUrlAction extends AbstractAction implements DocumentListener {
    … … public class OsmApiUrlInputPanel extends JPanel {  
    167168
    168169        @Override
    169170        public void actionPerformed(ActionEvent arg0) {
    170             final String url = getStrippedApiUrl();
     171            final String url = getApiUrl();
    171172            final ApiUrlTestTask task = new ApiUrlTestTask(OsmApiUrlInputPanel.this, url);
    172173            MainApplication.worker.submit(task);
    173174            Runnable r = () -> {
    … … public class OsmApiUrlInputPanel extends JPanel {  
    190191        }
    191192
    192193        protected final void updateEnabledState() {
    193             String url = getStrippedApiUrl();
     194            String url = getApiUrl();
    194195            boolean enabled = !url.isEmpty() && !url.equals(lastTestedUrl);
    195196            if (enabled) {
    196197                lblValid.setIcon(null);
    … … public class OsmApiUrlInputPanel extends JPanel {  
    275276
    276277    class ApiUrlPropagator extends FocusAdapter implements ActionListener {
    277278        protected void propagate() {
    278             propagate(getStrippedApiUrl());
     279            propagate(getApiUrl());
    279280        }
    280281
    281282        protected void propagate(String url) {

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

    diff --git src/org/openstreetmap/josm/io/OsmApi.java src/org/openstreetmap/josm/io/OsmApi.java
index 8895b26e7b..69643ebb18 100644
    public class OsmApi extends OsmConnection {  
    133133     *
    134134     */
    135135    public static OsmApi getOsmApi(String serverUrl) {
    136         OsmApi api = instances.get(serverUrl);
     136        String url = normalizeApiUrl(serverUrl);
     137        OsmApi api = instances.get(url);
    137138        if (api == null) {
    138             api = new OsmApi(serverUrl);
     139            api = new OsmApi(url);
    139140            cacheInstance(api);
    140141        }
    141142        return api;
    142143    }
    143144
     145    /**
     146     * Normalizes an OSM API URL: removes leading and trailing white space and any trailing {@code '/'}.
     147     * <p>
     148     * The URL may be entered with a trailing slash, but code which compares it to a known API URL or derives
     149     * another URL from it does not expect one: OAuth would not find the client id of the server, and the links
     150     * to the web site of the server (browse, history, user) would be built from the API path.
     151     * @param serverUrl the server URL, may be null
     152     * @return the normalized URL, or null if {@code serverUrl} is null
     153     * @since xxx
     154     */
     155    public static String normalizeApiUrl(String serverUrl) {
     156        if (serverUrl == null) {
     157            return null;
     158        }
     159        String url = Utils.strip(serverUrl);
     160        while (url.endsWith("/") && !url.endsWith("://")) {
     161            url = url.substring(0, url.length() - 1);
     162        }
     163        return url;
     164    }
     165
    144166    protected static void cacheInstance(OsmApi api) {
    145167        instances.put(api.getServerUrl(), api);
    146168    }
    … … public class OsmApi extends OsmConnection {  
    181203     */
    182204    protected OsmApi(String serverUrl) {
    183205        CheckParameterUtil.ensureParameterNotNull(serverUrl, "serverUrl");
    184         this.serverUrl = serverUrl;
     206        this.serverUrl = normalizeApiUrl(serverUrl);
    185207    }
    186208
    187209    /**

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

    diff --git test/unit/org/openstreetmap/josm/data/preferences/JosmUrlsTest.java test/unit/org/openstreetmap/josm/data/preferences/JosmUrlsTest.java
index cea11fd377..fce4aaae73 100644
    class JosmUrlsTest {  
    1919    void testGetBaseUserUrl() {
    2020        assertEquals("https://api06.dev.openstreetmap.org/user", Config.getUrls().getBaseUserUrl());
    2121    }
     22
     23    /**
     24     * An API URL entered with a trailing slash must give the same web site URLs as one without, so that
     25     * "History (web)" and the other links to the server are built from the web site and not from the API path.
     26     */
     27    @Test
     28    void testApiUrlWithTrailingSlash() {
     29        Config.getPref().put("osm-server.url", "https://api06.dev.openstreetmap.org/api/");
     30        assertEquals("https://api06.dev.openstreetmap.org", Config.getUrls().getOSMWebsiteDependingOnSelectedApi());
     31        assertEquals("https://api06.dev.openstreetmap.org", Config.getUrls().getBaseBrowseUrl());
     32        assertEquals("https://api06.dev.openstreetmap.org/user", Config.getUrls().getBaseUserUrl());
     33    }
     34
     35    /**
     36     * The default API URL is recognized even when it is entered with a trailing slash, so the links point to
     37     * the OSM web site rather than to the API host.
     38     */
     39    @Test
     40    void testDefaultApiUrlWithTrailingSlash() {
     41        Config.getPref().put("osm-server.url", Config.getUrls().getDefaultOsmApiUrl() + '/');
     42        assertEquals(Config.getUrls().getOSMWebsite(), Config.getUrls().getOSMWebsiteDependingOnSelectedApi());
     43    }
    2244}

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

    diff --git test/unit/org/openstreetmap/josm/io/OsmApiTest.java test/unit/org/openstreetmap/josm/io/OsmApiTest.java
index f5d9013f9a..79020c89da 100644
     
    22package org.openstreetmap.josm.io;
    33
    44import static org.junit.jupiter.api.Assertions.assertEquals;
     5import static org.junit.jupiter.api.Assertions.assertNull;
     6import static org.junit.jupiter.api.Assertions.assertSame;
    57
    68import java.io.ByteArrayInputStream;
    79import java.nio.charset.StandardCharsets;
    … … import org.junit.jupiter.api.Test;  
    1012import org.openstreetmap.josm.data.osm.Changeset;
    1113import org.openstreetmap.josm.data.osm.User;
    1214import org.openstreetmap.josm.gui.progress.NullProgressMonitor;
     15import org.openstreetmap.josm.spi.preferences.Config;
     16import org.openstreetmap.josm.testutils.annotations.BasicPreferences;
    1317
    1418/**
    1519 * Unit tests of {@link OsmApi} class.
    1620 */
     21@BasicPreferences
     22@org.openstreetmap.josm.testutils.annotations.OsmApi
    1723class OsmApiTest {
    1824    /**
    1925     * Non-regression test for <a href="https://josm.openstreetmap.de/ticket/12675">Bug #12675</a>.
    … … class OsmApiTest {  
    3642                NullProgressMonitor.INSTANCE).iterator().next();
    3743        assertEquals(User.getAnonymous(), cs2.getUser());
    3844    }
     45
     46    /**
     47     * Unit test of {@link OsmApi#normalizeApiUrl}: white space and trailing slashes are removed.
     48     */
     49    @Test
     50    void testNormalizeApiUrl() {
     51        assertEquals("https://example.com/api", OsmApi.normalizeApiUrl("https://example.com/api"));
     52        assertEquals("https://example.com/api", OsmApi.normalizeApiUrl("https://example.com/api/"));
     53        assertEquals("https://example.com/api", OsmApi.normalizeApiUrl("https://example.com/api///"));
     54        assertEquals("https://example.com/api", OsmApi.normalizeApiUrl("  https://example.com/api/\t"));
     55        assertEquals("https://example.com", OsmApi.normalizeApiUrl("https://example.com/"));
     56        assertEquals("", OsmApi.normalizeApiUrl("   "));
     57        assertNull(OsmApi.normalizeApiUrl(null));
     58        // the slashes of the scheme must not be eaten
     59        assertEquals("https://", OsmApi.normalizeApiUrl("https://"));
     60    }
     61
     62    /**
     63     * A URL which only differs by a trailing slash is the same API, and is stored without the slash.
     64     */
     65    @Test
     66    void testGetOsmApiIgnoresTrailingSlash() {
     67        OsmApi api = OsmApi.getOsmApi("https://example.com/api/");
     68        assertEquals("https://example.com/api", api.getServerUrl());
     69        assertSame(api, OsmApi.getOsmApi("https://example.com/api"));
     70        assertSame(api, OsmApi.getOsmApi(" https://example.com/api// "));
     71    }
     72
     73    /**
     74     * An API URL stored with a trailing slash is used without it, so that code comparing it to a known URL or
     75     * deriving another URL from it works. Non-regression test for the OAuth and "History (web)" breakage.
     76     */
     77    @Test
     78    void testTrailingSlashInPreference() {
     79        Config.getPref().put("osm-server.url", "https://example.com/api/");
     80        assertEquals("https://example.com/api", OsmApi.getOsmApi().getServerUrl());
     81        assertEquals("https://example.com/api/", OsmApi.getOsmApi().getBaseUrl());
     82    }
    3983}