[java-identity-provider] branch main updated: IDP-1689 Plugin update fails on Windows.

Rod Widdowson rdw at steadingsoftware.com
Wed Oct 14 12:24:50 UTC 2020


This is an automated email from the git hooks/post-receive script.

rdw pushed a commit to branch main
in repository java-identity-provider.

View the commit online:
http://git.shibboleth.net/view/?p=java-identity-provider.git;a=commit;h=1af4ee827ec206583a8d0ce10a6e80e007610e70

The following commit(s) were added to refs/heads/main by this push:
       new  1af4ee827 IDP-1689 Plugin update fails on Windows.
1af4ee827 is described below

commit 1af4ee827ec206583a8d0ce10a6e80e007610e70
Author: Rod Widdowson <rdw at steadingsoftware.com>
AuthorDate: Wed Oct 14 13:20:34 2020 +0100

    IDP-1689 Plugin update fails on Windows.
    
    https://issues.shibboleth.net/jira/browse/IDP-1689
    We need to copy the plugin jars somewhere separate before we make
    a ClasspathLoader from them otherwise the rename will fail because
    they are pinned open by the loader.
---
 .../idp/installer/plugin/impl/LoggingVisitor.java  | 94 ++++++++++++++++++++++
 .../idp/installer/plugin/impl/PluginInstaller.java | 58 ++++++++-----
 .../plugin/impl/PluginInstallerSupport.java        | 61 --------------
 .../idp/installer/plugin/impl/PluginState.java     |  7 +-
 4 files changed, 137 insertions(+), 83 deletions(-)

diff --git a/idp-installer/src/main/java/net/shibboleth/idp/installer/plugin/impl/LoggingVisitor.java b/idp-installer/src/main/java/net/shibboleth/idp/installer/plugin/impl/LoggingVisitor.java
new file mode 100644
index 000000000..98e472b07
--- /dev/null
+++ b/idp-installer/src/main/java/net/shibboleth/idp/installer/plugin/impl/LoggingVisitor.java
@@ -0,0 +1,94 @@
+/*
+ * Licensed to the University Corporation for Advanced Internet Development,
+ * Inc. (UCAID) under one or more contributor license agreements.  See the
+ * NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The UCAID licenses this file to You under the Apache
+ * License, Version 2.0 (the "License"); you may not use this file except in
+ * compliance with the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package net.shibboleth.idp.installer.plugin.impl;
+
+import java.io.BufferedInputStream;
+import java.io.BufferedOutputStream;
+import java.io.FileInputStream;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.nio.file.FileVisitResult;
+import java.nio.file.FileVisitor;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.SimpleFileVisitor;
+import java.nio.file.attribute.BasicFileAttributes;
+import java.util.ArrayList;
+import java.util.List;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * A @{link {@link FileVisitor} copies directory trees keeping a note of all copied target files.
+ */
+public final class LoggingVisitor extends SimpleFileVisitor<Path> {
+    /** How what files have we copied? */
+    private final List<Path> copiedFiles = new ArrayList<>();
+
+    /** logger. */
+    private final Logger log = LoggerFactory.getLogger(LoggingVisitor.class);
+
+    /** Path we are traversing. */
+    private final Path from;
+    
+    /** Path where we check for Duplicates. */
+    private final Path to;
+    /**
+     * Constructor.
+     *
+     * @param fromDir Path we are traversing
+     * @param toDir Path where we check for Duplicates
+     */
+    public LoggingVisitor(final Path fromDir, final Path toDir) {
+        from = fromDir;
+        to = toDir;
+    }
+
+    @Override
+    public FileVisitResult preVisitDirectory(final Path dir, final BasicFileAttributes attrs) throws IOException {
+        final Path relDir = from.relativize(dir);
+        final Path toDir = to.resolve(relDir);
+        if (!Files.exists(toDir)) {
+            log.trace("Creating directory {}", toDir);
+            Files.createDirectory(toDir);
+        }
+        return FileVisitResult.CONTINUE;
+    };
+
+    @Override
+    public FileVisitResult visitFile(final Path file, final BasicFileAttributes attrs) throws IOException {
+        final Path relFile = from.relativize(file);
+        final Path toFile = to.resolve(relFile);
+        copiedFiles.add(toFile);
+        try(final InputStream in = new BufferedInputStream(new FileInputStream(file.toFile()));
+            final OutputStream out = new BufferedOutputStream(new FileOutputStream(toFile.toFile()))) {
+            in.transferTo(out);
+        }
+        return FileVisitResult.CONTINUE;
+    }
+    
+    /** did we find a name clash?
+     * @return whether we found a name clash.
+     */
+    public List<Path> getCopiedList() {
+        return copiedFiles;
+    }
+}
\ No newline at end of file
diff --git a/idp-installer/src/main/java/net/shibboleth/idp/installer/plugin/impl/PluginInstaller.java b/idp-installer/src/main/java/net/shibboleth/idp/installer/plugin/impl/PluginInstaller.java
index b494961fe..29757ace8 100644
--- a/idp-installer/src/main/java/net/shibboleth/idp/installer/plugin/impl/PluginInstaller.java
+++ b/idp-installer/src/main/java/net/shibboleth/idp/installer/plugin/impl/PluginInstaller.java
@@ -25,6 +25,7 @@ import java.io.FileOutputStream;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
+import java.net.MalformedURLException;
 import java.net.URL;
 import java.net.URLClassLoader;
 import java.nio.file.DirectoryStream;
@@ -123,7 +124,7 @@ public final class PluginInstaller extends AbstractInitializableComponent implem
     private HttpClient httpClient;
 
     /** Dumping space for renamed files. */
-    @NonnullAfterInit private Path renamePath;
+    @NonnullAfterInit private Path workspacePath;
 
     /** DistDir. */
     @NonnullAfterInit private Path distPath;
@@ -141,7 +142,7 @@ public final class PluginInstaller extends AbstractInitializableComponent implem
     @NonnullAfterInit private ModuleContext moduleContext;
 
     /** The "plugins" classpath loader. AutoClosed. */
-    private URLClassLoader installedPluginLoader;
+    private URLClassLoader installedPluginsLoader;
 
     /** The "plugin under construction" classpath loader. AutoClosed. */
     private URLClassLoader installingPluginLoader;
@@ -360,7 +361,7 @@ public final class PluginInstaller extends AbstractInitializableComponent implem
      */
     private void checkRequiredModules() throws BuildException {
         final Set<String> requiredModules = new HashSet<>(description.getRequiredModules());
-        final Iterator<IdPModule> modules = ServiceLoader.load(IdPModule.class, getInstalledPluginLoader()).iterator();
+        final Iterator<IdPModule> modules = ServiceLoader.load(IdPModule.class, getInstalledPluginsLoader()).iterator();
         while (modules.hasNext() && !requiredModules.isEmpty()) {
             try {
                 final IdPModule module = modules.next();
@@ -434,7 +435,8 @@ public final class PluginInstaller extends AbstractInitializableComponent implem
             LOG.debug("{} not installed. files renamed", pluginId);
         } else {
             try {
-                PluginInstallerSupport.renameToTree(pluginsWebapp, renamePath,
+                PluginInstallerSupport.renameToTree(pluginsWebapp,
+                        workspacePath.resolve("rollback"),
                         getInstalledContents(),
                         rollback.getFilesRenamedAway());
             } catch (final IOException e) {
@@ -729,7 +731,7 @@ public final class PluginInstaller extends AbstractInitializableComponent implem
         moduleContext.setHttpClientSecurityParameters(securityParams);
         moduleContext.setHttpClient(httpClient);
         distPath = idpHome.resolve("dist");
-        renamePath = distPath.resolve("plugin-rollback");
+        workspacePath = distPath.resolve("plugin-workspace");
         pluginsWebapp = distPath.resolve("plugin-webapp");
         InstallerSupport.setReadOnly(distPath, false);
     }
@@ -739,27 +741,47 @@ public final class PluginInstaller extends AbstractInitializableComponent implem
      * @return an appropriate loader
      * @throws BuildException if a directory traversal fails.
      */
-    private synchronized URLClassLoader getInstalledPluginLoader() throws BuildException {
-        if (installedPluginLoader != null) {
-            return installedPluginLoader;
+    private synchronized URLClassLoader getInstalledPluginsLoader() throws BuildException {
+
+        if (installedPluginsLoader != null) {
+            return installedPluginsLoader;
         }
-        final List<URL> urls = new ArrayList<>();
+        final URL[] urls;
         final Path libs = pluginsWebapp.resolve("WEB-INF").resolve("lib");
         if (Files.exists(libs)) {
-            try (final DirectoryStream<Path> webInfLibs = Files.newDirectoryStream(libs)) {
-                for (final Path jar : webInfLibs) {
-                    urls.add(jar.toUri().toURL());
+            try {
+                if (!Files.exists(workspacePath)) {
+                    Files.createDirectories(workspacePath);
                 }
+                final Path pathToDir = Files.createTempDirectory(workspacePath, "classpath");
+                final LoggingVisitor visitor = new LoggingVisitor(libs, pathToDir);
+                try (final DirectoryStream<Path> webInfLibs = Files.newDirectoryStream(libs)) {
+                    for (final Path jar : webInfLibs) {
+                        visitor.visitFile(jar, null);
+                    }
+                }
+                urls = visitor.
+                        getCopiedList().
+                        stream().
+                        map( path -> {
+                            try {
+                                return path.toUri().toURL();
+                            } catch (final MalformedURLException e1) {
+                                throw new BuildException(e1);
+                            }
+                        }).
+                        toArray(URL[]::new);
             } catch (final IOException e) {
                 LOG.error("Error finding Plugins' classpath");
                 throw new BuildException(e);
             }
+        } else {
+            urls = new URL[0];
         }
-        installedPluginLoader = new URLClassLoader(urls.toArray(URL[]::new));
-        return installedPluginLoader;        
+        installedPluginsLoader = new URLClassLoader(urls);
+        return installedPluginsLoader;        
     }
 
-    
     /** Generate a {@link URLClassLoader} which looks at the
      * installing WEB-INF.
      * @return an appropriate loader
@@ -787,7 +809,7 @@ public final class PluginInstaller extends AbstractInitializableComponent implem
      */
     public List<IdPPlugin> getInstalledPlugins() throws BuildException {
        final Stream<Provider<IdPPlugin>> loaderStream =
-               ServiceLoader.load(IdPPlugin.class, getInstalledPluginLoader()).stream();
+               ServiceLoader.load(IdPPlugin.class, getInstalledPluginsLoader()).stream();
        return loaderStream.map(ServiceLoader.Provider::get).collect(Collectors.toList());
     }
 
@@ -822,11 +844,11 @@ public final class PluginInstaller extends AbstractInitializableComponent implem
 
     /** {@inheritDoc} */
     public void close() {
-        closeSilently(installedPluginLoader);
+        closeSilently(installedPluginsLoader);
         closeSilently(installingPluginLoader);
         PluginInstallerSupport.deleteTree(downloadDirectory);
         PluginInstallerSupport.deleteTree(unpackDirectory);
-        PluginInstallerSupport.deleteTree(renamePath);
+        PluginInstallerSupport.deleteTree(workspacePath);
         InstallerSupport.setReadOnly(distPath, true);
     }
     
diff --git a/idp-installer/src/main/java/net/shibboleth/idp/installer/plugin/impl/PluginInstallerSupport.java b/idp-installer/src/main/java/net/shibboleth/idp/installer/plugin/impl/PluginInstallerSupport.java
index 351b413c6..8ef91dab3 100644
--- a/idp-installer/src/main/java/net/shibboleth/idp/installer/plugin/impl/PluginInstallerSupport.java
+++ b/idp-installer/src/main/java/net/shibboleth/idp/installer/plugin/impl/PluginInstallerSupport.java
@@ -17,21 +17,14 @@
 
 package net.shibboleth.idp.installer.plugin.impl;
 
-import java.io.BufferedInputStream;
-import java.io.BufferedOutputStream;
 import java.io.File;
-import java.io.FileInputStream;
-import java.io.FileOutputStream;
 import java.io.IOException;
-import java.io.InputStream;
-import java.io.OutputStream;
 import java.nio.file.FileVisitResult;
 import java.nio.file.FileVisitor;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.SimpleFileVisitor;
 import java.nio.file.attribute.BasicFileAttributes;
-import java.util.ArrayList;
 import java.util.List;
 
 import javax.annotation.Nonnull;
@@ -199,60 +192,6 @@ public final class PluginInstallerSupport {
         }
     }
     
-    /**
-     * A @{link {@link FileVisitor} which detects (and logs) whether a copy would overwrite.
-     */
-    private static final class LoggingVisitor extends SimpleFileVisitor<Path> {
-        /** How what files have we copied? */
-        private final List<Path> copiedFiles = new ArrayList<>();
-
-        /** Path we are traversing. */
-        private final Path from;
-        
-        /** Path where we check for Duplicates. */
-        private final Path to;
-        /**
-         * Constructor.
-         *
-         * @param fromDir Path we are traversing
-         * @param toDir Path where we check for Duplicates
-         */
-        public LoggingVisitor(final Path fromDir, final Path toDir) {
-            from = fromDir;
-            to = toDir;
-        }
-
-        @Override
-        public FileVisitResult preVisitDirectory(final Path dir, final BasicFileAttributes attrs) throws IOException {
-            final Path relDir = from.relativize(dir);
-            final Path toDir = to.resolve(relDir);
-            if (!Files.exists(toDir)) {
-                LOG.trace("Creating directory {}", toDir);
-                Files.createDirectory(toDir);
-            }
-            return FileVisitResult.CONTINUE;
-        };
-
-        @Override
-        public FileVisitResult visitFile(final Path file, final BasicFileAttributes attrs) throws IOException {
-            final Path relFile = from.relativize(file);
-            final Path toFile = to.resolve(relFile);
-            copiedFiles.add(toFile);
-            try(final InputStream in = new BufferedInputStream(new FileInputStream(file.toFile()));
-                final OutputStream out = new BufferedOutputStream(new FileOutputStream(toFile.toFile()))) {
-                in.transferTo(out);
-            }
-            return FileVisitResult.CONTINUE;
-        }
-        
-        /** did we find a name clash?
-         * @return whether we found a name clash.
-         */
-        public List<Path> getCopiedList() {
-            return copiedFiles;
-        }
-    }
-
     /**
      * A @{link {@link FileVisitor} which deletes files.
      */
diff --git a/idp-installer/src/main/java/net/shibboleth/idp/installer/plugin/impl/PluginState.java b/idp-installer/src/main/java/net/shibboleth/idp/installer/plugin/impl/PluginState.java
index 11827c786..0b6fdf5e4 100644
--- a/idp-installer/src/main/java/net/shibboleth/idp/installer/plugin/impl/PluginState.java
+++ b/idp-installer/src/main/java/net/shibboleth/idp/installer/plugin/impl/PluginState.java
@@ -121,16 +121,15 @@ public class PluginState extends AbstractInitializableComponent {
         ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
         httpClient = Constraint.isNotNull(what, "HttpClient must be non null");
     }
-    
-    /** look up the key derived from the pluginId, the interfix and the version, but if that
-     * fails look for a templated definition. 
+
+    /** Look up the key derived from the pluginId, the interfix and the version, but if that
+     * fails look for a templated definition.
      * @param props what to look in
      * @param interfix the interface (between the ID and the version)
      * @param version the version.
      * @return the suitable value
      */
     @Nullable private String getDefaultedValue(final Properties props, final String interfix, final String version) {
-        
         String result = props.getProperty(plugin.getPluginId() + interfix + version);
         if (result != null) {
             return result;

-- 
To stop receiving notification emails like this one, please contact
the administrator of this repository.


More information about the commits mailing list