Permalink
Browse files

Merge pull request #151 from adamretter/develop

Fixes for permissions assignments when copying resources
  • Loading branch information...
2 parents 1de22a8 + 558c479 commit 136c6293cc0c37feeab3f196a6d1ceceade740f5 @wolfgangmm wolfgangmm committed Feb 13, 2014
@@ -292,7 +292,7 @@ private void before(DBBroker broker, Txn transaction, DocumentImpl document, boo
vDoc = vCollection.getDocument(broker, binUri);
} else {
vDoc = new DocumentImpl(broker.getBrokerPool(), vCollection, XmldbURI.createInternal(vFileName));
- vDoc.copyOf(document);
+ vDoc.copyOf(document, true);
vDoc.copyChildren(document);
}
@@ -902,7 +902,7 @@ final public Permission getPermissions() {
}
}
- final public Permission getPermissionsNoLock() {
+ public Permission getPermissionsNoLock() {
return permissions;
}
@@ -1649,7 +1649,7 @@ private IndexInfo validateXMLResourceInternal(final Txn transaction, final DBBro
} else {
//TODO : use a more elaborated method ? No triggers...
broker.removeXMLResource(transaction, oldDoc, false);
- oldDoc.copyOf(document);
+ oldDoc.copyOf(document, true);
indexer.setDocumentObject(oldDoc);
//old has become new at this point
document = oldDoc;
@@ -312,8 +312,13 @@ public DocumentMetadata getMetadata() {
* This is called by {@link Collection} when replacing a document.
*
* @param other a <code>DocumentImpl</code> value
+ * @param preserve Cause copyOf to preserve the following attributes of
+ * each source file in the copy: modification time,
+ * access time, file mode, user ID, and group ID,
+ * as allowed by permissions and Access Control
+ * Lists (ACLs)
*/
- public void copyOf(DocumentImpl other) {
+ public void copyOf(final DocumentImpl other, final boolean preserve) {
childAddress = null;
children = 0;
@@ -326,16 +331,21 @@ public void copyOf(DocumentImpl other) {
//copy metadata
metadata.copyOf(other.getMetadata());
- //update timestamp
- final long timestamp = System.currentTimeMillis();
- metadata.setCreated(timestamp);
- metadata.setLastModified(timestamp);
+ if(preserve) {
+ //copy permission
+ permissions = ((UnixStylePermission)other.permissions).copy();
+ //created and last modified are done by metadata.copyOf
+ //metadata.setCreated(other.getMetadata().getCreated());
+ //metadata.setLastModified(other.getMetadata().getLastModified());
+ } else {
+ //update timestamp
+ final long timestamp = System.currentTimeMillis();
+ metadata.setCreated(timestamp);
+ metadata.setLastModified(timestamp);
+ }
// reset pageCount: will be updated during storage
metadata.setPageCount(0);
-
- //copy permission
- permissions = ((UnixStylePermission)other.permissions).copy();
}
/**
@@ -48,6 +48,7 @@
import org.exist.Indexer;
import org.exist.backup.RawDataBackup;
import org.exist.collections.Collection;
+import org.exist.collections.Collection.CollectionEntry;
import org.exist.collections.Collection.SubCollectionEntry;
import org.exist.collections.CollectionCache;
import org.exist.collections.CollectionConfiguration;
@@ -996,32 +997,34 @@ private Collection openCollection(XmldbURI uri, long addr, int lockMode) throws
/**
* Checks all permissions in the tree to ensure that a copy operation will succeed
*/
- final void checkPermissionsForCopy(final Collection src, final XmldbURI destUri) throws PermissionDeniedException, LockException {
+ protected void checkPermissionsForCopy(final Collection src, final XmldbURI destUri, final XmldbURI newName) throws PermissionDeniedException, LockException {
if(!src.getPermissionsNoLock().validate(getSubject(), Permission.EXECUTE | Permission.READ)) {
throw new PermissionDeniedException("Permission denied to copy collection " + src.getURI() + " by " + getSubject().getName());
}
-
-
+
final Collection dest = getCollection(destUri);
- final XmldbURI newDestUri = destUri.append(src.getURI().lastSegment());
+ final XmldbURI newDestUri = destUri.append(newName);
final Collection newDest = getCollection(newDestUri);
if(dest != null) {
- if(!dest.getPermissionsNoLock().validate(getSubject(), Permission.EXECUTE | Permission.WRITE | Permission.READ)) {
+ //if(!dest.getPermissionsNoLock().validate(getSubject(), Permission.EXECUTE | Permission.WRITE | Permission.READ)) {
+ //TODO do we really need WRITE permission on the dest?
+ if(!dest.getPermissionsNoLock().validate(getSubject(), Permission.EXECUTE | Permission.WRITE)) {
throw new PermissionDeniedException("Permission denied to copy collection " + src.getURI() + " to " + dest.getURI() + " by " + getSubject().getName());
}
if(newDest != null) {
- if(!dest.getPermissionsNoLock().validate(getSubject(), Permission.EXECUTE | Permission.READ)) {
+ //TODO why do we need READ access on the dest collection?
+ /*if(!dest.getPermissionsNoLock().validate(getSubject(), Permission.EXECUTE | Permission.READ)) {
throw new PermissionDeniedException("Permission denied to copy collection " + src.getURI() + " to " + dest.getURI() + " by " + getSubject().getName());
- }
+ }*/
- if(newDest.isEmpty(this)) {
- if(!dest.getPermissionsNoLock().validate(getSubject(), Permission.WRITE)) {
- throw new PermissionDeniedException("Permission denied to copy collection " + src.getURI() + " to " + dest.getURI() + " by " + getSubject().getName());
+ //if(newDest.isEmpty(this)) {
+ if(!newDest.getPermissionsNoLock().validate(getSubject(), Permission.EXECUTE | Permission.WRITE)) {
+ throw new PermissionDeniedException("Permission denied to copy collection " + src.getURI() + " to " + newDest.getURI() + " by " + getSubject().getName());
}
- }
+ //}
}
}
@@ -1030,17 +1033,14 @@ final void checkPermissionsForCopy(final Collection src, final XmldbURI destUri)
if(!srcSubDoc.getPermissions().validate(getSubject(), Permission.READ)) {
throw new PermissionDeniedException("Permission denied to copy collection " + src.getURI() + " for resource " + srcSubDoc.getURI() + " by " + getSubject().getName());
}
-
+
+ //if the destination resource exists, we must have write access to replace it's metadata etc. (this follows the Linux convention)
if(newDest != null && !newDest.isEmpty(this)) {
final DocumentImpl newDestSubDoc = newDest.getDocument(this, srcSubDoc.getFileURI()); //TODO check this uri is just the filename!
if(newDestSubDoc != null) {
if(!newDestSubDoc.getPermissions().validate(getSubject(), Permission.WRITE)) {
throw new PermissionDeniedException("Permission denied to copy collection " + src.getURI() + " for resource " + newDestSubDoc.getURI() + " by " + getSubject().getName());
}
- } else {
- if(!dest.getPermissionsNoLock().validate(getSubject(), Permission.WRITE)) {
- throw new PermissionDeniedException("Permission denied to copy collection " + src.getURI() + " to " + dest.getURI() + " by " + getSubject().getName());
- }
}
}
}
@@ -1049,7 +1049,7 @@ final void checkPermissionsForCopy(final Collection src, final XmldbURI destUri)
final XmldbURI srcSubColUri = itSrcSubColUri.next();
final Collection srcSubCol = getCollection(src.getURI().append(srcSubColUri));
- checkPermissionsForCopy(srcSubCol, newDestUri);
+ checkPermissionsForCopy(srcSubCol, newDestUri, srcSubColUri);
}
}
@@ -1094,7 +1094,7 @@ public void copyCollection(final Txn transaction, final Collection collection, f
trigger.beforeCopyCollection(this, transaction, collection, dstURI);
//atomically check all permissions in the tree to ensure a copy operation will succeed before starting copying
- checkPermissionsForCopy(collection, destination.getURI());
+ checkPermissionsForCopy(collection, destination.getURI(), newName);
final DocumentTrigger docTrigger = new DocumentTriggers(this);
@@ -1110,29 +1110,47 @@ public void copyCollection(final Txn transaction, final Collection collection, f
private Collection doCopyCollection(final Txn transaction, final DocumentTrigger trigger, final Collection collection, final Collection destination, XmldbURI newName) throws PermissionDeniedException, IOException, EXistException, TriggerException, LockException {
- if(newName == null)
- {newName = collection.getURI().lastSegment();}
-
+ if(newName == null) {
+ newName = collection.getURI().lastSegment();
+ }
newName = destination.getURI().append(newName);
- if (LOG.isDebugEnabled())
- {LOG.debug("Copying collection to '" + newName + "'");}
+ if(LOG.isDebugEnabled()) {
+ LOG.debug("Copying collection to '" + newName + "'");
+ }
final Collection destCollection = getOrCreateCollection(transaction, newName);
for(final Iterator<DocumentImpl> i = collection.iterator(this); i.hasNext(); ) {
final DocumentImpl child = i.next();
- if (LOG.isDebugEnabled())
- {LOG.debug("Copying resource: '" + child.getURI() + "'");}
+ if(LOG.isDebugEnabled()) {
+ LOG.debug("Copying resource: '" + child.getURI() + "'");
+ }
+
+ //TODO The code below seems quite different to that in NativeBroker#copyResource presumably should be the same?
+
final XmldbURI newUri = destCollection.getURI().append(child.getFileURI());
trigger.beforeCopyDocument(this, transaction, child, newUri);
+ //are we overwriting an existing document?
+ final CollectionEntry oldDoc;
+ if(destCollection.hasDocument(this, child.getFileURI())) {
+ oldDoc = destCollection.getResourceEntry(this, child.getFileURI().toString());
+ } else {
+ oldDoc = null;
+ }
+
DocumentImpl createdDoc;
if (child.getResourceType() == DocumentImpl.XML_FILE) {
//TODO : put a lock on newDoc ?
final DocumentImpl newDoc = new DocumentImpl(pool, destCollection, child.getFileURI());
- newDoc.copyOf(child);
+ newDoc.copyOf(child, false);
+ if(oldDoc != null) {
+ //preserve permissions from existing doc we are replacing
+ newDoc.setPermissions(oldDoc.getPermissions()); //TODO use newDoc.copyOf(oldDoc) ideally, but we cannot currently access oldDoc without READ access to it, which we may not have (and should not need for this)!
+ }
+
newDoc.setDocId(getNextResourceId(transaction, destination));
copyXMLResource(transaction, child, newDoc);
storeXMLResource(transaction, newDoc);
@@ -1141,7 +1159,11 @@ private Collection doCopyCollection(final Txn transaction, final DocumentTrigger
createdDoc = newDoc;
} else {
final BinaryDocument newDoc = new BinaryDocument(pool, destCollection, child.getFileURI());
- newDoc.copyOf(child);
+ newDoc.copyOf(child, false);
+ if(oldDoc != null) {
+ //preserve permissions from existing doc we are replacing
+ newDoc.setPermissions(oldDoc.getPermissions()); //TODO use newDoc.copyOf(oldDoc) ideally, but we cannot currently access oldDoc without READ access to it, which we may not have (and should not need for this)!
+ }
newDoc.setDocId(getNextResourceId(transaction, destination));
InputStream is = null;
@@ -2524,9 +2546,8 @@ public void copyResource(Txn transaction, DocumentImpl doc, Collection destinati
}
} else {
DocumentImpl newDoc = new DocumentImpl(pool, destination, newName);
- newDoc.copyOf(doc);
+ newDoc.copyOf(doc, oldDoc != null);
newDoc.setDocId(getNextResourceId(transaction, destination));
- //newDoc.setPermissions(doc.getPermissions());
newDoc.getUpdateLock().acquire(Lock.WRITE_LOCK);
try {
copyXMLResource(transaction, doc, newDoc);
@@ -3018,7 +3039,7 @@ public Object start() {
}.run();
// create a copy of the old doc to copy the nodes into it
final DocumentImpl tempDoc = new DocumentImpl(pool, doc.getCollection(), doc.getFileURI());
- tempDoc.copyOf(doc);
+ tempDoc.copyOf(doc, true);
tempDoc.setDocId(doc.getDocId());
indexController.setDocument(doc, StreamListener.STORE);
final StreamListener listener = indexController.getStreamListener();
@@ -465,10 +465,11 @@ public void copyResource(XmldbURI resourcePath, XmldbURI destinationPath, XmldbU
throws XMLDBException {
resourcePath = parent.getPathURI().resolveCollectionPath(resourcePath);
- if (destinationPath == null)
- {destinationPath = resourcePath.removeLastSegment();}
- else
- {destinationPath = parent.getPathURI().resolveCollectionPath(destinationPath);}
+ if(destinationPath == null) {
+ destinationPath = resourcePath.removeLastSegment();
+ } else {
+ destinationPath = parent.getPathURI().resolveCollectionPath(destinationPath);
+ }
final Subject preserveSubject = brokerPool.getSubject();
final TransactionManager transact = brokerPool.getTransactionManager();
@@ -493,9 +494,9 @@ public void copyResource(XmldbURI resourcePath, XmldbURI destinationPath, XmldbU
transact.abort(transaction);
throw new XMLDBException(ErrorCodes.NO_SUCH_COLLECTION, "Collection " + destinationPath + " not found");
}
- if (newName == null) {
- newName = resourcePath.lastSegment();
- }
+ if(newName == null) {
+ newName = resourcePath.lastSegment();
+ }
broker.copyResource(transaction, doc, destination, newName);
transact.commit(transaction);
} catch ( final EXistException e ) {
@@ -520,12 +521,12 @@ public void copyResource(XmldbURI resourcePath, XmldbURI destinationPath, XmldbU
}
}
- public void setCollection( Collection parent ) throws XMLDBException {
+ public void setCollection(Collection parent) throws XMLDBException {
this.parent = (LocalCollection) parent;
}
- public void setProperty( String property,
- String value ) {
+ public void setProperty(String property,
+ String value) {
}
@Override
Oops, something went wrong.

0 comments on commit 136c629

Please sign in to comment.