shill: Remove uses of Base64Encode/Decode
Change to B64Encode/Decode to avoid memory leaks.
TEST=Unit tests pass
BUG=chromium:222827
Change-Id: I160fd8eae6d0d6d6307b1ab37b5d1029c3990abf
Reviewed-on: https://gerrit.chromium.org/gerrit/46172
Tested-by: Christopher Wiley <wiley@chromium.org>
Reviewed-by: Paul Stewart <pstew@chromium.org>
Commit-Queue: Christopher Wiley <wiley@chromium.org>
diff --git a/certificate_file.cc b/certificate_file.cc
index 2ec17e2..697399f 100644
--- a/certificate_file.cc
+++ b/certificate_file.cc
@@ -54,16 +54,12 @@
FilePath CertificateFile::CreateDERFromString(const string &pem_contents) {
string hex_data = ExtractHexData(pem_contents);
- gsize out_len = 0;
- guchar *data = glib_->Base64Decode(hex_data.c_str(), &out_len);
- if (!data || !out_len) {
+ string der_contents;
+ if (!glib_->B64Decode(hex_data, &der_contents)) {
LOG(ERROR) << "Could not decode hex data from input PEM";
- glib_->Free(data);
return FilePath();
}
- string der_contents(data, data + out_len);
- glib_->Free(data);
return WriteFile(der_contents);
}
diff --git a/certificate_file_unittest.cc b/certificate_file_unittest.cc
index 9c8dbfd..86202d0 100644
--- a/certificate_file_unittest.cc
+++ b/certificate_file_unittest.cc
@@ -97,17 +97,12 @@
TEST_F(CertificateFileTest, CreateDERFromString) {
// Create a DER file from the inner HEX data.
const string kPEMString = kPEMData;
- guchar fake_data[] = "this is a fake";
- const gsize kFakeLength = 5;
- const string kFakeData(fake_data, fake_data + kFakeLength);
- EXPECT_CALL(glib_, Base64Decode(StrEq(kPEMString), _))
- .WillOnce(Return(reinterpret_cast<guchar *>(NULL)))
- .WillOnce(DoAll(SetArgumentPointee<1>(0), Return(fake_data)))
- .WillOnce(DoAll(SetArgumentPointee<1>(kFakeLength),
- Return(fake_data)));
- EXPECT_CALL(glib_, Free(NULL)).Times(1);
- EXPECT_CALL(glib_, Free(fake_data)).Times(2);
- EXPECT_TRUE(certificate_file_.CreateDERFromString(kPEMData).empty());
+ const string fake_data("this is a fake");
+
+ EXPECT_CALL(glib_, B64Decode(StrEq(kPEMString), _))
+ .WillOnce(Return(false))
+ .WillOnce(DoAll(SetArgumentPointee<1>(fake_data),
+ Return(true)));
EXPECT_TRUE(certificate_file_.CreateDERFromString(kPEMData).empty());
FilePath outfile = certificate_file_.CreateDERFromString(kPEMData);
@@ -115,7 +110,7 @@
EXPECT_TRUE(file_util::PathExists(outfile));
string file_string;
EXPECT_TRUE(file_util::ReadFileToString(outfile, &file_string));
- EXPECT_EQ(kFakeData, file_string);
+ EXPECT_EQ(fake_data, file_string);
}
TEST_F(CertificateFileTest, ExtractHexData) {
diff --git a/crypto_des_cbc.cc b/crypto_des_cbc.cc
index 71ac52a..6dd09fb 100644
--- a/crypto_des_cbc.cc
+++ b/crypto_des_cbc.cc
@@ -44,15 +44,14 @@
b64_ciphertext.erase(0, strlen(kVersion2Prefix));
}
- gsize data_len = 0;
- guchar *gdata = glib_->Base64Decode(b64_ciphertext.c_str(), &data_len);
- if (!gdata) {
+ string decoded_data;
+ if (!glib_->B64Decode(b64_ciphertext, &decoded_data)) {
LOG(ERROR) << "Unable to base64-decode DEC-CBC ciphertext.";
return false;
}
- vector<char> data(gdata, gdata + data_len);
- glib_->Free(gdata);
+ vector<char> data(decoded_data.c_str(),
+ decoded_data.c_str() + decoded_data.length());
if (data.empty() || (data.size() % kBlockSize != 0)) {
LOG(ERROR) << "Invalid DES-CBC ciphertext size: " << data.size();
return false;
diff --git a/glib.cc b/glib.cc
index d5c0f91..f80a9d2 100644
--- a/glib.cc
+++ b/glib.cc
@@ -14,6 +14,7 @@
namespace shill {
+GLib::GLib() {}
GLib::~GLib() {}
std::string GLib::ConvertErrorToMessage(GError *error) {
@@ -26,18 +27,10 @@
return message;
}
-guchar *GLib::Base64Decode(const gchar *text, gsize *out_len) {
- return g_base64_decode(text, out_len);
-}
-
-gchar *GLib::Base64Encode(const guchar *data, gsize len) {
- return g_base64_encode(data, len);
-}
-
bool GLib::B64Decode(const string &input, string *output) {
CHECK(output);
gsize result_len = 0;
- guchar *result = Base64Decode(input.c_str(), &result_len);
+ guchar *result = g_base64_decode(input.c_str(), &result_len);
if (!result) {
LOG(ERROR) << "Failed in encoding.";
return false;
@@ -56,7 +49,7 @@
bool GLib::B64Encode(const string &input, string *output) {
CHECK(output);
- gchar *result = Base64Encode(
+ gchar *result = g_base64_encode(
reinterpret_cast<const unsigned char *>(input.c_str()), input.length());
if (!result) {
LOG(ERROR) << "Failed in encoding.";
diff --git a/glib.h b/glib.h
index 6e8e2cd..deb3496 100644
--- a/glib.h
+++ b/glib.h
@@ -10,21 +10,19 @@
#include <gio/gio.h>
#include <glib.h>
+#include <base/basictypes.h>
+
namespace shill {
// A GLib abstraction allowing GLib mocking in tests.
class GLib {
public:
+ GLib();
virtual ~GLib();
// Converts GLib's |error| to a string message and frees the GError object.
virtual std::string ConvertErrorToMessage(GError *error);
- // g_base64_decode
- virtual guchar *Base64Decode(const gchar *text, gsize *out_len);
- // g_base64_encode
- virtual gchar *Base64Encode(const guchar *data, gsize len);
-
// Thin wrappers around Base64Decode/Encode. Return true on success.
virtual bool B64Decode(const std::string &input, std::string *output);
virtual bool B64Encode(const std::string &input, std::string *output);
@@ -154,6 +152,9 @@
virtual void Strfreev(gchar **str_array);
// g_type_init
virtual void TypeInit();
+
+ private:
+ DISALLOW_COPY_AND_ASSIGN(GLib);
};
} // namespace shill
diff --git a/openvpn_management_server.cc b/openvpn_management_server.cc
index 10581f7..c170f21 100644
--- a/openvpn_management_server.cc
+++ b/openvpn_management_server.cc
@@ -274,20 +274,17 @@
driver_->FailService(Service::kFailureInternal, Service::kErrorDetailsNone);
return;
}
- gchar *b64_password =
- glib_->Base64Encode(reinterpret_cast<const guchar *>(password.data()),
- password.size());
- gchar *b64_otp =
- glib_->Base64Encode(reinterpret_cast<const guchar *>(otp.data()),
- otp.size());
- if (!b64_password || !b64_otp) {
+ string b64_password;
+ string b64_otp;
+ if (!glib_->B64Encode(password, &b64_password) ||
+ !glib_->B64Encode(otp, &b64_otp)) {
LOG(ERROR) << "Unable to base64-encode credentials.";
return;
}
SendUsername(tag, user);
- SendPassword(tag, StringPrintf("SCRV1:%s:%s", b64_password, b64_otp));
- glib_->Free(b64_otp);
- glib_->Free(b64_password);
+ SendPassword(tag, StringPrintf("SCRV1:%s:%s",
+ b64_password.c_str(),
+ b64_otp.c_str()));
// Don't reuse OTP.
driver_->args()->RemoveString(flimflam::kOpenVPNOTPProperty);
}