From: drh <>
Date: Mon, 26 Dec 2022 17:02:15 +0000 (+0000)
Subject: Prevent the sessions rebaser from generating changesets containing UPDATE records... 
X-Git-Tag: version-3.40.1~7
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=9940e04c67b9253d5d239e24fc154c24b4b2944b;p=thirdparty%2Fsqlite.git

Prevent the sessions rebaser from generating changesets containing UPDATE records for which non-PK fields are present in the old.* but not the new.* record. Also update the changeset iterator to work around such changesets.

FossilOrigin-Name: 629dbe254346dc0b78025bb73def853bd725201244baa35cf169cf425930e184
---

diff --git a/ext/session/sqlite3session.c b/ext/session/sqlite3session.c
index a892804b49..fd06f3b4d6 100644
--- a/ext/session/sqlite3session.c
+++ b/ext/session/sqlite3session.c
@@ -3327,6 +3327,22 @@ static int sessionChangesetNextOne(
       if( p->op==SQLITE_INSERT ) p->op = SQLITE_DELETE;
       else if( p->op==SQLITE_DELETE ) p->op = SQLITE_INSERT;
     }
+
+    /* If this is an UPDATE that is part of a changeset, then check that
+    ** there are no fields in the old.* record that are not (a) PK fields,
+    ** or (b) also present in the new.* record. 
+    **
+    ** Such records are technically corrupt, but the rebaser was at one
+    ** point generating them. Under most circumstances this is benign, but
+    ** can cause spurious SQLITE_RANGE errors when applying the changeset. */
+    if( p->bPatchset==0 && p->op==SQLITE_UPDATE){
+      for(i=0; i<p->nCol; i++){
+        if( p->abPK[i]==0 && p->apValue[i+p->nCol]==0 ){
+          sqlite3ValueFree(p->apValue[i]);
+          p->apValue[i] = 0;
+        }
+      }
+    }
   }
 
   return SQLITE_ROW;
@@ -5523,7 +5539,7 @@ static void sessionAppendPartialUpdate(
         if( !pIter->abPK[i] && a1[0] ) bData = 1;
         memcpy(pOut, a1, n1);
         pOut += n1;
-      }else if( a2[0]!=0xFF ){
+      }else if( a2[0]!=0xFF && a1[0] ){
         bData = 1;
         memcpy(pOut, a2, n2);
         pOut += n2;
diff --git a/ext/session/test_session.c b/ext/session/test_session.c
index f2fc77eee3..c1feb7802c 100644
--- a/ext/session/test_session.c
+++ b/ext/session/test_session.c
@@ -98,6 +98,19 @@ int sql_exec_changeset(
 }
 /************************************************************************/
 
+
+#ifdef SQLITE_DEBUG
+static int sqlite3_test_changeset(int, void *, char **);
+static void assert_changeset_is_ok(int n, void *p){
+  int rc = 0;
+  char *z = 0;
+  rc = sqlite3_test_changeset(n, p, &z);
+  assert( z==0 );
+}
+#else
+# define assert_changeset_is_ok(n,p)
+#endif
+
 /*
 ** Tclcmd: sql_exec_changeset DB SQL
 */
@@ -127,6 +140,7 @@ static int SQLITE_TCLAPI test_sql_exec_changeset(
     return TCL_ERROR;
   }
 
+  assert_changeset_is_ok(nChangeset, pChangeset);
   Tcl_SetObjResult(interp, Tcl_NewByteArrayObj(pChangeset, nChangeset));
   sqlite3_free(pChangeset);
   return TCL_OK;
@@ -295,6 +309,7 @@ static int SQLITE_TCLAPI test_session_cmd(
         }
       }
       if( rc==SQLITE_OK ){
+        assert_changeset_is_ok(o.n, o.p);
         Tcl_SetObjResult(interp, Tcl_NewByteArrayObj(o.p, o.n)); 
       }
       sqlite3_free(o.p);
@@ -953,6 +968,7 @@ static int SQLITE_TCLAPI test_sqlite3changeset_invert(
   if( rc!=SQLITE_OK ){
     rc = test_session_error(interp, rc, 0);
   }else{
+    assert_changeset_is_ok(sOut.n, sOut.p);
     Tcl_SetObjResult(interp,Tcl_NewByteArrayObj((unsigned char*)sOut.p,sOut.n));
   }
   sqlite3_free(sOut.p);
@@ -1001,6 +1017,7 @@ static int SQLITE_TCLAPI test_sqlite3changeset_concat(
   if( rc!=SQLITE_OK ){
     rc = test_session_error(interp, rc, 0);
   }else{
+    assert_changeset_is_ok(sOut.n, sOut.p);
     Tcl_SetObjResult(interp,Tcl_NewByteArrayObj((unsigned char*)sOut.p,sOut.n));
   }
   sqlite3_free(sOut.p);
@@ -1236,6 +1253,7 @@ static int SQLITE_TCLAPI test_rebaser_cmd(
       }
 
       if( rc==SQLITE_OK ){
+        assert_changeset_is_ok(sOut.n, sOut.p);
         Tcl_SetObjResult(interp, Tcl_NewByteArrayObj(sOut.p, sOut.n));
       }
       sqlite3_free(sOut.p);
@@ -1282,6 +1300,99 @@ static int SQLITE_TCLAPI test_sqlite3rebaser_create(
   return TCL_OK;
 }
 
+/*
+**
+*/
+static int sqlite3_test_changeset(
+  int nChangeset,
+  void *pChangeset,
+  char **pzErr
+){
+  sqlite3_changeset_iter *pIter = 0;
+  char *zErr = 0;
+  int rc = SQLITE_OK;
+  int bPatch = (nChangeset>0 && ((char*)pChangeset)[0]=='P');
+
+  rc = sqlite3changeset_start(&pIter, nChangeset, pChangeset);
+  if( rc==SQLITE_OK ){
+    int rc2;
+    while( rc==SQLITE_OK && SQLITE_ROW==sqlite3changeset_next(pIter) ){
+      unsigned char *aPk = 0;
+      int nCol = 0;
+      int op = 0;
+      const char *zTab = 0;
+
+      sqlite3changeset_pk(pIter, &aPk, &nCol);
+      sqlite3changeset_op(pIter, &zTab, &nCol, &op, 0);
+
+      if( op==SQLITE_UPDATE ){
+        int iCol;
+        for(iCol=0; iCol<nCol; iCol++){
+          sqlite3_value *pNew = 0;
+          sqlite3_value *pOld = 0;
+          sqlite3changeset_new(pIter, iCol, &pNew);
+          sqlite3changeset_old(pIter, iCol, &pOld);
+
+          if( aPk[iCol] ){
+            if( pOld==0 ) rc = SQLITE_ERROR;
+          }else if( bPatch ){
+            if( pOld ) rc = SQLITE_ERROR;
+          }else{
+            if( (pOld==0)!=(pNew==0) ) rc = SQLITE_ERROR;
+          }
+
+          if( rc!=SQLITE_OK ){
+            zErr = sqlite3_mprintf(
+                "unexpected SQLITE_UPDATE (bPatch=%d pk=%d pOld=%d pNew=%d)",
+                bPatch, (int)aPk[iCol], pOld!=0, pNew!=0
+            );
+            break;
+          }
+        }
+      }
+    }
+    rc2 = sqlite3changeset_finalize(pIter);
+    if( rc==SQLITE_OK ){
+      rc = rc2;
+    }
+  }
+
+  *pzErr = zErr;
+  return rc;
+}
+
+/*
+** test_changeset CHANGESET
+*/
+static int SQLITE_TCLAPI test_changeset(
+  void * clientData,
+  Tcl_Interp *interp,
+  int objc,
+  Tcl_Obj *CONST objv[]
+){
+  void *pChangeset = 0;           /* Buffer containing changeset */
+  int nChangeset = 0;             /* Size of buffer aChangeset in bytes */
+  int rc = SQLITE_OK;
+  char *z = 0;
+
+  if( objc!=2 ){
+    Tcl_WrongNumArgs(interp, 1, objv, "CHANGESET");
+    return TCL_ERROR;
+  }
+  pChangeset = (void *)Tcl_GetByteArrayFromObj(objv[1], &nChangeset);
+
+  Tcl_ResetResult(interp);
+  rc = sqlite3_test_changeset(nChangeset, pChangeset, &z);
+  if( rc!=SQLITE_OK ){
+    char *zErr = sqlite3_mprintf("(%d) - \"%s\"", rc, z);
+    Tcl_SetObjResult(interp, Tcl_NewStringObj(zErr, -1));
+    sqlite3_free(zErr);
+  }
+  sqlite3_free(z);
+
+  return rc ? TCL_ERROR : TCL_OK;
+}
+
 /*
 ** tclcmd: sqlite3rebaser_configure OP VALUE
 */
@@ -1337,6 +1448,7 @@ int TestSession_Init(Tcl_Interp *interp){
     { "sql_exec_changeset", test_sql_exec_changeset },
     { "sqlite3rebaser_create", test_sqlite3rebaser_create },
     { "sqlite3session_config", test_sqlite3session_config },
+    { "test_changeset", test_changeset },
   };
   int i;
 
diff --git a/manifest b/manifest
index 70f6f4d5e9..ca64f632dd 100644
--- a/manifest
+++ b/manifest
@@ -1,5 +1,5 @@
-C In\sthe\sunix\sbackend,\swhen\simplementing\sthe\sdefenses\sagainst\ssmall\sfile\ndescriptors,\sdelete\sa\sfile\sjust\screated\sif\sit\swas\sopened\swith\nO_EXCL|O_CREAT\sso\sthat\sit\scan\sbe\screated\sagain\sthe\snext\stime\sthrough\sthe\nloop.\s\sFix\sfor\sthe\sproblem\sdescribed\sby\n[forum:/forumpost/699af709ab3a8ccf|forum\spost\s699af709ab3a8ccf].
-D 2022-12-26T16:54:15.416
+C Prevent\sthe\ssessions\srebaser\sfrom\sgenerating\schangesets\scontaining\sUPDATE\srecords\sfor\swhich\snon-PK\sfields\sare\spresent\sin\sthe\sold.*\sbut\snot\sthe\snew.*\srecord.\sAlso\supdate\sthe\schangeset\siterator\sto\swork\saround\ssuch\schangesets.
+D 2022-12-26T17:02:15.138
 F .fossil-settings/empty-dirs dbb81e8fc0401ac46a1491ab34a7f2c7c0452f2f06b54ebb845d024ca8283ef1
 F .fossil-settings/ignore-glob 35175cdfcf539b2318cb04a9901442804be81cd677d8b889fcc9149c21f239ea
 F LICENSE.md df5091916dbb40e6e9686186587125e1b2ff51f022cc334e886c19a0e9982724
@@ -481,9 +481,9 @@ F ext/session/sessionrebase.test ccfa716b23bd1d3b03217ee58cfd90c78d4b99f53e6a9a2
 F ext/session/sessionsize.test 6f644aff31c7f1e4871e9ff3542766e18da68fc7e587b83a347ea9820a002dd8
 F ext/session/sessionstat1.test 218d351cf9fcd6648f125a26b607b140310160184723c2666091b54450a68fb5
 F ext/session/sessionwor.test 6fd9a2256442cebde5b2284936ae9e0d54bde692d0f5fd009ecef8511f4cf3fc
-F ext/session/sqlite3session.c 1d019c5caf51936ef24c761db63552b06e0e0d951c8740bba9639b17fa0cb107
+F ext/session/sqlite3session.c b4254dd6e785cdd206c9ca7118796cf82273627fe2d4fd647597f08c2f821f96
 F ext/session/sqlite3session.h 0907de79bc13a2e3af30a6dc29acc60792a3eaf7d33d44cf52500d0f3c2b2171
-F ext/session/test_session.c f433f68a8a8c64b0f5bc74dc725078f12483301ad4ae8375205eef790274a787
+F ext/session/test_session.c 6aa7e97407987e81431b26730889db4719efebebbb6fe7502802d19d58b35798
 F ext/userauth/sqlite3userauth.h 7f3ea8c4686db8e40b0a0e7a8e0b00fac13aa7a3
 F ext/userauth/user-auth.txt e6641021a9210364665fe625d067617d03f27b04
 F ext/userauth/userauth.c 7f00cded7dcaa5d47f54539b290a43d2e59f4b1eb5f447545fa865f002fc80cb
@@ -2055,9 +2055,9 @@ F vsixtest/vsixtest.tcl 6a9a6ab600c25a91a7acc6293828957a386a8a93
 F vsixtest/vsixtest.vcxproj.data 2ed517e100c66dc455b492e1a33350c1b20fbcdc
 F vsixtest/vsixtest.vcxproj.filters 37e51ffedcdb064aad6ff33b6148725226cd608e
 F vsixtest/vsixtest_TemporaryKey.pfx e5b1b036facdb453873e7084e1cae9102ccc67a0
-P 1e9dbb5a1bcbf66acc1073e23791d1eb804af207de9bd08f4f7425cd3ff2e5c6
-Q +c0cfe0582add87981826d124a0763482f51fae4b105b5a970dd56919f1d04d60
-R 477e461604419f2275b312435ec4a762
+P 174acf3284434adb8b6c643c85c059fdf5ea5e24e95852834251ea3b3c3d1a4d
+Q +f9cd23dffba06b1982c0a5e5362dba53eba768120a2daa985b4f649d3fea1427
+R 5bea58827f297eff0160f30142e8f17d
 U drh
-Z ee9526312e4811e0bb6246da4fd9a775
+Z 9a7f72b6866d9f559aedc8301f15c5c7
 # Remove this line to create a well-formed Fossil manifest.
diff --git a/manifest.uuid b/manifest.uuid
index a8b4ce2168..3ed89d0d54 100644
--- a/manifest.uuid
+++ b/manifest.uuid
@@ -1 +1 @@
-174acf3284434adb8b6c643c85c059fdf5ea5e24e95852834251ea3b3c3d1a4d
\ No newline at end of file
+629dbe254346dc0b78025bb73def853bd725201244baa35cf169cf425930e184
\ No newline at end of file