From 922c8e9024248a33ee5236432ae8c62c56b5977c Mon Sep 17 00:00:00 2001 From: "lambroslambrou@chromium.org" Date: Thu, 30 May 2013 05:20:29 +0000 Subject: unittests for Chromoting native messaging host. BUG=232200 Review URL: https://chromiumcodereview.appspot.com/14979008 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@203084 0039d316-1c4b-4281-b951-d872f2087c98 --- remoting/host/setup/native_messaging_host.cc | 9 +- remoting/host/setup/native_messaging_host.h | 10 +- remoting/host/setup/native_messaging_host_main.cc | 3 +- .../host/setup/native_messaging_host_unittest.cc | 517 +++++++++++++++++++++ remoting/host/setup/native_messaging_reader.cc | 31 +- .../host/setup/native_messaging_reader_unittest.cc | 163 +++++++ .../host/setup/native_messaging_writer_unittest.cc | 115 +++++ remoting/host/setup/test_util.cc | 32 ++ remoting/host/setup/test_util.h | 20 + remoting/remoting.gyp | 20 +- 10 files changed, 893 insertions(+), 27 deletions(-) create mode 100644 remoting/host/setup/native_messaging_host_unittest.cc create mode 100644 remoting/host/setup/native_messaging_reader_unittest.cc create mode 100644 remoting/host/setup/native_messaging_writer_unittest.cc create mode 100644 remoting/host/setup/test_util.cc create mode 100644 remoting/host/setup/test_util.h (limited to 'remoting') diff --git a/remoting/host/setup/native_messaging_host.cc b/remoting/host/setup/native_messaging_host.cc index 7ce09ea..28a0b10 100644 --- a/remoting/host/setup/native_messaging_host.cc +++ b/remoting/host/setup/native_messaging_host.cc @@ -37,6 +37,7 @@ scoped_ptr ConfigDictionaryFromMessage( namespace remoting { NativeMessagingHost::NativeMessagingHost( + scoped_ptr daemon_controller, base::PlatformFile input, base::PlatformFile output, scoped_refptr caller_task_runner, @@ -45,7 +46,7 @@ NativeMessagingHost::NativeMessagingHost( quit_closure_(quit_closure), native_messaging_reader_(input), native_messaging_writer_(output), - daemon_controller_(DaemonController::Create()), + daemon_controller_(daemon_controller.Pass()), weak_factory_(this) { weak_ptr_ = weak_factory_.GetWeakPtr(); } @@ -70,10 +71,16 @@ void NativeMessagingHost::Shutdown() { void NativeMessagingHost::ProcessMessage(scoped_ptr message) { DCHECK(caller_task_runner_->BelongsToCurrentThread()); + + // Don't process any more messages if Shutdown() has been called. + if (quit_closure_.is_null()) + return; + const base::DictionaryValue* message_dict; if (!message->GetAsDictionary(&message_dict)) { LOG(ERROR) << "Expected DictionaryValue"; Shutdown(); + return; } scoped_ptr response_dict(new base::DictionaryValue()); diff --git a/remoting/host/setup/native_messaging_host.h b/remoting/host/setup/native_messaging_host.h index 4bfd09a..9c375ad 100644 --- a/remoting/host/setup/native_messaging_host.h +++ b/remoting/host/setup/native_messaging_host.h @@ -26,6 +26,7 @@ namespace remoting { class NativeMessagingHost { public: NativeMessagingHost( + scoped_ptr daemon_controller, base::PlatformFile input, base::PlatformFile output, scoped_refptr caller_task_runner, @@ -79,11 +80,10 @@ class NativeMessagingHost { DaemonController::AsyncResult result); void SendConfigResponse(scoped_ptr response, scoped_ptr config); - void SendUsageStatsConsentResponse( - scoped_ptr response, - bool supported, - bool allowed, - bool set_by_policy); + void SendUsageStatsConsentResponse(scoped_ptr response, + bool supported, + bool allowed, + bool set_by_policy); void SendAsyncResult(scoped_ptr response, DaemonController::AsyncResult result); diff --git a/remoting/host/setup/native_messaging_host_main.cc b/remoting/host/setup/native_messaging_host_main.cc index 49eee24..35cad5e 100644 --- a/remoting/host/setup/native_messaging_host_main.cc +++ b/remoting/host/setup/native_messaging_host_main.cc @@ -33,7 +33,8 @@ int main(int argc, char** argv) { base::MessageLoop message_loop(base::MessageLoop::TYPE_IO); base::RunLoop run_loop; - remoting::NativeMessagingHost host(read_file, write_file, + remoting::NativeMessagingHost host(remoting::DaemonController::Create(), + read_file, write_file, message_loop.message_loop_proxy(), run_loop.QuitClosure()); host.Start(); diff --git a/remoting/host/setup/native_messaging_host_unittest.cc b/remoting/host/setup/native_messaging_host_unittest.cc new file mode 100644 index 0000000..b6fc4ab --- /dev/null +++ b/remoting/host/setup/native_messaging_host_unittest.cc @@ -0,0 +1,517 @@ +// Copyright 2013 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 "remoting/host/setup/native_messaging_host.h" + +#include "base/compiler_specific.h" +#include "base/json/json_reader.h" +#include "base/json/json_writer.h" +#include "base/message_loop.h" +#include "base/run_loop.h" +#include "base/stl_util.h" +#include "base/strings/stringize_macros.h" +#include "base/values.h" +#include "net/base/file_stream.h" +#include "net/base/net_util.h" +#include "remoting/host/pin_hash.h" +#include "remoting/host/setup/test_util.h" +#include "testing/gtest/include/gtest/gtest.h" + +namespace { + +void VerifyHelloResponse(const base::DictionaryValue* response) { + ASSERT_TRUE(response); + std::string value; + EXPECT_TRUE(response->GetString("type", &value)); + EXPECT_EQ("helloResponse", value); + EXPECT_TRUE(response->GetString("version", &value)); + EXPECT_EQ(STRINGIZE(VERSION), value); +} + +void VerifyGetHostNameResponse(const base::DictionaryValue* response) { + ASSERT_TRUE(response); + std::string value; + EXPECT_TRUE(response->GetString("type", &value)); + EXPECT_EQ("getHostNameResponse", value); + EXPECT_TRUE(response->GetString("hostname", &value)); + EXPECT_EQ(net::GetHostName(), value); +} + +void VerifyGetPinHashResponse(const base::DictionaryValue* response) { + ASSERT_TRUE(response); + std::string value; + EXPECT_TRUE(response->GetString("type", &value)); + EXPECT_EQ("getPinHashResponse", value); + EXPECT_TRUE(response->GetString("hash", &value)); + EXPECT_EQ(remoting::MakeHostPinHash("my_host", "1234"), value); +} + +void VerifyGenerateKeyPairResponse(const base::DictionaryValue* response) { + ASSERT_TRUE(response); + std::string value; + EXPECT_TRUE(response->GetString("type", &value)); + EXPECT_EQ("generateKeyPairResponse", value); + EXPECT_TRUE(response->GetString("private_key", &value)); + EXPECT_TRUE(response->GetString("public_key", &value)); +} + +void VerifyGetDaemonConfigResponse(const base::DictionaryValue* response) { + ASSERT_TRUE(response); + std::string value; + EXPECT_TRUE(response->GetString("type", &value)); + EXPECT_EQ("getDaemonConfigResponse", value); + const base::DictionaryValue* config = NULL; + EXPECT_TRUE(response->GetDictionary("config", &config)); + EXPECT_TRUE(base::DictionaryValue().Equals(config)); +} + +void VerifyGetUsageStatsConsentResponse(const base::DictionaryValue* response) { + ASSERT_TRUE(response); + std::string value; + EXPECT_TRUE(response->GetString("type", &value)); + EXPECT_EQ("getUsageStatsConsentResponse", value); + bool supported, allowed, set_by_policy; + EXPECT_TRUE(response->GetBoolean("supported", &supported)); + EXPECT_TRUE(response->GetBoolean("allowed", &allowed)); + EXPECT_TRUE(response->GetBoolean("set_by_policy", &set_by_policy)); + EXPECT_TRUE(supported); + EXPECT_TRUE(allowed); + EXPECT_TRUE(set_by_policy); +} + +void VerifyStopDaemonResponse(const base::DictionaryValue* response) { + ASSERT_TRUE(response); + std::string value; + EXPECT_TRUE(response->GetString("type", &value)); + EXPECT_EQ("stopDaemonResponse", value); + int result; + EXPECT_TRUE(response->GetInteger("result", &result)); + EXPECT_EQ(0, result); +} + +void VerifyGetDaemonStateResponse(const base::DictionaryValue* response) { + ASSERT_TRUE(response); + std::string value; + EXPECT_TRUE(response->GetString("type", &value)); + EXPECT_EQ("getDaemonStateResponse", value); + int result; + EXPECT_TRUE(response->GetInteger("state", &result)); + EXPECT_EQ(4, result); +} + +void VerifyUpdateDaemonConfigResponse(const base::DictionaryValue* response) { + ASSERT_TRUE(response); + std::string value; + EXPECT_TRUE(response->GetString("type", &value)); + EXPECT_EQ("updateDaemonConfigResponse", value); + int result; + EXPECT_TRUE(response->GetInteger("result", &result)); + EXPECT_EQ(0, result); +} + +void VerifyStartDaemonResponse(const base::DictionaryValue* response) { + ASSERT_TRUE(response); + std::string value; + EXPECT_TRUE(response->GetString("type", &value)); + EXPECT_EQ("startDaemonResponse", value); + int result; + EXPECT_TRUE(response->GetInteger("result", &result)); + EXPECT_EQ(0, result); +} + +} // namespace + +namespace remoting { + +class MockDaemonController : public DaemonController { + public: + MockDaemonController(); + virtual ~MockDaemonController(); + + virtual State GetState() OVERRIDE; + virtual void GetConfig(const GetConfigCallback& callback) OVERRIDE; + virtual void SetConfigAndStart(scoped_ptr config, + bool consent, + const CompletionCallback& callback) OVERRIDE; + virtual void UpdateConfig(scoped_ptr config, + const CompletionCallback& callback) OVERRIDE; + virtual void Stop(const CompletionCallback& callback) OVERRIDE; + virtual void SetWindow(void* window_handle) OVERRIDE; + virtual void GetVersion(const GetVersionCallback& callback) OVERRIDE; + virtual void GetUsageStatsConsent( + const GetUsageStatsConsentCallback& callback) OVERRIDE; + + // Returns a record of functions called, so that unittests can verify these + // were called in the proper sequence. + std::string call_log() { return call_log_; } + + private: + std::string call_log_; + + DISALLOW_COPY_AND_ASSIGN(MockDaemonController); +}; + +MockDaemonController::MockDaemonController() {} + +MockDaemonController::~MockDaemonController() {} + +DaemonController::State MockDaemonController::GetState() { + call_log_ += "GetState:"; + return DaemonController::STATE_STARTED; +} + +void MockDaemonController::GetConfig(const GetConfigCallback& callback) { + call_log_ += "GetConfig:"; + scoped_ptr config(new base::DictionaryValue()); + callback.Run(config.Pass()); +} + +void MockDaemonController::SetConfigAndStart( + scoped_ptr config, bool consent, + const CompletionCallback& callback) { + call_log_ += "SetConfigAndStart:"; + + // Verify parameters passed in. + if (consent && config && config->HasKey("start")) { + callback.Run(DaemonController::RESULT_OK); + } else { + callback.Run(DaemonController::RESULT_FAILED); + } +} + +void MockDaemonController::UpdateConfig( + scoped_ptr config, + const CompletionCallback& callback) { + call_log_ += "UpdateConfig:"; + if (config && config->HasKey("update")) { + callback.Run(DaemonController::RESULT_OK); + } else { + callback.Run(DaemonController::RESULT_FAILED); + } +} + +void MockDaemonController::Stop(const CompletionCallback& callback) { + call_log_ += "Stop:"; + callback.Run(DaemonController::RESULT_OK); +} + +void MockDaemonController::SetWindow(void* window_handle) {} + +void MockDaemonController::GetVersion(const GetVersionCallback& callback) { + // Unused - NativeMessagingHost returns the compiled-in version string + // instead of calling this method. +} + +void MockDaemonController::GetUsageStatsConsent( + const GetUsageStatsConsentCallback& callback) { + call_log_ += "GetUsageStatsConsent:"; + callback.Run(true, true, true); +} + +class NativeMessagingHostTest : public testing::Test { + public: + NativeMessagingHostTest(); + virtual ~NativeMessagingHostTest(); + + virtual void SetUp() OVERRIDE; + virtual void TearDown() OVERRIDE; + + void Run(); + + scoped_ptr ReadMessageFromOutputPipe(); + + void WriteMessageToInputPipe(const base::Value& message); + + // The Host process should shut down when it receives a malformed request. + // This is tested by sending a known-good request, followed by |message|, + // followed by the known-good request again. The response file should only + // contain a single response from the first good request. + void TestBadRequest(const base::Value& message); + + protected: + // Reference to the MockDaemonController, which is owned by |host_|. + MockDaemonController* daemon_controller_; + std::string call_log_; + + private: + // Each test creates two unidirectional pipes: "input" and "output". + // NativeMessagingHost reads from input_read_handle and writes to + // output_write_handle. The unittest supplies data to input_write_handle, and + // verifies output from output_read_handle. + // + // unittest -> [input] -> NativeMessagingHost -> [output] -> unittest + base::PlatformFile input_read_handle_; + base::PlatformFile input_write_handle_; + base::PlatformFile output_read_handle_; + base::PlatformFile output_write_handle_; + + base::MessageLoop message_loop_; + base::RunLoop run_loop_; + scoped_ptr host_; + + DISALLOW_COPY_AND_ASSIGN(NativeMessagingHostTest); +}; + +NativeMessagingHostTest::NativeMessagingHostTest() + : message_loop_(base::MessageLoop::TYPE_IO) {} + +NativeMessagingHostTest::~NativeMessagingHostTest() {} + +void NativeMessagingHostTest::SetUp() { + ASSERT_TRUE(MakePipe(&input_read_handle_, &input_write_handle_)); + ASSERT_TRUE(MakePipe(&output_read_handle_, &output_write_handle_)); + + daemon_controller_ = new MockDaemonController(); + scoped_ptr daemon_controller(daemon_controller_); + host_.reset(new NativeMessagingHost(daemon_controller.Pass(), + input_read_handle_, output_write_handle_, + message_loop_.message_loop_proxy(), + run_loop_.QuitClosure())); +} + +void NativeMessagingHostTest::TearDown() { + // The NativeMessagingHost dtor closes the handles that are passed to it. + // |input_write_handle_| gets closed just before starting the host. So the + // only handle left to close is |output_read_handle_|. + base::ClosePlatformFile(output_read_handle_); +} + +void NativeMessagingHostTest::Run() { + // Close the write-end of input, so that the host sees EOF after reading + // messages and won't block waiting for more input. + base::ClosePlatformFile(input_write_handle_); + host_->Start(); + run_loop_.Run(); + + // Destroy |host_| so that it closes its end of the output pipe, so that + // TestBadRequest() will see EOF and won't block waiting for more data. + // Since |host_| owns |daemon_controller_|, capture its call log first. + call_log_ = daemon_controller_->call_log(); + host_.reset(NULL); +} + +scoped_ptr +NativeMessagingHostTest::ReadMessageFromOutputPipe() { + uint32 length; + int read_result = base::ReadPlatformFileAtCurrentPos( + output_read_handle_, reinterpret_cast(&length), sizeof(length)); + if (read_result != sizeof(length)) { + return scoped_ptr(); + } + + std::string message_json(length, '\0'); + read_result = base::ReadPlatformFileAtCurrentPos( + output_read_handle_, string_as_array(&message_json), length); + if (read_result != static_cast(length)) { + return scoped_ptr(); + } + + scoped_ptr message(base::JSONReader::Read(message_json)); + if (!message || !message->IsType(base::Value::TYPE_DICTIONARY)) { + return scoped_ptr(); + } + + return scoped_ptr( + static_cast(message.release())); +} + +void NativeMessagingHostTest::WriteMessageToInputPipe( + const base::Value& message) { + std::string message_json; + base::JSONWriter::Write(&message, &message_json); + + uint32 length = message_json.length(); + base::WritePlatformFileAtCurrentPos(input_write_handle_, + reinterpret_cast(&length), + sizeof(length)); + base::WritePlatformFileAtCurrentPos(input_write_handle_, message_json.data(), + length); +} + +void NativeMessagingHostTest::TestBadRequest(const base::Value& message) { + base::DictionaryValue good_message; + good_message.SetString("type", "hello"); + + WriteMessageToInputPipe(good_message); + WriteMessageToInputPipe(message); + WriteMessageToInputPipe(good_message); + + Run(); + + // Read from output pipe, and verify responses. + scoped_ptr response = + ReadMessageFromOutputPipe(); + VerifyHelloResponse(response.get()); + + response = ReadMessageFromOutputPipe(); + EXPECT_FALSE(response); +} + +// Test all valid request-types. +TEST_F(NativeMessagingHostTest, All) { + base::DictionaryValue message; + message.SetString("type", "hello"); + WriteMessageToInputPipe(message); + + message.SetString("type", "getHostName"); + WriteMessageToInputPipe(message); + + message.SetString("type", "getPinHash"); + message.SetString("hostId", "my_host"); + message.SetString("pin", "1234"); + WriteMessageToInputPipe(message); + + message.Clear(); + message.SetString("type", "generateKeyPair"); + WriteMessageToInputPipe(message); + + message.SetString("type", "getDaemonConfig"); + WriteMessageToInputPipe(message); + + message.SetString("type", "getUsageStatsConsent"); + WriteMessageToInputPipe(message); + + message.SetString("type", "stopDaemon"); + WriteMessageToInputPipe(message); + + message.SetString("type", "getDaemonState"); + WriteMessageToInputPipe(message); + + // Following messages require a "config" dictionary. + base::DictionaryValue config; + config.SetBoolean("update", true); + message.Set("config", config.DeepCopy()); + message.SetString("type", "updateDaemonConfig"); + WriteMessageToInputPipe(message); + + config.Clear(); + config.SetBoolean("start", true); + message.Set("config", config.DeepCopy()); + message.SetBoolean("consent", true); + message.SetString("type", "startDaemon"); + WriteMessageToInputPipe(message); + + Run(); + + // Read from output pipe, and verify responses. + scoped_ptr response = ReadMessageFromOutputPipe(); + VerifyHelloResponse(response.get()); + + response = ReadMessageFromOutputPipe(); + VerifyGetHostNameResponse(response.get()); + + response = ReadMessageFromOutputPipe(); + VerifyGetPinHashResponse(response.get()); + + response = ReadMessageFromOutputPipe(); + VerifyGenerateKeyPairResponse(response.get()); + + response = ReadMessageFromOutputPipe(); + VerifyGetDaemonConfigResponse(response.get()); + + response = ReadMessageFromOutputPipe(); + VerifyGetUsageStatsConsentResponse(response.get()); + + response = ReadMessageFromOutputPipe(); + VerifyStopDaemonResponse(response.get()); + + response = ReadMessageFromOutputPipe(); + VerifyGetDaemonStateResponse(response.get()); + + response = ReadMessageFromOutputPipe(); + VerifyUpdateDaemonConfigResponse(response.get()); + + response = ReadMessageFromOutputPipe(); + VerifyStartDaemonResponse(response.get()); + + // Verify that DaemonController methods were called in the correct sequence. + // This detects cases where NativeMessagingHost might call a wrong method + // that takes the same parameters and writes out the same response. + EXPECT_EQ("GetConfig:GetUsageStatsConsent:Stop:GetState:UpdateConfig:" + "SetConfigAndStart:", call_log_); +} + +// Verify that response ID matches request ID. +TEST_F(NativeMessagingHostTest, Id) { + base::DictionaryValue message; + message.SetString("type", "hello"); + WriteMessageToInputPipe(message); + message.SetString("id", "42"); + WriteMessageToInputPipe(message); + + Run(); + + scoped_ptr response = + ReadMessageFromOutputPipe(); + EXPECT_TRUE(response); + std::string value; + EXPECT_FALSE(response->GetString("id", &value)); + + response = ReadMessageFromOutputPipe(); + EXPECT_TRUE(response); + EXPECT_TRUE(response->GetString("id", &value)); + EXPECT_EQ("42", value); +} + +// Verify non-Dictionary requests are rejected. +TEST_F(NativeMessagingHostTest, WrongFormat) { + base::ListValue message; + TestBadRequest(message); +} + +// Verify requests with no type are rejected. +TEST_F(NativeMessagingHostTest, MissingType) { + base::DictionaryValue message; + TestBadRequest(message); +} + +// Verify rejection if type is unrecognized. +TEST_F(NativeMessagingHostTest, InvalidType) { + base::DictionaryValue message; + message.SetString("type", "xxx"); + TestBadRequest(message); +} + +// Verify rejection if getPinHash request has no hostId. +TEST_F(NativeMessagingHostTest, GetPinHashNoHostId) { + base::DictionaryValue message; + message.SetString("type", "getPinHash"); + message.SetString("pin", "1234"); + TestBadRequest(message); +} + +// Verify rejection if getPinHash request has no pin. +TEST_F(NativeMessagingHostTest, GetPinHashNoPin) { + base::DictionaryValue message; + message.SetString("type", "getPinHash"); + message.SetString("hostId", "my_host"); + TestBadRequest(message); +} + +// Verify rejection if updateDaemonConfig request has invalid config. +TEST_F(NativeMessagingHostTest, UpdateDaemonConfigInvalidConfig) { + base::DictionaryValue message; + message.SetString("type", "updateDaemonConfig"); + message.SetString("config", "xxx"); + TestBadRequest(message); +} + +// Verify rejection if startDaemon request has invalid config. +TEST_F(NativeMessagingHostTest, StartDaemonInvalidConfig) { + base::DictionaryValue message; + message.SetString("type", "startDaemon"); + message.SetString("config", "xxx"); + message.SetBoolean("consent", true); + TestBadRequest(message); +} + +// Verify rejection if startDaemon request has no "consent" parameter. +TEST_F(NativeMessagingHostTest, StartDaemonNoConsent) { + base::DictionaryValue message; + message.SetString("type", "startDaemon"); + message.Set("config", base::DictionaryValue().DeepCopy()); + TestBadRequest(message); +} + +} // namespace remoting diff --git a/remoting/host/setup/native_messaging_reader.cc b/remoting/host/setup/native_messaging_reader.cc index e98ac52..7151653 100644 --- a/remoting/host/setup/native_messaging_reader.cc +++ b/remoting/host/setup/native_messaging_reader.cc @@ -73,8 +73,7 @@ NativeMessagingReader::Core::Core( read_task_runner_(read_task_runner) { } -NativeMessagingReader::Core::~Core() { -} +NativeMessagingReader::Core::~Core() {} void NativeMessagingReader::Core::ReadMessage() { DCHECK(read_task_runner_->RunsTasksOnCurrentThread()); @@ -85,8 +84,11 @@ void NativeMessagingReader::Core::ReadMessage() { int read_result = read_stream_.ReadUntilComplete( reinterpret_cast(&message_length), kMessageHeaderSize); if (read_result != kMessageHeaderSize) { - LOG(ERROR) << "Failed to read message header, read returned " - << read_result; + // 0 means EOF which is normal and should not be logged as an error. + if (read_result != 0) { + LOG(ERROR) << "Failed to read message header, read returned " + << read_result; + } NotifyEof(); return; } @@ -98,8 +100,8 @@ void NativeMessagingReader::Core::ReadMessage() { } std::string message_json(message_length, '\0'); - read_result = read_stream_.ReadUntilComplete( - string_as_array(&message_json), message_length); + read_result = read_stream_.ReadUntilComplete(string_as_array(&message_json), + message_length); if (read_result != static_cast(message_length)) { LOG(ERROR) << "Failed to read message body, read returned " << read_result; @@ -115,16 +117,17 @@ void NativeMessagingReader::Core::ReadMessage() { } // Notify callback of new message. - caller_task_runner_->PostTask(FROM_HERE, base::Bind( - &NativeMessagingReader::InvokeMessageCallback, reader_, - base::Passed(&message))); + caller_task_runner_->PostTask( + FROM_HERE, base::Bind(&NativeMessagingReader::InvokeMessageCallback, + reader_, base::Passed(&message))); } } void NativeMessagingReader::Core::NotifyEof() { DCHECK(read_task_runner_->RunsTasksOnCurrentThread()); - caller_task_runner_->PostTask(FROM_HERE, base::Bind( - &NativeMessagingReader::InvokeEofCallback, reader_)); + caller_task_runner_->PostTask( + FROM_HERE, + base::Bind(&NativeMessagingReader::InvokeEofCallback, reader_)); } NativeMessagingReader::NativeMessagingReader(base::PlatformFile handle) @@ -147,9 +150,9 @@ void NativeMessagingReader::Start(MessageCallback message_callback, // base::Unretained is safe since |core_| is only deleted via the // DeleteSoon task which is posted from this class's dtor. - read_task_runner_->PostTask(FROM_HERE, base::Bind( - &NativeMessagingReader::Core::ReadMessage, - base::Unretained(core_.get()))); + read_task_runner_->PostTask( + FROM_HERE, base::Bind(&NativeMessagingReader::Core::ReadMessage, + base::Unretained(core_.get()))); } void NativeMessagingReader::InvokeMessageCallback( diff --git a/remoting/host/setup/native_messaging_reader_unittest.cc b/remoting/host/setup/native_messaging_reader_unittest.cc new file mode 100644 index 0000000..a33ca3b --- /dev/null +++ b/remoting/host/setup/native_messaging_reader_unittest.cc @@ -0,0 +1,163 @@ +// Copyright 2013 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 "remoting/host/setup/native_messaging_reader.h" + +#include "base/basictypes.h" +#include "base/bind.h" +#include "base/memory/scoped_ptr.h" +#include "base/message_loop.h" +#include "base/platform_file.h" +#include "base/run_loop.h" +#include "base/values.h" +#include "remoting/host/setup/test_util.h" +#include "testing/gtest/include/gtest/gtest.h" + +namespace remoting { + +class NativeMessagingReaderTest : public testing::Test { + public: + NativeMessagingReaderTest(); + virtual ~NativeMessagingReaderTest(); + + virtual void SetUp() OVERRIDE; + virtual void TearDown() OVERRIDE; + + // Starts the reader and runs the MessageLoop to completion. + void Run(); + + // MessageCallback passed to the Reader. Stores |message| so it can be + // verified by tests. + void OnMessage(scoped_ptr message); + + // Writes a message (header+body) to the write-end of the pipe. + void WriteMessage(std::string message); + + // Writes some data to the write-end of the pipe. + void WriteData(const char* data, int length); + + protected: + scoped_ptr reader_; + base::PlatformFile read_handle_; + base::PlatformFile write_handle_; + scoped_ptr message_; + + private: + // MessageLoop declared here, since the NativeMessageReader ctor requires a + // MessageLoop to have been created. + base::MessageLoop message_loop_; + base::RunLoop run_loop_; +}; + +NativeMessagingReaderTest::NativeMessagingReaderTest() + : message_loop_(base::MessageLoop::TYPE_IO) { +} + +NativeMessagingReaderTest::~NativeMessagingReaderTest() {} + +void NativeMessagingReaderTest::SetUp() { + ASSERT_TRUE(MakePipe(&read_handle_, &write_handle_)); + reader_.reset(new NativeMessagingReader(read_handle_)); +} + +void NativeMessagingReaderTest::TearDown() { + // |read_handle_| is owned by NativeMessagingReader's FileStream, so don't + // try to close it here. Also, |write_handle_| gets closed by Run(). +} + +void NativeMessagingReaderTest::Run() { + // Close the write-end, so the reader doesn't block waiting for more data. + base::ClosePlatformFile(write_handle_); + + // base::Unretained is safe since no further tasks can run after + // RunLoop::Run() returns. + reader_->Start( + base::Bind(&NativeMessagingReaderTest::OnMessage, base::Unretained(this)), + run_loop_.QuitClosure()); + run_loop_.Run(); +} + +void NativeMessagingReaderTest::OnMessage(scoped_ptr message) { + message_ = message.Pass(); +} + +void NativeMessagingReaderTest::WriteMessage(std::string message) { + uint32 length = message.length(); + WriteData(reinterpret_cast(&length), 4); + WriteData(message.data(), length); +} + +void NativeMessagingReaderTest::WriteData(const char* data, int length) { + int written = base::WritePlatformFileAtCurrentPos(write_handle_, data, + length); + ASSERT_EQ(length, written); +} + +TEST_F(NativeMessagingReaderTest, GoodMessage) { + WriteMessage("{\"foo\": 42}"); + Run(); + EXPECT_TRUE(message_); + base::DictionaryValue* message_dict; + EXPECT_TRUE(message_->GetAsDictionary(&message_dict)); + int result; + EXPECT_TRUE(message_dict->GetInteger("foo", &result)); + EXPECT_EQ(42, result); +} + +TEST_F(NativeMessagingReaderTest, InvalidLength) { + uint32 length = 0xffffffff; + WriteData(reinterpret_cast(&length), 4); + Run(); + EXPECT_FALSE(message_); +} + +TEST_F(NativeMessagingReaderTest, EmptyFile) { + Run(); + EXPECT_FALSE(message_); +} + +TEST_F(NativeMessagingReaderTest, ShortHeader) { + // Write only 3 bytes - the message length header is supposed to be 4 bytes. + WriteData("xxx", 3); + Run(); + EXPECT_FALSE(message_); +} + +TEST_F(NativeMessagingReaderTest, EmptyBody) { + uint32 length = 1; + WriteData(reinterpret_cast(&length), 4); + Run(); + EXPECT_FALSE(message_); +} + +TEST_F(NativeMessagingReaderTest, ShortBody) { + uint32 length = 2; + WriteData(reinterpret_cast(&length), 4); + + // Only write 1 byte, where the header indicates there should be 2 bytes. + WriteData("x", 1); + Run(); + EXPECT_FALSE(message_); +} + +TEST_F(NativeMessagingReaderTest, InvalidJSON) { + std::string text = "{"; + WriteMessage(text); + Run(); + EXPECT_FALSE(message_); +} + +TEST_F(NativeMessagingReaderTest, SecondMessage) { + WriteMessage("{}"); + WriteMessage("{\"foo\": 42}"); + Run(); + EXPECT_TRUE(message_); + base::DictionaryValue* message_dict; + EXPECT_TRUE(message_->GetAsDictionary(&message_dict)); + int result; + EXPECT_TRUE(message_dict->GetInteger("foo", &result)); + EXPECT_EQ(42, result); +} + +} // namespace remoting diff --git a/remoting/host/setup/native_messaging_writer_unittest.cc b/remoting/host/setup/native_messaging_writer_unittest.cc new file mode 100644 index 0000000..c0cfbd1 --- /dev/null +++ b/remoting/host/setup/native_messaging_writer_unittest.cc @@ -0,0 +1,115 @@ +// Copyright 2013 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 "remoting/host/setup/native_messaging_writer.h" + +#include "base/basictypes.h" +#include "base/json/json_reader.h" +#include "base/memory/scoped_ptr.h" +#include "base/platform_file.h" +#include "base/stl_util.h" +#include "base/values.h" +#include "remoting/host/setup/test_util.h" +#include "testing/gtest/include/gtest/gtest.h" + +namespace remoting { + +class NativeMessagingWriterTest : public testing::Test { + public: + NativeMessagingWriterTest(); + virtual ~NativeMessagingWriterTest(); + + virtual void SetUp() OVERRIDE; + virtual void TearDown() OVERRIDE; + + protected: + scoped_ptr writer_; + base::PlatformFile read_handle_; + base::PlatformFile write_handle_; + bool read_handle_open_; +}; + +NativeMessagingWriterTest::NativeMessagingWriterTest() {} + +NativeMessagingWriterTest::~NativeMessagingWriterTest() {} + +void NativeMessagingWriterTest::SetUp() { + ASSERT_TRUE(MakePipe(&read_handle_, &write_handle_)); + writer_.reset(new NativeMessagingWriter(write_handle_)); + read_handle_open_ = true; +} + +void NativeMessagingWriterTest::TearDown() { + // |write_handle_| is owned by NativeMessagingWriter's FileStream, so don't + // try to close it here. And close |read_handle_| only if it hasn't + // already been closed. + if (read_handle_open_) + base::ClosePlatformFile(read_handle_); +} + +TEST_F(NativeMessagingWriterTest, GoodMessage) { + base::DictionaryValue message; + message.SetInteger("foo", 42); + EXPECT_TRUE(writer_->WriteMessage(message)); + + // Read from the pipe and verify the content. + uint32 length; + int read = base::ReadPlatformFileAtCurrentPos( + read_handle_, reinterpret_cast(&length), 4); + EXPECT_EQ(4, read); + std::string content(length, '\0'); + read = base::ReadPlatformFileAtCurrentPos(read_handle_, + string_as_array(&content), length); + EXPECT_EQ(static_cast(length), read); + + // |content| should now contain serialized |message|. + scoped_ptr written_message(base::JSONReader::Read(content)); + EXPECT_TRUE(message.Equals(written_message.get())); + + // Nothing more should have been written. Close the write-end of the pipe, + // and verify the read end immediately hits EOF. + writer_.reset(NULL); + char unused; + read = base::ReadPlatformFileAtCurrentPos(read_handle_, &unused, 1); + EXPECT_LE(read, 0); +} + +TEST_F(NativeMessagingWriterTest, SecondMessage) { + base::DictionaryValue message1; + base::DictionaryValue message2; + message2.SetInteger("foo", 42); + EXPECT_TRUE(writer_->WriteMessage(message1)); + EXPECT_TRUE(writer_->WriteMessage(message2)); + writer_.reset(NULL); + + // Read two messages. + uint32 length; + int read; + std::string content; + for (int i = 0; i < 2; i++) { + read = base::ReadPlatformFileAtCurrentPos( + read_handle_, reinterpret_cast(&length), 4); + EXPECT_EQ(4, read) << "i = " << i; + content.resize(length); + read = base::ReadPlatformFileAtCurrentPos(read_handle_, + string_as_array(&content), + length); + EXPECT_EQ(static_cast(length), read) << "i = " << i; + } + + // |content| should now contain serialized |message2|. + scoped_ptr written_message2(base::JSONReader::Read(content)); + EXPECT_TRUE(message2.Equals(written_message2.get())); +} + +TEST_F(NativeMessagingWriterTest, FailedWrite) { + // Close the read end so that writing fails immediately. + base::ClosePlatformFile(read_handle_); + read_handle_open_ = false; + + base::DictionaryValue message; + EXPECT_FALSE(writer_->WriteMessage(message)); +} + +} // namespace remoting diff --git a/remoting/host/setup/test_util.cc b/remoting/host/setup/test_util.cc new file mode 100644 index 0000000..9266c74 --- /dev/null +++ b/remoting/host/setup/test_util.cc @@ -0,0 +1,32 @@ +// Copyright 2013 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 "remoting/host/setup/test_util.h" + +#if defined(OS_WIN) +#include +#elif defined(OS_POSIX) +#include +#endif + +namespace remoting { + +bool MakePipe(base::PlatformFile* read_handle, + base::PlatformFile* write_handle) { +#if defined(OS_WIN) + return CreatePipe(read_handle, write_handle, NULL, 0) != FALSE; +#elif defined(OS_POSIX) + int fds[2]; + if (pipe(fds) == 0) { + *read_handle = fds[0]; + *write_handle = fds[1]; + return true; + } + return false; +#else +#error Not implemented +#endif +} + +} // namepsace remoting diff --git a/remoting/host/setup/test_util.h b/remoting/host/setup/test_util.h new file mode 100644 index 0000000..77a46ab --- /dev/null +++ b/remoting/host/setup/test_util.h @@ -0,0 +1,20 @@ +// Copyright 2013 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. + +#ifndef REMOTING_HOST_SETUP_TEST_UTIL_H_ +#define REMOTING_HOST_SETUP_TEST_UTIL_H_ + +#include "base/platform_file.h" + +namespace remoting { + +// Creates an anonymous, unidirectional pipe, returning true if successful. On +// success, the caller is responsible for closing both ends using +// base::ClosePlatformFile(). +bool MakePipe(base::PlatformFile* read_handle, + base::PlatformFile* write_handle); + +} // namespace remoting + +#endif // REMOTING_HOST_SETUP_TEST_UTIL_H_ diff --git a/remoting/remoting.gyp b/remoting/remoting.gyp index f99e399..7dfc95d 100644 --- a/remoting/remoting.gyp +++ b/remoting/remoting.gyp @@ -612,6 +612,9 @@ '../google_apis/google_apis.gyp:google_apis', 'remoting_host', ], + 'defines': [ + 'VERSION=<(version_full)', + ], 'sources': [ 'host/setup/daemon_controller.h', 'host/setup/daemon_controller_linux.cc', @@ -621,10 +624,18 @@ 'host/setup/daemon_installer_win.h', 'host/setup/host_starter.cc', 'host/setup/host_starter.h', + 'host/setup/native_messaging_host.cc', + 'host/setup/native_messaging_host.h', + 'host/setup/native_messaging_reader.cc', + 'host/setup/native_messaging_reader.h', + 'host/setup/native_messaging_writer.cc', + 'host/setup/native_messaging_writer.h', 'host/setup/oauth_helper.cc', 'host/setup/oauth_helper.h', 'host/setup/pin_validator.cc', 'host/setup/pin_validator.h', + 'host/setup/test_util.cc', + 'host/setup/test_util.h', 'host/setup/win/auth_code_getter.cc', 'host/setup/win/auth_code_getter.h', ], @@ -735,13 +746,7 @@ 'VERSION=<(version_full)', ], 'sources': [ - 'host/setup/native_messaging_host.cc', - 'host/setup/native_messaging_host.h', 'host/setup/native_messaging_host_main.cc', - 'host/setup/native_messaging_reader.cc', - 'host/setup/native_messaging_reader.h', - 'host/setup/native_messaging_writer.cc', - 'host/setup/native_messaging_writer.h', ], 'conditions': [ ['OS=="linux" and linux_use_tcmalloc==1', { @@ -2672,6 +2677,9 @@ 'host/resizing_host_observer_unittest.cc', 'host/screen_resolution_unittest.cc', 'host/server_log_entry_unittest.cc', + 'host/setup/native_messaging_host_unittest.cc', + 'host/setup/native_messaging_reader_unittest.cc', + 'host/setup/native_messaging_writer_unittest.cc', 'host/setup/oauth_helper_unittest.cc', 'host/setup/pin_validator_unittest.cc', 'host/token_validator_factory_impl_unittest.cc', -- cgit v1.1