fix some xattr sync bugs

Sat, 19 Oct 2019 15:46:33 +0200

author
Olaf Wintermann <olaf.wintermann@gmail.com>
date
Sat, 19 Oct 2019 15:46:33 +0200
changeset 663
888aa263c0f1
parent 662
ab34fc9ecf1d
child 664
d066a89d5e87

fix some xattr sync bugs

dav/sync.c file | annotate | diff | comparison | revisions
dav/sync.h file | annotate | diff | comparison | revisions
dav/xattrtool.c file | annotate | diff | comparison | revisions
test/bin-test/dav-home/sync.xml file | annotate | diff | comparison | revisions
test/bin-test/test-dav-sync-metadata1.sh file | annotate | diff | comparison | revisions
--- a/dav/sync.c	Sat Oct 19 11:15:04 2019 +0200
+++ b/dav/sync.c	Sat Oct 19 15:46:33 2019 +0200
@@ -3236,16 +3236,17 @@
         }
     }
     
-    DavXmlNode *xattr_prop = dav_get_property_ns(res, DAV_PROPS_NS, "xattributes");
-    if(xattr_prop) {
-        XAttributes *xattr = xml_get_attributes(xattr_prop);
-        if(xattr) {
-            if(!sync_store_xattr(dir, path, xattr)) {
-                if(local->xattr_hash) {
-                    free(local->xattr_hash);
-                }
-                local->xattr_hash = xattr->hash;
+    if((dir->metadata & FINFO_XATTR) == FINFO_XATTR) {
+        DavXmlNode *xattr_prop = dav_get_property_ns(res, DAV_PROPS_NS, "xattributes");
+        XAttributes *xattr = NULL;
+        if(xattr_prop) {
+            xattr = xml_get_attributes(xattr_prop);
+        }
+        if(!sync_store_xattr(dir, path, xattr)) {
+            if(local->xattr_hash) {
+                free(local->xattr_hash);
             }
+            local->xattr_hash = xattr ? xattr->hash : NULL;
         }
     }
     
@@ -3257,16 +3258,53 @@
 }
 
 int sync_store_xattr(SyncDirectory *dir, const char *path, XAttributes *xattr) {
-    for(int i=0;i<xattr->nattr;i++) {
+    // create a map of all currently available local attributes
+    ssize_t nelm = 0;
+    char **list = xattr_list(path, &nelm);
+    UcxMap *current_xattr = NULL;
+    if(nelm > 0) {
+        current_xattr = ucx_map_new(nelm + 8);
+        for(int i=0;i<nelm;i++) {
+            // use the xattr name as key and store any value
+            ucx_map_cstr_put(current_xattr, list[i], list[i]);
+        }
+    }
+    if(list) {
+        free(list);
+    }
+    
+    // store extended attributes
+    size_t nattr = xattr ? xattr->nattr : 0;
+    for(int i=0;i<nattr;i++) {
         sstr_t value = xattr->values[i];
         if(xattr_set(path, xattr->names[i], value.ptr, value.length)) {
-            fprintf(
-                    stderr,
+            fprintf(stderr,
                     "Cannot store xattr '%s' for file: %s\n",
                     xattr->names[i],
                     path);
         }
-    }
+        
+        if(current_xattr) {
+            // to detect which xattributes are removed, we remove all new
+            // attributes from the map and all remaining attributes must
+            // be removed with xattr_remove
+            char *value = ucx_map_cstr_remove(current_xattr, xattr->names[i]);
+            if(value) {
+                free(value);
+            }
+        }
+    }
+    
+    if(current_xattr) {
+        UcxMapIterator i = ucx_map_iterator(current_xattr);
+        char *value = NULL;
+        UCX_MAP_FOREACH(key, value, i) {
+            (void)xattr_remove(path, value); // don't print error
+            free(value);
+        }
+        ucx_map_free(current_xattr);
+    }
+    
     return 0;
 }
 
@@ -3628,20 +3666,20 @@
 }
 
 static void update_metadata_hashes(LocalResource *local, MetadataHashes hashes) {
-    if(hashes.tags) {
+    if(hashes.update_tags) {
         if(local->tags_hash) {
             free(local->tags_hash);
             local->tags_hash = NULL;
         }
         local->tags_hash = hashes.tags;
     }
-    if(hashes.tags_remote) {
+    if(hashes.update_tags_remote) {
         if(local->remote_tags_hash) {
             free(local->remote_tags_hash);
         }
         local->remote_tags_hash = hashes.tags_remote;
     }
-    if(hashes.xattr) {
+    if(hashes.update_xattr) {
         if(local->xattr_hash) {
             free(local->xattr_hash);
         }
@@ -4196,14 +4234,15 @@
         DavResource *res,
         LocalResource *local)
 {
-    MetadataHashes hashes = {NULL, NULL, NULL};
+    MetadataHashes hashes = {NULL, NULL, NULL, 0, 0, 0};
     if(dir->tagconfig) {
         // get local tags
         DavBool changed = 0;
         char *tags_hash = NULL;
         UcxList *tags = sync_get_file_tags(dir, local, &changed, &tags_hash);
         if(changed || local->tags_updated) {
-            hashes.tags = tags_hash;            
+            hashes.tags = tags_hash;      
+            hashes.update_tags = 1;
             
             DavBool store_tags = TRUE;
             // get remote tags
@@ -4268,9 +4307,14 @@
     if(local->xattr_updated) {
         if(local->xattr) {
             resource_set_xattr(res, local->xattr);
-            hashes.xattr = strdup(local->xattr->hash);
+            hashes.xattr = local->xattr ? strdup(local->xattr->hash) : NULL;
+            hashes.update_xattr = 1;
         } else {
             dav_remove_property(res, "idavprops:xattributes");
+            if(local->xattr_hash) {
+                free(local->xattr_hash);
+                local->xattr_hash = NULL;
+            }
         }
     }
     return hashes;
--- a/dav/sync.h	Sat Oct 19 11:15:04 2019 +0200
+++ b/dav/sync.h	Sat Oct 19 15:46:33 2019 +0200
@@ -65,6 +65,9 @@
     char *tags;
     char *tags_remote;
     char *xattr;
+    DavBool update_tags;
+    DavBool update_tags_remote;
+    DavBool update_xattr;
 } MetadataHashes;
 
 typedef struct MovedFile {
--- a/dav/xattrtool.c	Sat Oct 19 11:15:04 2019 +0200
+++ b/dav/xattrtool.c	Sat Oct 19 15:46:33 2019 +0200
@@ -40,6 +40,7 @@
 int attrtool_list(int argc, char **argv, int values);
 int attrtool_get(int argc, char **argv, int raw);
 int attrtool_set(int argc, char **argv);
+int attrtool_rm(int argc, char **argv);
 
 void print_usage(char *cmd) {
     fprintf(stderr, "usage %s:\n", cmd);
@@ -47,7 +48,7 @@
     fprintf(stderr, "   listvalues <file>\n");
     fprintf(stderr, "   get <file> <name>\n");
     fprintf(stderr, "   set <file> <name> <value>\n");
-    fprintf(stderr, "   getraw <file> <name>\n");
+    fprintf(stderr, "   remove <file> <name>\n");
 }
 
 int main(int argc, char **argv) {
@@ -62,10 +63,10 @@
         return attrtool_list(argc, argv, 1);
     } else if(!strcmp(argv[1], "get")) {
         return attrtool_get(argc, argv, 0);
-    } else if(!strcmp(argv[1], "getraw")) {
-        return attrtool_get(argc, argv, 1);
     } else if(!strcmp(argv[1], "set")) {
         return attrtool_set(argc, argv);
+    } else if(!strcmp(argv[1], "rm") || !strcmp(argv[1], "remove")) {
+        return attrtool_rm(argc, argv);
     } else {
         fprintf(stderr, "Unknown command\n");
         print_usage(argv[0]);
@@ -158,3 +159,21 @@
     return ret;
 }
 
+int attrtool_rm(int argc, char **argv) {
+    if(argc < 4) {
+        fprintf(stderr, "Too few arguments\n");
+        print_usage(argv[1]);
+        return 1;
+    }
+    
+    char *file = argv[2];
+    char *attr = argv[3];
+    int ret = 0;
+    
+    if(xattr_remove(file, attr)) {
+        perror("xattr_remove");
+        ret = 1;
+    }
+    
+    return ret;
+}
--- a/test/bin-test/dav-home/sync.xml	Sat Oct 19 11:15:04 2019 +0200
+++ b/test/bin-test/dav-home/sync.xml	Sat Oct 19 15:46:33 2019 +0200
@@ -120,6 +120,8 @@
 		<trash>.trash</trash>
 		<database>dav-sync-tests-test4a-db.xml</database>
 		
+		<hashing>true</hashing>
+		
 		<tagconfig>
 			<local-store format="csv">xattr</local-store>
 		</tagconfig>
@@ -134,6 +136,8 @@
 		<trash>.trash</trash>
 		<database>dav-sync-tests-test4b-db.xml</database>
 		
+		<hashing>true</hashing>
+		
 		<tagconfig>
 			<local-store format="csv">xattr</local-store>
 		</tagconfig>
--- a/test/bin-test/test-dav-sync-metadata1.sh	Sat Oct 19 11:15:04 2019 +0200
+++ b/test/bin-test/test-dav-sync-metadata1.sh	Sat Oct 19 15:46:33 2019 +0200
@@ -38,6 +38,8 @@
 	exit 1
 fi
 
+XATTR=../../build/xattrtool
+
 # checks if tmp-sync/out.txt contains a specific text
 # arg1: pattern
 # arg2: errormsg
@@ -164,6 +166,7 @@
 	exit 2
 fi
 
+
 # ----------------------------------------------------------------------------
 # test 3: modify file1 and push/pull
 # expected result: file content synced, mtime also synced
@@ -187,3 +190,226 @@
 	echo "file1: mtime not synced"
 	exit 2
 fi
+
+
+# ----------------------------------------------------------------------------
+# test 4: add xattr to files, push/pull
+# expected result: xattr synced
+
+# test if xattr are supported
+../../build/xattrtool set tmp-sync/test4a/file1 attr1 testv  2> /dev/null
+ATTR_TEST=`../../build/xattrtool get tmp-sync/test4a/file1 attr1 2> /dev/null`
+if [ $ATTR_TEST != "testv" ]; then
+	echo "xattr unsupported, skip"
+	exit 2
+fi
+
+sleep 2
+
+# add xattr to file without content modification
+$XATTR set tmp-sync/test4a/file1 attr2 value2
+$XATTR set tmp-sync/test4a/file1 attr3 hello
+$XATTR set tmp-sync/test4a/file1 attr4 hello
+touch tmp-sync/test4a/file1
+
+# modify file and add xattr
+echo "test4-mod1" >> tmp-sync/test4a/dir1/file2
+$XATTR set tmp-sync/test4a/dir1/file2 msg helloworld
+
+# create new file with extended attributes
+echo "newfile" > tmp-sync/test4a/file3
+$XATTR set tmp-sync/test4a/file3 msg helloworld
+
+dav_sync_push test4a "test 4: push failed"
+check_tmpout "0 conflicts" "test 4: wrong conflict counter (push)"
+check_tmpout "0 errors" "test 4: wrong error counter (push)"
+check_tmpout "put: /file3" "test 4: file 3 not pushed"
+check_tmpout "put: /dir1/file2" "test 4: file 2 not pushed"
+check_tmpout "update: /file1" "test 4: file 1 not pushed"
+
+dav_sync_pull test4b "test 4: pull failed"
+check_tmpout "0 conflicts" "test 4: wrong conflict counter (pull)"
+check_tmpout "0 errors" "test 4: wrong error counter (pull)"
+check_tmpout "get: /file3" "test 4: file 3 not pulled"
+check_tmpout "get: /dir1/file2" "test 4: file 2 not pulled"
+check_tmpout "update: /file1" "test 4: file 1 not pulled"
+
+FILE1_ATTR2=`$XATTR get tmp-sync/test4b/file1 attr2 2> /dev/null`
+FILE1_ATTR3=`$XATTR get tmp-sync/test4b/file1 attr3 2> /dev/null`
+FILE1_ATTR4=`$XATTR get tmp-sync/test4b/file1 attr4 2> /dev/null`
+
+FILE2_ATTR0=`$XATTR get tmp-sync/test4b/dir1/file2 msg 2> /dev/null`
+
+FILE3_ATTR0=`$XATTR get tmp-sync/test4b/file3 msg 2> /dev/null`
+
+
+if [ "$FILE1_ATTR2" != "value2" ]; then
+	echo "file1 attr2 has wrong value: $FILE1_ATTR2"
+	exit 2
+fi
+if [ "$FILE1_ATTR3" != "hello" ]; then
+	echo "file1 attr3 has wrong value: $FILE1_ATTR3"
+	exit 2
+fi
+if [ "$FILE1_ATTR4" != "hello" ]; then
+	echo "file1 attr4 has wrong value: $FILE1_ATTR4"
+	exit 2
+fi
+
+if [ "$FILE2_ATTR0" != "helloworld" ]; then
+	echo "file2 attr has wrong value: $FILE2_ATTR0"
+	exit 2
+fi
+
+if [ "$FILE3_ATTR0" != "helloworld" ]; then
+	echo "file3 attr has wrong value: $FILE3_ATTR0"
+	exit 2
+fi
+
+
+# ----------------------------------------------------------------------------
+# test 5: do nothing, push/pull
+# expected result: no update, no files pushed/pulled
+
+sleep 2
+
+dav_sync_push test4a "test 5: push failed"
+check_tmpout "0 files pushed" "test 5: wrong push counter"
+check_tmpout "0 conflicts" "test 5: wrong conflict counter (push)"
+check_tmpout "0 errors" "test 5: wrong error counter (push)"
+LN=`cat tmp-sync/out.txt | wc -l`
+if [ "$LN" != "1" ]; then
+	echo "test 5: wrong output (push)"
+	exit 2
+fi
+
+dav_sync_pull test4b "test 5: pull failed"
+check_tmpout "0 files pulled" "test 5: wrong pull counter"
+check_tmpout "0 conflicts" "test 5: wrong conflict counter (pull)"
+check_tmpout "0 errors" "test 5: wrong error counter (pull)"
+LN=`cat tmp-sync/out.txt | wc -l`
+if [ "$LN" != "1" ]; then
+	echo "test 5: wrong output (pull)"
+	exit 2
+fi
+
+
+# ----------------------------------------------------------------------------
+# test 6: add additional xattr to files, push/pull
+# expected result: xattr synced
+
+sleep 2
+
+$XATTR set tmp-sync/test4a/file1 test5 f1value5
+
+echo "test6-mod1" >> tmp-sync/test4a/file3
+$XATTR set tmp-sync/test4a/file3 test5 f3value5
+
+dav_sync_push test4a "test 6: push failed"
+check_tmpout "0 conflicts" "test 6: wrong conflict counter (push)"
+check_tmpout "0 errors" "test 6: wrong error counter (push)"
+check_tmpout "put: /file3" "test 6: file 3 not pushed"
+check_tmpout "update: /file1" "test 6: file 1 not pushed"
+
+dav_sync_pull test4b "test 6: pull failed"
+check_tmpout "0 conflicts" "test 6: wrong conflict counter (pull)"
+check_tmpout "0 errors" "test 6: wrong error counter (pull)"
+check_tmpout "get: /file3" "test 6: file 3 not pulled"
+check_tmpout "update: /file1" "test 6: file 1 not pulled"
+
+TEST5=`$XATTR get tmp-sync/test4b/file1 test5 2> /dev/null`
+if [ "$TEST5" != "f1value5" ]; then
+	echo "test 6: file1 attr has wrong value"
+	exit 2
+fi
+TEST5=`$XATTR get tmp-sync/test4b/file3 test5 2> /dev/null`
+if [ "$TEST5" != "f3value5" ]; then
+	echo "test 6: file3 attr has wrong value"
+	exit 2
+fi
+
+
+# ----------------------------------------------------------------------------
+# test 7: do nothing, push/pull
+# expected result: no update, no files pushed/pulled
+
+sleep 2
+
+dav_sync_push test4a "test 7: push failed"
+check_tmpout "0 files pushed" "test 7: wrong push counter"
+check_tmpout "0 conflicts" "test 7: wrong conflict counter (push)"
+check_tmpout "0 errors" "test 7: wrong error counter (push)"
+LN=`cat tmp-sync/out.txt | wc -l`
+if [ "$LN" != "1" ]; then
+	echo "test 7: wrong output (push)"
+	exit 2
+fi
+
+dav_sync_pull test4b "test 7: pull failed"
+check_tmpout "0 files pulled" "test 7: wrong pull counter"
+check_tmpout "0 conflicts" "test 7: wrong conflict counter (pull)"
+check_tmpout "0 errors" "test 7: wrong error counter (pull)"
+LN=`cat tmp-sync/out.txt | wc -l`
+if [ "$LN" != "1" ]; then
+	echo "test 7: wrong output (pull)"
+	exit 2
+fi
+
+
+# ----------------------------------------------------------------------------
+# test 8: remove xattr
+# expected result: xattr removed in test4b
+
+$XATTR remove tmp-sync/test4a/dir1/file2 msg
+if [ $? -ne 0 ]; then
+	echo "test 8: xattrtool remove failed"
+	exit 2
+fi
+
+touch tmp-sync/test4a/dir1/file2
+
+dav_sync_push test4a "test 8: push failed"
+check_tmpout "0 conflicts" "test 8: wrong conflict counter (push)"
+check_tmpout "0 errors" "test 8: wrong error counter (push)"
+check_tmpout "update: /dir1/file2" "test 8: file 1 not pushed"
+
+dav_sync_pull test4b "test 8: pull failed"
+check_tmpout "0 conflicts" "test 8: wrong conflict counter (pull)"
+check_tmpout "0 errors" "test 8: wrong error counter (pull)"
+check_tmpout "update: /dir1/file2" "test 8: file 1 not pulled"
+
+TEST6=`$XATTR get tmp-sync/test4b/dir1/file2 msg 2> /dev/null`
+if [ "$TEST6" != "" ]; then
+	echo "test 8: xattr msg not removed"
+	exit 2
+fi
+
+
+# ----------------------------------------------------------------------------
+# test 9: do nothing, push/pull
+# expected result: no update, no files pushed/pulled
+
+sleep 2
+
+dav_sync_push test4a "test 9: push failed"
+check_tmpout "0 files pushed" "test 9: wrong push counter"
+check_tmpout "0 conflicts" "test 9: wrong conflict counter (push)"
+check_tmpout "0 errors" "test 9: wrong error counter (push)"
+LN=`cat tmp-sync/out.txt | wc -l`
+if [ "$LN" != "1" ]; then
+	echo "test 7: wrong output (push)"
+	exit 2
+fi
+
+dav_sync_pull test4b "test 9: pull failed"
+check_tmpout "0 files pulled" "test 9: wrong pull counter"
+check_tmpout "0 conflicts" "test 9: wrong conflict counter (pull)"
+check_tmpout "0 errors" "test 9: wrong error counter (pull)"
+LN=`cat tmp-sync/out.txt | wc -l`
+if [ "$LN" != "1" ]; then
+	echo "test 9: wrong output (pull)"
+	exit 2
+fi
+
+
+

mercurial