diff options
-rw-r--r-- | dbus/end_to_end_async_unittest.cc | 38 | ||||
-rw-r--r-- | dbus/end_to_end_sync_unittest.cc | 32 | ||||
-rw-r--r-- | dbus/exported_object.cc | 2 | ||||
-rw-r--r-- | dbus/message.cc | 44 | ||||
-rw-r--r-- | dbus/message.h | 12 | ||||
-rw-r--r-- | dbus/message_unittest.cc | 42 | ||||
-rw-r--r-- | dbus/object_proxy.cc | 25 |
7 files changed, 148 insertions, 47 deletions
diff --git a/dbus/end_to_end_async_unittest.cc b/dbus/end_to_end_async_unittest.cc index a4438e4..d5e278f 100644 --- a/dbus/end_to_end_async_unittest.cc +++ b/dbus/end_to_end_async_unittest.cc @@ -453,6 +453,44 @@ TEST_F(EndToEndAsyncTest, BrokenMethodWithErrorCallback) { ASSERT_EQ(DBUS_ERROR_FAILED, error_names_[0]); } +TEST_F(EndToEndAsyncTest, InvalidObjectPath) { + // Trailing '/' is only allowed for the root path. + const dbus::ObjectPath invalid_object_path("/org/chromium/TestObject/"); + + // Replace object proxy with new one. + object_proxy_ = bus_->GetObjectProxy("org.chromium.TestService", + invalid_object_path); + + dbus::MethodCall method_call("org.chromium.TestInterface", "Echo"); + + const int timeout_ms = dbus::ObjectProxy::TIMEOUT_USE_DEFAULT; + CallMethodWithErrorCallback(&method_call, timeout_ms); + WaitForErrors(1); + + // Should fail because of the invalid path. + ASSERT_TRUE(response_strings_.empty()); + ASSERT_EQ("", error_names_[0]); +} + +TEST_F(EndToEndAsyncTest, InvalidServiceName) { + // Bus name cannot contain '/'. + const std::string invalid_service_name = ":1/2"; + + // Replace object proxy with new one. + object_proxy_ = bus_->GetObjectProxy( + invalid_service_name, dbus::ObjectPath("org.chromium.TestObject")); + + dbus::MethodCall method_call("org.chromium.TestInterface", "Echo"); + + const int timeout_ms = dbus::ObjectProxy::TIMEOUT_USE_DEFAULT; + CallMethodWithErrorCallback(&method_call, timeout_ms); + WaitForErrors(1); + + // Should fail because of the invalid bus name. + ASSERT_TRUE(response_strings_.empty()); + ASSERT_EQ("", error_names_[0]); +} + TEST_F(EndToEndAsyncTest, EmptyResponseCallback) { const char* kHello = "hello"; diff --git a/dbus/end_to_end_sync_unittest.cc b/dbus/end_to_end_sync_unittest.cc index 26a9011..9cddea4 100644 --- a/dbus/end_to_end_sync_unittest.cc +++ b/dbus/end_to_end_sync_unittest.cc @@ -104,3 +104,35 @@ TEST_F(EndToEndSyncTest, BrokenMethod) { object_proxy_->CallMethodAndBlock(&method_call, timeout_ms)); ASSERT_FALSE(response.get()); } + +TEST_F(EndToEndSyncTest, InvalidObjectPath) { + // Trailing '/' is only allowed for the root path. + const dbus::ObjectPath invalid_object_path("/org/chromium/TestObject/"); + + // Replace object proxy with new one. + object_proxy_ = client_bus_->GetObjectProxy("org.chromium.TestService", + invalid_object_path); + + dbus::MethodCall method_call("org.chromium.TestInterface", "Echo"); + + const int timeout_ms = dbus::ObjectProxy::TIMEOUT_USE_DEFAULT; + scoped_ptr<dbus::Response> response( + object_proxy_->CallMethodAndBlock(&method_call, timeout_ms)); + ASSERT_FALSE(response.get()); +} + +TEST_F(EndToEndSyncTest, InvalidServiceName) { + // Bus name cannot contain '/'. + const std::string invalid_service_name = ":1/2"; + + // Replace object proxy with new one. + object_proxy_ = client_bus_->GetObjectProxy( + invalid_service_name, dbus::ObjectPath("org.chromium.TestObject")); + + dbus::MethodCall method_call("org.chromium.TestInterface", "Echo"); + + const int timeout_ms = dbus::ObjectProxy::TIMEOUT_USE_DEFAULT; + scoped_ptr<dbus::Response> response( + object_proxy_->CallMethodAndBlock(&method_call, timeout_ms)); + ASSERT_FALSE(response.get()); +} diff --git a/dbus/exported_object.cc b/dbus/exported_object.cc index 7c4bada..f9ddb62 100644 --- a/dbus/exported_object.cc +++ b/dbus/exported_object.cc @@ -90,7 +90,7 @@ void ExportedObject::ExportMethod(const std::string& interface_name, void ExportedObject::SendSignal(Signal* signal) { // For signals, the object path should be set to the path to the sender // object, which is this exported object here. - signal->SetPath(object_path_); + CHECK(signal->SetPath(object_path_)); // Increment the reference count so we can safely reference the // underlying signal message until the signal sending is complete. This diff --git a/dbus/message.cc b/dbus/message.cc index 400c1bc..5b45d42 100644 --- a/dbus/message.cc +++ b/dbus/message.cc @@ -249,40 +249,28 @@ std::string Message::ToString() { return headers + "\n" + ToStringInternal("", &reader); } -void Message::SetDestination(const std::string& destination) { - const bool success = dbus_message_set_destination(raw_message_, - destination.c_str()); - CHECK(success) << "Unable to allocate memory"; +bool Message::SetDestination(const std::string& destination) { + return dbus_message_set_destination(raw_message_, destination.c_str()); } -void Message::SetPath(const ObjectPath& path) { - const bool success = dbus_message_set_path(raw_message_, - path.value().c_str()); - CHECK(success) << "Unable to allocate memory"; +bool Message::SetPath(const ObjectPath& path) { + return dbus_message_set_path(raw_message_, path.value().c_str()); } -void Message::SetInterface(const std::string& interface) { - const bool success = dbus_message_set_interface(raw_message_, - interface.c_str()); - CHECK(success) << "Unable to allocate memory"; +bool Message::SetInterface(const std::string& interface) { + return dbus_message_set_interface(raw_message_, interface.c_str()); } -void Message::SetMember(const std::string& member) { - const bool success = dbus_message_set_member(raw_message_, - member.c_str()); - CHECK(success) << "Unable to allocate memory"; +bool Message::SetMember(const std::string& member) { + return dbus_message_set_member(raw_message_, member.c_str()); } -void Message::SetErrorName(const std::string& error_name) { - const bool success = dbus_message_set_error_name(raw_message_, - error_name.c_str()); - CHECK(success) << "Unable to allocate memory"; +bool Message::SetErrorName(const std::string& error_name) { + return dbus_message_set_error_name(raw_message_, error_name.c_str()); } -void Message::SetSender(const std::string& sender) { - const bool success = dbus_message_set_sender(raw_message_, - sender.c_str()); - CHECK(success) << "Unable to allocate memory"; +bool Message::SetSender(const std::string& sender) { + return dbus_message_set_sender(raw_message_, sender.c_str()); } void Message::SetSerial(uint32 serial) { @@ -345,8 +333,8 @@ MethodCall::MethodCall(const std::string& interface_name, : Message() { Init(dbus_message_new(DBUS_MESSAGE_TYPE_METHOD_CALL)); - SetInterface(interface_name); - SetMember(method_name); + CHECK(SetInterface(interface_name)); + CHECK(SetMember(method_name)); } MethodCall::MethodCall() : Message() { @@ -368,8 +356,8 @@ Signal::Signal(const std::string& interface_name, : Message() { Init(dbus_message_new(DBUS_MESSAGE_TYPE_SIGNAL)); - SetInterface(interface_name); - SetMember(method_name); + CHECK(SetInterface(interface_name)); + CHECK(SetMember(method_name)); } Signal::Signal() : Message() { diff --git a/dbus/message.h b/dbus/message.h index 75e4045..0b9834d 100644 --- a/dbus/message.h +++ b/dbus/message.h @@ -90,12 +90,12 @@ class Message { DBusMessage* raw_message() { return raw_message_; } // Sets the destination, the path, the interface, the member, etc. - void SetDestination(const std::string& destination); - void SetPath(const ObjectPath& path); - void SetInterface(const std::string& interface); - void SetMember(const std::string& member); - void SetErrorName(const std::string& error_name); - void SetSender(const std::string& sender); + bool SetDestination(const std::string& destination); + bool SetPath(const ObjectPath& path); + bool SetInterface(const std::string& interface); + bool SetMember(const std::string& member); + bool SetErrorName(const std::string& error_name); + bool SetSender(const std::string& sender); void SetSerial(uint32 serial); void SetReplySerial(uint32 reply_serial); // SetSignature() does not exist as we cannot do it. diff --git a/dbus/message_unittest.cc b/dbus/message_unittest.cc index 9d87c2a..d78d90f2 100644 --- a/dbus/message_unittest.cc +++ b/dbus/message_unittest.cc @@ -565,12 +565,12 @@ TEST(MessageTest, GetAndSetHeaders) { EXPECT_EQ(0U, message->GetSerial()); EXPECT_EQ(0U, message->GetReplySerial()); - message->SetDestination("org.chromium.destination"); - message->SetPath(dbus::ObjectPath("/org/chromium/path")); - message->SetInterface("org.chromium.interface"); - message->SetMember("member"); - message->SetErrorName("org.chromium.error"); - message->SetSender(":1.2"); + EXPECT_TRUE(message->SetDestination("org.chromium.destination")); + EXPECT_TRUE(message->SetPath(dbus::ObjectPath("/org/chromium/path"))); + EXPECT_TRUE(message->SetInterface("org.chromium.interface")); + EXPECT_TRUE(message->SetMember("member")); + EXPECT_TRUE(message->SetErrorName("org.chromium.error")); + EXPECT_TRUE(message->SetSender(":1.2")); message->SetSerial(123); message->SetReplySerial(456); @@ -583,3 +583,33 @@ TEST(MessageTest, GetAndSetHeaders) { EXPECT_EQ(123U, message->GetSerial()); EXPECT_EQ(456U, message->GetReplySerial()); } + +TEST(MessageTest, SetInvalidHeaders) { + scoped_ptr<dbus::Response> message(dbus::Response::CreateEmpty()); + EXPECT_EQ("", message->GetDestination()); + EXPECT_EQ(dbus::ObjectPath(""), message->GetPath()); + EXPECT_EQ("", message->GetInterface()); + EXPECT_EQ("", message->GetMember()); + EXPECT_EQ("", message->GetErrorName()); + EXPECT_EQ("", message->GetSender()); + + // Empty element between periods. + EXPECT_FALSE(message->SetDestination("org..chromium")); + // Trailing '/' is only allowed for the root path. + EXPECT_FALSE(message->SetPath(dbus::ObjectPath("/org/chromium/"))); + // Interface name cannot contain '/'. + EXPECT_FALSE(message->SetInterface("org/chromium/interface")); + // Member name cannot begin with a digit. + EXPECT_FALSE(message->SetMember("1member")); + // Error name cannot begin with a period. + EXPECT_FALSE(message->SetErrorName(".org.chromium.error")); + // Disallowed characters. + EXPECT_FALSE(message->SetSender("?!#*")); + + EXPECT_EQ("", message->GetDestination()); + EXPECT_EQ(dbus::ObjectPath(""), message->GetPath()); + EXPECT_EQ("", message->GetInterface()); + EXPECT_EQ("", message->GetMember()); + EXPECT_EQ("", message->GetErrorName()); + EXPECT_EQ("", message->GetSender()); +} diff --git a/dbus/object_proxy.cc b/dbus/object_proxy.cc index 051d8f1..4d7aa25 100644 --- a/dbus/object_proxy.cc +++ b/dbus/object_proxy.cc @@ -63,11 +63,11 @@ Response* ObjectProxy::CallMethodAndBlock(MethodCall* method_call, int timeout_ms) { bus_->AssertOnDBusThread(); - if (!bus_->Connect()) + if (!bus_->Connect() || + !method_call->SetDestination(service_name_) || + !method_call->SetPath(object_path_)) return NULL; - method_call->SetDestination(service_name_); - method_call->SetPath(object_path_); DBusMessage* request_message = method_call->raw_message(); ScopedDBusError error; @@ -108,15 +108,28 @@ void ObjectProxy::CallMethodWithErrorCallback(MethodCall* method_call, ErrorCallback error_callback) { bus_->AssertOnOriginThread(); - method_call->SetDestination(service_name_); - method_call->SetPath(object_path_); + const base::TimeTicks start_time = base::TimeTicks::Now(); + + if (!method_call->SetDestination(service_name_) || + !method_call->SetPath(object_path_)) { + // In case of a failure, run the error callback with NULL. + DBusMessage* response_message = NULL; + base::Closure task = base::Bind(&ObjectProxy::RunResponseCallback, + this, + callback, + error_callback, + start_time, + response_message); + bus_->PostTaskToOriginThread(FROM_HERE, task); + return; + } + // Increment the reference count so we can safely reference the // underlying request message until the method call is complete. This // will be unref'ed in StartAsyncMethodCall(). DBusMessage* request_message = method_call->raw_message(); dbus_message_ref(request_message); - const base::TimeTicks start_time = base::TimeTicks::Now(); base::Closure task = base::Bind(&ObjectProxy::StartAsyncMethodCall, this, timeout_ms, |