[FIXED JENKINS-9942] Convert system path separator to "/"

The ZipArchiver was visit()'d with a relative path that uses the system's path separator, but zip archives require a forward slash for platform compatibility. This fix replaces the platform-dependent File.separatorChar with a forward slash.
jenkinsci · Jan 3, 2012 · 7e48d4b · 7e48d4b
1 parent f33e714
commit 7e48d4b
Show file tree

Hide file tree

Showing 3 changed files with 135 additions and 2 deletions.
diff --git a/changelog.html b/changelog.html
@@ -55,7 +55,9 @@
 <!-- Record your changes in the trunk here. -->
 <div id="trunk" style="display:none"><!--=TRUNK-BEGIN=-->
 <ul class=image>
-  <li class=>
+  <li class="bug">
+    Incorrect path delimiter used in ZipArchiver when creating archive on Windows.
+    <a href="https://issues.jenkins-ci.org/browse/JENKINS-9942">issue 9942</a>
 </ul>
 </div><!--=TRUNK-END=-->
 

diff --git a/core/src/main/java/hudson/util/io/ZipArchiver.java b/core/src/main/java/hudson/util/io/ZipArchiver.java
@@ -48,9 +48,14 @@ final class ZipArchiver extends Archiver {
         zip.setEncoding(System.getProperty("file.encoding"));
     }
 
-    public void visit(File f, String relativePath) throws IOException {
+    public void visit(final File f, final String _relativePath) throws IOException {
         int mode = IOUtils.mode(f);
 
+        // On Windows, the elements of relativePath are separated by 
+        // back-slashes (\), but Zip files need to have their path elements separated
+        // by forward-slashes (/)
+        String relativePath = _relativePath.replace('\\', '/');
+
         if(f.isDirectory()) {
             ZipEntry dirZipEntry = new ZipEntry(relativePath+'/');
             // Setting this bit explicitly is needed by some unzipping applications (see JENKINS-3294).

diff --git a/core/src/test/java/hudson/util/io/ZipArchiverTest.java b/core/src/test/java/hudson/util/io/ZipArchiverTest.java
@@ -0,0 +1,126 @@
+package hudson.util.io;
+
+import junit.framework.TestCase;
+import org.jvnet.hudson.test.Bug;
+
+import java.io.File;
+import java.io.OutputStream;
+import java.io.FileOutputStream;
+import java.io.IOException;
+
+import java.util.Enumeration;
+import java.util.zip.ZipFile;
+import java.util.zip.ZipEntry;
+
+import org.apache.commons.logging.LogFactory;
+import org.apache.commons.logging.Log;
+
+public class ZipArchiverTest extends TestCase {
+    private final Log logger = LogFactory.getLog(getClass());
+
+    private File tmpDir;
+
+    @Override
+    protected void setUp() {
+        try {
+            // initialize temp directory
+            tmpDir = File.createTempFile("temp", ".dir");
+            tmpDir.delete();
+            tmpDir.mkdir();
+        } catch (IOException e) {
+            fail("unable to create temp directory", e);
+        }
+    }
+
+    @Override
+    protected void tearDown() {
+        deleteDir(tmpDir);
+    }
+
+    @Bug(9942)
+    public void testBackwardsSlashesOnWindows()  {
+        // create foo/bar/baz/Test.txt
+        File tmpFile = null;
+        try {
+            File baz = new File(new File(new File(tmpDir, "foo"), "bar"), "baz");
+            baz.mkdirs();
+
+            tmpFile = new File(baz, "Test.txt");
+            tmpFile.createNewFile();
+        } catch (IOException e) {
+            fail("unable to prepare source directory for zipping", e);
+        }
+
+        // a file to store the zip archive in
+        File zipFile = null;
+
+        // create zip from tmpDir
+        ZipArchiver archiver = null;
+
+        try {
+            zipFile = File.createTempFile("test", ".zip");
+            archiver = new ZipArchiver(new FileOutputStream(zipFile));
+
+            archiver.visit(tmpFile, "foo\\bar\\baz\\Test.txt");
+        } catch (Exception e) {
+            fail("exception driving ZipArchiver", e);
+        } finally {
+            try {
+                archiver.close();
+            } catch (IOException e) {
+                // ignored
+            }
+
+            archiver = null;
+        }
+
+        // examine zip contents and assert that none of the entry names (paths) have
+        // back-slashes ("\")
+        String zipEntryName = null;
+
+        ZipFile zipFileVerify = null; 
+        try {
+            zipFileVerify = new ZipFile(zipFile); 
+
+            zipEntryName = ((ZipEntry) zipFileVerify.entries().nextElement()).getName();
+        } catch (Exception e) {
+            fail("failure enumerating zip entries", e);
+        } finally {
+            try {
+                zipFileVerify.close();
+            } catch (IOException e) {
+                // ignored
+            }
+        }
+
+        assertEquals("foo/bar/baz/Test.txt", zipEntryName);
+    }
+
+    /**
+     * Convenience method for failing with a cause.
+     * 
+     * @param msg the failure description
+     * @param cause the root cause of the failure
+     */
+    private final void fail(final String msg, final Throwable cause) {
+        logger.error(msg, cause);
+        fail(msg);
+    }
+
+    /**
+     * Recursively deletes a directory and all of its children.
+     *
+     * @param f the File (neé, directory) to delete
+     */
+    private final void deleteDir(final File f) {
+        for (File c : f.listFiles()) {
+            if (c.isDirectory()) {
+                deleteDir(c);
+            } else {
+                c.delete();
+            }
+        }
+
+        f.delete();
+    }
+}