WebSocket Pepper API: PPB_WebSocket::close must accept undefined reason

webkit/plugins/ppapi/ppb_websocket_impl.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 "webkit/plugins/ppapi/ppb_websocket_impl.h"
 
 #include <set>
 #include <string>
 
 #include "base/basictypes.h"
@@ skipping 205 matching lines @@
   if (code != static_cast<uint16_t>(WebSocket::CloseEventCodeNotSpecified)) {
     if (!(code == WebSocket::CloseEventCodeNormalClosure ||
         (WebSocket::CloseEventCodeMinimumUserDefined <= code &&
         code <= WebSocket::CloseEventCodeMaximumUserDefined)))
     // RFC 6455 limits applications to use reserved connection close code in
     // section 7.4.2.. The WebSocket API (http://www.w3.org/TR/websockets/)
     // defines this out of range error as InvalidAccessError in JavaScript.
     return PP_ERROR_NOACCESS;
   }
 
-  // Validate |reason|.
-  // TODO(toyoshim): Returns PP_ERROR_BADARGUMENT if |reason| contains any
-  // surrogates.
-  scoped_refptr<StringVar> reason_string = StringVar::FromPPVar(reason);
-  if (!reason_string || reason_string->value().size() > kMaxReasonSizeInBytes)
-    return PP_ERROR_BADARGUMENT;
+  scoped_refptr<StringVar> reason_string;
+  WebString web_reason;
+  // |reason| must be ignored if it is PP_VARTYPE_UNDEFINED.
+  if (reason.type != PP_VARTYPE_UNDEFINED) {
+    // Validate |reason|.
+    // TODO(toyoshim): Returns PP_ERROR_BADARGUMENT if |reason| contains any
+    // surrogates.

[dmichael (off chromium), 2012/04/23 15:14:46]

I'm not sure what you mean here, but I'm guessing it suffices to check that the
string is valid UTF-8? VarFromUTF8 does that, so by the time you get the PP_Var,
it's already validated. I think you can remove this comment.

[Takashi Toyoshima, 2012/04/24 06:44:57]

Thank you.
I check VarFromUTF8 implementation. As you said, it includes surrogates check.

FYI; Surrogate pair is defined in UTF-16 to expand the number of characters in
16-bit. It means UTF-16 has multi-word characters. These pairs consist of
special characters from 0xd800 to 0xdfff. UTF-8 must not include these codes
after it is decoded to Unicode.

+    reason_string = StringVar::FromPPVar(reason);
+    if (!reason_string ||
+        reason_string->value().size() > kMaxReasonSizeInBytes)
+      return PP_ERROR_BADARGUMENT;
+    web_reason = WebString::fromUTF8(reason_string->value());
+  }
 
   // Check state.
   if (state_ == PP_WEBSOCKETREADYSTATE_CLOSING ||
       state_ == PP_WEBSOCKETREADYSTATE_CLOSED)
     return PP_ERROR_INPROGRESS;
 
   // Validate |callback| (Doesn't support blocking callback)
   if (!callback.func)
     return PP_ERROR_BLOCKS_MAIN_THREAD;
 
@@ skipping 16 matching lines @@
     wait_for_receive_ = false;
     receive_callback_var_ = NULL;
 
     // Need to do a "Post" to avoid reentering the plugin.
     receive_callback_->PostAbort();
     receive_callback_ = NULL;
   }
 
   // Close connection.
   state_ = PP_WEBSOCKETREADYSTATE_CLOSING;
-  WebString web_reason = WebString::fromUTF8(reason_string->value());
   websocket_->close(code, web_reason);
 
   return PP_OK_COMPLETIONPENDING;
 }
 
 int32_t PPB_WebSocket_Impl::ReceiveMessage(PP_Var* message,
                                            PP_CompletionCallback callback) {
   // Check state.
   if (state_ == PP_WEBSOCKETREADYSTATE_INVALID ||
       state_ == PP_WEBSOCKETREADYSTATE_CONNECTING)
@@ skipping 245 matching lines @@
 
   *receive_callback_var_ = received_messages_.front()->GetPPVar();
   received_messages_.pop();
   receive_callback_var_ = NULL;
   wait_for_receive_ = false;
   return PP_OK;
 }
 
 }  // namespace ppapi
 }  // namespace webkit