CSync: Proper error for invalid filenames #2777 #3128 #2240

csync/src/csync.h

@@ -100,7 +100,8 @@ enum csync_status_codes_e {
     CSYNC_STATUS_INDIVIDUAL_IS_INVALID_CHARS,
     CSYNC_STATUS_INDIVIDUAL_EXCLUDE_LONG_FILENAME,
     CYSNC_STATUS_FILE_LOCKED_OR_OPEN,
-    CSYNC_STATUS_INDIVIDUAL_EXCLUDE_HIDDEN
+    CSYNC_STATUS_INDIVIDUAL_EXCLUDE_HIDDEN,
+    CSYNC_STATUS_INVALID_CHARACTERS
 };
 
 typedef enum csync_status_codes_e CSYNC_STATUS;
@@ -217,6 +218,8 @@ struct csync_vio_file_stat_s {
   enum csync_vio_file_type_e type;
 
   enum csync_vio_file_flags_e flags;
+
+  char *original_name; // only set if locale conversion fails
 };
 
 csync_vio_file_stat_t *csync_vio_file_stat_new(void);

csync/src/csync_update.c

@@ -664,6 +664,14 @@ int csync_ftw(CSYNC *ctx, const char *uri, csync_walker_fn fn,
     int flen;
     int flag;
 
+    /* Conversion error */
+    if (dirent->name == NULL && dirent->original_name) {
+        ctx->status_code = CSYNC_STATUS_INVALID_CHARACTERS;
+        ctx->error_string = dirent->original_name; // take ownership
+        dirent->original_name = NULL;
+        goto error;
+    }
+
     d_name = dirent->name;
     if (d_name == NULL) {
       ctx->status_code = CSYNC_STATUS_READDIR_ERROR;

csync/src/vio/csync_vio_local_unix.c

@@ -104,6 +104,12 @@ csync_vio_file_stat_t *csync_vio_local_readdir(csync_vio_handle_t *dhandle) {
       goto err;
   }
   file_stat->name = c_utf8_from_locale(dirent->d_name);
+  if (file_stat->name == NULL) {
+      //file_stat->original_name = c_strdup(dirent->d_name);
+      asprintf(&file_stat->original_name, "%s/%s", handle->path, dirent->d_name);
+      CSYNC_LOG(CSYNC_LOG_PRIORITY_WARN, "Invalid characters in file/directory name, please rename: \"%s\" (%s)",
+                dirent->d_name, handle->path);
+  }
 
   /* Check for availability of d_type, see manpage. */
 #if defined(_DIRENT_HAVE_D_TYPE) || defined(__APPLE__)

csync/tests/vio_tests/check_vio_ext.c

@@ -46,6 +46,7 @@ static mbchar_t wd_buffer[WD_BUFFER_SIZE];
 typedef struct {
     CSYNC *csync;
     char  *result;
+    char *ignored_dir;
 } statevar;
 
 /* remove the complete test dir */
@@ -204,8 +205,12 @@ static void traverse_dir(void **state, const char *dir, int *cnt)
 
     while( (dirent = csync_vio_readdir(csync, dh)) ) {
         assert_non_null(dirent);
-        assert_non_null(dirent->name);
+        if (dirent->original_name) {
+            sv->ignored_dir = c_strdup(dirent->original_name);
+            continue;
+        }
 
+        assert_non_null(dirent->name);
         assert_int_equal( dirent->fields & CSYNC_VIO_FILE_STAT_FIELDS_TYPE, CSYNC_VIO_FILE_STAT_FIELDS_TYPE );
 
         if( c_streq( dirent->name, "..") || c_streq( dirent->name, "." )) {
@@ -416,13 +421,46 @@ static void check_readdir_longtree(void **state)
     assert_string_equal( sv->result, result);
 }
 
+// https://github.com/owncloud/client/issues/3128 https://github.com/owncloud/client/issues/2777
+static void check_readdir_bigunicode(void **state)
+{
+    statevar *sv = (statevar*) *state;
+//    1: ? ASCII: 239 - EF
+//    2: ? ASCII: 187 - BB
+//    3: ? ASCII: 191 - BF
+//    4: ASCII: 32    - 20
+
+    char *p = 0;
+    asprintf( &p, "%s/%s", CSYNC_TEST_DIR, "goodone/" );
+    int rc = _tmkdir(p, MKDIR_MASK);
+    assert_int_equal(rc, 0);
+    SAFE_FREE(p);
+
+    const char *t1 = "goodone/ugly\xEF\xBB\xBF\x32" ".txt";
+    asprintf( &p, "%s/%s", CSYNC_TEST_DIR, t1 );
+    rc = _tmkdir(p, MKDIR_MASK);
+    SAFE_FREE(p);
+
+    assert_int_equal(rc, 0);
+
+    int files_cnt = 0;
+    traverse_dir(state, CSYNC_TEST_DIR, &files_cnt);
+    // Only the directory with good name is returned
+    assert_string_equal( sv->result,
+                         "<DIR> C:/tmp/csync_test/goodone"
+     );
+    // Bad one is recognized though.. !
+    assert_string_equal( sv->ignored_dir, CSYNC_TEST_DIR "/goodone/" "ugly\xEF\xBB\xBF\x32" ".txt");
+    assert_int_equal(files_cnt, 0);
+}
+
 int torture_run_tests(void)
 {
     const UnitTest tests[] = {
         unit_test_setup_teardown(check_readdir_shorttree, setup_testenv, teardown),
         unit_test_setup_teardown(check_readdir_with_content, setup_testenv, teardown),
         unit_test_setup_teardown(check_readdir_longtree, setup_testenv, teardown),
-
+        unit_test_setup_teardown(check_readdir_bigunicode, setup_testenv, teardown),
     };
 
     return run_tests(tests);

src/libsync/syncengine.cpp

@@ -166,6 +166,11 @@ QString SyncEngine::csyncErrorToString(CSYNC_STATUS err)
     case CSYNC_STATUS_OPENDIR_ERROR:
         errStr = tr("An error occurred while opening a directory");
         break;
+    case CSYNC_STATUS_READDIR_ERROR:
+        errStr = tr("Error while reading directory.");
+        break;
+    case CSYNC_STATUS_INVALID_CHARACTERS:
+        // Handled in callee
     default:
         errStr = tr("An internal error number %1 occurred.").arg( (int) err );
     }
@@ -543,6 +548,10 @@ void SyncEngine::handleSyncError(CSYNC *ctx, const char *state) {
         }
         errStr += QString::fromUtf8(errMsg);
     }
+    // Special handling CSYNC_STATUS_INVALID_CHARACTERS
+    if (err == CSYNC_STATUS_INVALID_CHARACTERS) {
+        errStr = tr("Invalid characters, please rename \"%1\"").arg(errMsg);
+    }
 
     // if there is csyncs url modifier in the error message, replace it.
     if( errStr.contains("ownclouds://") ) errStr.replace("ownclouds://", "https://");