On 08 Sep 2016, at 12:55 pm -0400, Andrew Gregory wrote:
On 09/07/16 at 10:28pm, Ivy Foster wrote:
On 07 Sep 2016, at 10:05 pm -0400, Andrew Gregory wrote:
This runs the risk of conflicting with another db if dbext is "" or ".bak", for example, if I have repos core and core.bak, syncing core would clobber the core.bak db.
Fair enough. I'll keep that in mind when addressing the note below.
+ if (rename(newdb, olddb) == -1) { + ret = -1; + handle->pm_errno = ALPM_ERR_DB_BACKUP; + goto cleanup; + }
This is backwards. Instead of moving the good db out of the way and hoping we can move it back if something goes wrong, we should download the new database to a temp file then move it into place if it's good.
Okay. I'll change that around tomorrow. Given the note before this one, perhaps the thing to do is to download the new db to a tmpfile named by mkstemp.
As long as we're downloading directly into the db directory, ensuring we don't conflict with another db is impossible because, now that the extension is configurable, there are no limits on db file names. Even a tmpfile could conflict if the conflicting db file hasn't been downloaded yet. I see two ways around this: place restrictions on db names/extensions or download into a separate directory and move/symlink them over once validated.
Using a separate directory with symlinks is probably a bit more fault tolerant because both the old and new versions of the db and its sig could coexist the entire time, but I doubt the small gain is worth the complexity.
I am inclined to say that we should just reject database names beginning with '_' and use that as a prefix for any temporary files. That should be sufficient to prevent any conflicts and is trivial to check in register_syncdb.
I mean, the names returned by mkstemp are probably sufficient protection. You provide a prefix and it provides six random characters to follow. Personally, I'm willing to bet that nobody's using the dbext ".db.tmp.XXXXXX", where each X is a random character...unless they're deliberately *trying* to collide. Either way, I'll have a look at it later this afternoon. iff