You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by kw...@apache.org on 2022/11/15 13:47:38 UTC

[sling-org-apache-sling-tooling-support-install] branch master updated: SLING-11672 Migrate from Felix HTTP Whiteboard to OSGi Whiteboard (#4)

This is an automated email from the ASF dual-hosted git repository.

kwin pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-tooling-support-install.git


The following commit(s) were added to refs/heads/master by this push:
     new 765faa6  SLING-11672 Migrate from Felix HTTP Whiteboard to OSGi Whiteboard (#4)
765faa6 is described below

commit 765faa6f6ce849307d794d2f51468651ba5d8a13
Author: Konrad Windszus <kw...@apache.org>
AuthorDate: Tue Nov 15 14:47:34 2022 +0100

    SLING-11672 Migrate from Felix HTTP Whiteboard to OSGi Whiteboard (#4)
    
    Update dependencies
    Make refresh packages optional
    Add test
---
 pom.xml                                            |  80 ++++-
 .../support/install/impl/InstallServlet.java       | 322 ++++++++++-----------
 .../support/install/impl/InstallationResult.java   |   5 +-
 .../support/install/impl/InstallServletTest.java   |  92 ++++++
 .../exploded-bundle1/META-INF/MANIFEST.MF          |   7 +
 .../exploded-bundle1/folder/nestedfolder/test      |   1 +
 6 files changed, 326 insertions(+), 181 deletions(-)

diff --git a/pom.xml b/pom.xml
index 87737b9..432c468 100644
--- a/pom.xml
+++ b/pom.xml
@@ -23,25 +23,26 @@
     <parent>
         <groupId>org.apache.sling</groupId>
         <artifactId>sling-bundle-parent</artifactId>
-        <version>48</version>
+        <version>49</version>
     </parent>
 
     <artifactId>org.apache.sling.tooling.support.install</artifactId>
     <version>1.0.7-SNAPSHOT</version>
 
     <name>Apache Sling Tooling Support Install</name>
-
+    <description>ReST endpoint for installing/updating a bundle from a directory or a JAR</description>
     <scm>
         <connection>scm:git:https://gitbox.apache.org/repos/asf/sling-org-apache-sling-tooling-support-install.git</connection>
         <developerConnection>scm:git:https://gitbox.apache.org/repos/asf/sling-org-apache-sling-tooling-support-install.git</developerConnection>
         <url>https://gitbox.apache.org/repos/asf?p=sling-org-apache-sling-tooling-support-install.git</url>
         <tag>HEAD</tag>
     </scm>
-    
+
     <dependencies>
         <dependency>
             <groupId>org.slf4j</groupId>
             <artifactId>slf4j-api</artifactId>
+            <scope>provided</scope>
         </dependency>
         <dependency>
             <groupId>javax.servlet</groupId>
@@ -50,7 +51,12 @@
         </dependency>
         <dependency>
             <groupId>org.osgi</groupId>
-            <artifactId>osgi.core</artifactId>
+            <artifactId>org.osgi.framework</artifactId>
+            <scope>provided</scope>
+        </dependency>
+        <dependency>
+            <groupId>org.osgi</groupId>
+            <artifactId>org.osgi.service.http.whiteboard</artifactId>
             <scope>provided</scope>
         </dependency>
         <dependency>
@@ -64,23 +70,67 @@
             <version>2.4</version>
             <scope>provided</scope>
         </dependency>
+        
+        <!-- embedded dependency -->
         <dependency>
-            <groupId>org.apache.commons</groupId>
-            <artifactId>commons-lang3</artifactId>
-            <version>3.0</version>
+            <groupId>org.apache.felix</groupId>
+            <artifactId>org.apache.felix.utils</artifactId>
+            <version>1.11.8</version>
             <scope>provided</scope>
         </dependency>
         <dependency>
-            <groupId>commons-fileupload</groupId>
-            <artifactId>commons-fileupload</artifactId>
-            <version>1.2.2</version>
-            <scope>provided</scope>
+            <groupId>org.junit.jupiter</groupId>
+            <artifactId>junit-jupiter</artifactId>
+            <version>5.9.1</version>
+            <scope>test</scope>
         </dependency>
         <dependency>
-            <groupId>org.apache.felix</groupId>
-            <artifactId>org.apache.felix.utils</artifactId>
-            <version>1.9.0</version>
-            <scope>provided</scope>
+          <groupId>org.mockito</groupId>
+          <artifactId>mockito-core</artifactId>
+          <version>4.8.1</version>
+          <scope>test</scope>
+        </dependency>
+        <dependency>
+            <groupId>org.slf4j</groupId>
+            <artifactId>slf4j-simple</artifactId>
+            <scope>test</scope>
         </dependency>
     </dependencies>
+    <build>
+        <plugins>
+            <plugin>
+                <groupId>org.apache.rat</groupId>
+                <artifactId>apache-rat-plugin</artifactId>
+                <configuration>
+                    <excludes combine.children="append">
+                        <!-- Used by maven-remote-resources-plugin -->
+                        <exclude>src/test/resources/exploded-bundle1/**</exclude>
+                    </excludes>
+                </configuration>
+            </plugin>
+            <plugin>
+                <groupId>org.apache.maven.plugins</groupId>
+                <artifactId>maven-dependency-plugin</artifactId>
+                <executions>
+                    <execution>
+                        <id>copy-test-bundle</id>
+                        <phase>process-test-resources</phase>
+                        <goals>
+                            <goal>copy</goal>
+                        </goals>
+                        <configuration>
+                            <artifactItems>
+                                <artifactItem>
+                                    <groupId>org.apache.sling</groupId>
+                                    <artifactId>org.apache.sling.commons.messaging</artifactId>
+                                    <version>1.0.0</version>
+                                </artifactItem>
+                            </artifactItems>
+                            <outputDirectory>${project.build.testOutputDirectory}</outputDirectory>
+                        </configuration>
+                    </execution>
+                </executions>
+            </plugin>
+        </plugins>
+    </build>
 </project>
diff --git a/src/main/java/org/apache/sling/tooling/support/install/impl/InstallServlet.java b/src/main/java/org/apache/sling/tooling/support/install/impl/InstallServlet.java
index 9501f9a..b45d155 100644
--- a/src/main/java/org/apache/sling/tooling/support/install/impl/InstallServlet.java
+++ b/src/main/java/org/apache/sling/tooling/support/install/impl/InstallServlet.java
@@ -16,17 +16,20 @@
  */
 package org.apache.sling.tooling.support.install.impl;
 
-import java.io.File;
-import java.io.FileInputStream;
-import java.io.FileNotFoundException;
-import java.io.FileOutputStream;
+import java.io.BufferedInputStream;
 import java.io.IOException;
 import java.io.InputStream;
-import java.util.List;
+import java.io.UncheckedIOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.Collection;
+import java.util.Collections;
 import java.util.jar.JarFile;
 import java.util.jar.JarInputStream;
 import java.util.jar.JarOutputStream;
 import java.util.jar.Manifest;
+import java.util.stream.Stream;
 import java.util.zip.Deflater;
 import java.util.zip.ZipEntry;
 import java.util.zip.ZipOutputStream;
@@ -36,31 +39,29 @@ import javax.servlet.ServletException;
 import javax.servlet.http.HttpServlet;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
+import javax.servlet.http.Part;
 
-import org.apache.commons.fileupload.FileItem;
-import org.apache.commons.fileupload.FileUploadException;
-import org.apache.commons.fileupload.disk.DiskFileItemFactory;
-import org.apache.commons.fileupload.servlet.ServletFileUpload;
 import org.apache.commons.io.IOUtils;
+import org.apache.commons.io.input.BoundedInputStream;
+import org.apache.commons.io.input.CloseShieldInputStream;
 import org.osgi.framework.Bundle;
 import org.osgi.framework.BundleContext;
 import org.osgi.framework.BundleException;
 import org.osgi.framework.Constants;
+import org.osgi.framework.wiring.FrameworkWiring;
 import org.osgi.service.component.annotations.Activate;
 import org.osgi.service.component.annotations.Component;
-import org.osgi.service.component.annotations.Reference;
-import org.osgi.service.packageadmin.PackageAdmin;
+import org.osgi.service.http.whiteboard.propertytypes.HttpWhiteboardServletMultipart;
+import org.osgi.service.http.whiteboard.propertytypes.HttpWhiteboardServletPattern;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 /**
- * Prototype for installing/updating a bundle from a directory
+ * ReST endpoint for installing/updating a bundle from a directory or a JAR
  */
-@Component(service = Servlet.class,
-    property = {
-            Constants.SERVICE_VENDOR + "=The Apache Software Foundation",
-            "alias=/system/sling/tooling/install"
-    })
+@Component(service = Servlet.class)
+@HttpWhiteboardServletPattern("/system/sling/tooling/install")
+@HttpWhiteboardServletMultipart(enabled = true, fileSizeThreshold = InstallServlet.UPLOAD_IN_MEMORY_SIZE_THRESHOLD)
 public class InstallServlet extends HttpServlet {
 
     private static final long serialVersionUID = -8820366266126231409L;
@@ -69,15 +70,13 @@ public class InstallServlet extends HttpServlet {
 
     private static final String DIR = "dir";
 
-    private static final int UPLOAD_IN_MEMORY_SIZE_THRESHOLD = 512 * 1024 * 1024;
+    public static final int UPLOAD_IN_MEMORY_SIZE_THRESHOLD = 512 * 1024 * 1024;
+    public static final int MANIFEST_SIZE_IN_INPUTSTREAM = 2 * 1024 * 1024;
 
-    private BundleContext bundleContext;
-
-    @Reference
-    private PackageAdmin packageAdmin;
+    private final BundleContext bundleContext;
 
     @Activate
-    protected void activate(final BundleContext bundleContext) {
+    public InstallServlet(final BundleContext bundleContext) {
         this.bundleContext = bundleContext;
     }
 
@@ -85,88 +84,81 @@ public class InstallServlet extends HttpServlet {
     protected void doPost(HttpServletRequest req, HttpServletResponse resp)
             throws ServletException, IOException {
         final String dirPath = req.getParameter(DIR);
+        final boolean refreshPackages = Boolean.parseBoolean(req.getParameter(dirPath));
+        boolean isMultipart = req.getContentType() != null && req.getContentType().toLowerCase().indexOf("multipart/form-data") > -1;
+        try {
+            if (dirPath == null && !isMultipart) {
+                logger.error("No dir parameter specified : {} and no multipart content found", req.getParameterMap());
+                resp.setStatus(500);
+                InstallationResult result = new InstallationResult(false, "No dir parameter specified: "
+                        + req.getParameterMap() + " and no multipart content found");
+                result.render(resp.getWriter());
+                return;
+            }
+            if (isMultipart) {
+                installBundleFromJar(req, resp, refreshPackages);
+            } else {
+                installBundleFromDirectory(resp, Paths.get(dirPath), refreshPackages);
+            }
+        } catch (IOException e) {
+            throw new ServletException(e);
+        }
+    }
 
-        boolean isMultipart = ServletFileUpload.isMultipartContent(req);
+    private void installBundleFromJar(HttpServletRequest req, HttpServletResponse resp, boolean refreshPackages) throws IOException, ServletException {
 
-        if (dirPath == null && !isMultipart) {
-            logger.error("No dir parameter specified : {} and no multipart content found", req.getParameterMap());
-            resp.setStatus(500);
-            InstallationResult result = new InstallationResult(false, "No dir parameter specified: "
-                    + req.getParameterMap() + " and no multipart content found");
-            result.render(resp.getWriter());
+        Collection<Part> parts = req.getParts();
+        if (parts.size() != 1) {
+            logAndWriteError("Found " + parts.size() + " items to process, but only updating 1 bundle is supported", resp);
             return;
         }
 
-        if (isMultipart) {
-            installBasedOnUploadedJar(req, resp);
-        } else {
-            installBasedOnDirectory(resp, new File(dirPath));
+        Part part = parts.iterator().next();
+
+        try (InputStream input = part.getInputStream()) {
+            installBundleFromJar(input, refreshPackages);
+            InstallationResult result = new InstallationResult(true, null);
+            resp.setStatus(200);
+            result.render(resp.getWriter());
+        } catch (IllegalArgumentException e) {
+            logAndWriteError(e, resp);
         }
     }
 
-    private void installBasedOnUploadedJar(HttpServletRequest req, HttpServletResponse resp) throws IOException {
-
-        InstallationResult result = null;
-
-        try {
-            DiskFileItemFactory factory = new DiskFileItemFactory();
-            // try to hold even largish bundles in memory to potentially improve performance
-            factory.setSizeThreshold(UPLOAD_IN_MEMORY_SIZE_THRESHOLD);
-
-            ServletFileUpload upload = new ServletFileUpload();
-            upload.setFileItemFactory(factory);
-
-            @SuppressWarnings("unchecked")
-            List<FileItem> items = upload.parseRequest(req);
-            if (items.size() != 1) {
-                logAndWriteError("Found " + items.size() + " items to process, but only updating 1 bundle is supported", resp);
-                return;
-            }
-
-            FileItem item = items.get(0);
-
-            JarInputStream jar = null;
-            InputStream rawInput = null;
-            try {
-                jar = new JarInputStream(item.getInputStream());
+    /**
+     * 
+     * @param inputStream
+     * @param refreshPackages
+     * @throws IOException
+     * @throws IllegalArgumentException if the provided input stream does not contain a valid OSGi bundle
+     */
+    Bundle installBundleFromJar(InputStream inputStream, boolean refreshPackages) throws IOException {
+        try (InputStream input = new BufferedInputStream(new CloseShieldInputStream(inputStream), MANIFEST_SIZE_IN_INPUTSTREAM)) {
+            input.mark(MANIFEST_SIZE_IN_INPUTSTREAM);
+            final String version;
+            final String symbolicName;
+            try (JarInputStream jar = new JarInputStream(new BoundedInputStream(new CloseShieldInputStream(input), MANIFEST_SIZE_IN_INPUTSTREAM))) {
+                
                 Manifest manifest = jar.getManifest();
                 if (manifest == null) {
-                    logAndWriteError("Uploaded jar file does not contain a manifest", resp);
-                    return;
+                    throw new IllegalArgumentException("Uploaded jar file does not contain a manifest");
                 }
-
-                final String symbolicName = manifest.getMainAttributes().getValue(Constants.BUNDLE_SYMBOLICNAME);
-
+                symbolicName = manifest.getMainAttributes().getValue(Constants.BUNDLE_SYMBOLICNAME);
                 if (symbolicName == null) {
-                    logAndWriteError("Manifest does not have a " + Constants.BUNDLE_SYMBOLICNAME, resp);
-                    return;
+                    throw new IllegalArgumentException("Manifest does not have a " + Constants.BUNDLE_SYMBOLICNAME);
                 }
-
-                final String version = manifest.getMainAttributes().getValue(Constants.BUNDLE_VERSION);
-
-                // the JarInputStream is used only for validation, we need a fresh input stream for updating
-                rawInput = item.getInputStream();
-
-                Bundle found = getBundle(symbolicName);
-                try {
-                    installOrUpdateBundle(found, rawInput, "inputstream:" + symbolicName + "-" + version + ".jar");
-
-                    result = new InstallationResult(true, null);
-                    resp.setStatus(200);
-                    result.render(resp.getWriter());
-                    return;
-                } catch (BundleException e) {
-                    logAndWriteError("Unable to install/update bundle " + symbolicName, e, resp);
-                    return;
-                }
-            } finally {
-                IOUtils.closeQuietly(jar);
-                IOUtils.closeQuietly(rawInput);
+                version = manifest.getMainAttributes().getValue(Constants.BUNDLE_VERSION);
             }
+            // go back to beginning of input stream
+            // the JarInputStream is used only for validation, we need a fresh input stream for updating
+            input.reset();
 
-        } catch (FileUploadException e) {
-            logAndWriteError("Failed parsing uploaded bundle", e, resp);
-            return;
+            Bundle found = getBundle(symbolicName);
+            try {
+                return installOrUpdateBundle(found, input, "inputstream:" + symbolicName + "-" + version + ".jar", refreshPackages);
+            } catch (BundleException e) {
+                throw new IllegalArgumentException("Unable to install/update bundle " + symbolicName, e);
+            }
         }
     }
 
@@ -176,79 +168,87 @@ public class InstallServlet extends HttpServlet {
         new InstallationResult(false, message).render(resp.getWriter());
     }
 
-    private void logAndWriteError(String message, Exception e, HttpServletResponse resp) throws IOException {
-        logger.info(message, e);
+    private void logAndWriteError(Exception e, HttpServletResponse resp) throws IOException {
+        logger.info(e.getMessage(), e);
         resp.setStatus(500);
-        new InstallationResult(false, message + " : " + e.getMessage()).render(resp.getWriter());
+        new InstallationResult(false, e.getMessage()).render(resp.getWriter());
     }
 
-    private void installBasedOnDirectory(HttpServletResponse resp, final File dir) throws FileNotFoundException,
-            IOException {
-
-        InstallationResult result = null;
+    private void installBundleFromDirectory(HttpServletResponse resp, final Path dir, boolean refreshPackages) throws IOException {
+        try {
+            installBundleFromDirectory(dir, refreshPackages);
+            InstallationResult result = new InstallationResult(true, null);
+            resp.setStatus(200);
+            result.render(resp.getWriter());
+        } catch (IllegalArgumentException e) {
+            logAndWriteError(e, resp);
+        }
+    }
 
-        if ( dir.exists() && dir.isDirectory() ) {
+    /**
+     * 
+     * @param dir
+     * @param refreshPackages
+     * @throws IOException
+     * @throws IllegalArgumentException if the provided directory does not contain a valid exploded OSGi bundle
+     */
+    Bundle installBundleFromDirectory(final Path dir, boolean refreshPackages) throws IOException {
+        if (Files.isDirectory(dir)) {
             logger.info("Checking dir {} for bundle install", dir);
-            final File manifestFile = new File(dir, JarFile.MANIFEST_NAME);
-            if ( manifestFile.exists() ) {
-                FileInputStream fis = null;
-                try {
-                    fis = new FileInputStream(manifestFile);
+            final Path manifestFile = dir.resolve(JarFile.MANIFEST_NAME);
+            if (Files.exists(dir)) {
+                try (InputStream fis = Files.newInputStream(manifestFile)) {
                     final Manifest mf = new Manifest(fis);
 
                     final String symbolicName = mf.getMainAttributes().getValue(Constants.BUNDLE_SYMBOLICNAME);
-                    if ( symbolicName != null ) {
+                    if (symbolicName != null) {
                         // search bundle
                         Bundle found = getBundle(symbolicName);
 
-                        final File tempFile = File.createTempFile(dir.getName(), "bundle");
+                        Path tmpJarFile = Files.createTempFile(dir.getFileName().toString(), "bundle");
                         try {
-                            createJar(dir, tempFile, mf);
-
-                            final InputStream in = new FileInputStream(tempFile);
-                            try {
-                                String location = dir.getAbsolutePath();
-
-                                installOrUpdateBundle(found, in, location);
-                                result = new InstallationResult(true, null);
-                                resp.setStatus(200);
-                                result.render(resp.getWriter());
-                                return;
-                            } catch ( final BundleException be ) {
-                                logAndWriteError("Unable to install/update bundle from dir " + dir, be, resp);
+                            createJar(dir, tmpJarFile, mf);
+                            try (InputStream in = Files.newInputStream(tmpJarFile)) {
+                                String location = dir.toAbsolutePath().toString();
+                                return installOrUpdateBundle(found, in, location, refreshPackages);
+                            } catch (final BundleException be) {
+                                throw new IllegalArgumentException("Unable to install/update bundle from dir " + dir, be);
                             }
                         } finally {
-                            tempFile.delete();
+                            Files.delete(tmpJarFile);
                         }
                     } else {
-                        logAndWriteError("Manifest in " + dir + " does not have a symbolic name", resp);
+                        throw new IllegalArgumentException("Manifest in " + dir + " does not have a symbolic name");
                     }
-                } finally {
-                    IOUtils.closeQuietly(fis);
                 }
             } else {
-                result = new InstallationResult(false, "Dir " + dir + " does not have a manifest");
-                logAndWriteError("Dir " + dir + " does not have a manifest", resp);
+                throw new IllegalArgumentException("Dir " + dir + " does not have a manifest");
             }
         } else {
-            result = new InstallationResult(false, "Dir " + dir + " does not exist");
-            logAndWriteError("Dir " + dir + " does not exist", resp);
+            throw new IllegalArgumentException("Dir " + dir + " does not exist");
         }
     }
 
-    private void installOrUpdateBundle(Bundle bundle, final InputStream in, String location) throws BundleException {
+    private Bundle installOrUpdateBundle(Bundle bundle, final InputStream in, String location, boolean refreshPackages) throws BundleException {
         if (bundle != null) {
             // update
             bundle.update(in);
         } else {
             // install
-            final Bundle b = bundleContext.installBundle(location, in);
-            b.start();
+            bundle = bundleContext.installBundle(location, in);
+            bundle.start();
         }
-
+        if (refreshPackages) {
+            refreshBundle(bundle);
+        }
+        return bundle;
+    }
+    
+    private void refreshBundle(Bundle bundle) {
+        FrameworkWiring frameworkWiring = bundleContext.getBundle(Constants.SYSTEM_BUNDLE_ID).adapt(FrameworkWiring.class);
         // take into account added/removed packages for updated bundles and newly satisfied optional package imports
         // for new installed bundles
-        packageAdmin.refreshPackages(new Bundle[] { bundle });
+        frameworkWiring.refreshBundles(Collections.singleton(bundle));
     }
 
     private Bundle getBundle(final String symbolicName) {
@@ -262,10 +262,8 @@ public class InstallServlet extends HttpServlet {
         return found;
     }
 
-    private static void createJar(final File sourceDir, final File jarFile, final Manifest mf)
-    throws IOException {
-        final JarOutputStream zos = new JarOutputStream(new FileOutputStream(jarFile));
-        try {
+    private static void createJar(final Path sourceDir, final Path jarFile, final Manifest mf) throws IOException {
+        try (JarOutputStream zos = new JarOutputStream(Files.newOutputStream(jarFile))) {
             zos.setLevel(Deflater.NO_COMPRESSION);
             // manifest first
             final ZipEntry anEntry = new ZipEntry(JarFile.MANIFEST_NAME);
@@ -273,38 +271,36 @@ public class InstallServlet extends HttpServlet {
             mf.write(zos);
             zos.closeEntry();
             zipDir(sourceDir, zos, "");
-        } finally {
-            try {
-                zos.close();
-            } catch ( final IOException ignore ) {
-                // ignore
-            }
         }
     }
 
-    public static void zipDir(final File sourceDir, final ZipOutputStream zos, final String path)
-    throws IOException {
-        final byte[] readBuffer = new byte[8192];
-        int bytesIn = 0;
+    public static void zipDir(final Path sourceDir, final ZipOutputStream zos, final String prefix) throws IOException {
+        try (Stream<Path> stream = Files.list(sourceDir)) {
+            stream.forEach(p -> 
+            {
+                try {
+                    zipFileOrDir(p, zos, prefix);
+                } catch (IOException e) {
+                    throw new UncheckedIOException(e);
+                }
+            });
+        } catch (UncheckedIOException ioe) {
+            throw ioe.getCause();
+        }
+    }
 
-        for(final File f : sourceDir.listFiles()) {
-            if (f.isDirectory()) {
-                final String prefix = path + f.getName() + "/";
-                zos.putNextEntry(new ZipEntry(prefix));
-                zipDir(f, zos, prefix);
-            } else {
-                final String entry = path + f.getName();
-                if ( !JarFile.MANIFEST_NAME.equals(entry) ) {
-                    final FileInputStream fis = new FileInputStream(f);
-                    try {
-                        final ZipEntry anEntry = new ZipEntry(entry);
-                        zos.putNextEntry(anEntry);
-                        while ( (bytesIn = fis.read(readBuffer)) != -1) {
-                            zos.write(readBuffer, 0, bytesIn);
-                        }
-                    } finally {
-                        fis.close();
-                    }
+    private static void zipFileOrDir(final Path sourceFileOrDir, final ZipOutputStream zos, final String prefix) throws IOException {
+        if (Files.isDirectory(sourceFileOrDir)) {
+            final String newPrefix = prefix + sourceFileOrDir.getFileName() + "/";
+            zos.putNextEntry(new ZipEntry(newPrefix));
+            zipDir(sourceFileOrDir, zos, newPrefix);
+        } else {
+            final String entry = prefix + sourceFileOrDir.getFileName();
+            if (!JarFile.MANIFEST_NAME.equals(entry)) {
+                try (InputStream fis = Files.newInputStream(sourceFileOrDir)) {
+                    final ZipEntry anEntry = new ZipEntry(entry);
+                    zos.putNextEntry(anEntry);
+                    IOUtils.copy(fis, zos);
                 }
             }
         }
diff --git a/src/main/java/org/apache/sling/tooling/support/install/impl/InstallationResult.java b/src/main/java/org/apache/sling/tooling/support/install/impl/InstallationResult.java
index 25f9924..d97894f 100644
--- a/src/main/java/org/apache/sling/tooling/support/install/impl/InstallationResult.java
+++ b/src/main/java/org/apache/sling/tooling/support/install/impl/InstallationResult.java
@@ -19,13 +19,12 @@ package org.apache.sling.tooling.support.install.impl;
 import java.io.IOException;
 import java.io.Writer;
 
-import org.apache.commons.lang3.StringUtils;
 import org.apache.felix.utils.json.JSONWriter;
 
 public class InstallationResult {
 
     private final boolean status;
-    private final String message;
+    private final String message; // may be null
 
     public InstallationResult(boolean status, String message) {
         this.status = status;
@@ -38,7 +37,7 @@ public class InstallationResult {
             JSONWriter writer = new JSONWriter(out);
             writer.object();
             writer.key("status").value(status ? "OK" : "FAILURE");
-            if (!StringUtils.isEmpty(message)) {
+            if (message != null) {
                 writer.key("message").value(message);
             }
             writer.endObject();
diff --git a/src/test/java/org/apache/sling/tooling/support/install/impl/InstallServletTest.java b/src/test/java/org/apache/sling/tooling/support/install/impl/InstallServletTest.java
new file mode 100644
index 0000000..72cc577
--- /dev/null
+++ b/src/test/java/org/apache/sling/tooling/support/install/impl/InstallServletTest.java
@@ -0,0 +1,92 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF 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 org.apache.sling.tooling.support.install.impl;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.net.URISyntaxException;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.Objects;
+import java.util.jar.JarInputStream;
+
+import org.apache.commons.io.IOUtils;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.mockito.Mockito;
+import org.mockito.stubbing.Answer2;
+import org.osgi.framework.Bundle;
+import org.osgi.framework.BundleContext;
+import org.osgi.framework.BundleException;
+import org.osgi.framework.Constants;
+
+import static org.mockito.AdditionalAnswers.answer;
+
+class InstallServletTest {
+
+    private InstallServlet servlet;
+    private BundleContext bundleContext;
+    private ByteArrayOutputStream output; // data of installed bundle's JAR
+
+    @BeforeEach
+    void setUp() throws BundleException {
+        output = new ByteArrayOutputStream();
+        bundleContext = Mockito.mock(BundleContext.class);
+        Mockito.when(bundleContext.getBundles()).thenReturn(new Bundle[0]);
+        Mockito.when(bundleContext.installBundle(Mockito.anyString(), Mockito.any(InputStream.class)))
+            .then(answer(new Answer2<Bundle, String, InputStream>() {
+                @Override
+                public Bundle answer(String location, InputStream inputStream) throws Throwable {
+                    IOUtils.copy(inputStream, output);
+                    return Mockito.mock(Bundle.class);
+                }
+            }));
+        servlet = new InstallServlet(bundleContext);
+    }
+
+    @Test
+    void testInstallJar() throws IOException, BundleException {
+        try (InputStream input = Objects.requireNonNull(getClass().getResourceAsStream("/org.apache.sling.commons.messaging-1.0.0.jar"))) {
+            servlet.installBundleFromJar(input, false);
+        }
+        Mockito.verify(bundleContext).installBundle(Mockito.eq("inputstream:org.apache.sling.commons.messaging-1.0.0.jar"), Mockito.any(InputStream.class));
+        assertBundle(output.toByteArray(), 17, "org.apache.sling.commons.messaging");
+    }
+
+    @Test
+    void testInstallDirectory() throws IOException, URISyntaxException, BundleException {
+        Path sourceDir = Paths.get(getClass().getResource("/exploded-bundle1").toURI());
+        servlet.installBundleFromDirectory(sourceDir, false);
+        Mockito.verify(bundleContext).installBundle(Mockito.eq(sourceDir.toString()), Mockito.any(InputStream.class));
+        assertBundle(output.toByteArray(), 5, "test-bundle1");
+    }
+
+    static void assertBundle(byte[] data, int expectedNumEntries, String expectedBSN) throws IOException {
+        int numEntries = 1;
+        try (JarInputStream jarInputStream = new JarInputStream(new ByteArrayInputStream(data))) {
+            assertEquals(expectedBSN, jarInputStream.getManifest().getMainAttributes().getValue(Constants.BUNDLE_SYMBOLICNAME));
+            while(jarInputStream.getNextEntry() != null) {
+                numEntries++;
+            }
+        }
+        assertEquals(expectedNumEntries, numEntries);
+    }
+}
diff --git a/src/test/resources/exploded-bundle1/META-INF/MANIFEST.MF b/src/test/resources/exploded-bundle1/META-INF/MANIFEST.MF
new file mode 100644
index 0000000..addc068
--- /dev/null
+++ b/src/test/resources/exploded-bundle1/META-INF/MANIFEST.MF
@@ -0,0 +1,7 @@
+Manifest-Version: 1.0
+Bundle-SymbolicName: test-bundle1
+Bundle-Name: Apache Sling Tooling Support Install
+Bundle-Version: 1.0.0.SNAPSHOT
+Created-By: Maven JAR Plugin 3.2.2
+Specification-Version: 1.0
+
diff --git a/src/test/resources/exploded-bundle1/folder/nestedfolder/test b/src/test/resources/exploded-bundle1/folder/nestedfolder/test
new file mode 100644
index 0000000..30d74d2
--- /dev/null
+++ b/src/test/resources/exploded-bundle1/folder/nestedfolder/test
@@ -0,0 +1 @@
+test
\ No newline at end of file