Changeset 13300 in josm for trunk

Timestamp:

2018-01-08T02:45:59+01:00 (7 years ago)

Author:

Don-vip

Message:

fix #15742 - check downloaded plugin is valid *before* we delete any existing one (patch by ris)

Location:

trunk

Files:

• src/org/openstreetmap/josm/plugins/PluginHandler.java (modified) (2 diffs)
• test/data/__files/plugin (added)
• test/data/__files/plugin/corrupted_plugin.jar (added)
• test/data/__files/plugin/dummy_plugin.jar (added)
• test/data/plugin/corrupted_plugin.MANIFEST.MF (added)
• test/unit/org/openstreetmap/josm/TestUtils.java (modified) (1 diff)
• test/unit/org/openstreetmap/josm/actions/downloadtasks/AbstractDownloadTaskTestParent.java (modified) (2 diffs)
• test/unit/org/openstreetmap/josm/actions/downloadtasks/PluginDownloadTaskTest.java (added)
• test/unit/org/openstreetmap/josm/testutils/JOSMTestRules.java (modified) (9 diffs)

trunk/src/org/openstreetmap/josm/plugins/PluginHandler.java

@@ -1258 +1258 @@
             File plugin = new File(filePath.substring(0, filePath.length() - 4));
             String pluginName = updatedPlugin.getName().substring(0, updatedPlugin.getName().length() - 8);
-            if (plugin.exists() && !plugin.delete() && dowarn) {
-                Logging.warn(tr("Failed to delete outdated plugin ''{0}''.", plugin.toString()));
-                Logging.warn(tr("Failed to install already downloaded plugin ''{0}''. " +
-                        "Skipping installation. JOSM is still going to load the old plugin version.",
-                        pluginName));
-                continue;
-            }
             try {
                 // Check the plugin is a valid and accessible JAR file before installing it (fix #7754)
@@ -1273 +1266 @@
                             plugin.toString(), updatedPlugin.toString(), e.getLocalizedMessage()), e);
                 }
+                continue;
+            }
+            if (plugin.exists() && !plugin.delete() && dowarn) {
+                Logging.warn(tr("Failed to delete outdated plugin ''{0}''.", plugin.toString()));
+                Logging.warn(tr("Failed to install already downloaded plugin ''{0}''. " +
+                        "Skipping installation. JOSM is still going to load the old plugin version.",
+                        pluginName));
                 continue;
             }

trunk/test/unit/org/openstreetmap/josm/TestUtils.java

@@ -188 +188 @@
 
     /**
+     * Sets a private static field value.
+     * @param cls object class
+     * @param fieldName private field name
+     * @param value replacement value
+     * @throws ReflectiveOperationException if a reflection operation error occurs
+     */
+    public static void setPrivateStaticField(Class<?> cls, String fieldName, final Object value) throws ReflectiveOperationException {
+        Field f = cls.getDeclaredField(fieldName);
+        AccessController.doPrivileged((PrivilegedAction<Void>) () -> {
+            f.setAccessible(true);
+            return null;
+        });
+        f.set(null, value);
+    }
+
+    /**
      * Returns an instance of {@link AbstractProgressMonitor} which keeps track of the monitor state,
      * but does not show the progress.

trunk/test/unit/org/openstreetmap/josm/actions/downloadtasks/AbstractDownloadTaskTestParent.java

@@ -40 +40 @@
 
     /**
+     * Returns the {@code Content-Type} with which to serve the file referenced
+     * by {@link #getRemoteFile()}
+     * @return the {@code Content-Type} string for file {@link #getRemoteFile()}
+     */
+    protected String getRemoteContentType() {
+        return "text/xml";
+    }
+
+    /**
      * Returns the http URL to remote test file to download.
      * @return the http URL to remote test file to download
@@ -54 +63 @@
                 .willReturn(aResponse()
                     .withStatus(200)
-                    .withHeader("Content-Type", "text/xml")
+                    .withHeader("Content-Type", getRemoteContentType())
                     .withBodyFile(getRemoteFile())));
     }

trunk/test/unit/org/openstreetmap/josm/testutils/JOSMTestRules.java

@@ -3 +3 @@
 
 import java.awt.Color;
+import java.io.ByteArrayInputStream;
 import java.io.File;
 import java.io.IOException;
@@ -16 +17 @@
 import org.openstreetmap.josm.JOSMFixture;
 import org.openstreetmap.josm.Main;
+import org.openstreetmap.josm.TestUtils;
 import org.openstreetmap.josm.actions.DeleteAction;
 import org.openstreetmap.josm.command.DeleteCommand;
 import org.openstreetmap.josm.data.UserIdentityManager;
+import org.openstreetmap.josm.data.Version;
 import org.openstreetmap.josm.data.osm.User;
 import org.openstreetmap.josm.data.osm.event.SelectionEventManager;
@@ -59 +62 @@
     private String i18n = null;
     private TileSourceRule tileSourceRule;
+    private String assumeRevisionString;
+    private Version originalVersion;
     private boolean platform;
     private boolean useProjection;
@@ -133 +138 @@
     public JOSMTestRules platform() {
         platform = true;
+        return this;
+    }
+
+    /**
+     * Mock this test's assumed JOSM version (as reported by {@link Version}).
+     * @param revisionProperties mock contents of JOSM's {@code REVISION} properties file
+     * @return this instance, for easy chaining
+     */
+    public JOSMTestRules assumeRevision(final String revisionProperties) {
+        this.assumeRevisionString = revisionProperties;
         return this;
     }
@@ -277 +292 @@
     /**
      * Use the {@link Main#main}, {@code Main.contentPanePrivate}, {@code Main.mainPanel},
-     *         {@link Main#menu}, {@link Main#toolbar} global variables in this test.
+     *         global variables in this test.
      * @return this instance, for easy chaining
      * @since 12557
@@ -285 +300 @@
         main = true;
         return this;
+    }
+
+    private static class MockVersion extends Version {
+        MockVersion(final String propertiesString) {
+            super.initFromRevisionInfo(
+                new ByteArrayInputStream(propertiesString.getBytes())
+            );
+        }
     }
 
@@ -318 +341 @@
      * Set up before running a test
      * @throws InitializationError If an error occured while creating the required environment.
-     */
-    protected void before() throws InitializationError {
+     * @throws ReflectiveOperationException if a reflective access error occurs
+     */
+    protected void before() throws InitializationError, ReflectiveOperationException {
         // Tests are running headless by default.
         System.setProperty("java.awt.headless", "true");
 
         cleanUpFromJosmFixture();
+
+        if (this.assumeRevisionString != null) {
+            this.originalVersion = Version.getInstance();
+            final Version replacementVersion = new MockVersion(this.assumeRevisionString);
+            TestUtils.setPrivateStaticField(Version.class, "instance", replacementVersion);
+        }
 
         Config.setPreferencesInstance(Main.pref);
@@ -465 +495 @@
     /**
      * Clean up after running a test
+     * @throws ReflectiveOperationException if a reflective access error occurs
      */
     @SuppressFBWarnings("DM_GC")
-    protected void after() {
+    protected void after() throws ReflectiveOperationException {
         // Sync AWT Thread
         GuiHelper.runInEDTAndWait(new Runnable() {
@@ -481 +512 @@
         Main.pref.resetToInitialState();
         Main.platform = null;
+
+        if (this.assumeRevisionString != null && this.originalVersion != null) {
+            TestUtils.setPrivateStaticField(Version.class, "instance", this.originalVersion);
+        }
+
         // Parts of JOSM uses weak references - destroy them.
         System.gc();