Support decoding GenericNamedValue based policy.

Even if it arrives in the ChromeSettingsProto message where Chrome expects to find explicitly typed policy.
This is necessary for as long as CPanel/D3 deliver old-style policy. We include this fix on the client side to make a corresponding workaround in DMServer unnecessary.

BUG=chromium-os:14102
TEST=UserPolicyCacheTest.OldStylePolicy; manual test against DMServer without the server-side workaround

Review URL: http://codereview.chromium.org/6840014

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@81729 0039d316-1c4b-4281-b951-d872f2087c98

10 files changed, 275 insertions, 118 deletions

diff --git a/chrome/app/policy/cloud_policy_codegen.gyp b/chrome/app/policy/cloud_policy_codegen.gyp
index ef3bb59..723d091 100644
--- a/chrome/app/policy/cloud_policy_codegen.gyp
+++ b/chrome/app/policy/cloud_policy_codegen.gyp
@@ -102,6 +102,7 @@
         '<(proto_rel_path)/chrome_device_policy.proto',
         '<(proto_rel_path)/device_management_backend.proto',
         '<(proto_rel_path)/device_management_local.proto',
+        '<(proto_rel_path)/old_generic_format.proto',
       ],
       'rules': [
         {
@@ -117,18 +118,16 @@
           ],
           'action': [
             '<(PRODUCT_DIR)/<(EXECUTABLE_PREFIX)protoc<(EXECUTABLE_SUFFIX)',
-            '--proto_path=<(policy_out_dir)/policy',
             '--proto_path=<(proto_rel_path)',
             '<(proto_rel_path)/<(RULE_INPUT_NAME)',
             '--cpp_out=<(protoc_out_dir)/<(proto_path_substr)',
             '--python_out=<(PRODUCT_DIR)/pyproto/device_management_pb',
           ],
           'message': 'Generating C++ and Python code from <(RULE_INPUT_PATH)',
-        }
+        },
       ],
       'dependencies': [
         '<(DEPTH)/third_party/protobuf/protobuf.gyp:protoc#host',
-        'cloud_policy_proto_compile',
       ],
       'direct_dependent_settings': {
         'include_dirs': [
@@ -153,6 +152,8 @@
         '<(protobuf_decoder_path)',
         '<(protoc_out_dir)/<(proto_path_substr)/cloud_policy.pb.h',
         '<(protoc_out_dir)/<(proto_path_substr)/cloud_policy.pb.cc',
+        '<(protoc_out_dir)/<(proto_path_substr)/old_generic_format.pb.h',
+        '<(protoc_out_dir)/<(proto_path_substr)/old_generic_format.pb.cc',
         '<(DEPTH)/chrome/browser/policy/policy_map.h',
         '<(DEPTH)/chrome/browser/policy/policy_map.cc',
       ],
diff --git a/chrome/browser/policy/configuration_policy_provider.h b/chrome/browser/policy/configuration_policy_provider.h
index 9371a65..1cbc848 100644
--- a/chrome/browser/policy/configuration_policy_provider.h
+++ b/chrome/browser/policy/configuration_policy_provider.h
@@ -78,6 +78,10 @@ class ConfigurationPolicyProvider {
  private:
   friend class ConfigurationPolicyObserverRegistrar;
 
+  // Temporarily needed for access to ApplyPolicyValueTree as long as we need
+  // to support old-style policy.
+  friend class UserPolicyCache;
+
   virtual void AddObserver(ConfigurationPolicyProvider::Observer* observer) = 0;
   virtual void RemoveObserver(
       ConfigurationPolicyProvider::Observer* observer) = 0;
diff --git a/chrome/browser/policy/device_management_service_browsertest.cc b/chrome/browser/policy/device_management_service_browsertest.cc
index c26e8a3..2a1ae15 100644
--- a/chrome/browser/policy/device_management_service_browsertest.cc
+++ b/chrome/browser/policy/device_management_service_browsertest.cc
@@ -165,10 +165,9 @@ IN_PROC_BROWSER_TEST_F(DeviceManagementServiceIntegrationTest,
     EXPECT_CALL(delegate, HandlePolicyResponse(_))
         .WillOnce(InvokeWithoutArgs(QuitMessageLoop));
     em::DevicePolicyRequest request;
-    request.set_policy_scope(kChromePolicyScope);
-    em::DevicePolicySettingRequest* setting_request =
-        request.add_setting_request();
-    setting_request->set_key(kChromeDevicePolicySettingKey);
+    em::PolicyFetchRequest* fetch_request = request.add_request();
+    fetch_request->set_signature_type(em::PolicyFetchRequest::SHA1_RSA);
+    fetch_request->set_policy_type(kChromeUserPolicyType);
     backend->ProcessPolicyRequest(token_, "testid", request, &delegate);
 
     MessageLoop::current()->Run();
diff --git a/chrome/browser/policy/device_management_service_unittest.cc b/chrome/browser/policy/device_management_service_unittest.cc
index b501d2c..00a6f17 100644
--- a/chrome/browser/policy/device_management_service_unittest.cc
+++ b/chrome/browser/policy/device_management_service_unittest.cc
@@ -364,61 +364,6 @@ TEST_F(DeviceManagementServiceTest, UnregisterRequest) {
                                           response_data);
 }
 
-TEST_F(DeviceManagementServiceTest, PolicyRequest) {
-  DevicePolicyResponseDelegateMock mock;
-  em::DevicePolicyResponse expected_response;
-  em::DevicePolicySetting* policy_setting = expected_response.add_setting();
-  policy_setting->set_policy_key(kChromeDevicePolicySettingKey);
-  policy_setting->set_watermark("fresh");
-  em::GenericSetting* policy_value = policy_setting->mutable_policy_value();
-  em::GenericNamedValue* named_value = policy_value->add_named_value();
-  named_value->set_name("HomepageLocation");
-  named_value->mutable_value()->set_value_type(
-      em::GenericValue::VALUE_TYPE_STRING);
-  named_value->mutable_value()->set_string_value("http://www.chromium.org");
-  named_value = policy_value->add_named_value();
-  named_value->set_name("HomepageIsNewTabPage");
-  named_value->mutable_value()->set_value_type(
-      em::GenericValue::VALUE_TYPE_BOOL);
-  named_value->mutable_value()->set_bool_value(false);
-  EXPECT_CALL(mock, HandlePolicyResponse(MessageEquals(expected_response)));
-
-  em::DevicePolicyRequest request;
-  request.set_policy_scope(kChromePolicyScope);
-  em::DevicePolicySettingRequest* setting_request =
-      request.add_setting_request();
-  setting_request->set_key(kChromeDevicePolicySettingKey);
-  setting_request->set_watermark("stale");
-  backend_->ProcessPolicyRequest(kDMToken, kDeviceId, request, &mock);
-  TestURLFetcher* fetcher = factory_.GetFetcherByID(0);
-  ASSERT_TRUE(fetcher);
-
-  CheckURLAndQueryParams(fetcher->original_url(),
-                         DeviceManagementBackendImpl::kValueRequestPolicy,
-                         kDeviceId);
-
-  em::DeviceManagementRequest expected_request_wrapper;
-  expected_request_wrapper.mutable_policy_request()->CopyFrom(request);
-  std::string expected_request_data;
-  ASSERT_TRUE(expected_request_wrapper.SerializeToString(
-      &expected_request_data));
-  EXPECT_EQ(expected_request_data, fetcher->upload_data());
-
-  // Generate the response.
-  std::string response_data;
-  em::DeviceManagementResponse response_wrapper;
-  response_wrapper.set_error(em::DeviceManagementResponse::SUCCESS);
-  response_wrapper.mutable_policy_response()->CopyFrom(expected_response);
-  ASSERT_TRUE(response_wrapper.SerializeToString(&response_data));
-  net::URLRequestStatus status(net::URLRequestStatus::SUCCESS, 0);
-  fetcher->delegate()->OnURLFetchComplete(fetcher,
-                                          GURL(kServiceUrl),
-                                          status,
-                                          200,
-                                          ResponseCookies(),
-                                          response_data);
-}
-
 TEST_F(DeviceManagementServiceTest, CancelRegisterRequest) {
   DeviceRegisterResponseDelegateMock mock;
   EXPECT_CALL(mock, HandleRegisterResponse(_)).Times(0);
diff --git a/chrome/browser/policy/proto/device_management_backend.proto b/chrome/browser/policy/proto/device_management_backend.proto
index 4b45f7b..9d3923f 100644
--- a/chrome/browser/policy/proto/device_management_backend.proto
+++ b/chrome/browser/policy/proto/device_management_backend.proto
@@ -24,57 +24,6 @@ message ChromeInitialSettingsProto {
   optional EnrollmentProvision enrollment_provision = 1 [default = UNMANAGED];
 }
 
-// A setting is a set of generic name value pairs.
-// TODO(gfeher): remove this after Chrome OS TT is over.
-message GenericSetting {
-  repeated GenericNamedValue named_value = 1;
-}
-
-// Generic value container.
-message GenericValue {
-  enum ValueType {
-    VALUE_TYPE_BOOL = 1;
-    VALUE_TYPE_INT64 = 2;
-    VALUE_TYPE_STRING = 3;
-    VALUE_TYPE_DOUBLE = 4;
-    VALUE_TYPE_BYTES = 5;
-    VALUE_TYPE_BOOL_ARRAY = 6;
-    VALUE_TYPE_INT64_ARRAY = 7;
-    VALUE_TYPE_STRING_ARRAY = 8;
-    VALUE_TYPE_DOUBLE_ARRAY = 9;
-  }
-
-  optional ValueType value_type = 1 [default = VALUE_TYPE_STRING];
-
-  // basic value types
-  optional bool bool_value = 2;
-  optional int64 int64_value = 3;
-  optional string string_value = 4;
-  optional double double_value = 5;
-  optional bytes bytes_value = 6;
-  repeated bool bool_array = 7;
-  repeated int64 int64_array = 8;
-  repeated string string_array = 9;
-  repeated double double_array = 10;
-}
-
-// Generic name value pair container.
-message GenericNamedValue {
-  required string name = 1;
-  optional GenericValue value = 2;
-}
-
-// Identify a single device policy setting key/value pair.
-// TODO(gfeher): remove this after Chrome OS TT is over.
-message DevicePolicySetting {
-  // key of the policy setting
-  required string policy_key = 1;
-  // value of the setting
-  optional GenericSetting policy_value = 2;
-  // watermark for setting value.
-  optional string watermark = 3;
-}
-
 // Request from device to server to register device.
 message DeviceRegisterRequest {
   // Reregister device without erasing server state.  It can be used
@@ -257,10 +206,6 @@ message DevicePolicyRequest {
 
 // Response from server to device for reading policies.
 message DevicePolicyResponse {
-  // the result of the settings.
-  // TODO(gfeher): remove this after Chrome OS TT is over.
-  repeated DevicePolicySetting setting = 1;
-
   // The policy fetch response.
   repeated PolicyFetchResponse response = 3;
 }
diff --git a/chrome/browser/policy/proto/old_generic_format.proto b/chrome/browser/policy/proto/old_generic_format.proto
new file mode 100644
index 0000000..dac2850
--- /dev/null
+++ b/chrome/browser/policy/proto/old_generic_format.proto
@@ -0,0 +1,59 @@
+// Copyright (c) 2011 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.
+
+syntax = "proto2";
+
+option optimize_for = LITE_RUNTIME;
+
+package enterprise_management;
+
+// This file keeps the deprecated GenericNamedValue based format for policies
+// available. It is intended to be removed (along with all code that makes
+// use of it) as soon as all server-side components (CPanel, D3) have been
+// migrated to provide the new, explicitly typed format to clients.
+
+// A setting is a set of generic name value pairs.
+message GenericSetting {
+  repeated GenericNamedValue named_value = 1;
+}
+
+// Generic value container.
+message GenericValue {
+  enum ValueType {
+    VALUE_TYPE_BOOL = 1;
+    VALUE_TYPE_INT64 = 2;
+    VALUE_TYPE_STRING = 3;
+    VALUE_TYPE_DOUBLE = 4;
+    VALUE_TYPE_BYTES = 5;
+    VALUE_TYPE_BOOL_ARRAY = 6;
+    VALUE_TYPE_INT64_ARRAY = 7;
+    VALUE_TYPE_STRING_ARRAY = 8;
+    VALUE_TYPE_DOUBLE_ARRAY = 9;
+  }
+
+  optional ValueType value_type = 1 [default = VALUE_TYPE_STRING];
+
+  // basic value types
+  optional bool bool_value = 2;
+  optional int64 int64_value = 3;
+  optional string string_value = 4;
+  optional double double_value = 5;
+  optional bytes bytes_value = 6;
+  repeated bool bool_array = 7;
+  repeated int64 int64_array = 8;
+  repeated string string_array = 9;
+  repeated double double_array = 10;
+}
+
+// Generic name value pair container.
+message GenericNamedValue {
+  required string name = 1;
+  optional GenericValue value = 2;
+}
+
+// Wrapper that contains the above. Designed to be a partial view of the
+// data the server currently delivers.
+message LegacyChromeSettingsProto {
+  repeated GenericNamedValue named_value = 2;
+} 
diff --git a/chrome/browser/policy/user_policy_cache.cc b/chrome/browser/policy/user_policy_cache.cc
index 12845f0..ae14f83 100644
--- a/chrome/browser/policy/user_policy_cache.cc
+++ b/chrome/browser/policy/user_policy_cache.cc
@@ -4,14 +4,18 @@
 
 #include "chrome/browser/policy/user_policy_cache.h"
 
+#include <limits>
 #include <string>
 
 #include "base/file_util.h"
 #include "base/logging.h"
 #include "base/task.h"
+#include "base/values.h"
+#include "chrome/browser/policy/configuration_policy_pref_store.h"
 #include "chrome/browser/policy/policy_map.h"
 #include "chrome/browser/policy/proto/cloud_policy.pb.h"
 #include "chrome/browser/policy/proto/device_management_local.pb.h"
+#include "chrome/browser/policy/proto/old_generic_format.pb.h"
 #include "content/browser/browser_thread.h"
 #include "policy/configuration_policy_type.h"
 
@@ -147,7 +151,144 @@ bool UserPolicyCache::DecodePolicyData(const em::PolicyData& policy_data,
     return false;
   }
   DecodePolicy(policy, mandatory, recommended);
+  MaybeDecodeOldstylePolicy(policy_data.policy_value(), mandatory, recommended);
   return true;
 }
 
+// Everything below is only needed for supporting old-style GenericNamedValue
+// based policy data and can be removed once this support is no longer needed.
+
+using google::protobuf::RepeatedField;
+using google::protobuf::RepeatedPtrField;
+
+class PolicyMapProxy : public ConfigurationPolicyStoreInterface {
+ public:
+  // Does not take ownership of |policy_map|, and callers need to make sure
+  // that |policy_map| outlives this PolicyMapProxy.
+  explicit PolicyMapProxy(PolicyMap* policy_map)
+      : policy_map_(policy_map) {}
+  virtual ~PolicyMapProxy() {}
+  virtual void Apply(ConfigurationPolicyType policy, Value* value) {
+    policy_map_->Set(policy, value);
+  }
+
+ private:
+  PolicyMap* policy_map_;
+
+  DISALLOW_COPY_AND_ASSIGN(PolicyMapProxy);
+};
+
+void UserPolicyCache::MaybeDecodeOldstylePolicy(
+    const std::string& policy_data,
+    PolicyMap* mandatory,
+    PolicyMap* recommended) {
+  // Return immediately if we already have policy information in the maps.
+  if (!mandatory->empty() || !recommended->empty())
+    return;
+  em::LegacyChromeSettingsProto policy;
+  // Return if the input string doesn't match the protobuf definition.
+  if (!policy.ParseFromString(policy_data))
+    return;
+  // Return if there's no old-style policy to decode.
+  if (policy.named_value_size() == 0)
+    return;
+
+  // Inspect GenericNamedValues and decode them.
+  DictionaryValue result;
+  RepeatedPtrField<em::GenericNamedValue>::const_iterator named_value;
+  for (named_value = policy.named_value().begin();
+       named_value != policy.named_value().end();
+       ++named_value) {
+    if (named_value->has_value()) {
+      Value* decoded_value = DecodeValue(named_value->value());
+      if (decoded_value)
+        result.Set(named_value->name(), decoded_value);
+    }
+  }
+  // Hack: Let one of the providers do the transformation from DictionaryValue
+  // to PolicyMap, since they have the required code anyway.
+  PolicyMapProxy map_proxy(mandatory);
+  GetManagedPolicyProvider()->ApplyPolicyValueTree(&result, &map_proxy);
+}
+
+Value* UserPolicyCache::DecodeIntegerValue(
+    google::protobuf::int64 value) const {
+  if (value < std::numeric_limits<int>::min() ||
+      value > std::numeric_limits<int>::max()) {
+    LOG(WARNING) << "Integer value " << value
+                 << " out of numeric limits, ignoring.";
+    return NULL;
+  }
+
+  return Value::CreateIntegerValue(static_cast<int>(value));
+}
+
+Value* UserPolicyCache::DecodeValue(const em::GenericValue& value) const {
+  if (!value.has_value_type())
+    return NULL;
+
+  switch (value.value_type()) {
+    case em::GenericValue::VALUE_TYPE_BOOL:
+      if (value.has_bool_value())
+        return Value::CreateBooleanValue(value.bool_value());
+      return NULL;
+    case em::GenericValue::VALUE_TYPE_INT64:
+      if (value.has_int64_value())
+        return DecodeIntegerValue(value.int64_value());
+      return NULL;
+    case em::GenericValue::VALUE_TYPE_STRING:
+      if (value.has_string_value())
+        return Value::CreateStringValue(value.string_value());
+      return NULL;
+    case em::GenericValue::VALUE_TYPE_DOUBLE:
+      if (value.has_double_value())
+        return Value::CreateDoubleValue(value.double_value());
+      return NULL;
+    case em::GenericValue::VALUE_TYPE_BYTES:
+      if (value.has_bytes_value()) {
+        std::string bytes = value.bytes_value();
+        return BinaryValue::CreateWithCopiedBuffer(bytes.c_str(), bytes.size());
+      }
+      return NULL;
+    case em::GenericValue::VALUE_TYPE_BOOL_ARRAY: {
+      ListValue* list = new ListValue;
+      RepeatedField<bool>::const_iterator i;
+      for (i = value.bool_array().begin(); i != value.bool_array().end(); ++i)
+        list->Append(Value::CreateBooleanValue(*i));
+      return list;
+    }
+    case em::GenericValue::VALUE_TYPE_INT64_ARRAY: {
+      ListValue* list = new ListValue;
+      RepeatedField<google::protobuf::int64>::const_iterator i;
+      for (i = value.int64_array().begin();
+           i != value.int64_array().end(); ++i) {
+        Value* int_value = DecodeIntegerValue(*i);
+        if (int_value)
+          list->Append(int_value);
+      }
+      return list;
+    }
+    case em::GenericValue::VALUE_TYPE_STRING_ARRAY: {
+      ListValue* list = new ListValue;
+      RepeatedPtrField<std::string>::const_iterator i;
+      for (i = value.string_array().begin();
+           i != value.string_array().end(); ++i)
+        list->Append(Value::CreateStringValue(*i));
+      return list;
+    }
+    case em::GenericValue::VALUE_TYPE_DOUBLE_ARRAY: {
+      ListValue* list = new ListValue;
+      RepeatedField<double>::const_iterator i;
+      for (i = value.double_array().begin();
+           i != value.double_array().end(); ++i)
+        list->Append(Value::CreateDoubleValue(*i));
+      return list;
+    }
+    default:
+      NOTREACHED() << "Unhandled value type";
+  }
+
+  return NULL;
+}
+
 }  // namespace policy
diff --git a/chrome/browser/policy/user_policy_cache.h b/chrome/browser/policy/user_policy_cache.h
index 316497f..3d6b56bc 100644
--- a/chrome/browser/policy/user_policy_cache.h
+++ b/chrome/browser/policy/user_policy_cache.h
@@ -6,9 +6,17 @@
 #define CHROME_BROWSER_POLICY_USER_POLICY_CACHE_H_
 #pragma once
 
+#include <string>
+
 #include "base/file_path.h"
 #include "chrome/browser/policy/cloud_policy_cache_base.h"
 
+// <Old-style policy support> (see comment below)
+namespace enterprise_management {
+class GenericValue;
+}  // namespace enterprise_management
+// </Old-style policy support>
+
 namespace policy {
 
 // CloudPolicyCacheBase implementation that persists policy information
@@ -32,6 +40,23 @@ class UserPolicyCache : public CloudPolicyCacheBase {
                                 PolicyMap* mandatory,
                                 PolicyMap* recommended) OVERRIDE;
 
+  // <Old-style policy support>
+  // The following member functions are needed to support old-style policy and
+  // can be removed once all server-side components (CPanel, D3) have been
+  // migrated to providing the new policy format.
+
+  // If |mandatory| and |recommended| are both empty, and |policy_data|
+  // contains a field named "repeated GenericNamedValue named_value = 2;",
+  // this field is decoded into |mandatory|.
+  void MaybeDecodeOldstylePolicy(const std::string& policy_data,
+                                 PolicyMap* mandatory,
+                                 PolicyMap* recommended);
+
+  Value* DecodeIntegerValue(google::protobuf::int64 value) const;
+  Value* DecodeValue(const em::GenericValue& value) const;
+
+  // </Old-style policy support>
+
   // The file in which we store a cached version of the policy information.
   const FilePath backing_file_path_;
 
diff --git a/chrome/browser/policy/user_policy_cache_unittest.cc b/chrome/browser/policy/user_policy_cache_unittest.cc
index 8e74535..fdf9102 100644
--- a/chrome/browser/policy/user_policy_cache_unittest.cc
+++ b/chrome/browser/policy/user_policy_cache_unittest.cc
@@ -15,6 +15,7 @@
 #include "chrome/browser/policy/proto/cloud_policy.pb.h"
 #include "chrome/browser/policy/proto/device_management_backend.pb.h"
 #include "chrome/browser/policy/proto/device_management_local.pb.h"
+#include "chrome/browser/policy/proto/old_generic_format.pb.h"
 #include "content/browser/browser_thread.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -335,4 +336,41 @@ TEST_F(UserPolicyCacheTest, FreshPolicyOverride) {
   EXPECT_TRUE(expected.Equals(mandatory_policy(cache)));
 }
 
+// Test case for the temporary support for GenericNamedValues in the
+// CloudPolicySettings protobuf. Can be removed when this support is no longer
+// required.
+TEST_F(UserPolicyCacheTest, OldStylePolicy) {
+  UserPolicyCache cache(test_file());
+  em::PolicyFetchResponse* policy = new em::PolicyFetchResponse();
+  em::PolicyData signed_response;
+  em::LegacyChromeSettingsProto settings;
+  em::GenericNamedValue* named_value = settings.add_named_value();
+  named_value->set_name("HomepageLocation");
+  em::GenericValue* value_container = named_value->mutable_value();
+  value_container->set_value_type(em::GenericValue::VALUE_TYPE_STRING);
+  value_container->set_string_value("http://www.example.com");
+  EXPECT_TRUE(
+      settings.SerializeToString(signed_response.mutable_policy_value()));
+  base::TimeDelta timestamp =
+      base::Time::NowFromSystemTime() - base::Time::UnixEpoch();
+  signed_response.set_timestamp(timestamp.InMilliseconds());
+  EXPECT_TRUE(
+      signed_response.SerializeToString(policy->mutable_policy_data()));
+
+  SetPolicy(&cache, policy, true);
+  PolicyMap expected;
+  expected.Set(kPolicyHomepageLocation,
+               Value::CreateStringValue("http://www.example.com"));
+  PolicyMap empty;
+  EXPECT_TRUE(expected.Equals(mandatory_policy(cache)));
+  EXPECT_TRUE(empty.Equals(recommended_policy(cache)));
+  // If new-style policy comes in, it should override old-style policy.
+  policy = CreateHomepagePolicy("http://www.example.com",
+                                base::Time::NowFromSystemTime(),
+                                em::PolicyOptions::RECOMMENDED);
+  SetPolicy(&cache, policy, true);
+  EXPECT_TRUE(expected.Equals(recommended_policy(cache)));
+  EXPECT_TRUE(empty.Equals(mandatory_policy(cache)));
+}
+
 }  // namespace policy
diff --git a/net/tools/testserver/device_management.py b/net/tools/testserver/device_management.py
index c4ec75e..b62b2c0 100644
--- a/net/tools/testserver/device_management.py
+++ b/net/tools/testserver/device_management.py
@@ -352,7 +352,7 @@ class RequestHandler(object):
     if field.type == field.TYPE_BOOL:
       assert type(field_value) == bool
     elif field.type == field.TYPE_STRING:
-      assert type(field_value) == str
+      assert type(field_value) == str or type(field_value) == unicode
     elif field.type == field.TYPE_INT64:
       assert type(field_value) == int
     elif (field.type == field.TYPE_MESSAGE and