From 3bde14b5cf88272a43c0594a695fcbcd0690e957 Mon Sep 17 00:00:00 2001
From: Matthew Oliver <matt@oliver.net.au>
Date: Thu, 1 Dec 2016 09:46:53 +1100
Subject: [PATCH] Quarantine malformed database schema SQLite errors

Currently if an sqlite3.DatabaseError is thrown when caused by
a corrupted database schema, it get logged and the database is isn't
quarantined.

This patch adds the malformed database schema case to the list of
SQLite errors in possibly_quarantine that will trigger the db to be
quarantined.

Also it improved the possibly_quarantined unit test to test all existing
exceptions, and catches exceptions based on the real world except we use
in code.

Closes-Bug: #1646247

Change-Id: Id9452c88f8394a2a910c34c69361442543aa206d
---
 swift/common/db.py                           |   2 +
 test/unit/common/malformed_schema_example.db | Bin 0 -> 49152 bytes
 test/unit/common/test_db.py                  |  61 +++++++++++++------
 3 files changed, 44 insertions(+), 19 deletions(-)
 create mode 100644 test/unit/common/malformed_schema_example.db

diff --git a/swift/common/db.py b/swift/common/db.py
index 1f06694f10..fc5c057daf 100644
--- a/swift/common/db.py
+++ b/swift/common/db.py
@@ -331,6 +331,8 @@ class DatabaseBroker(object):
         """
         if 'database disk image is malformed' in str(exc_value):
             exc_hint = 'malformed'
+        elif 'malformed database schema' in str(exc_value):
+            exc_hint = 'malformed'
         elif 'file is encrypted or is not a database' in str(exc_value):
             exc_hint = 'corrupted'
         elif 'disk I/O error' in str(exc_value):
diff --git a/test/unit/common/malformed_schema_example.db b/test/unit/common/malformed_schema_example.db
new file mode 100644
index 0000000000000000000000000000000000000000..bdd46e61eff78f03faaf14f3adf3a3cc489eafd2
GIT binary patch
literal 49152
zcmeI2L2nz!700=@EYh-MIxZSUFak^w6eJ;0AQefL;ixHXEgd!z>5!|+23ip7<x1Xw
z;wt2_mJ1Yt9HcOU9(u_a=)I?&3N*KVhJKD5dPv)YJF_#pJ3}sMCAAVZ`4g73GxOe?
zncuuO^LFUw2Rn{ulLNPRWP0T7#I=N`C4NpwB9XXGf9B~=@JZ8$nV>-bYSQz#$Lop0
zzn))W!s-7onR)(aoH*)100ck)1V8`;KmY_l00ck)1V8`;UOj;;^9#)AwO4N$7C`_6
zKmY_l00ck)1V8`;KmY_l00gFoz#JP2-v6g(K`;^!009sH0T2KI5C8!X009sH0T4I`
z0{H&_9H7D^2!H?xfB*=900@8p2!H?xfB*<g83BC%KV=Jo;eY@LfB*=900@8p2!H?x
zfB*=9z&Q}W_y6Yr6(&Id1V8`;KmY_l00ck)1V8`;Kw!!U;QfEf76iiq0T2KI5C8!X
z009sH0T2KI5CDO5Ab|h>e-2P#5(Gd11V8`;KmY_l00ck)1V8`;ri=jl{$ES~J3;^9
z0|Fob0w4eaAOHd&00JNY0w4eaAn?5-Fq4=`q*DC-zne&RzjqVFgg^iUKmY_l00ck)
z1V8`;KmY_l;5#C4n$~`lP8SO4bUKyV-CHOuXj)1$t<u_Naebq>`u66^>gw9+Em1CR
zY^<!Vmo`epmwz@l*VfIFbx<fiI=ER_E3UN*n^tkHVA+;+^XB@-fn|-DeevSOLhALz
z{Osq6tN%=YvGC>9zs~>T%HQVxbou$^a`NxVU(J4g@y{0)wD;0qeMi91EeL=B2!Oy3
ziNNXheDX$iG4-tJ^_;^)yXQXo(6+qR;J9siHZR<5=w(AEMx%P~p57pe1i7PE>J6Rj
z-QQ+1^_pm&Bhu%Ney>_nYFJ}a-_h?Hq*1Om^<1S~-B~7CVW#VP#O!q3CwBWb*`{*T
zwApQwYOSd^jBNh4Voa}XA6!XhZY^r3NvGSkPn?sM7-ZY-*q+^Pb<HC&nUt7KwYIH4
zAn}sXG)PWZSSHLw{{Gx-W@$-#=$Vf?cK@S}<Jqmg{n5bgTEoR;fUSIIM<+wFIc9B{
zYUa~lX-hvOhx;Gqs$ZVXEG}w?K778w{P*nL26Itj_^5du$u;Wx)otoBhR(c}0Vjt3
zz{o$#q>?w*v#C#hEnME|TJDk4J#6)#b}ixgN?ov$A^?;&Ay3K`gZ2`Ifg+^1YkV{a
z&GBkHWDLS1v)wX1vPGIkqheHd^<4I+{Va{2T75s8S9)V=`|s&AL^+Qw(zTzg_|GFz
z^emH{M8@g%?VhKMa3UElFUauFWs#*AP{umVWM)+|?!Y^AhkQn<8zQ5$jq@2r=<H<1
zt9>gzb7>M8?gPpQCz2WCGm3%#6Edfji^)tjt37+)->?+7R|+!$5r6wqnn+HWEL+c>
zedN+p$7%b!8QH5<KiJckN5uTS?3lW!7i@u}GSXJUc1C*AR?2%j1}Q4m^0#JYGrL(W
z;h(MIATtLZwN203mNMQ}Nhw}Bt!Od>Wf_&i1%-jqG#v~sOk$vnw^dS#7wL(YVy7Pa
z{(m;{abn?(D<^Y*Nq#)@1ucAgpXax)CvV)$r~dd8aqL-c*E5~2-D~wdGsyVV%3x8%
z!KXHgUCj3ty1N^Cd7IQL(h;bN`*P|IFV4J3&gmYwq3<<ygP4}(4yX%nk($20B1)<w
zhuT6YEDwqGY?{!DW0M)P!U|RJc;K}>=g6i|j*f**rA`&+`QeDhaJ8z`vya^#8;xRE
zbQa$@#=vkt!#0N7$Cwf6vJ5*MhTJwr{n1m;?zaYgyFJ{0)S{|BHv5l-EPGTN>E6Z{
z+-a)~%uxq@(Z~yN0xj#Y**%mOo6Ab|s@jq5ne=_f6qbXcDs<8cU5jUm{_a{5I%8VT
zVq$foiB;7^-*bEBq1`%mJC5~~?!I&n4uOm|smeW@PQh|IEaq9R%g+bSI<}3ci1Y>P
z(|2D>{y3jceYW)*$9^J#pev%5#GdiXs=iOu7C*Af&EUn)a>9Tn^f{DW_DeC*NrjYn
z@dd{_I28|lcvLTJI-3C{2-pBaeaXXFz3R=hg8DG47v35@%Ff7YN1cvVj`J#}5Z@-b
z3zdmo)W|2OM4UfK0<KBlb1ge}++R@<6!U~GrBWB}+?J#z<Ka3{jjvE?)ORIEVMhR+
z9of+<2Khz3S|h^K89;K}_jTx7#F(MeP@qGNo7C@1*OHlhUi-tFemWMq8x5fYS1rv2
z=boRCsclj=5*$t}cao8bW7W^kr25dKhZ+>$<VjtSh-Hh}$iT3pP7n>9^Q^W*g>TY{
z>;yrw8Re4E1j0@t511tG9}#|D+O`kOLB}K6mwh)Q(GvM_)H@HrA!w4q$PqClDlbJ4
zt3x7^wfNvHjm)=Dibfc(vpP{H&eKN}icuf28^VY}q2z~I{`5{dnR)B2;1x!SMCtLe
z5-n1L6e<&kdB0KJEjJ#L_w|R-K+oz_lYEI!;*&S8CU3mEnELccz2X$EE)AIX8B9mL
z?i`gC7aQVwQoZ?%&2y!m{E-E3=gL&Kls%OcNUlVGxt;b3A3-}GF=7U!`ESVuuG4pZ
zYlnUDYoOSrpuN%_;+mpMqL*juBB`?;+b3=>=bwbWUH`Jve~OZBafi!N3wyy|pU-}$
zt`ENo2ofC@o|D#RRC?s%iL3iKUfKFA@%3SMYF}@Cmi&9Tv#$^9Ve<Nj8#a33Sy#?X
z^$H=*#<Jpb%aR*KcZuS*e%vrrr=))AyX|-^RKhnk>Yuf`LG%aJrqNUfE(X9;`+hD@
s%s%0T(7{<e_~}Gkq_IXG0bGl`^E2t9L;6bLoB27L4fk{Q6zAvv1<Jcrs{jB1

literal 0
HcmV?d00001

diff --git a/test/unit/common/test_db.py b/test/unit/common/test_db.py
index 45949c9007..dd580b2399 100644
--- a/test/unit/common/test_db.py
+++ b/test/unit/common/test_db.py
@@ -749,6 +749,22 @@ class TestDatabaseBroker(unittest.TestCase):
             broker = DatabaseBroker(os.path.join(dbpath, '1.db'))
             broker.db_type = 'test'
             exc = None
+            try:
+                with broker.get() as conn:
+                    conn.execute('SELECT * FROM test')
+            except Exception as err:
+                exc = err
+            self.assertEqual(
+                str(exc),
+                'Quarantined %s to %s due to malformed database' %
+                (dbpath, qpath))
+            # Test malformed schema database
+            copy(os.path.join(os.path.dirname(__file__),
+                              'malformed_schema_example.db'),
+                 os.path.join(dbpath, '1.db'))
+            broker = DatabaseBroker(os.path.join(dbpath, '1.db'))
+            broker.db_type = 'test'
+            exc = None
             try:
                 with broker.get() as conn:
                     conn.execute('SELECT * FROM test')
@@ -1214,29 +1230,36 @@ class TestDatabaseBroker(unittest.TestCase):
             message = str(e)
         self.assertEqual(message, '400 Bad Request')
 
-    def test_possibly_quarantine_disk_error(self):
+    def test_possibly_quarantine_db_errors(self):
         dbpath = os.path.join(self.testdir, 'dev', 'dbs', 'par', 'pre', 'db')
-        mkdirs(dbpath)
         qpath = os.path.join(self.testdir, 'dev', 'quarantined', 'tests', 'db')
-        broker = DatabaseBroker(os.path.join(dbpath, '1.db'))
-        broker.db_type = 'test'
+        # Data is a list of Excpetions to be raised and expected values in the
+        # log
+        data = [
+            (sqlite3.DatabaseError('database disk image is malformed'),
+             'malformed'),
+            (sqlite3.DatabaseError('malformed database schema'), 'malformed'),
+            (sqlite3.DatabaseError('file is encrypted or is not a database'),
+             'corrupted'),
+            (sqlite3.OperationalError('disk I/O error'),
+             'disk error while accessing')]
 
-        def stub():
-            raise sqlite3.OperationalError('disk I/O error')
-
-        try:
-            stub()
-        except Exception:
+        for i, (ex, hint) in enumerate(data):
+            mkdirs(dbpath)
+            broker = DatabaseBroker(os.path.join(dbpath, '%d.db' % (i)))
+            broker.db_type = 'test'
             try:
-                broker.possibly_quarantine(*sys.exc_info())
-            except Exception as exc:
-                self.assertEqual(
-                    str(exc),
-                    'Quarantined %s to %s due to disk error '
-                    'while accessing database' %
-                    (dbpath, qpath))
-            else:
-                self.fail('Expected an exception to be raised')
+                raise ex
+            except (sqlite3.DatabaseError, DatabaseConnectionError):
+                try:
+                    broker.possibly_quarantine(*sys.exc_info())
+                except Exception as exc:
+                    self.assertEqual(
+                        str(exc),
+                        'Quarantined %s to %s due to %s database' %
+                        (dbpath, qpath, hint))
+                else:
+                    self.fail('Expected an exception to be raised')
 
 if __name__ == '__main__':
     unittest.main()