[Exist-commits] SF.net SVN: exist:[11685] stable/eXist-1.4.x/src/org/exist/storage/ NativeBroker.java

[<di...@us...>]

Revision: 11685
          http://exist.svn.sourceforge.net/exist/?rev=11685&view=rev
Author:   dizzzz
Date:     2010-04-16 20:08:21 +0000 (Fri, 16 Apr 2010)

Log Message:
-----------
[bugfix] port of rev11673
Thomas White reported both in Sourceforge bug tracker (2981937) and
eXist-devel list a couple of bugs related to collections containing
both binary resources and subcollections on deletion and movement. These
bugs lead to an out of sync database, where the binary resource listed
internally and the files that actually are stored in the file system at
the location they are expected differ. So, errors like unreachable binary
resources are fired:

514140 [qtp11513418-62] WARN org.eclipse.jetty.util.log - Nested in javax.servlet.ServletException: An error occurred: d:\eXist2\webapp\WEB-INF\data\fs\db\exe\res\exe-res\js\jquery.taconite.js (The system cannot find the path specified):
java.io.FileNotFoundException: d:\eXist2\webapp\WEB-INF\data\fs\db\exe\res\exe-res\js\jquery.taconite.js (The system cannot find the path specified)

Even worse, the fact that files stay in the file system after collections
deletions is a security issue as well.

Both bugs come from the limitations of File.renameTo method. If you try
moving a directory or a file to a destination directory where there is
already a file or a directory with the same name, the operation is not made.

When a collection A with binary resources and subcollections B,C,D,...
with also binary resources, is going to be removed, the recursive
implementation of NativeBroker.removeCollection calls first
NativeBroker.removeCollection on B,C,D,..., and then it renames the
collection A, so it is placed on the binary log dir of the current
transaction. As the binary log contents are only disposed once the
transaction has properly finished, the File.renameTo operation on A
does not work properly, because there is already a directory with
that name in the binary log dir, leaving those contents in its origin.

Something similar happens when NativeBroker.moveCollection is called and
there is already either a binary resource or a collection containing a
binary resource with that name. The reason: the movement operation at
binary resources level is done using File.renameTo call. In this scenario
the operation fails, but binary resources in that moved collection are
registered as so, leading to java.io.FileNotFoundException in further
accesses.

So, this patch fixes both bugs. The first one is fixed moving the code
which handle binary resource collection removal just before the recursive
calls on subcollections. The second one is fixed checking whether a
destination collection exists at filesystem level just before the movement,
so previous contents are moved to the binary log dir and registered using
a RenameBinaryLoggable object.

Revision Links:
--------------
    http://exist.svn.sourceforge.net/exist/?rev=11673&view=rev

Modified Paths:
--------------
    stable/eXist-1.4.x/src/org/exist/storage/NativeBroker.java

Modified: stable/eXist-1.4.x/src/org/exist/storage/NativeBroker.java
===================================================================
--- stable/eXist-1.4.x/src/org/exist/storage/NativeBroker.java	2010-04-15 04:36:23 UTC (rev 11684)
+++ stable/eXist-1.4.x/src/org/exist/storage/NativeBroker.java	2010-04-16 20:08:21 UTC (rev 11685)
@@ -853,20 +853,35 @@
     }
 
 	private void moveBinaryFork(Txn transaction, File sourceDir, Collection destination, XmldbURI newName) throws IOException {
-        File targetDir = getCollectionFile(fsDir,destination.getURI().append(newName),false);
-        if (sourceDir.exists()) {
-           targetDir.getParentFile().mkdirs();
-           if (sourceDir.renameTo(targetDir)) {
-              Loggable loggable = new RenameBinaryLoggable(this,transaction,sourceDir,targetDir);
-              try {
-                 logManager.writeToLog(loggable);
-              } catch (TransactionException e) {
-                  LOG.warn(e.getMessage(), e);
-              }
-           } else {
-              LOG.fatal("Cannot move "+sourceDir+" to "+targetDir);
-           }
-        }
+		File targetDir = getCollectionFile(fsDir,destination.getURI().append(newName),false);
+		if (sourceDir.exists()) {
+			if(targetDir.exists()) {
+				File targetDelDir = getCollectionFile(fsBackupDir,transaction,destination.getURI().append(newName),true);
+				targetDelDir.getParentFile().mkdirs();
+	
+				if (targetDir.renameTo(targetDelDir)) {
+					Loggable loggable = new RenameBinaryLoggable(this,transaction,targetDir,targetDelDir);
+					try {
+						logManager.writeToLog(loggable);
+					} catch (TransactionException e) {
+						LOG.warn(e.getMessage(), e);
+					}
+				} else {
+					LOG.fatal("Cannot rename "+targetDir+" to "+targetDelDir);
+				}
+			}
+			targetDir.getParentFile().mkdirs();
+			if (sourceDir.renameTo(targetDir)) {
+				Loggable loggable = new RenameBinaryLoggable(this,transaction,sourceDir,targetDir);
+				try {
+					logManager.writeToLog(loggable);
+				} catch (TransactionException e) {
+					LOG.warn(e.getMessage(), e);
+				}
+			} else {
+				LOG.fatal("Cannot move "+sourceDir+" to "+targetDir);
+			}
+		}
 	}
 
 	private void moveCollectionRecursive(Txn transaction, Collection collection, Collection destination, XmldbURI newName) throws PermissionDeniedException, IOException, LockException {
@@ -1004,6 +1019,20 @@
                 pool.getConfigurationManager().invalidateAll(uri);
 
                 // remove child collections
+                if (sourceDir.exists()) {
+                   targetDir.getParentFile().mkdirs();
+                   if (sourceDir.renameTo(targetDir)) {
+                     Loggable loggable = new RenameBinaryLoggable(this,transaction,sourceDir,targetDir);
+                     try {
+                        logManager.writeToLog(loggable);
+                     } catch (TransactionException e) {
+                         LOG.warn(e.getMessage(), e);
+                     }
+
+                   } else {
+                      LOG.fatal("Cannot rename "+sourceDir+" to "+targetDir);
+                   }
+                }
                 if(LOG.isDebugEnabled())
                     LOG.debug("Removing children collections from their parent '" + collName + "'...");
 
@@ -1172,20 +1201,6 @@
                     freeResourceId(transaction, doc.getDocId());
                 }
 
-                if (sourceDir.exists()) {
-                   targetDir.getParentFile().mkdirs();
-                   if (sourceDir.renameTo(targetDir)) {
-                     Loggable loggable = new RenameBinaryLoggable(this,transaction,sourceDir,targetDir);
-                     try {
-                        logManager.writeToLog(loggable);
-                     } catch (TransactionException e) {
-                         LOG.warn(e.getMessage(), e);
-                     }
-
-                   } else {
-                      LOG.fatal("Cannot rename "+sourceDir+" to "+targetDir);
-                   }
-                }
                 if(LOG.isDebugEnabled())
                     LOG.debug("Removing collection '" + collName + "' took " + (System.currentTimeMillis() - start));
 


This was sent by the SourceForge.net collaborative development platform, the world's largest Open Source development site.