>From 4c785fc864e7b8cf9dddf8687736db81e9b00d08 Mon Sep 17 00:00:00 2001 From: Joseph Artsimovich Date: Wed, 23 Apr 2014 11:14:27 +0100 Subject: [PATCH 1/2] Make SoupCookieJarDB robust to database file corruption. SoupCookieJarDB stores cookies in a SQLite database. Until now, a corrupted database file resulted in warnings in a log and cookies not being stored to or loaded from the database. This change forces a database file to be deleted and re-created whenever the open operation or a query to the database fails. --- libsoup/soup-cookie-jar-db.c | 83 ++++++++++++++---- tests/Makefile.am | 1 + tests/cookiejar-db-test.c | 186 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 251 insertions(+), 19 deletions(-) create mode 100644 tests/cookiejar-db-test.c diff --git a/libsoup/soup-cookie-jar-db.c b/libsoup/soup-cookie-jar-db.c index 8f21baa..2054c5c 100644 --- a/libsoup/soup-cookie-jar-db.c +++ b/libsoup/soup-cookie-jar-db.c @@ -13,6 +13,8 @@ #include +#include +#include #include #include "soup-cookie-jar-db.h" @@ -48,6 +50,7 @@ typedef struct { G_DEFINE_TYPE (SoupCookieJarDB, soup_cookie_jar_db, SOUP_TYPE_COOKIE_JAR) static void load (SoupCookieJar *jar); +static gboolean open_db (SoupCookieJar *jar, gboolean delete_db_file); static void soup_cookie_jar_db_init (SoupCookieJarDB *db) @@ -188,30 +191,54 @@ callback (void *data, int argc, char **argv, char **colname) } static void -try_create_table (sqlite3 *db) +try_create_table (SoupCookieJar *jar, gboolean delete_db_file) { char *error = NULL; + SoupCookieJarDBPrivate *priv = SOUP_COOKIE_JAR_DB_GET_PRIVATE (jar); - if (sqlite3_exec (db, CREATE_TABLE, NULL, NULL, &error)) { - g_warning ("Failed to execute query: %s", error); - sqlite3_free (error); + if (delete_db_file) { + if (0 != open_db (jar, /*delete_db_file=*/TRUE)) { + /* open_db() prints errors by itself. */ + return; + } + } + + if (sqlite3_exec (priv->db, CREATE_TABLE, NULL, NULL, &error)) { + if (delete_db_file) { + g_warning ("Failed to execute query: %s", error); + sqlite3_free (error); + } else { + g_info ("Failed to create table. Deleting the database and retrying."); + sqlite3_free (error); + try_create_table(jar, /*delete_db_file=*/TRUE); + } } } static void -exec_query_with_try_create_table (sqlite3 *db, +exec_query_with_try_create_table (SoupCookieJar *jar, const char *sql, int (*callback)(void*,int,char**,char**), void *argument) { char *error = NULL; gboolean try_create = TRUE; + SoupCookieJarDBPrivate *priv = SOUP_COOKIE_JAR_DB_GET_PRIVATE (jar); try_exec: - if (sqlite3_exec (db, sql, callback, argument, &error)) { + /* Note that try_create_table() below can leave us with + * priv->db == NULL, so this check needs to go below try_exec. */ + if (!priv->db) { + if (0 != open_db (jar, /*delete_db_file=*/TRUE)) { + /* open_db() prints errors by itself. */ + return; + } + } + + if (sqlite3_exec (priv->db, sql, callback, argument, &error)) { if (try_create) { try_create = FALSE; - try_create_table (db); + try_create_table (jar, /*delete_db_file=*/FALSE); sqlite3_free (error); error = NULL; goto try_exec; @@ -224,23 +251,41 @@ try_exec: /* Follows sqlite3 convention; returns TRUE on error */ static gboolean -open_db (SoupCookieJar *jar) +open_db (SoupCookieJar *jar, gboolean delete_db_file) { SoupCookieJarDBPrivate *priv = SOUP_COOKIE_JAR_DB_GET_PRIVATE (jar); char *error = NULL; + if (delete_db_file) { + g_clear_pointer (&priv->db, sqlite3_close); + + if (0 != g_unlink (priv->filename)) { + g_warning("Unable to delete %s", priv->filename); + return TRUE; + } + } + if (sqlite3_open (priv->filename, &priv->db)) { - sqlite3_close (priv->db); - priv->db = NULL; - g_warning ("Can't open %s", priv->filename); - return TRUE; + g_clear_pointer (&priv->db, sqlite3_close); + if (delete_db_file) { + g_warning ("Can't open %s", priv->filename); + return TRUE; + } else { + return open_db (jar, /*delete_db_file=*/TRUE); + } } if (sqlite3_exec (priv->db, "PRAGMA synchronous = OFF; PRAGMA secure_delete = 1;", NULL, NULL, &error)) { - g_warning ("Failed to execute query: %s", error); - sqlite3_free (error); + if (delete_db_file) { + g_warning ("Failed to execute query: %s", error); + sqlite3_free (error); + return TRUE; + } else { + sqlite3_free (error); + return open_db (jar, /*delete_db_file=*/TRUE); + } } return FALSE; @@ -253,11 +298,11 @@ load (SoupCookieJar *jar) SOUP_COOKIE_JAR_DB_GET_PRIVATE (jar); if (priv->db == NULL) { - if (open_db (jar)) + if (open_db (jar, /*delete_db_file=*/FALSE)) return; } - exec_query_with_try_create_table (priv->db, QUERY_ALL, callback, jar); + exec_query_with_try_create_table (jar, QUERY_ALL, callback, jar); } static void @@ -270,7 +315,7 @@ soup_cookie_jar_db_changed (SoupCookieJar *jar, char *query; if (priv->db == NULL) { - if (open_db (jar)) + if (open_db (jar, /*delete_db_file=*/FALSE)) return; } @@ -278,7 +323,7 @@ soup_cookie_jar_db_changed (SoupCookieJar *jar, query = sqlite3_mprintf (QUERY_DELETE, old_cookie->name, old_cookie->domain); - exec_query_with_try_create_table (priv->db, query, NULL, NULL); + exec_query_with_try_create_table (jar, query, NULL, NULL); sqlite3_free (query); } @@ -294,7 +339,7 @@ soup_cookie_jar_db_changed (SoupCookieJar *jar, expires, new_cookie->secure, new_cookie->http_only); - exec_query_with_try_create_table (priv->db, query, NULL, NULL); + exec_query_with_try_create_table (jar, query, NULL, NULL); sqlite3_free (query); } } diff --git a/tests/Makefile.am b/tests/Makefile.am index a8b9d01..6a44cc3 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -18,6 +18,7 @@ test_programs = \ context-test \ continue-test \ cookies-test \ + cookiejar-db-test \ date \ forms-test \ header-parsing \ diff --git a/tests/cookiejar-db-test.c b/tests/cookiejar-db-test.c new file mode 100644 index 0000000..b1663bb --- /dev/null +++ b/tests/cookiejar-db-test.c @@ -0,0 +1,186 @@ +/* -*- Mode: C; tab-width: 8; indent-tabs-mode: t; c-basic-offset: 8 -*- */ +/* + * Copyright (C) 2014 YouView TV Ltd + */ + +#include "test-utils.h" + +#include +#include + +#include +#include +#include + +static void rmdir_recursive (const gchar *dir_path) +{ + GError* error; + GDir *dir; + const gchar *entry; + + dir = g_dir_open (dir_path, 0, &error); + if (!dir) { + g_warning ("Failed to delete directory \"%s\": %s", + dir_path, error->message); + g_error_free (error); + return; + } + + while ((entry = g_dir_read_name (dir))) { + gchar *entry_path = g_strdup_printf("%s/%s", dir_path, entry); + if (-1 == g_unlink (entry_path)) { + /* If unlinking failed, we assume it's a directory. */ + rmdir_recursive (entry_path); + } + g_free (entry_path); + } + + g_dir_close (dir); + if (-1 == g_rmdir (dir_path)) + { + g_warning ("Failed to delete directory \"%s\"", dir_path); + } +} + +static void +perform_basic_test (const gchar *db_file_path) +{ + SoupCookieJar *jar1, *jar2; + SoupCookie *cookie; + SoupURI *uri; + gchar *keyval; + + jar1 = soup_cookie_jar_db_new (db_file_path, FALSE); + g_test_queue_unref (jar1); + + /* Add a cookie to jar1. */ + cookie = soup_cookie_new ("testcookie", "value", "test.com", "/", + SOUP_COOKIE_MAX_AGE_ONE_DAY); + soup_cookie_jar_add_cookie (jar1, cookie); + + /* Open the same database file again, as jar2. */ + jar2 = soup_cookie_jar_db_new (db_file_path, FALSE); + g_test_queue_unref (jar2); + + uri = soup_uri_new("http://test.com/"); + g_test_queue_destroy ((GDestroyNotify)soup_uri_free, uri); + + /* Read the cookie back from jar2. */ + keyval = soup_cookie_jar_get_cookies (jar2, uri, TRUE); + if (0 != g_strcmp0 (keyval, "testcookie=value")) { + g_printerr ("Unexpected cookies retrieved: %s\n", keyval); + g_test_fail (); + return; + } +} + +static void +do_basic_test (void) +{ + gchar *temp_dir; + gchar *db_file_path; + + temp_dir = g_dir_make_tmp("soup-cookiejar-db-basic-test-XXXXXX", NULL); + g_test_queue_destroy ((GDestroyNotify)rmdir_recursive, temp_dir); + + db_file_path = g_strdup_printf ("%s/db.sqlite", temp_dir); + g_test_queue_free (db_file_path); + + perform_basic_test (db_file_path); +} + +static void +do_empty_file_recovery_test (void) +{ + gchar *temp_dir; + gchar *db_file_path; + + temp_dir = g_dir_make_tmp( + "soup-cookiejar-db-empty-file-recovery-XXXXXX", NULL); + g_test_queue_destroy ((GDestroyNotify)rmdir_recursive, temp_dir); + + db_file_path = g_strdup_printf ("%s/db.sqlite", temp_dir); + g_test_queue_free (db_file_path); + + /* Create an empty database file. */ + fclose(g_fopen(db_file_path, "w")); + + perform_basic_test (db_file_path); +} + +static void +do_broken_file_recovery_test (void) +{ + gchar *temp_dir; + gchar *db_file_path; + FILE *db_file; + char buf[2048]; + + temp_dir = g_dir_make_tmp( + "soup-cookiejar-db-broken-file-recovery-XXXXXX", NULL); + g_test_queue_destroy ((GDestroyNotify)rmdir_recursive, temp_dir); + + db_file_path = g_strdup_printf ("%s/db.sqlite", temp_dir); + g_test_queue_free (db_file_path); + + /* Fill buf with 0xff bytes and write that to db.sqlite file. */ + db_file = g_fopen(db_file_path, "w"); + memset(buf, 0xff, sizeof(buf)); + fwrite(buf, sizeof(buf), 1, db_file); + fclose(db_file); + + perform_basic_test (db_file_path); +} + +static void +do_broken_schema_recovery_test (void) +{ + gchar *temp_dir; + gchar *db_file_path; + sqlite3 *db = NULL; + char* errmsg = NULL; + const char *query = "CREATE TABLE moz_cookies(id INTEGER)"; + + temp_dir = g_dir_make_tmp("soup-cookiejar-db-schema-recovery-XXXXXX", NULL); + g_test_queue_destroy ((GDestroyNotify)rmdir_recursive, temp_dir); + + db_file_path = g_strdup_printf ("%s/db.sqlite", temp_dir); + g_test_queue_free (db_file_path); + + if (sqlite3_open (db_file_path, &db)) { + g_printerr ("Unable to open %s\n", db_file_path); + g_test_fail (); + return; + } + + if (sqlite3_exec(db, query, NULL, NULL, &errmsg)) { + g_printerr ("Error executing query: %s", errmsg); + sqlite3_free(errmsg); + return; + } + + sqlite3_close(db); + + perform_basic_test (db_file_path); +} + +int +main (int argc, char **argv) +{ + int ret; + + test_init (argc, argv, NULL); + + g_test_add_func ("/cookiejar-db/basic-test", do_basic_test); + g_test_add_func ("/cookiejar-db/recovery/empty-file", + do_empty_file_recovery_test); + g_test_add_func ("/cookiejar-db/recovery/broken-file", + do_broken_file_recovery_test); + g_test_add_func ("/cookiejar-db/recovery/broken-schema", + do_broken_schema_recovery_test); + + ret = g_test_run (); + + test_cleanup (); + return ret; +} -- 1.7.1