1714874 - "Dbtest r/w succeeded in an readonly directory" fails with SQLite 3.34 and newer

Status: RESOLVED FIXED

(NSS :: Test, defect, P3)

Description

[mbakke]

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 IceCat/78.0

Steps to reproduce:

After updating SQLite from 3.32.3 to 3.35.5, dbtests.sh started failing. Further investigation revealed that SQLite 3.33.0 works fine, but 3.34.0 does not.

Actual results:

---

| test opening the database r/w in a readonly directory

dbdir selected is /tmp/guix-build-nss-3.61-1.92dcda9.drv-0/tests_results/security/localhost.1/ronlydir

ERROR: Directory "/tmp/guix-build-nss-3.61-1.92dcda9.drv-0/tests_results/security/localhost.1/ronlydir" is not writeable.
database checked is /tmp/guix-build-nss-3.61-1.92dcda9.drv-0/tests_results/security/localhost.1/ronlydir/secmod.db
ERROR: File "/tmp/guix-build-nss-3.61-1.92dcda9.drv-0/tests_results/security/localhost.1/ronlydir/secmod.db" does not exist.
database checked is /tmp/guix-build-nss-3.61-1.92dcda9.drv-0/tests_results/security/localhost.1/ronlydir/cert8.db
ERROR: File "/tmp/guix-build-nss-3.61-1.92dcda9.drv-0/tests_results/security/localhost.1/ronlydir/cert8.db" does not exist.
database checked is /tmp/guix-build-nss-3.61-1.92dcda9.drv-0/tests_results/security/localhost.1/ronlydir/key3.db
ERROR: File "/tmp/guix-build-nss-3.61-1.92dcda9.drv-0/tests_results/security/localhost.1/ronlydir/key3.db" does not exist.
dbtests.sh: #10: Dbtest r/w succeeded in a readonly directory 0 - FAILED

(excuse the 3.61 version in the output, this also happens with 3.66).

Expected results:

---

| test opening the database r/w in a readonly directory

dbdir selected is /tmp/guix-build-nss-3.66.drv-0/nss-3.66/tests_results/security/localhost.1/ronlydir

ERROR: Directory "/tmp/guix-build-nss-3.66.drv-0/nss-3.66/tests_results/security/localhost.1/ronlydir" is not writeable.
database checked is /tmp/guix-build-nss-3.66.drv-0/nss-3.66/tests_results/security/localhost.1/ronlydir/secmod.db
ERROR: File "/tmp/guix-build-nss-3.66.drv-0/nss-3.66/tests_results/security/localhost.1/ronlydir/secmod.db" does not exist.
database checked is /tmp/guix-build-nss-3.66.drv-0/nss-3.66/tests_results/security/localhost.1/ronlydir/cert8.db
ERROR: File "/tmp/guix-build-nss-3.66.drv-0/nss-3.66/tests_results/security/localhost.1/ronlydir/cert8.db" does not exist.
database checked is /tmp/guix-build-nss-3.66.drv-0/nss-3.66/tests_results/security/localhost.1/ronlydir/key3.db
ERROR: File "/tmp/guix-build-nss-3.66.drv-0/nss-3.66/tests_results/security/localhost.1/ronlydir/key3.db" does not exist.
dbtest: function failed: SEC_ERROR_READ_ONLY: security library: read-only database.
dbtests.sh: #10: Dbtest r/w didn't work in an readonly dir 46 - PASSED

Comment 1

[mbakke]

Here is a diff of the two SQLite revisions:

https://github.com/mackyle/sqlite/compare/version-3.33.0...version-3.34.0

I could not figure out how to do it in the Fossil timeline.

Comment 2

[mbakke]

Errh, the canonical git mirror is here:

https://github.com/sqlite/sqlite

I wrote a script to bisect the issue which identified https://github.com/sqlite/sqlite/commit/1ca037f41e5590aed29fbb1e61bd9175f7bc387f as the first bad commit. Diff included here for posterity (except Fossil "manifest" changes):

commit 1ca037f41e5590aed29fbb1e61bd9175f7bc387f
Author: drh <[e-mail]>
Date:   Mon Oct 12 13:24:00 2020 +0000

    Fix BEGIN IMMEDIATE and BEGIN EXCLUSIVE so that they work even if one or
    more of the database files in the connection are read-only.  Test cases
    for this are in TH3.
    
    FossilOrigin-Name: 2fa08c3963f008d4723c3f4f4496abcb6d4b575c85ba4a911a6aed5730b5948b

diff --git a/src/build.c b/src/build.c
index 907d4403d..e7efca87b 100644
--- a/src/build.c
+++ b/src/build.c
@@ -4657,7 +4657,16 @@ void sqlite3BeginTransaction(Parse *pParse, int type){
   if( !v ) return;
   if( type!=TK_DEFERRED ){
     for(i=0; i<db->nDb; i++){
-      sqlite3VdbeAddOp2(v, OP_Transaction, i, (type==TK_EXCLUSIVE)+1);
+      int eTxnType;
+      Btree *pBt = db->aDb[i].pBt;
+      if( pBt && sqlite3BtreeIsReadonly(pBt) ){
+        eTxnType = 0;  /* Read txn */
+      }else if( type==TK_EXCLUSIVE ){
+        eTxnType = 2;  /* Exclusive txn */
+      }else{
+        eTxnType = 1;  /* Write txn */
+      }
+      sqlite3VdbeAddOp2(v, OP_Transaction, i, eTxnType);
       sqlite3VdbeUsesBtree(v, i);
     }
   }
diff --git a/src/vdbe.c b/src/vdbe.c
index 676a6e29b..5823ebd92 100644
--- a/src/vdbe.c
+++ b/src/vdbe.c
@@ -3487,7 +3487,8 @@ case OP_AutoCommit: {
 ** active.
 ** If P2 is non-zero, then a write-transaction is started, or if a 
 ** read-transaction is already active, it is upgraded to a write-transaction.
-** If P2 is zero, then a read-transaction is started.
+** If P2 is zero, then a read-transaction is started.  If P2 is 2 or more
+** then an exclusive transaction is started.
 **
 ** P1 is the index of the database file on which the transaction is
 ** started.  Index 0 is the main database file and index 1 is the
@@ -3521,6 +3522,7 @@ case OP_Transaction: {
 
   assert( p->bIsReader );
   assert( p->readOnly==0 || pOp->p2==0 );
+  assert( pOp->p2>=0 && pOp->p2<=2 );
   assert( pOp->p1>=0 && pOp->p1<db->nDb );
   assert( DbMaskTest(p->btreeMask, pOp->p1) );
   if( pOp->p2 && (db->flags & SQLITE_QueryOnly)!=0 ){

Comment 3

[Benjamin Beurdouche [:beurdouche]]

Could you please check if this is still happening with NSS 3.69?

Comment 4

[Patrick McCarty]

I have been tracking this test failure as well after updating to sqlite3 3.34.0 a few months ago.

Checking nss build logs, I see that the test was still failing in nss 3.68 (with sqlite3 3.36.0), and it began passing in nss 3.69 with the message "Dbtest r/w didn't work in an readonly dir 46 - PASSED".

Comment 5

[Benjamin Beurdouche [:beurdouche]]

I believe this was fixed in https://bugzilla.mozilla.org/show_bug.cgi?id=1721476 by
https://hg.mozilla.org/projects/nss/rev/f2d34a957599067e31bc035ed5df357c620fc035