Importing the same bundle twice returns duplicate flag

commandline.c

@@ -1098,21 +1098,29 @@ int app_rhizome_add_file(int argc, const char *const *argv, const struct command
     if (config.debug.rhizome) DEBUGF("manifest file %s does not exist -- creating new manifest", manifestpath);
   }
   
-  if (rhizome_stat_file(m, filepath))
+  if (rhizome_stat_file(m, filepath)){
+    rhizome_manifest_free(m);
     return -1;
+  }
   
-  if (rhizome_fill_manifest(m, filepath, *authorSidHex?&authorSid:NULL, bskhex?&bsk:NULL))
+  if (rhizome_fill_manifest(m, filepath, *authorSidHex?&authorSid:NULL, bskhex?&bsk:NULL)){
+    rhizome_manifest_free(m);
     return -1;
+  }
   
   if (m->fileLength){
-    if (rhizome_add_file(m, filepath))
+    if (rhizome_add_file(m, filepath)){
+      rhizome_manifest_free(m);
       return -1;
+    }
   }
   
   rhizome_manifest *mout = NULL;
   int ret=rhizome_manifest_finalise(m,&mout);
-  if (ret<0)
+  if (ret<0){
+    rhizome_manifest_free(m);
     return -1;
+  }
   
   if (manifestpath[0] 
       && rhizome_write_manifest_file(mout, manifestpath, 0) == -1)

rhizome.c

@@ -141,18 +141,11 @@ int rhizome_bundle_import_files(rhizome_manifest *m, const char *manifest_path,
   if (status<0)
     return status;
   
-  status = rhizome_manifest_check_duplicate(m, NULL);
-  if (status<0)
+  status = rhizome_manifest_check_duplicate(m, NULL, 0);
+  if (status)
     return status;
-  
-  if (status==0){
-    if (rhizome_add_manifest(m, 1) == -1) { // ttl = 1
-      return WHY("rhizome_add_manifest() failed");
-    }
-  }else
-    INFO("Duplicate found in store");
-  
-  return status;
+    
+  return rhizome_add_manifest(m, 1);
 }
 
 /* Import a bundle from a finalised manifest struct.  The dataFileName element must give the path
@@ -165,7 +158,7 @@ int rhizome_bundle_import(rhizome_manifest *m, int ttl)
 {
   if (config.debug.rhizome)
     DEBUGF("(m=%p, ttl=%d)", m, ttl);
-  int ret = rhizome_manifest_check_duplicate(m, NULL);
+  int ret = rhizome_manifest_check_duplicate(m, NULL, 0);
   if (ret == 0) {
     ret = rhizome_add_manifest(m, ttl);
     if (ret == -1)
@@ -185,6 +178,12 @@ int rhizome_manifest_check_sanity(rhizome_manifest *m_in)
       return WHY("Manifest missing 'service' field");
   if (rhizome_manifest_get_ll(m_in, "date") == -1)
       return WHY("Manifest missing 'date' field");
+  
+  /* Get manifest version number. */
+  m_in->version = rhizome_manifest_get_ll(m_in, "version");
+  if (m_in->version==-1)
+    return WHY("Manifest must have a version number");
+  
   if (strcasecmp(service, RHIZOME_SERVICE_FILE) == 0) {
     const char *name = rhizome_manifest_get(m_in, "name", NULL, 0);
     if (name == NULL)
@@ -259,17 +258,12 @@ int rhizome_manifest_bind_id(rhizome_manifest *m_in)
 /* Check if a manifest is already stored for the same payload with the same details.
    This catches the case of "rhizome add file <filename>" on the same file more than once.
    (Debounce!) */
-int rhizome_manifest_check_duplicate(rhizome_manifest *m_in, rhizome_manifest **m_out)
+int rhizome_manifest_check_duplicate(rhizome_manifest *m_in, rhizome_manifest **m_out, int check_author)
 {
-  // if a manifest was supplied with an ID, don't bother to check for a duplicate.
-  // we only want to filter out added files with no existing manifest.
-  if (m_in->haveSecret!=NEW_BUNDLE_ID)
-    return 0;
-  
   if (config.debug.rhizome) DEBUG("Checking for duplicate");
   if (m_out) *m_out = NULL; 
   rhizome_manifest *dupm = NULL;
-  if (rhizome_find_duplicate(m_in, &dupm) == -1)
+  if (rhizome_find_duplicate(m_in, &dupm, check_author) == -1)
     return WHY("Errors encountered searching for duplicate manifest");
   if (dupm) {
     /* If the caller wants the duplicate manifest, it must be finalised, otherwise discarded. */
@@ -297,58 +291,41 @@ int rhizome_add_manifest(rhizome_manifest *m_in,int ttl)
   m_in->ttl = ttl < 0 ? 0 : ttl > 254 ? 254 : ttl;
 
   if (rhizome_manifest_check_sanity(m_in))
-    return WHY("Sanity checks on manifest failed");
+    return -1;
 
   if (m_in->fileLength){
     if (!rhizome_exists(m_in->fileHexHash))
       return WHY("File has not been imported");
   }
 
-  /* Get manifest version number. */
-  m_in->version = rhizome_manifest_get_ll(m_in, "version");
-  if (m_in->version==-1)
-    return WHY("Manifest must have a version number");
-
   /* If the manifest already has an ID */
   char id[SID_STRLEN + 1];
-  if (rhizome_manifest_get(m_in, "id", id, SID_STRLEN + 1)) {
-    str_toupper_inplace(id);
-    /* Discard the new manifest unless it is newer than the most recent known version with the same ID */
-    long long storedversion = -1;
-    switch (sqlite_exec_int64(&storedversion, "SELECT version from manifests where id='%s';", id)) {
-      case -1:
-	return WHY("Select failed");
-      case 0:
-	if (config.debug.rhizome) DEBUG("No existing manifest");
-	break;
-      case 1:
-	if (config.debug.rhizome) DEBUGF("Found existing version=%lld, new version=%lld", storedversion, m_in->version);
-	if (m_in->version < storedversion)
-	  return WHY("Newer version exists");
-	if (m_in->version == storedversion)
-	  return WHY("Same version of manifest exists, not adding");
-	break;
-      default:
-	return WHY("Select found too many rows!");
-    }
-  } else {
-    /* no manifest ID */
+  if (!rhizome_manifest_get(m_in, "id", id, SID_STRLEN + 1))
+  /* no manifest ID */
     return WHY("Manifest does not have an ID");   
+  
+  str_toupper_inplace(id);
+  /* Discard the new manifest unless it is newer than the most recent known version with the same ID */
+  long long storedversion = -1;
+  switch (sqlite_exec_int64(&storedversion, "SELECT version from manifests where id='%s';", id)) {
+    case -1:
+      return WHY("Select failed");
+    case 0:
+      if (config.debug.rhizome) DEBUG("No existing manifest");
+      break;
+    case 1:
+      if (config.debug.rhizome) DEBUGF("Found existing version=%lld, new version=%lld", storedversion, m_in->version);
+      if (m_in->version < storedversion)
+	return WHY("Newer version exists");
+      if (m_in->version == storedversion)
+	return WHY("Same version of manifest exists, not adding");
+      break;
+    default:
+      return WHY("Select found too many rows!");
   }
 
   /* Okay, it is written, and can be put directly into the rhizome database now */
-  if (rhizome_store_bundle(m_in) == -1)
-    return WHY("rhizome_store_bundle() failed.");
-
-  // This message used in tests; do not modify or remove.
-  const char *service = rhizome_manifest_get(m_in, "service", NULL, 0);
-  INFOF("RHIZOME ADD MANIFEST service=%s bid=%s version=%lld",
-      service ? service : "NULL",
-      alloca_tohex_sid(m_in->cryptoSignPublic),
-      m_in->version
-      );
-  monitor_announce_bundle(m_in);
-  return 0;
+  return rhizome_store_bundle(m_in);
 }
 
 /* Update an existing Rhizome bundle */

rhizome.h

@@ -252,7 +252,7 @@ int rhizome_fill_manifest(rhizome_manifest *m, const char *filepath, const sid_t
 
 int rhizome_manifest_verify(rhizome_manifest *m);
 int rhizome_manifest_check_sanity(rhizome_manifest *m_in);
-int rhizome_manifest_check_duplicate(rhizome_manifest *m_in,rhizome_manifest **m_out);
+int rhizome_manifest_check_duplicate(rhizome_manifest *m_in,rhizome_manifest **m_out, int check_author);
 
 int rhizome_manifest_bind_id(rhizome_manifest *m_in);
 int rhizome_manifest_finalise(rhizome_manifest *m, rhizome_manifest **mout);
@@ -304,7 +304,7 @@ int _sqlite_exec_strbuf(struct __sourceloc, strbuf sb, const char *sqlformat,...
 double rhizome_manifest_get_double(rhizome_manifest *m,char *var,double default_value);
 int rhizome_manifest_extract_signature(rhizome_manifest *m,int *ofs);
 int rhizome_update_file_priority(const char *fileid);
-int rhizome_find_duplicate(const rhizome_manifest *m, rhizome_manifest **found);
+int rhizome_find_duplicate(const rhizome_manifest *m, rhizome_manifest **found, int check_author);
 int rhizome_manifest_to_bar(rhizome_manifest *m,unsigned char *bar);
 long long rhizome_bar_version(unsigned char *bar);
 unsigned long long rhizome_bar_bidprefix_ll(unsigned char *bar);

rhizome_bundle.c

@@ -638,29 +638,30 @@ int rhizome_manifest_finalise(rhizome_manifest *m, rhizome_manifest **mout)
   IN();
   int ret=0;
   
-  if (rhizome_manifest_check_duplicate(m, mout) == 2) {
-    /* duplicate found -- verify it so that we can write it out later */
-    rhizome_manifest_verify(*mout);
-    ret=2;
-  } else {
-    *mout=m;
-    
-    /* Convert to final form for signing and writing to disk */
-    if (rhizome_manifest_pack_variables(m))
-      RETURN(WHY("Could not convert manifest to wire format"));
-    
-    /* Sign it */
-    if (rhizome_manifest_selfsign(m))
-      RETURN(WHY("Could not sign manifest"));
-    
-    /* mark manifest as finalised */
-    m->finalised=1;
-    if (rhizome_add_manifest(m, 255 /* TTL */)) {
-      rhizome_manifest_free(m);
-      RETURN(WHY("Manifest not added to Rhizome database"));
+  // if a manifest was supplied with an ID, don't bother to check for a duplicate.
+  // we only want to filter out added files with no existing manifest.
+  if (m->haveSecret==NEW_BUNDLE_ID){
+    if (rhizome_manifest_check_duplicate(m, mout, 1) == 2) {
+      /* duplicate found -- verify it so that we can write it out later */
+      rhizome_manifest_verify(*mout);
+      RETURN(2);
     }
   }
   
+  *mout=m;
+  
+  /* Convert to final form for signing and writing to disk */
+  if (rhizome_manifest_pack_variables(m))
+    RETURN(WHY("Could not convert manifest to wire format"));
+  
+  /* Sign it */
+  if (rhizome_manifest_selfsign(m))
+    RETURN(WHY("Could not sign manifest"));
+  
+  /* mark manifest as finalised */
+  m->finalised=1;
+  ret=rhizome_add_manifest(m, 255 /* TTL */);
+  
   RETURN(ret);
 }
 

rhizome_database.c

@@ -855,8 +855,17 @@ int rhizome_store_bundle(rhizome_manifest *m)
     sqlite3_finalize(stmt);
     stmt = NULL;
   }
-  if (sqlite_exec_void_retry(&retry, "COMMIT;") == SQLITE_OK)
+  if (sqlite_exec_void_retry(&retry, "COMMIT;") == SQLITE_OK){
+    // This message used in tests; do not modify or remove.
+    const char *service = rhizome_manifest_get(m, "service", NULL, 0);
+    INFOF("RHIZOME ADD MANIFEST service=%s bid=%s version=%lld",
+	  service ? service : "NULL",
+	  alloca_tohex_sid(m->cryptoSignPublic),
+	  m->version
+	  );
+    monitor_announce_bundle(m);
     return 0;
+  }
 rollback:
   if (stmt)
     sqlite3_finalize(stmt);
@@ -1066,7 +1075,7 @@ int rhizome_update_file_priority(const char *fileid)
 
    @author Andrew Bettison <andrew@servalproject.com>
  */
-int rhizome_find_duplicate(const rhizome_manifest *m, rhizome_manifest **found)
+int rhizome_find_duplicate(const rhizome_manifest *m, rhizome_manifest **found, int check_author)
 {
   // TODO, add service, name, sender & recipient to manifests table so we can simply query them.
   
@@ -1202,28 +1211,33 @@ int rhizome_find_duplicate(const rhizome_manifest *m, rhizome_manifest **found)
 	  if (blob_name && !strcmp(blob_name, name)) {
 	    if (config.debug.rhizome)
 	      strbuf_sprintf(b, " name=\"%s\"", blob_name);
-	    ret = 1;
-	  }
+	  }else
+	    ++inconsistent;
 	} else if (strcasecmp(service, RHIZOME_SERVICE_MESHMS) == 0) {
 	  const char *blob_sender = rhizome_manifest_get(blob_m, "sender", NULL, 0);
 	  const char *blob_recipient = rhizome_manifest_get(blob_m, "recipient", NULL, 0);
 	  if (blob_sender && !strcasecmp(blob_sender, sender) && blob_recipient && !strcasecmp(blob_recipient, recipient)) {
 	    if (config.debug.rhizome)
 	      strbuf_sprintf(b, " sender=%s recipient=%s", blob_sender, blob_recipient);
-	    ret = 1;
-	  }
-	}
-	if (ret == 1) {
-	  // check that we can re-author this manifest
-	  if (rhizome_extract_privatekey(blob_m, NULL)==0){
-	    *found = blob_m;
-	    DEBUGF("Found duplicate payload: service=%s%s version=%llu hexhash=%s",
-		    blob_service, strbuf_str(b), blob_m->version, blob_m->fileHexHash, q_author ? q_author : ""
-		  );
-	    break;
-	  }
+	  }else
+	    ++inconsistent;
 	}
       }
+      
+      if ((!inconsistent) && check_author) {
+	// check that we can re-author this manifest
+	if (rhizome_extract_privatekey(blob_m, NULL))
+	  ++inconsistent;
+      }
+      
+      if (!inconsistent) {
+	*found = blob_m;
+	DEBUGF("Found duplicate payload: service=%s%s version=%llu hexhash=%s",
+		blob_service, strbuf_str(b), blob_m->version, blob_m->fileHexHash, q_author ? q_author : ""
+	      );
+	ret = 1;
+	break;
+      }
     }
     if (blob_m)
       rhizome_manifest_free(blob_m);

rhizome_store.c

@@ -309,6 +309,8 @@ int rhizome_import_file(rhizome_manifest *m, const char *filepath)
 
 int rhizome_stat_file(rhizome_manifest *m, const char *filepath)
 {
+  long long existing = rhizome_manifest_get_ll(m, "filesize");
+  
   m->fileLength = 0;
   if (filepath[0]) {
     struct stat stat;
@@ -317,6 +319,14 @@ int rhizome_stat_file(rhizome_manifest *m, const char *filepath)
     m->fileLength = stat.st_size;
   }
   
+  // fail if the file is shorter than specified by the manifest
+  if (existing > m->fileLength)
+    return WHY("Manifest length is longer than the file");
+  
+  // if the file is longer than specified by the manifest, ignore the end.
+  if (existing!=-1 && existing < m->fileLength)
+    m->fileLength = existing;
+  
   rhizome_manifest_set_ll(m, "filesize", m->fileLength);
   
   if (m->fileLength == 0){

tests/rhizomeops

@@ -762,6 +762,8 @@ setup_ImportForeignBundle() {
 test_ImportForeignBundle() {
    executeOk_servald rhizome import bundle fileA fileA.manifest
    assert_stdout_import_bundle fileA
+   execute --exit-status=2 --stdout --stderr $servald rhizome import bundle fileA fileA.manifest
+   assert_stdout_import_bundle fileA
    executeOk_servald rhizome list ''
    assert_rhizome_list --fromhere=0 fileA
 }