Changeset 15423 in josm

Timestamp:

2019-10-05T19:23:20+02:00 (5 years ago)

Author:

Don-vip

Message:

fix #17240 - Presets: verify that wiki links are no redirect

Location:

trunk

Files:

• src/org/openstreetmap/josm/gui/tagging/presets/items/Link.java (modified) (4 diffs)
• src/org/openstreetmap/josm/tools/HttpClient.java (modified) (1 diff)
• test/unit/org/openstreetmap/josm/gui/preferences/map/TaggingPresetPreferenceTestIT.java (modified) (5 diffs)

trunk/src/org/openstreetmap/josm/gui/tagging/presets/items/Link.java

@@ -7 +7 @@
 import java.util.Arrays;
 import java.util.Collection;
-import java.util.List;
 import java.util.Optional;
 
@@ -22 +21 @@
 /**
  * Hyperlink type.
+ * @since 8863
  */
 public class Link extends TextItem {
@@ -37 +37 @@
     public boolean addToPanel(JPanel p, Collection<OsmPrimitive> sel, boolean presetInitiallyMatches) {
         initializeLocaleText(tr("More information about this feature"));
+        Optional.ofNullable(buildUrlLabel()).ifPresent(label -> p.add(label, GBC.eol().insets(0, 10, 0, 0).fill(GBC.HORIZONTAL)));
+        return false;
+    }
+
+    protected UrlLabel buildUrlLabel() {
+        final String url = getUrl();
         if (wiki != null) {
-            final String url = Config.getUrls().getOSMWiki() + "/wiki/" + wiki;
-            final UrlLabel label = new UrlLabel(url, locale_text, 2) {
+            return new UrlLabel(url, locale_text, 2) {
                 @Override
                 public void mouseClicked(MouseEvent e) {
                     if (SwingUtilities.isLeftMouseButton(e)) {
                         // Open localized page if exists
-                        final List<String> pages = Arrays.asList(
+                        HelpAction.displayHelp(Arrays.asList(
                                 LanguageInfo.getWikiLanguagePrefix(LanguageInfo.LocaleType.OSM_WIKI) + wiki,
-                                wiki);
-                        HelpAction.displayHelp(pages);
+                                wiki));
                     } else {
                         super.mouseClicked(e);
@@ -53 +57 @@
                 }
             };
-            p.add(label, GBC.eol().insets(0, 10, 0, 0).fill(GBC.HORIZONTAL));
         } else if (href != null || locale_href != null) {
-            final String url = Optional.ofNullable(locale_href).orElse(href);
-            final UrlLabel label = new UrlLabel(url, locale_text, 2);
-            p.add(label, GBC.eol().insets(0, 10, 0, 0).fill(GBC.HORIZONTAL));
+            return new UrlLabel(url, locale_text, 2);
         }
-        return false;
+        return null;
+    }
+
+    /**
+     * Returns the link URL.
+     * @return the link URL
+     * @since 15423
+     */
+    public String getUrl() {
+        if (wiki != null) {
+            return Config.getUrls().getOSMWiki() + "/wiki/" + wiki;
+        } else if (href != null || locale_href != null) {
+            return Optional.ofNullable(locale_href).orElse(href);
+        }
+        return null;
     }
 

trunk/src/org/openstreetmap/josm/tools/HttpClient.java

@@ -821 +821 @@
     }
 
-    private static boolean isRedirect(final int statusCode) {
+    /**
+     * Determines if the given status code is an HTTP redirection.
+     * @param statusCode HTTP status code
+     * @return {@code true} if the given status code is an HTTP redirection
+     * @since 15423
+     */
+    public static boolean isRedirect(final int statusCode) {
         switch (statusCode) {
             case HttpURLConnection.HTTP_MOVED_PERM: // 301

trunk/test/unit/org/openstreetmap/josm/gui/preferences/map/TaggingPresetPreferenceTestIT.java

@@ -8 +8 @@
 
 import java.io.IOException;
+import java.net.URL;
 import java.util.Collection;
 import java.util.HashSet;
@@ -28 +29 @@
 import org.openstreetmap.josm.gui.tagging.presets.TaggingPreset;
 import org.openstreetmap.josm.gui.tagging.presets.TaggingPresetReader;
+import org.openstreetmap.josm.gui.tagging.presets.items.Link;
 import org.openstreetmap.josm.spi.preferences.Config;
 import org.openstreetmap.josm.testutils.JOSMTestRules;
+import org.openstreetmap.josm.tools.HttpClient;
+import org.openstreetmap.josm.tools.HttpClient.Response;
 import org.openstreetmap.josm.tools.ImageProvider;
 import org.openstreetmap.josm.tools.Logging;
@@ -113 +117 @@
         assertFalse(presets.isEmpty());
         // wait for asynchronous icon loading
-        presets.stream().map(TaggingPreset::getIconLoadingTask).filter(Objects::nonNull).forEach(t -> {
+        presets.parallelStream().map(TaggingPreset::getIconLoadingTask).filter(Objects::nonNull).forEach(t -> {
             try {
                 t.get(30, TimeUnit.SECONDS);
             } catch (InterruptedException | ExecutionException | TimeoutException e) {
+                Logging.error(e);
+            }
+        });
+        // check that links are correct and not redirections
+        presets.parallelStream().flatMap(x -> x.data.stream().filter(i -> i instanceof Link).map(i -> ((Link) i).getUrl())).forEach(u -> {
+            try {
+                Response cr = HttpClient.create(new URL(u)).setMaxRedirects(-1).connect();
+                final int code = cr.getResponseCode();
+                if (HttpClient.isRedirect(code)) {
+                    addOrIgnoreError(messages, "Found HTTP redirection for " + u + " -> " + code + " -> " + cr.getHeaderField("Location"));
+                } else if (code >= 400) {
+                    addOrIgnoreError(messages, "Found HTTP error for " + u + " -> " + code);
+                }
+            } catch (IOException e) {
                 Logging.error(e);
             }
@@ -125 +143 @@
             if (message.contains(TaggingPreset.PRESET_ICON_ERROR_MSG_PREFIX)) {
                 error = true;
-                if (isIgnoredSubstring(message)) {
-                    ignoredErrors.add(message);
-                } else {
-                    messages.add(message);
-                }
+                addOrIgnoreError(messages, message);
             }
         }
@@ -136 +150 @@
         }
     }
+
+    void addOrIgnoreError(Set<String> messages, String message) {
+        if (isIgnoredSubstring(message)) {
+            ignoredErrors.add(message);
+        } else {
+            messages.add(message);
+        }
+    }
 }