Commit 394a5057 authored by Harveer Singh's avatar Harveer Singh Committed by dsmith@mozilla.com
Browse files

Bug 2047853: Persisting response credential type in cache entries table. a=diannaS DONTBUILD 

This re-lands D311986, backed out from esr153 in b81b4a1b for crashing
every dom/cache xpcshell migration test, and squashes in the Bug 2009295
schema migration it depends on.

Root cause: D311986 bumps kLatestSchemaVersion from 29 to 31 but registers
only MigrateFrom30To31. Schema version 30 comes from Bug 2009295 (D278397),
which adds the response_headers(entry_id, name) index and was never uplifted
to esr153. With nothing registered for version 29, Migrate() walked a database
up to 29, found no matching entry in sMigrationList, and looped without the
version advancing:

  Assertion failure: currentVersion > lastVersion, at dom/cache/DBSchema.cpp:2659

On the release channel MOZ_DIAGNOSTIC_ASSERT is compiled out, so this is an
infinite loop on the Cache IO thread rather than a crash. Every existing
esr153 profile is at schema 29, so it would have hung Cache API and Service
Worker storage for upgrading users, not just broken tests.

Squashed commits:

  Bug 2009295 (D278397), by Marcelo Camargo - closes the gap at version 29.
  Uplifted whole rather than schema-only so that versions 30 and 31 mean the
  same thing on esr153 as on mozilla-central. Profiles are shared across
  channels, and a partial uplift would leave an esr153 database at version 31
  permanently missing the index, with no migration left to ever add it.

  Bug 2047853 (D311986) - unchanged from the original uplift.

With both applied, dom/cache/DBSchema.cpp matches mozilla-central except for
the unrelated nsID::Parse signature change from Bug 2054573.

Original Revision: https://phabricator.services.mozilla.com/D278397

Original Revision: https://phabricator.services.mozilla.com/D311986

Differential Revision: https://phabricator.services.mozilla.com/D317102
parent 28124d78

dom/cache/DBSchema.cpp

+94 −28
Original line number Diff line number Diff line
@@ -79,7 +79,7 @@ const int32_t kHackyDowngradeSchemaVersion = 25; 
const int32_t kHackyPaddingSizePresentVersion = 27;

//

// Update this whenever the DB schema is changed.

const int32_t kLatestSchemaVersion = 29;

const int32_t kLatestSchemaVersion = 31;

// ---------

// The following constants define the SQL schema.  These are defined in the

// same order the SQL should be executed in CreateOrMigrateSchema().  They are
@@ -148,7 +148,8 @@ const char kTableEntries[] = 
    "request_url_fragment TEXT NOT NULL, "

    "response_padding_size INTEGER NULL, "

    "request_body_disk_size INTEGER NULL, "

    "response_body_disk_size INTEGER NULL "

    "response_body_disk_size INTEGER NULL, "

    "response_credentials INTEGER NOT NULL "

    // New columns must be added at the end of table to migrate and

    // validate properly.

    ")";
@@ -185,6 +186,11 @@ const char kIndexResponseHeadersName[] = 
    "CREATE INDEX response_headers_name_index "

    "ON response_headers (name)";



// Index for fast lookup of headers by entry_id, particularly for vary checks

const char kIndexResponseHeadersEntryId[] =

    "CREATE INDEX response_headers_entry_id_index "

    "ON response_headers (entry_id, name)";



const char kTableResponseUrlList[] =

    "CREATE TABLE response_url_list ("

    "url TEXT NOT NULL, "
@@ -544,6 +550,8 @@ nsresult CreateOrMigrateSchema(nsIFile& aDBDir, mozIStorageConnection& aConn) { 
        aConn.ExecuteSimpleSQL(nsLiteralCString(kTableResponseHeaders))));

    QM_TRY(MOZ_TO_RESULT(

        aConn.ExecuteSimpleSQL(nsLiteralCString(kIndexResponseHeadersName))));

    QM_TRY(MOZ_TO_RESULT(aConn.ExecuteSimpleSQL(

        nsLiteralCString(kIndexResponseHeadersEntryId))));

    QM_TRY(MOZ_TO_RESULT(

        aConn.ExecuteSimpleSQL(nsLiteralCString(kTableResponseUrlList))));

    QM_TRY(
@@ -1062,12 +1070,18 @@ Result<EntryIds, nsresult> QueryCache(mozIStorageConnection& aConn, 
    return Result<EntryIds, nsresult>{std::in_place};

  }



  // We use truncated SHA-1 hashes (64 bits) in the WHERE clause for O(log n)

  // index lookup performance. The full URL strings are retrieved in the SELECT

  // and verified post-query to handle the extremely rare case of hash

  // collisions. This approach avoids O(n) string comparisons in the SQL query

  // while maintaining correctness.

  //

  // We use a simplified query that avoids the expensive LEFT JOIN, GROUP BY,

  // and COUNT() operations. The vary count check is done separately only when

  // needed, avoiding ~300μs overhead per query.

  nsAutoCString query(

      "SELECT id, COUNT(response_headers.name) AS vary_count, response_type "

      "SELECT id, response_type, request_url_no_query, request_url_query "

      "FROM entries "

      "LEFT OUTER JOIN response_headers ON "

      "entries.id=response_headers.entry_id "

      "AND response_headers.name='vary' COLLATE NOCASE "

      "WHERE entries.cache_id=:cache_id "

      "AND entries.request_url_no_query_hash=:url_no_query_hash ");
@@ -1075,13 +1089,7 @@ Result<EntryIds, nsresult> QueryCache(mozIStorageConnection& aConn, 
    query.AppendLiteral("AND entries.request_url_query_hash=:url_query_hash ");

  }



  query.AppendLiteral("AND entries.request_url_no_query=:url_no_query ");



  if (!aParams.ignoreSearch()) {

    query.AppendLiteral("AND entries.request_url_query=:url_query ");

  }



  query.AppendLiteral("GROUP BY entries.id ORDER BY entries.id;");

  query.AppendLiteral("ORDER BY entries.id;");



  QM_TRY_INSPECT(const auto& state, MOZ_TO_RESULT_INVOKE_MEMBER_TYPED(

                                        nsCOMPtr<mozIStorageStatement>, aConn,
@@ -1108,14 +1116,6 @@ Result<EntryIds, nsresult> QueryCache(mozIStorageConnection& aConn, 
        state->BindUTF8StringAsBlobByName("url_query_hash"_ns, urlQueryHash)));

  }



  QM_TRY(MOZ_TO_RESULT(state->BindUTF8StringByName(

      "url_no_query"_ns, aRequest.urlWithoutQuery())));



  if (!aParams.ignoreSearch()) {

    QM_TRY(MOZ_TO_RESULT(

        state->BindUTF8StringByName("url_query"_ns, aRequest.urlQuery())));

  }



  EntryIds entryIdList;



  QM_TRY(CollectWhile(
@@ -1130,17 +1130,35 @@ Result<EntryIds, nsresult> QueryCache(mozIStorageConnection& aConn, 
        QM_TRY_INSPECT(const EntryId& entryId,

                       MOZ_TO_RESULT_INVOKE_MEMBER(state, GetInt32, 0));



        QM_TRY_INSPECT(const int32_t& varyCount,

        QM_TRY_INSPECT(const int32_t& responseType,

                       MOZ_TO_RESULT_INVOKE_MEMBER(state, GetInt32, 1));



        QM_TRY_INSPECT(const int32_t& responseType,

                       MOZ_TO_RESULT_INVOKE_MEMBER(state, GetInt32, 2));

        QM_TRY_INSPECT(const auto& cachedUrlNoQuery,

                       MOZ_TO_RESULT_INVOKE_MEMBER_TYPED(nsAutoCString, state,

                                                         GetUTF8String, 2));



        QM_TRY_INSPECT(const auto& cachedUrlQuery,

                       MOZ_TO_RESULT_INVOKE_MEMBER_TYPED(nsAutoCString, state,

                                                         GetUTF8String, 3));



        // Verify URLs match to handle potential hash collisions. We use

        // truncated SHA-1 (64 bits) for indexing performance, but verify the

        // full URL post-query to ensure correctness. Hash collisions are

        // extremely rare (~1 in 92 million for 20k entries) but possible.

        if (!aRequest.urlWithoutQuery().Equals(cachedUrlNoQuery)) {

          return Ok{};

        }



        if (!aParams.ignoreSearch() &&

            !aRequest.urlQuery().Equals(cachedUrlQuery)) {

          return Ok{};

        }



        auto ignoreVary =

            aParams.ignoreVary() ||

            responseType == static_cast<int>(ResponseType::Opaque);



        if (!ignoreVary && varyCount > 0) {

        if (!ignoreVary) {

          QM_TRY_INSPECT(const bool& matchedByVary,

                         MatchByVaryHeader(aConn, aRequest, entryId));

          if (!matchedByVary) {
@@ -1182,8 +1200,10 @@ Result<bool, nsresult> MatchByVaryHeader(mozIStorageConnection& aConn, 
                })));

      }()));



  // Should not have called this function if this was not the case

  MOZ_DIAGNOSTIC_ASSERT(!varyValues.IsEmpty());

  // If there are no vary headers, the request matches

  if (varyValues.IsEmpty()) {

    return true;

  }



  QM_TRY_INSPECT(const auto& state,

                 MOZ_TO_RESULT_INVOKE_MEMBER_TYPED(
@@ -1726,6 +1746,7 @@ nsresult InsertEntry(mozIStorageConnection& aConn, CacheId aCacheId, 
                       "response_security_info_id, "

                       "response_principal_info, "

                       "response_padding_size, "

                       "response_credentials, "

                       "cache_id "

                       ") VALUES ("

                       ":request_method, "
@@ -1754,6 +1775,7 @@ nsresult InsertEntry(mozIStorageConnection& aConn, CacheId aCacheId, 
                       ":response_security_info_id, "

                       ":response_principal_info, "

                       ":response_padding_size, "

                       ":response_credentials, "

                       ":cache_id "

                       ");"_ns));
@@ -1876,6 +1898,10 @@ nsresult InsertEntry(mozIStorageConnection& aConn, CacheId aCacheId, 
                                                  aResponse.paddingSize())));

    }



    QM_TRY(MOZ_TO_RESULT(

        state->BindInt32ByName("response_credentials"_ns,

                               static_cast<int32_t>(aResponse.credentials()))));



    QM_TRY(MOZ_TO_RESULT(state->BindInt64ByName("cache_id"_ns, aCacheId)));



    QM_TRY(MOZ_TO_RESULT(state->Execute()));
@@ -1987,7 +2013,7 @@ Result<SavedResponse, nsresult> ReadResponse(mozIStorageConnection& aConn, 
          "entries.response_principal_info, "

          "entries.response_padding_size, "

          "security_info.data, "

          "entries.request_credentials "

          "entries.response_credentials "

          "FROM entries "

          "LEFT OUTER JOIN security_info "

          "ON entries.response_security_info_id=security_info.id "
@@ -2467,6 +2493,8 @@ nsresult Validate(mozIStorageConnection& aConn) { 
      Expect("request_headers", "table", kTableRequestHeaders),

      Expect("response_headers", "table", kTableResponseHeaders),

      Expect("response_headers_name_index", "index", kIndexResponseHeadersName),

      Expect("response_headers_entry_id_index", "index",

             kIndexResponseHeadersEntryId),

      Expect("response_url_list", "table", kTableResponseUrlList),

      Expect("storage", "table", kTableStorage),

      Expect("sqlite_autoindex_storage_1", "index"),  // auto-gen by sqlite
@@ -2573,6 +2601,10 @@ nsresult MigrateFrom27To28(nsIFile& aDBDir, mozIStorageConnection& aConn, 
                           bool& aRewriteSchema);

nsresult MigrateFrom28To29(nsIFile& aDBDir, mozIStorageConnection& aConn,

                           bool& aRewriteSchema);

nsresult MigrateFrom29To30(nsIFile& aDBDir, mozIStorageConnection& aConn,

                           bool& aRewriteSchema);

nsresult MigrateFrom30To31(nsIFile& aDBDir, mozIStorageConnection& aConn,

                           bool& aRewriteSchema);

// Configure migration functions to run for the given starting version.

constexpr Migration sMigrationList[] = {

    Migration{15, MigrateFrom15To16}, Migration{16, MigrateFrom16To17},
@@ -2582,6 +2614,7 @@ constexpr Migration sMigrationList[] = { 
    Migration{23, MigrateFrom23To24}, Migration{24, MigrateFrom24To25},

    Migration{25, MigrateFrom25To26}, Migration{26, MigrateFrom26To27},

    Migration{27, MigrateFrom27To28}, Migration{28, MigrateFrom28To29},

    Migration{29, MigrateFrom29To30}, Migration{30, MigrateFrom30To31},

};



nsresult RewriteEntriesSchema(mozIStorageConnection& aConn) {
@@ -3228,5 +3261,38 @@ nsresult MigrateFrom28To29(nsIFile& aDBDir, mozIStorageConnection& aConn, 
  return NS_OK;

}



nsresult MigrateFrom29To30(nsIFile& aDBDir, mozIStorageConnection& aConn,

                           bool& aRewriteSchema) {

  MOZ_ASSERT(!NS_IsMainThread());



  // Add an index on response_headers(entry_id, name) to dramatically speed up

  // vary header lookups during cache matching operations.

  QM_TRY(MOZ_TO_RESULT(

      aConn.ExecuteSimpleSQL(nsLiteralCString(kIndexResponseHeadersEntryId))));



  QM_TRY(MOZ_TO_RESULT(aConn.SetSchemaVersion(30)));



  return NS_OK;

}



nsresult MigrateFrom30To31(nsIFile& aDBDir, mozIStorageConnection& aConn,

                           bool& aRewriteSchema) {

  MOZ_ASSERT(!NS_IsMainThread());



  // Existing response objects would be getting credential type set to 2,

  // which means that these response objects can no longer be loaded by

  // credentialless cross-origin embedders which is fine; a network fetch

  // would need to be performed for such objects.

  QM_TRY(MOZ_TO_RESULT(aConn.ExecuteSimpleSQL(

      "ALTER TABLE entries "

      "ADD COLUMN response_credentials INTEGER NOT NULL DEFAULT 2;"_ns)));



  QM_TRY(MOZ_TO_RESULT(aConn.SetSchemaVersion(31)));



  aRewriteSchema = true;



  return NS_OK;

}



}  // anonymous namespace

}  // namespace mozilla::dom::cache::db

dom/cache/TypeUtils.cpp

+3 −0
Original line number Diff line number Diff line
@@ -199,6 +199,9 @@ void TypeUtils::ToCacheResponseWithoutBody(CacheResponse& aOut, 


  aOut.paddingInfo() = aIn.GetPaddingInfo();

  aOut.paddingSize() = aIn.GetPaddingSize();



  // Record the credentials mode of the fetch that produced this response.

  aOut.credentials() = aIn.GetCredentialsMode();

}



void TypeUtils::ToCacheResponse(JSContext* aCx, CacheResponse& aOut,