From 8cf58297d9743bdbfdddb605cc9cefe0c6273ee5 Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Thu, 21 Jan 2010 18:45:21 +0100 Subject: TicketUploader and FileTransfer plugins: fixes after a round of testing for one, FileTransfer now would not use current dir as a storage for temp files. Signed-off-by: Denys Vlasenko --- lib/Plugins/FileTransfer.cpp | 171 +++++++++++++++++++++++-------------------- 1 file changed, 91 insertions(+), 80 deletions(-) (limited to 'lib/Plugins/FileTransfer.cpp') diff --git a/lib/Plugins/FileTransfer.cpp b/lib/Plugins/FileTransfer.cpp index e28fbbee..4d317c8d 100644 --- a/lib/Plugins/FileTransfer.cpp +++ b/lib/Plugins/FileTransfer.cpp @@ -22,9 +22,6 @@ #ifndef _GNU_SOURCE # define _GNU_SOURCE #endif -#include -#include -#include #include #include #include @@ -60,22 +57,21 @@ void CFileTransfer::SendFile(const char *pURL, const char *pFilename) update_client(_("Sending archive %s to %s"), pFilename, pURL); - string wholeURL = concat_path_file(pURL, pFilename); + string wholeURL = concat_path_file(pURL, strrchr(pFilename, '/') ? : pFilename); int count = m_nRetryCount; while (1) { - FILE *f; - struct stat buf; - CURL *curl; - - f = fopen(pFilename, "r"); + FILE *f = fopen(pFilename, "r"); if (!f) { throw CABRTException(EXCEP_PLUGIN, "Can't open archive file '%s'", pFilename); } + + struct stat buf; fstat(fileno(f), &buf); /* never fails */ - curl = xcurl_easy_init(); + + CURL *curl = xcurl_easy_init(); /* enable uploading */ curl_easy_setopt(curl, CURLOPT_UPLOAD, 1L); /* specify target */ @@ -83,8 +79,10 @@ void CFileTransfer::SendFile(const char *pURL, const char *pFilename) /* FILE handle: passed to the default callback, it will fread() it */ curl_easy_setopt(curl, CURLOPT_READDATA, f); curl_easy_setopt(curl, CURLOPT_INFILESIZE_LARGE, (curl_off_t)buf.st_size); + /* everything is done here; result 0 means success */ int result = curl_easy_perform(curl); + curl_easy_cleanup(curl); fclose(f); if (result == 0 || --count <= 0) @@ -95,11 +93,9 @@ void CFileTransfer::SendFile(const char *pURL, const char *pFilename) } /* -walks through the directory and applies a function with one -parameter "something" to each filename, -now used in create_zip, but can be useful for some future -archivers as well -*/ + * Walks through the directory and applies a function with one + * parameter "something" to each filename. + */ static void traverse_directory(const char *directory, void *something, void (*func)(void *, const char *)) { @@ -122,20 +118,26 @@ static void traverse_directory(const char *directory, void *something, closedir(dp); } -static void add_to_zip(void * z, const char * filename) +static void add_to_zip(void *z, const char *filename) { struct zip_source *s; - s = zip_source_file( (struct zip *)z, filename, 0, 0); - zip_add( (struct zip *)z, filename, s); + s = zip_source_file((struct zip *)z, filename, /*start:*/ 0, /*len:*/ 0); + if (s) + { + if (zip_add((struct zip *)z, filename, s) == -1) + { + zip_source_free(s); + } + /* else: don't call zip_source_free(s), successful zip_add consumes it */ + } } - -static void create_zip(const char * archive_name, const char * directory) +static void create_zip(const char *archive_name, const char *directory) { - struct zip * z; + struct zip *z; - z = zip_open(archive_name, ZIP_CREATE, NULL); + z = zip_open(archive_name, ZIP_CREATE, /*errorp:*/ NULL); if (z == NULL) { return; @@ -144,7 +146,7 @@ static void create_zip(const char * archive_name, const char * directory) zip_close(z); } -static void create_tar(const char * archive_name, const char * directory) +static void create_tar(const char *archive_name, const char *directory) { TAR *tar; @@ -156,89 +158,86 @@ static void create_tar(const char * archive_name, const char * directory) tar_close(tar); } -static void create_targz(const char * archive_name, const char * directory) +static void create_targz(const char *archive_name, const char *directory) { - char * name_without_gz; - char buf[BUFSIZ]; - FILE * f; - ssize_t bytesRead; - gzFile gz; - - name_without_gz = xstrdup(archive_name); + char *name_without_gz = xstrdup(archive_name); strrchr(name_without_gz, '.')[0] = '\0'; + create_tar(name_without_gz, directory); - f = fopen(name_without_gz, "r"); - if (f == NULL) + int fd = open(name_without_gz, O_RDONLY); + if (fd < 0) { -//TODO: we leak uncompressed tar file on disk?? + remove(name_without_gz); free(name_without_gz); return; } - gz = gzopen(archive_name, "w"); + + gzFile gz = gzopen(archive_name, "w"); if (gz == NULL) { - fclose(f); + close(fd); + remove(name_without_gz); free(name_without_gz); return; } - while ((bytesRead = fread(buf, 1, BUFSIZ, f)) > 0) + char buf[BUFSIZ]; + ssize_t bytesRead; + while ((bytesRead = full_read(fd, buf, BUFSIZ)) > 0) { - gzwrite(gz, buf, bytesRead); + gzwrite(gz, buf, bytesRead); // TODO: check that return value == bytesRead } + gzclose(gz); - fclose(f); + close(fd); remove(name_without_gz); free(name_without_gz); } static void create_tarbz2(const char * archive_name, const char * directory) { - char * name_without_bz2; - char buf[BUFSIZ]; - FILE * f; - ssize_t bytesRead; - int tarFD; - int bzError; - BZFILE * bz; -#define BLOCK_MULTIPLIER 7 - - name_without_bz2 = xstrdup(archive_name); + char *name_without_bz2 = xstrdup(archive_name); strrchr(name_without_bz2, '.')[0] = '\0'; + create_tar(name_without_bz2, directory); - tarFD = open(name_without_bz2, O_RDONLY); + int tarFD = open(name_without_bz2, O_RDONLY); if (tarFD == -1) { + remove(name_without_bz2); free(name_without_bz2); return; } - f = fopen(archive_name, "w"); + FILE *f = fopen(archive_name, "w"); if (f == NULL) { -//TODO: we leak uncompressed tar file on disk?? close(tarFD); + remove(name_without_bz2); free(name_without_bz2); return; } - bz = BZ2_bzWriteOpen(&bzError, f, BLOCK_MULTIPLIER, 0, 0); + int bzError; + BZFILE *bz = BZ2_bzWriteOpen(&bzError, f, /*BLOCK_MULTIPLIER:*/ 7, 0, 0); if (bz == NULL) { - close(tarFD); fclose(f); + close(tarFD); + remove(name_without_bz2); free(name_without_bz2); return; } + char buf[BUFSIZ]; + ssize_t bytesRead; while ((bytesRead = read(tarFD, buf, BUFSIZ)) > 0) { BZ2_bzWrite(&bzError, bz, buf, bytesRead); } - BZ2_bzWriteClose(&bzError, bz, 0, NULL, NULL); - close(tarFD); + BZ2_bzWriteClose(&bzError, bz, 0, NULL, NULL); fclose(f); + close(tarFD); remove(name_without_bz2); free(name_without_bz2); } @@ -289,23 +288,31 @@ void CFileTransfer::Run(const char *pActionDir, const char *pArgs) { update_client(_("File Transfer: Creating a report...")); + if (strcmp(pArgs, "store") == 0) + { + /* Remember pActiveDir for later sending */ + FILE *dirlist = fopen(FILETRANSFER_DIRLIST, "a"); + fprintf(dirlist, "%s\n", pActionDir); + fclose(dirlist); + VERB3 log("Remembered '%s' for future file transfer", pActionDir); + return; + } + char hostname[HBLEN]; gethostname(hostname, HBLEN-1); hostname[HBLEN-1] = '\0'; - fstream dirlist; - if (strcmp(pArgs, "store") == 0) + char tmpdir_name[] = "/tmp/abrtuploadXXXXXX"; + /* mkdtemp does mkdir(xxx, 0700), should be safe (is it?) */ + if (mkdtemp(tmpdir_name) == NULL) { - /* store pActiveDir for later sending */ - dirlist.open(FILETRANSFER_DIRLIST, fstream::out | fstream::app); - dirlist << pActionDir << endl; - dirlist.close(); + throw CABRTException(EXCEP_PLUGIN, "Can't mkdir a temporary directory in /tmp"); } - else if (strcmp(pArgs, "one") == 0) + + if (strcmp(pArgs, "one") == 0) { - /* just send one archive */ -//TODO: where are we creating it??!! In cwd, which may well be / ??!!! - string archivename = ssprintf("%s-%s%s", hostname, DirBase(pActionDir).c_str(), m_sArchiveType.c_str()); + /* Just send one archive */ + string archivename = ssprintf("%s/%s-%s%s", tmpdir_name, hostname, DirBase(pActionDir).c_str(), m_sArchiveType.c_str()); try { CreateArchive(archivename.c_str(), pActionDir); @@ -319,35 +326,43 @@ void CFileTransfer::Run(const char *pActionDir, const char *pArgs) } else { - dirlist.open(FILETRANSFER_DIRLIST, fstream::in); - if (dirlist.fail()) + /* Tar up and send all remebered directories */ + FILE *dirlist = fopen(FILETRANSFER_DIRLIST, "r"); + if (!dirlist) { - /* this means there are no reports to send (no crashes, hurray) - which is perfectly OK */ - return; + /* not an error */ + VERB3 log("No saved crashes to transfer"); + goto del_tmp_dir; } - string dirname; - while (getline(dirlist, dirname), dirlist.good()) + char dirname[PATH_MAX]; + while (fgets(dirname, sizeof(dirname), dirlist) != NULL) { - string archivename = ssprintf("%s-%s%s", hostname, DirBase(dirname.c_str()).c_str(), m_sArchiveType.c_str()); + strchrnul(dirname, '\n')[0] = '\0'; + string archivename = ssprintf("%s/%s-%s%s", tmpdir_name, hostname, DirBase(dirname).c_str(), m_sArchiveType.c_str()); try { - CreateArchive(archivename.c_str(), dirname.c_str()); + VERB3 log("Creating archive '%s' of dir '%s'", archivename.c_str(), dirname); + CreateArchive(archivename.c_str(), dirname); + VERB3 log("Sending archive to '%s'", m_sURL.c_str()); SendFile(m_sURL.c_str(), archivename.c_str()); } catch (CABRTException& e) { error_msg(_("Can't create and send an archive %s"), e.what()); } + VERB3 log("Deleting archive '%s'", archivename.c_str()); unlink(archivename.c_str()); } - dirlist.close(); + fclose(dirlist); /* all the files we're able to send should be sent now, starting over with clean table */ unlink(FILETRANSFER_DIRLIST); } + + del_tmp_dir: + rmdir(tmpdir_name); } void CFileTransfer::SetSettings(const map_plugin_settings_t& pSettings) @@ -361,10 +376,6 @@ void CFileTransfer::SetSettings(const map_plugin_settings_t& pSettings) { m_sURL = it->second; } - else - { - error_msg(_("FileTransfer: URL not specified")); - } it = pSettings.find("RetryCount"); if (it != end) -- cgit