Can't move folder to a UNC path

Jay McGaffigan added a comment - 27/Oct/10 12:09 PM - edited I've sent this to wayne meissner but the problem is in the jnr-posix JavaSecuredFile class. It uses java.io.File#renameTo which is pretty fragile in the face of windows filesystem to filesystem moves. I've sent the following patch to Wayne. It preserves teh renameTo semantics but if that fails it uses a FileChannel style copy. index f8abb6b..1bd1a80 100644 --- a/src/org/jruby/ext/posix/JavaSecuredFile.java +++ b/src/org/jruby/ext/posix/JavaSecuredFile.java @@ -32,6 +32,10 @@ import java.io.FileFilter; import java.io.FilenameFilter; import java.io.IOException; import java.net.URI; +import java.io.FileInputStream; +import java.io.FileOutputStream; +import java.nio.channels.FileChannel; +import java.io.Closeable; /** * <p>This file catches any SecurityExceptions raised when access @@ -251,7 +255,10 @@ public class JavaSecuredFile extends File { @Override public boolean renameTo(File dest) { try { - return super.renameTo(dest); + if (super.renameTo(dest)) { + return true; + } + return moveIt(this, dest); } catch (SecurityException ex) { return false; } @@ -274,5 +281,166 @@ public class JavaSecuredFile extends File { return false; } } + + + /** Move a File + * The renameTo method does not allow action across NFS mounted filesystems + * this method is the workaround + * + * @param fromFile The existing File + * @param toFile The new File + * @return <code>true</code> if and only if the renaming succeeded; + * <code>false</code> otherwise + */ + private static final boolean moveIt(File fromFile, File toFile) { + + // delete if copy was successful, otherwise move will fail + if (copyIt(fromFile, toFile)) { + return removeIt(fromFile); + } + return false; + } + /** Copy a Directory + * if renameTo has failed and our source is a directory we have to start delving into + * copying all the directories. + * One thing to note is that I need to "preserve" the mv semantics. This means: + * if the destination is a directory that exists we add the source directory under it. + * so: mv stuff /Volumes/share would become /Volumes/share/stuff if /Volumes/share exists + * if the distination directory does not exist. We create that and add all the files in the + * source directory UNDER this new directory. + * so: mv stuff /Volumes/share/stuff3 would put the contents of stuff into stuff3 if stuff3 does not exist. + * + * @param fromDirectory the existing directory + * @param toDirectory the destination directory + * @return <code>true</code> if the copy succeeded + * <code>false</code> if there was a failure + * @throws IOException if we encounter issues + */ + private static final boolean copyDirectory (File fromDirectory, File toDirectory) throws IOException { + File destDirectory = null; + if (toDirectory.exists()) { + destDirectory = new File(toDirectory.getPath(), fromDirectory.getName()); + } else { + destDirectory = toDirectory; + } + + if (destDirectory.mkdirs() == false){ + throw new IOException("Could not create destination directory"); + } + + File[] files = fromDirectory.listFiles(); + boolean success = false; + for (File file:files){ + File toFile = new File(destDirectory, file.getName()); + if (file.isDirectory()) { + copyDirectory(file, toFile); + } else { + if (copyFile(file, toFile) == false) { + throw new IOException("Could not copy file"); + } + } + } + return true; + } + + /** Copy a File + * The renameTo method does not allow action across NFS mounted filesystems + * this method is the workaround + * + * @param fromFile The existing File + * @param toFile The new File + * @return <code>true</code> if and only if the renaming succeeded; + * <code>false</code> otherwise + */ + private static final boolean copyFile (File fromFile, File toFile) { + FileInputStream in = null; + FileOutputStream out = null; + try { + in = new FileInputStream(fromFile); + out = new FileOutputStream(toFile); + + + FileChannel src = in.getChannel(); + FileChannel dst = out.getChannel(); + + long pos = 0; + long transmitted = -1; + //Transfer bytes until we get 0. (need to cover the last chunk) + while (transmitted != 0) { + transmitted = src.transferTo(pos, 8192, dst); + pos += transmitted; + } + + // cleanupif files are not the same length + if (fromFile.length() != toFile.length()) { + toFile.delete(); + return false; + } + + return true; + } catch (IOException e) { + return false; + } finally { + closeIt(out); + closeIt(in); + } + } + + private static final void closeIt(Closeable closeable){ + try { + if (closeable != null) { + closeable.close(); + } + } catch (IOException e) { + //Do nothing + } + } + + /** + * copy a file check if directories are involved and handle appropriately + * + * @param fromFile from + * @param toFile to + * @return <code>true</code> success + * <code>false</code> failure somewhere + */ + private static final boolean copyIt(File fromFile, File toFile) { + try { + if (fromFile.isDirectory()) { + return copyDirectory(fromFile, toFile); + } else { + return copyFile(fromFile, toFile); + } + } catch (IOException e) { + return false; + } + } + + /** Remove a file. If it's a directory recursively remove it + * + * @param fromFile + * @return <code>true</code> success + * <code>false</code> could not remove files + */ + private static final boolean removeIt(File fromFile){ + boolean success = false; + if (fromFile.isDirectory()) { + File[] files = fromFile.listFiles(); + for (File file : files){ + if (file.isDirectory()) { + success = removeIt(file); + } else { + success = file.delete(); + } + if (success == false) { + //Break out of the loop on failure means we don't need to go on. + return false; + } + } + } + //AFter deleting all the children we can delete this one. + return fromFile.delete(); + } + }

Charles Oliver Nutter added a comment - 01/Nov/10 9:36 AM Patch provided, but requires updating jnr-posix release. Marking for 1.6.

Jay McGaffigan added a comment - 01/Nov/10 9:50 PM I had to update the patch to support directories too

Hiro Asari added a comment - 05/Dec/10 8:07 PM Jay, Can you attach your patch as a git patch? git diff --format-patch That way we can give you the credit for the fix.

Charles Oliver Nutter added a comment - 07/Feb/11 11:07 AM Getting a bit late in 1.6 cycle for this. Has it been integrated in the offending library?

Thomas E Enebo added a comment - 07/Feb/11 11:10 AM Hoping to land this when I land real Windows exec behavior (hoping for 1.6.0 still).

George added a comment - 14/Feb/13 2:06 PM - edited Not sure if this was fixed using Jay's patch, or http://jira.codehaus.org/browse/JRUBY-2724 fixed it, but it appears to be fixed in 1.7.2

George added a comment - 14/Feb/13 2:27 PM Correction, the move seems fixed, but a mkdir -p still fails. Using Jay's patch, I can still get this to work, but 1.7.2 hasn't fixed it.