[Sync] Handle invalid specifics field numbers gracefully

sync/engine/syncer_proto_util.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "sync/engine/syncer_proto_util.h"
 
 #include "base/format_macros.h"
 #include "base/stringprintf.h"
 #include "google_apis/google_api_keys.h"
 #include "sync/engine/net/server_connection_manager.h"
@@ skipping 99 matching lines @@
 }  // namespace
 
 // static
 void SyncerProtoUtil::HandleMigrationDoneResponse(
   const ClientToServerResponse* response,
   sessions::SyncSession* session) {
   LOG_IF(ERROR, 0 >= response->migrated_data_type_id_size())
       << "MIGRATION_DONE but no types specified.";
   ModelTypeSet to_migrate;
   for (int i = 0; i < response->migrated_data_type_id_size(); i++) {
-    to_migrate.Put(GetModelTypeFromSpecificsFieldNumber(
-        response->migrated_data_type_id(i)));
+    int field_number = response->migrated_data_type_id(i);
+    ModelType model_type = GetModelTypeFromSpecificsFieldNumber(field_number);
+    if (!IsRealDataType(model_type)) {
+      NOTREACHED() << "Unknown field number " << field_number;
+      continue;
+    }
+    to_migrate.Put(model_type);
   }
   // TODO(akalin): This should be a set union.
   session->mutable_status_controller()->
       set_types_needing_local_migration(to_migrate);
 }
 
 // static
 bool SyncerProtoUtil::VerifyResponseBirthday(syncable::Directory* dir,
     const ClientToServerResponse* response) {
 
@@ skipping 183 matching lines @@
       error.error_type());
   sync_protocol_error.error_description = error.error_description();
   sync_protocol_error.url = error.url();
   sync_protocol_error.action = ConvertClientActionPBToLocalClientAction(
       error.action());
 
   if (error.error_data_type_ids_size() > 0) {
     // THROTTLED is currently the only error code that uses |error_data_types|.
     DCHECK_EQ(error.error_type(), sync_pb::SyncEnums::THROTTLED);
     for (int i = 0; i < error.error_data_type_ids_size(); ++i) {
-      sync_protocol_error.error_data_types.Put(
-          GetModelTypeFromSpecificsFieldNumber(
-              error.error_data_type_ids(i)));
+      int field_number = error.error_data_type_ids(i);
+      ModelType model_type =
+          GetModelTypeFromSpecificsFieldNumber(field_number);
+      if (!IsRealDataType(model_type)) {
+        NOTREACHED() << "Unknown field number " << field_number;

[tim (not reviewing), 2012/12/10 23:49:37]

Is there any way to add a test for this?

[akalin, 2012/12/11 00:15:50]

Yeah.  Are you okay with doing it in another CL?  I'd like to keep this one
small in case we merge it.

+        continue;
+      }
+      sync_protocol_error.error_data_types.Put(model_type);
     }
   }
 
   return sync_protocol_error;
 }
 
 // TODO(lipalani) : Rename these function names as per the CR for issue 7740067.
 SyncProtocolError ConvertLegacyErrorCodeToNewError(
     const sync_pb::SyncEnums::ErrorType& error_type) {
   SyncProtocolError error;
@@ skipping 260 matching lines @@
 std::string SyncerProtoUtil::ClientToServerResponseDebugString(
     const ClientToServerResponse& response) {
   // Add more handlers as needed.
   std::string output;
   if (response.has_get_updates())
     output.append(GetUpdatesResponseString(response.get_updates()));
   return output;
 }
 
 }  // namespace syncer