summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--dbus/end_to_end_async_unittest.cc38
-rw-r--r--dbus/end_to_end_sync_unittest.cc32
-rw-r--r--dbus/exported_object.cc2
-rw-r--r--dbus/message.cc44
-rw-r--r--dbus/message.h12
-rw-r--r--dbus/message_unittest.cc42
-rw-r--r--dbus/object_proxy.cc25
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,