Remove special handling for strings in var serialization.

ppapi/proxy/serialized_var.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 "ppapi/proxy/serialized_var.h"
 
 #include "base/logging.h"
 #include "ipc/ipc_message_utils.h"
 #include "ppapi/proxy/dispatcher.h"
 #include "ppapi/proxy/interface_proxy.h"
 #include "ppapi/proxy/ppapi_param_traits.h"
 #include "ppapi/proxy/var_serialization_rules.h"
+#include "ppapi/shared_impl/var.h"
 
 namespace ppapi {
 namespace proxy {
 
 // SerializedVar::Inner --------------------------------------------------------
 
 SerializedVar::Inner::Inner()
     : serialization_rules_(NULL),
       var_(PP_MakeUndefined()),
-      tracker_string_ptr_(NULL),
       cleanup_mode_(CLEANUP_NONE),
       dispatcher_for_end_send_pass_ref_(NULL) {
 #ifndef NDEBUG
   has_been_serialized_ = false;
   has_been_deserialized_ = false;
 #endif
 }
 
 SerializedVar::Inner::Inner(VarSerializationRules* serialization_rules)
     : serialization_rules_(serialization_rules),
       var_(PP_MakeUndefined()),
-      tracker_string_ptr_(NULL),
       cleanup_mode_(CLEANUP_NONE),
       dispatcher_for_end_send_pass_ref_(NULL) {
 #ifndef NDEBUG
   has_been_serialized_ = false;
   has_been_deserialized_ = false;
 #endif
 }
 
 SerializedVar::Inner::~Inner() {
   switch (cleanup_mode_) {
     case END_SEND_PASS_REF:
       DCHECK(dispatcher_for_end_send_pass_ref_);
       serialization_rules_->EndSendPassRef(var_,
                                            dispatcher_for_end_send_pass_ref_);
       break;
     case END_RECEIVE_CALLER_OWNED:
       serialization_rules_->EndReceiveCallerOwned(var_);
       break;
     default:
       break;
   }
 }
 
 PP_Var SerializedVar::Inner::GetVar() const {
   DCHECK(serialization_rules_);
-
-  // If we're a string var, we should have already converted the string value
-  // to a var ID.
-  DCHECK(var_.type != PP_VARTYPE_STRING || var_.value.as_id != 0);
-  return var_;
-}
-
-PP_Var SerializedVar::Inner::GetIncompleteVar() const {
-  DCHECK(serialization_rules_);
   return var_;
 }
 
 void SerializedVar::Inner::SetVar(PP_Var var) {
   // Sanity check, when updating the var we should have received a
   // serialization rules pointer already.
   DCHECK(serialization_rules_);
   var_ = var;
 }
 
-scoped_ptr<std::string> SerializedVar::Inner::GetStringDestructive() {
-  DCHECK(serialization_rules_);
-  return string_from_ipc_.Pass();
-}
-
-const std::string** SerializedVar::Inner::GetStringPtrPtr() {
-  DCHECK(serialization_rules_);
-  // The caller will set our string pointer, and we promise not to change it.
-  // This path is taken for the "Send" side of SerializedVars, and we will only
-  // read the string in those cases, so it's safe for us to point directly to a
-  // string in the VarTracker.
-  return &tracker_string_ptr_;
-}
-
 void SerializedVar::Inner::ForceSetVarValueForTest(PP_Var value) {
   var_ = value;
 }
 
-void SerializedVar::Inner::ForceSetStringValueForTest(const std::string& str) {
-  // We don't need to change tracker_string_ptr_, as that is only used for
-  // serializing, and we're emulating a SerializedVar that was received from
-  // IPC.
-  string_from_ipc_.reset(new std::string(str));
-}
-
 void SerializedVar::Inner::WriteToMessage(IPC::Message* m) const {
   // When writing to the IPC messages, a serization rules handler should
   // always have been set.
   //
   // When sending a message, it should be difficult to trigger this if you're
   // using the SerializedVarSendInput class and giving a non-NULL dispatcher.
   // Make sure you're using the proper "Send" helper class.
   //
   // It should be more common to see this when handling an incoming message
   // that returns a var. This means the message handler didn't write to the
   // output parameter, or possibly you used the wrong helper class
   // (normally SerializedVarReturnValue).
   DCHECK(serialization_rules_);
 
 #ifndef NDEBUG
   // We should only be serializing something once.
   DCHECK(!has_been_serialized_);
   has_been_serialized_ = true;
 #endif
 
-  // If the var is not a string type, we should not have ended up with any
-  // string data.
-  DCHECK(var_.type == PP_VARTYPE_STRING || !tracker_string_ptr_);
-
   m->WriteInt(static_cast<int>(var_.type));
   switch (var_.type) {
     case PP_VARTYPE_UNDEFINED:
     case PP_VARTYPE_NULL:
       // These don't need any data associated with them other than the type we
       // just serialized.
       break;
     case PP_VARTYPE_BOOL:
       m->WriteBool(PP_ToBool(var_.value.as_bool));
       break;
     case PP_VARTYPE_INT32:
       m->WriteInt(var_.value.as_int);
       break;
     case PP_VARTYPE_DOUBLE:
       IPC::ParamTraits<double>::Write(m, var_.value.as_double);
       break;
-    case PP_VARTYPE_STRING:
+    case PP_VARTYPE_STRING: {
       // TODO(brettw) in the case of an invalid string ID, it would be nice
       // to send something to the other side such that a 0 ID would be
       // generated there. Then the function implementing the interface can
       // handle the invalid string as if it was in process rather than seeing
       // what looks like a valid empty string.
-      m->WriteString(tracker_string_ptr_ ? *tracker_string_ptr_
-                                           : std::string());
+      StringVar* string_var = StringVar::FromPPVar(var_);
+      m->WriteString(string_var ? *string_var->ptr() : std::string());
       break;
+    }
     case PP_VARTYPE_ARRAY_BUFFER:
       // TODO(dmichael): Proxy ArrayBuffer.
       NOTIMPLEMENTED();
       break;
     case PP_VARTYPE_OBJECT:
       m->WriteInt64(var_.value.as_id);
       break;
     case PP_VARTYPE_ARRAY:
     case PP_VARTYPE_DICTIONARY:
       // TODO(brettw) when these are supported, implement this.
@@ skipping 34 matching lines @@
       success = m->ReadBool(iter, &bool_value);
       var_.value.as_bool = PP_FromBool(bool_value);
       break;
     }
     case PP_VARTYPE_INT32:
       success = m->ReadInt(iter, &var_.value.as_int);
       break;
     case PP_VARTYPE_DOUBLE:
       success = IPC::ParamTraits<double>::Read(m, iter, &var_.value.as_double);
       break;
-    case PP_VARTYPE_STRING:
-      DCHECK(!tracker_string_ptr_ && !string_from_ipc_.get());
-      string_from_ipc_.reset(new std::string);
-      success = m->ReadString(iter, string_from_ipc_.get());
-      var_.value.as_id = 0;
+    case PP_VARTYPE_STRING: {
+      std::string string_from_ipc;
+      success = m->ReadString(iter, &string_from_ipc);
+      var_ = StringVar::SwapValidatedUTF8StringIntoPPVar(&string_from_ipc);

[dmichael (off chromium), 2012/02/07 16:35:31]

It just occurred to me that we maybe should still do the extra UTF8 validation
on the browser side when we receive a string, since the plugin side of the proxy
is (going to be) untrusted, and downstream code probably assumes valid UTF8.

What do you think? :-/

[brettw, 2012/02/07 16:56:31]

The validating we do is mostly because we wanted to force people to do the right
thing rather than having weird behaviors. I think we can skip this extra step.
Chrome's behavior for non-UTF-8 strings should be well-defined, and I'm not
particularly worried about security problems here.

       break;
+    }
     case PP_VARTYPE_OBJECT:
       success = m->ReadInt64(iter, &var_.value.as_id);
       break;
     case PP_VARTYPE_ARRAY:
     case PP_VARTYPE_DICTIONARY:
       // TODO(brettw) when these types are supported, implement this.
       NOTIMPLEMENTED();
       break;
     default:
       // Leave success as false.
@@ skipping 30 matching lines @@
 }
 
 SerializedVar::~SerializedVar() {
 }
 
 // SerializedVarSendInput ------------------------------------------------------
 
 SerializedVarSendInput::SerializedVarSendInput(Dispatcher* dispatcher,
                                                const PP_Var& var)
     : SerializedVar(dispatcher->serialization_rules()) {
-  inner_->SetVar(dispatcher->serialization_rules()->SendCallerOwned(
-      var, inner_->GetStringPtrPtr()));
+  inner_->SetVar(dispatcher->serialization_rules()->SendCallerOwned(var));
 }
 
 // static
 void SerializedVarSendInput::ConvertVector(Dispatcher* dispatcher,
                                            const PP_Var* input,
                                            size_t input_count,
                                            std::vector<SerializedVar>* output) {
   output->reserve(input_count);
   for (size_t i = 0; i < input_count; i++)
     output->push_back(SerializedVarSendInput(dispatcher, input[i]));
 }
 
 // ReceiveSerializedVarReturnValue ---------------------------------------------
 
 ReceiveSerializedVarReturnValue::ReceiveSerializedVarReturnValue() {
 }
 
 ReceiveSerializedVarReturnValue::ReceiveSerializedVarReturnValue(
     const SerializedVar& serialized)
     : SerializedVar(serialized) {
 }
 
 PP_Var ReceiveSerializedVarReturnValue::Return(Dispatcher* dispatcher) {
   inner_->set_serialization_rules(dispatcher->serialization_rules());
   inner_->SetVar(inner_->serialization_rules()->ReceivePassRef(
-      inner_->GetIncompleteVar(), inner_->GetStringDestructive(), dispatcher));
+      inner_->GetVar(), dispatcher));
   return inner_->GetVar();
 }
 
 // ReceiveSerializedException --------------------------------------------------
 
 ReceiveSerializedException::ReceiveSerializedException(Dispatcher* dispatcher,
                                                        PP_Var* exception)
     : SerializedVar(dispatcher->serialization_rules()),
       dispatcher_(dispatcher),
       exception_(exception) {
 }
 
 ReceiveSerializedException::~ReceiveSerializedException() {
   if (exception_) {
     // When an output exception is specified, it will take ownership of the
     // reference.
     inner_->SetVar(
-        inner_->serialization_rules()->ReceivePassRef(
-            inner_->GetIncompleteVar(),
-            inner_->GetStringDestructive(),
-            dispatcher_));
+        inner_->serialization_rules()->ReceivePassRef(inner_->GetVar(),
+                                                      dispatcher_));
     *exception_ = inner_->GetVar();
   } else {
     // When no output exception is specified, the browser thinks we have a ref
     // to an object that we don't want (this will happen only in the plugin
     // since the browser will always specify an out exception for the plugin to
     // write into).
     //
     // Strings don't need this handling since we can just avoid creating a
     // Var from the std::string in the first place.
     if (inner_->GetVar().type == PP_VARTYPE_OBJECT)
@@ skipping 52 matching lines @@
 PP_Var SerializedVarReceiveInput::Get(Dispatcher* dispatcher) {
   serialized_.inner_->set_serialization_rules(
       dispatcher->serialization_rules());
 
   // Ensure that when the serialized var goes out of scope it cleans up the
   // stuff we're making in BeginReceiveCallerOwned.
   serialized_.inner_->SetCleanupModeToEndReceiveCallerOwned();
 
   serialized_.inner_->SetVar(
       serialized_.inner_->serialization_rules()->BeginReceiveCallerOwned(
-          serialized_.inner_->GetIncompleteVar(),
-          serialized_.inner_->GetStringDestructive(),
+          serialized_.inner_->GetVar(),
           dispatcher));
   return serialized_.inner_->GetVar();
 }
 
 // SerializedVarVectorReceiveInput ---------------------------------------------
 
 SerializedVarVectorReceiveInput::SerializedVarVectorReceiveInput(
     const std::vector<SerializedVar>& serialized)
     : serialized_(serialized) {
 }
 
 SerializedVarVectorReceiveInput::~SerializedVarVectorReceiveInput() {
   for (size_t i = 0; i < deserialized_.size(); i++) {
     serialized_[i].inner_->serialization_rules()->EndReceiveCallerOwned(
         deserialized_[i]);
   }
 }
 
 PP_Var* SerializedVarVectorReceiveInput::Get(Dispatcher* dispatcher,
                                              uint32_t* array_size) {
   deserialized_.resize(serialized_.size());
   for (size_t i = 0; i < serialized_.size(); i++) {
     // The vectors must be able to clean themselves up after this call is
     // torn down.
     serialized_[i].inner_->set_serialization_rules(
         dispatcher->serialization_rules());
 
     serialized_[i].inner_->SetVar(
         serialized_[i].inner_->serialization_rules()->BeginReceiveCallerOwned(
-            serialized_[i].inner_->GetIncompleteVar(),
-            serialized_[i].inner_->GetStringDestructive(),
+            serialized_[i].inner_->GetVar(),
             dispatcher));
     deserialized_[i] = serialized_[i].inner_->GetVar();
   }
 
   *array_size = static_cast<uint32_t>(serialized_.size());
   return deserialized_.empty() ? NULL : &deserialized_[0];
 }
 
 // SerializedVarReturnValue ----------------------------------------------------
 
 SerializedVarReturnValue::SerializedVarReturnValue(SerializedVar* serialized)
     : serialized_(serialized) {
 }
 
 void SerializedVarReturnValue::Return(Dispatcher* dispatcher,
                                       const PP_Var& var) {
   serialized_->inner_->set_serialization_rules(
       dispatcher->serialization_rules());
 
   // Var must clean up after our BeginSendPassRef call.
   serialized_->inner_->SetCleanupModeToEndSendPassRef(dispatcher);
 
   serialized_->inner_->SetVar(
-      dispatcher->serialization_rules()->BeginSendPassRef(
-          var,
-          serialized_->inner_->GetStringPtrPtr()));
+      dispatcher->serialization_rules()->BeginSendPassRef(var));
 }
 
 // static
 SerializedVar SerializedVarReturnValue::Convert(Dispatcher* dispatcher,
                                                 const PP_Var& var) {
   // Mimic what happens in the normal case.
   SerializedVar result;
   SerializedVarReturnValue retvalue(&result);
   retvalue.Return(dispatcher, var);
   return result;
 }
 
 // SerializedVarOutParam -------------------------------------------------------
 
 SerializedVarOutParam::SerializedVarOutParam(SerializedVar* serialized)
     : serialized_(serialized),
       writable_var_(PP_MakeUndefined()),
       dispatcher_(NULL) {
 }
 
 SerializedVarOutParam::~SerializedVarOutParam() {
   if (serialized_->inner_->serialization_rules()) {
     // When unset, OutParam wasn't called. We'll just leave the var untouched
     // in that case.
     serialized_->inner_->SetVar(
         serialized_->inner_->serialization_rules()->BeginSendPassRef(
-            writable_var_, serialized_->inner_->GetStringPtrPtr()));
+            writable_var_));
 
     // Normally the current object will be created on the stack to wrap a
     // SerializedVar and won't have a scope around the actual IPC send. So we
     // need to tell the SerializedVar to do the begin/end send pass ref calls.
     serialized_->inner_->SetCleanupModeToEndSendPassRef(dispatcher_);
   }
 }
 
 PP_Var* SerializedVarOutParam::OutParam(Dispatcher* dispatcher) {
   dispatcher_ = dispatcher;
@@ skipping 39 matching lines @@
 }
 
 SerializedVarTestConstructor::SerializedVarTestConstructor(
     const PP_Var& pod_var) {
   DCHECK(pod_var.type != PP_VARTYPE_STRING);
   inner_->ForceSetVarValueForTest(pod_var);
 }
 
 SerializedVarTestConstructor::SerializedVarTestConstructor(
     const std::string& str) {
-  PP_Var string_var = {};
-  string_var.type = PP_VARTYPE_STRING;
-  string_var.value.as_id = 0;
-  inner_->ForceSetVarValueForTest(string_var);
-  inner_->ForceSetStringValueForTest(str);
+  inner_->ForceSetVarValueForTest(StringVar::StringToPPVar(str));
 }
 
 SerializedVarTestReader::SerializedVarTestReader(const SerializedVar& var)
     : SerializedVar(var) {
 }
 
 }  // namespace proxy
 }  // namespace ppapi