DatabaseCorruptionHandler causes NPE

it is trying to get attachedDb list (by executing a pragma) after closing
the database.
also added unittests.

Change-Id: I7dce665ec7354402cdef6fbe055455f5798e123c

2 files changed, 105 insertions, 4 deletions

diff --git a/core/java/android/database/DefaultDatabaseErrorHandler.java b/core/java/android/database/DefaultDatabaseErrorHandler.java
index 98aa54a..0bad37a 100644
--- a/core/java/android/database/DefaultDatabaseErrorHandler.java
+++ b/core/java/android/database/DefaultDatabaseErrorHandler.java
@@ -16,6 +16,7 @@
 package android.database;
 
 import java.io.File;
+import java.util.ArrayList;
 
 import android.database.sqlite.SQLiteDatabase;
 import android.database.sqlite.SQLiteException;
@@ -46,7 +47,7 @@ public final class DefaultDatabaseErrorHandler implements DatabaseErrorHandler {
             // make the application crash on database open operation. To avoid this problem,
             // the application should provide its own {@link DatabaseErrorHandler} impl class
             // to delete ALL files of the database (including the attached databases).
-            if (!dbObj.getPath().equalsIgnoreCase(":memory")) {
+            if (!dbObj.getPath().equalsIgnoreCase(":memory:")) {
                 // not memory database.
                 try {
                     new File(dbObj.getPath()).delete();
@@ -57,8 +58,11 @@ public final class DefaultDatabaseErrorHandler implements DatabaseErrorHandler {
             return;
         }
 
+        ArrayList<Pair<String, String>> attachedDbs = null;
         try {
             // Close the database, which will cause subsequent operations to fail.
+            // before that, get the attached database list first.
+            attachedDbs = dbObj.getAttachedDbs();
             try {
                 dbObj.close();
             } catch (SQLiteException e) {
@@ -66,10 +70,13 @@ public final class DefaultDatabaseErrorHandler implements DatabaseErrorHandler {
             }
         } finally {
             // Delete all files of this corrupt database and/or attached databases
-            for (Pair<String, String> p : dbObj.getAttachedDbs()) {
-                Log.e(TAG, "deleting the database file: " + p.second);
-                if (!p.second.equalsIgnoreCase(":memory:")) {
+            if (attachedDbs != null) {
+                for (Pair<String, String> p : attachedDbs) {
                     // delete file if it is a non-memory database file
+                    if (p.second.equalsIgnoreCase(":memory:") || p.second.trim().length() == 0) {
+                        continue;
+                    }
+                    Log.e(TAG, "deleting the database file: " + p.second);
                     try {
                         new File(p.second).delete();
                     } catch (Exception e) {
diff --git a/core/tests/coretests/src/android/database/DatabaseGeneralTest.java b/core/tests/coretests/src/android/database/DatabaseGeneralTest.java
index 350d600..d9a234a 100644
--- a/core/tests/coretests/src/android/database/DatabaseGeneralTest.java
+++ b/core/tests/coretests/src/android/database/DatabaseGeneralTest.java
@@ -26,9 +26,11 @@ import android.os.Handler;
 import android.os.Parcel;
 import android.test.AndroidTestCase;
 import android.test.PerformanceTestCase;
+import android.test.suitebuilder.annotation.LargeTest;
 import android.test.suitebuilder.annotation.MediumTest;
 import android.test.suitebuilder.annotation.SmallTest;
 import android.util.Log;
+import android.util.Pair;
 
 import junit.framework.Assert;
 
@@ -1138,4 +1140,96 @@ public class DatabaseGeneralTest extends AndroidTestCase implements PerformanceT
             assertTrue(e.getMessage().contains("cannot set cacheSize to a value less than"));
         }
     }
+
+    @LargeTest
+    public void testDefaultDatabaseErrorHandler() {
+        DefaultDatabaseErrorHandler errorHandler = new DefaultDatabaseErrorHandler();
+
+        // close the database. and call corruption handler.
+        // it should delete the database file.
+        File dbfile = new File(mDatabase.getPath());
+        mDatabase.close();
+        assertFalse(mDatabase.isOpen());
+        assertTrue(dbfile.exists());
+        try {
+            errorHandler.onCorruption(mDatabase);
+            assertFalse(dbfile.exists());
+        } catch (Exception e) {
+            fail("unexpected");
+        }
+
+        // create an in-memory database. and corruption handler shouldn't try to delete it
+        SQLiteDatabase memoryDb = SQLiteDatabase.openOrCreateDatabase(":memory:", null);
+        assertNotNull(memoryDb);
+        memoryDb.close();
+        assertFalse(memoryDb.isOpen());
+        try {
+            errorHandler.onCorruption(memoryDb);
+        } catch (Exception e) {
+            fail("unexpected");
+        }
+
+        // create a database, keep it open, call corruption handler. database file should be deleted
+        SQLiteDatabase dbObj = SQLiteDatabase.openOrCreateDatabase(mDatabase.getPath(), null);
+        assertTrue(dbfile.exists());
+        assertNotNull(dbObj);
+        assertTrue(dbObj.isOpen());
+        try {
+            errorHandler.onCorruption(dbObj);
+            assertFalse(dbfile.exists());
+        } catch (Exception e) {
+            fail("unexpected");
+        }
+
+        // create a database, attach 2 more databases to it
+        //    attached database # 1: ":memory:"
+        //    attached database # 2: mDatabase.getPath() + "1";
+        // call corruption handler. database files including the one for attached database # 2
+        // should be deleted
+        String attachedDb1File = mDatabase.getPath() + "1";
+        dbObj = SQLiteDatabase.openOrCreateDatabase(mDatabase.getPath(), null);
+        dbObj.execSQL("ATTACH DATABASE ':memory:' as memoryDb");
+        dbObj.execSQL("ATTACH DATABASE '" +  attachedDb1File + "' as attachedDb1");
+        assertTrue(dbfile.exists());
+        assertTrue(new File(attachedDb1File).exists());
+        assertNotNull(dbObj);
+        assertTrue(dbObj.isOpen());
+        ArrayList<Pair<String, String>> attachedDbs = dbObj.getAttachedDbs();
+        try {
+            errorHandler.onCorruption(dbObj);
+            assertFalse(dbfile.exists());
+            assertFalse(new File(attachedDb1File).exists());
+        } catch (Exception e) {
+            fail("unexpected");
+        }
+
+        // same as above, except this is a bit of stress testing. attach 5 database files
+        // and make sure they are all removed.
+        int N = 5;
+        ArrayList<String> attachedDbFiles = new ArrayList<String>(N);
+        for (int i = 0; i < N; i++) {
+            attachedDbFiles.add(mDatabase.getPath() + i);
+        }
+        dbObj = SQLiteDatabase.openOrCreateDatabase(mDatabase.getPath(), null);
+        dbObj.execSQL("ATTACH DATABASE ':memory:' as memoryDb");
+        for (int i = 0; i < N; i++) {
+            dbObj.execSQL("ATTACH DATABASE '" +  attachedDbFiles.get(i) + "' as attachedDb" + i);
+        }
+        assertTrue(dbfile.exists());
+        for (int i = 0; i < N; i++) {
+            assertTrue(new File(attachedDbFiles.get(i)).exists());
+        }
+        assertNotNull(dbObj);
+        assertTrue(dbObj.isOpen());
+        attachedDbs = dbObj.getAttachedDbs();
+        try {
+            errorHandler.onCorruption(dbObj);
+            assertFalse(dbfile.exists());
+            for (int i = 0; i < N; i++) {
+                assertFalse(new File(attachedDbFiles.get(i)).exists());
+            }
+        } catch (Exception e) {
+            fail("unexpected");
+        }
+    }
 }