<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[287322] trunk/Source/WebKit</title>
</head>
<body>

<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt;  }
#msg dl a { font-weight: bold}
#msg dl a:link    { color:#fc3; }
#msg dl a:active  { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff  {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/287322">287322</a></dd>
<dt>Author</dt> <dd>katherine_cheney@apple.com</dd>
<dt>Date</dt> <dd>2021-12-21 10:31:16 -0800 (Tue, 21 Dec 2021)</dd>
</dl>

<h3>Log Message</h3>
<pre>Harden PCM and ITP databases against crashes
https://bugs.webkit.org/show_bug.cgi?id=234528
<rdar://problem/86741319>

Reviewed by Brent Fulgham.

This patch does two things. First, it specifies a column type for the
new destination token, keyID and signature columns. This was causing
the crashes in rdar://86347439 by comparing the defined CREATE TABLE query
with types to the existing query with no types. Second, it adds
hardening to the migration code to abort migration if one of the steps
fails. This will help prevent future crashes like rdar://86347439
by aborting a migration early if a failure occurs and not leaving the
db in a messy state.

No new tests. No behavior change, this will harden against flaky issues
that may cause a migration to fail part way through, like I/O errors.

* NetworkProcess/DatabaseUtilities.cpp:
(WebKit::DatabaseUtilities::migrateDataToNewTablesIfNecessary):
* NetworkProcess/PrivateClickMeasurement/PrivateClickMeasurementDatabase.cpp:
(WebKit::PCM::Database::addDestinationTokenColumnsIfNecessary):</pre>

<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceWebKitChangeLog">trunk/Source/WebKit/ChangeLog</a></li>
<li><a href="#trunkSourceWebKitNetworkProcessDatabaseUtilitiescpp">trunk/Source/WebKit/NetworkProcess/DatabaseUtilities.cpp</a></li>
<li><a href="#trunkSourceWebKitNetworkProcessPrivateClickMeasurementPrivateClickMeasurementDatabasecpp">trunk/Source/WebKit/NetworkProcess/PrivateClickMeasurement/PrivateClickMeasurementDatabase.cpp</a></li>
</ul>

</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceWebKitChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit/ChangeLog (287321 => 287322)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebKit/ChangeLog    2021-12-21 18:17:38 UTC (rev 287321)
+++ trunk/Source/WebKit/ChangeLog       2021-12-21 18:31:16 UTC (rev 287322)
</span><span class="lines">@@ -1,3 +1,28 @@
</span><ins>+2021-12-21  Kate Cheney  <katherine_cheney@apple.com>
+
+        Harden PCM and ITP databases against crashes
+        https://bugs.webkit.org/show_bug.cgi?id=234528
+        <rdar://problem/86741319>
+
+        Reviewed by Brent Fulgham.
+
+        This patch does two things. First, it specifies a column type for the
+        new destination token, keyID and signature columns. This was causing
+        the crashes in rdar://86347439 by comparing the defined CREATE TABLE query
+        with types to the existing query with no types. Second, it adds
+        hardening to the migration code to abort migration if one of the steps
+        fails. This will help prevent future crashes like rdar://86347439
+        by aborting a migration early if a failure occurs and not leaving the
+        db in a messy state.
+
+        No new tests. No behavior change, this will harden against flaky issues
+        that may cause a migration to fail part way through, like I/O errors.
+
+        * NetworkProcess/DatabaseUtilities.cpp:
+        (WebKit::DatabaseUtilities::migrateDataToNewTablesIfNecessary):
+        * NetworkProcess/PrivateClickMeasurement/PrivateClickMeasurementDatabase.cpp:
+        (WebKit::PCM::Database::addDestinationTokenColumnsIfNecessary):
+
</ins><span class="cx"> 2021-12-21  Wenson Hsieh  <wenson_hsieh@apple.com>
</span><span class="cx"> 
</span><span class="cx">         Add support for a UI delegate method to decide how to handle detected modal containers
</span></span></pre></div>
<a id="trunkSourceWebKitNetworkProcessDatabaseUtilitiescpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit/NetworkProcess/DatabaseUtilities.cpp (287321 => 287322)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebKit/NetworkProcess/DatabaseUtilities.cpp 2021-12-21 18:17:38 UTC (rev 287321)
+++ trunk/Source/WebKit/NetworkProcess/DatabaseUtilities.cpp    2021-12-21 18:31:16 UTC (rev 287322)
</span><span class="lines">@@ -273,15 +273,18 @@
</span><span class="cx"> 
</span><span class="cx"> void DatabaseUtilities::migrateDataToNewTablesIfNecessary()
</span><span class="cx"> {
</span><ins>+    ASSERT(!RunLoop::isMain());
</ins><span class="cx">     if (!needsUpdatedSchema())
</span><span class="cx">         return;
</span><span class="cx"> 
</span><del>-    auto transactionScope = beginTransactionIfNecessary();
</del><ins>+    WebCore::SQLiteTransaction transaction(m_database);
+    transaction.begin();
</ins><span class="cx"> 
</span><span class="cx">     for (auto& table : expectedTableAndIndexQueries().keys()) {
</span><span class="cx">         auto alterTable = m_database.prepareStatementSlow(makeString("ALTER TABLE ", table, " RENAME TO _", table));
</span><span class="cx">         if (!alterTable || alterTable->step() != SQLITE_DONE) {
</span><span class="cx">             RELEASE_LOG_ERROR(PrivateClickMeasurement, "%p - DatabaseUtilities::migrateDataToNewTablesIfNecessary failed to rename table, error message: %s", this, m_database.lastErrorMsg());
</span><ins>+            transaction.rollback();
</ins><span class="cx">             ASSERT_NOT_REACHED();
</span><span class="cx">             return;
</span><span class="cx">         }
</span><span class="lines">@@ -288,6 +291,7 @@
</span><span class="cx">     }
</span><span class="cx"> 
</span><span class="cx">     if (!createSchema()) {
</span><ins>+        transaction.rollback();
</ins><span class="cx">         ASSERT_NOT_REACHED();
</span><span class="cx">         return;
</span><span class="cx">     }
</span><span class="lines">@@ -296,6 +300,7 @@
</span><span class="cx">     for (auto& table : sortedTables()) {
</span><span class="cx">         auto migrateTableData = insertDistinctValuesInTableStatement(m_database, table);
</span><span class="cx">         if (!migrateTableData || migrateTableData->step() != SQLITE_DONE) {
</span><ins>+            transaction.rollback();
</ins><span class="cx">             RELEASE_LOG_ERROR(PrivateClickMeasurement, "%p - DatabaseUtilities::migrateDataToNewTablesIfNecessary (table %s) failed to migrate schema, error message: %s", this, table.characters(), m_database.lastErrorMsg());
</span><span class="cx">             ASSERT_NOT_REACHED();
</span><span class="cx">             return;
</span><span class="lines">@@ -306,6 +311,7 @@
</span><span class="cx">     for (auto& table : sortedTables()) {
</span><span class="cx">         auto dropTableQuery = m_database.prepareStatementSlow(makeString("DROP TABLE _", table));
</span><span class="cx">         if (!dropTableQuery || dropTableQuery->step() != SQLITE_DONE) {
</span><ins>+            transaction.rollback();
</ins><span class="cx">             RELEASE_LOG_ERROR(PrivateClickMeasurement, "%p - DatabaseUtilities::migrateDataToNewTablesIfNecessary failed to drop temporary tables, error message: %s", this, m_database.lastErrorMsg());
</span><span class="cx">             ASSERT_NOT_REACHED();
</span><span class="cx">             return;
</span><span class="lines">@@ -314,8 +320,12 @@
</span><span class="cx"> 
</span><span class="cx">     if (!createUniqueIndices()) {
</span><span class="cx">         RELEASE_LOG_ERROR(PrivateClickMeasurement, "%p - DatabaseUtilities::migrateDataToNewTablesIfNecessary failed to create unique indices, error message: %s", this, m_database.lastErrorMsg());
</span><ins>+        transaction.rollback();
</ins><span class="cx">         ASSERT_NOT_REACHED();
</span><ins>+        return;
</ins><span class="cx">     }
</span><ins>+
+    transaction.commit();
</ins><span class="cx"> }
</span><span class="cx"> 
</span><span class="cx"> } // namespace WebKit
</span></span></pre></div>
<a id="trunkSourceWebKitNetworkProcessPrivateClickMeasurementPrivateClickMeasurementDatabasecpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit/NetworkProcess/PrivateClickMeasurement/PrivateClickMeasurementDatabase.cpp (287321 => 287322)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebKit/NetworkProcess/PrivateClickMeasurement/PrivateClickMeasurementDatabase.cpp   2021-12-21 18:17:38 UTC (rev 287321)
+++ trunk/Source/WebKit/NetworkProcess/PrivateClickMeasurement/PrivateClickMeasurementDatabase.cpp      2021-12-21 18:31:16 UTC (rev 287322)
</span><span class="lines">@@ -715,9 +715,9 @@
</span><span class="cx">     String destinationKeyIDColumnName("destinationKeyID"_s);
</span><span class="cx">     auto columns = columnsForTable(attributedTableName);
</span><span class="cx">     if (!columns.size() || columns.last() != destinationKeyIDColumnName) {
</span><del>-        addMissingColumnToTable(attributedTableName, "destinationToken"_s);
-        addMissingColumnToTable(attributedTableName, "destinationSignature"_s);
-        addMissingColumnToTable(attributedTableName, destinationKeyIDColumnName);
</del><ins>+        addMissingColumnToTable(attributedTableName, "destinationToken TEXT"_s);
+        addMissingColumnToTable(attributedTableName, "destinationSignature TEXT"_s);
+        addMissingColumnToTable(attributedTableName, "destinationKeyID TEXT");
</ins><span class="cx">     }
</span><span class="cx"> }
</span><span class="cx"> 
</span></span></pre>
</div>
</div>

</body>
</html>