Skip to content

Commit

Permalink
The comment in base64.h implies that base::Base64Encode() can return …
Browse files Browse the repository at this point in the history
…false, but

this cannot happen in practice. Fix the comment.

The implementation of Base64Encode() attempts to check for the return value
MODP_B64_ERROR as a failure, but modp_b64_encode() cannot return this
value. Remove the check.

Remove unneeded integer cast.

Change the return type to void.

BUG=323357
TEST=base_unittests, compile all
TBR=jochen@chromium.org,miket@chromium.org,joi@chromium.org,akalin@chromium.org,sergeyu@chromium.org

Review URL: https://codereview.chromium.org/86913002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@239759 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
ricea@chromium.org committed Dec 10, 2013
1 parent 9377d2a commit 0553072
Show file tree
Hide file tree
Showing 67 changed files with 125 additions and 252 deletions.
10 changes: 2 additions & 8 deletions base/base64.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,21 +8,15 @@

namespace base {

bool Base64Encode(const StringPiece& input, std::string* output) {
void Base64Encode(const StringPiece& input, std::string* output) {
std::string temp;
temp.resize(modp_b64_encode_len(input.size())); // makes room for null byte

// null terminates result since result is base64 text!
int input_size = static_cast<int>(input.size());

// modp_b64_encode_len() returns at least 1, so temp[0] is safe to use.
size_t output_size = modp_b64_encode(&(temp[0]), input.data(), input_size);
if (output_size == MODP_B64_ERROR)
return false;
size_t output_size = modp_b64_encode(&(temp[0]), input.data(), input.size());

temp.resize(output_size); // strips off null byte
output->swap(temp);
return true;
}

bool Base64Decode(const StringPiece& input, std::string* output) {
Expand Down
5 changes: 2 additions & 3 deletions base/base64.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,8 @@

namespace base {

// Encodes the input string in base64. Returns true if successful and false
// otherwise. The output string is only modified if successful.
BASE_EXPORT bool Base64Encode(const StringPiece& input, std::string* output);
// Encodes the input string in base64.
BASE_EXPORT void Base64Encode(const StringPiece& input, std::string* output);

// Decodes the base64 input string. Returns true if successful and false
// otherwise. The output string is only modified if successful.
Expand Down
3 changes: 1 addition & 2 deletions base/base64_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,7 @@ TEST(Base64Test, Basic) {
std::string decoded;
bool ok;

ok = Base64Encode(kText, &encoded);
EXPECT_TRUE(ok);
Base64Encode(kText, &encoded);
EXPECT_EQ(kBase64Text, encoded);

ok = Base64Decode(encoded, &decoded);
Expand Down
7 changes: 3 additions & 4 deletions chrome/browser/bookmarks/bookmark_html_writer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -270,10 +270,9 @@ class Writer : public base::RefCountedThreadSafe<Writer> {
favicon_data.assign(reinterpret_cast<const char*>(data->front()),
data->size());
std::string favicon_base64_encoded;
if (base::Base64Encode(favicon_data, &favicon_base64_encoded)) {
GURL favicon_url("data:image/png;base64," + favicon_base64_encoded);
favicon_string = favicon_url.spec();
}
base::Base64Encode(favicon_data, &favicon_base64_encoded);
GURL favicon_url("data:image/png;base64," + favicon_base64_encoded);
favicon_string = favicon_url.spec();
}

if (!WriteIndent() ||
Expand Down
5 changes: 1 addition & 4 deletions chrome/browser/chromeos/settings/device_settings_cache.cc
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,7 @@ bool Store(const em::PolicyData& policy, PrefService* local_state) {
if (local_state) {
std::string policy_string = policy.SerializeAsString();
std::string encoded;
if (!base::Base64Encode(policy_string, &encoded)) {
LOG(ERROR) << "Can't encode policy in base64.";
return false;
}
base::Base64Encode(policy_string, &encoded);
local_state->SetString(prefs::kDeviceSettingsCache, encoded);
return true;
}
Expand Down
7 changes: 3 additions & 4 deletions chrome/browser/devtools/adb/android_rsa.cc
Original file line number Diff line number Diff line change
Expand Up @@ -196,10 +196,9 @@ crypto::RSAPrivateKey* AndroidRSAPrivateKey(Profile* profile) {
return NULL;

std::string key_string(key_info.begin(), key_info.end());
if (base::Base64Encode(key_string, &encoded_key)) {
profile->GetPrefs()->SetString(prefs::kDevToolsAdbKey,
encoded_key);
}
base::Base64Encode(key_string, &encoded_key);
profile->GetPrefs()->SetString(prefs::kDevToolsAdbKey,
encoded_key);
}
return key.release();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,7 @@ ExtensionUpdater* GetExtensionUpdater(Profile* profile) {

GURL GetImageURLFromData(std::string contents) {
std::string contents_base64;
if (!base::Base64Encode(contents, &contents_base64))
return GURL();
base::Base64Encode(contents, &contents_base64);

// TODO(dvh): make use of chrome::kDataScheme. Filed as crbug/297301.
const char kDataURLPrefix[] = "data:image;base64,";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -371,11 +371,7 @@ void EPKPChallengeMachineKey::SignChallengeCallback(
}

std::string encoded_response;
if (!base::Base64Encode(response, &encoded_response)) {
SetError(kResponseBadBase64Error);
SendResponse(false);
return;
}
base::Base64Encode(response, &encoded_response);

results_ = api_epkp::ChallengeMachineKey::Results::Create(encoded_response);
SendResponse(true);
Expand Down Expand Up @@ -544,11 +540,7 @@ void EPKPChallengeUserKey::RegisterKeyCallback(
}

std::string encoded_response;
if (!base::Base64Encode(response, &encoded_response)) {
SetError(kResponseBadBase64Error);
SendResponse(false);
return;
}
base::Base64Encode(response, &encoded_response);

results_ = api_epkp::ChallengeUserKey::Results::Create(encoded_response);
SendResponse(true);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,7 @@ std::string RepresentationToString(const gfx::ImageSkia& image, float scale) {
std::string raw_str(static_cast<const char*>(bitmap_pickle.data()),
bitmap_pickle.size());
std::string base64_str;
if (!base::Base64Encode(raw_str, &base64_str))
return std::string();
base::Base64Encode(raw_str, &base64_str);
return base64_str;
}

Expand Down
3 changes: 1 addition & 2 deletions chrome/browser/extensions/api/identity/web_auth_flow.cc
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,7 @@ void WebAuthFlow::Start() {
// in OnShellWindowAdded.
std::string random_bytes;
crypto::RandBytes(WriteInto(&random_bytes, 33), 32);
bool success = base::Base64Encode(random_bytes, &shell_window_key_);
DCHECK(success);
base::Base64Encode(random_bytes, &shell_window_key_);

// identityPrivate.onWebFlowRequest(shell_window_key, provider_url_, mode_)
scoped_ptr<base::ListValue> args(new base::ListValue());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,10 +79,7 @@ class CryptoVerifyImpl : public NetworkingPrivateServiceClient::CryptoVerify {
return;
}

if (!base::Base64Encode(ciphertext, base64_encoded_ciphertext)) {
*error = "EncodeError";
return;
}
base::Base64Encode(ciphertext, base64_encoded_ciphertext);
}
};

Expand Down
3 changes: 1 addition & 2 deletions chrome/browser/extensions/api/proxy/proxy_api_helpers.cc
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,7 @@ bool CreateDataURLFromPACScript(const std::string& pac_script,
std::string* pac_script_url_base64_encoded) {
// Encode pac_script in base64.
std::string pac_script_base64_encoded;
if (!base::Base64Encode(pac_script, &pac_script_base64_encoded))
return false;
base::Base64Encode(pac_script, &pac_script_base64_encoded);

// Make it a correct data url.
*pac_script_url_base64_encoded =
Expand Down
17 changes: 8 additions & 9 deletions chrome/browser/extensions/extension_protocols.cc
Original file line number Diff line number Diff line change
Expand Up @@ -86,15 +86,14 @@ net::HttpResponseHeaders* BuildHttpHeaders(
last_modified_time.ToInternalValue());
hash = base::SHA1HashString(hash);
std::string etag;
if (base::Base64Encode(hash, &etag)) {
raw_headers.append(1, '\0');
raw_headers.append("ETag: \"");
raw_headers.append(etag);
raw_headers.append("\"");
// Also force revalidation.
raw_headers.append(1, '\0');
raw_headers.append("cache-control: no-cache");
}
base::Base64Encode(hash, &etag);
raw_headers.append(1, '\0');
raw_headers.append("ETag: \"");
raw_headers.append(etag);
raw_headers.append("\"");
// Also force revalidation.
raw_headers.append(1, '\0');
raw_headers.append("cache-control: no-cache");
}

raw_headers.append(2, '\0');
Expand Down
3 changes: 2 additions & 1 deletion chrome/browser/extensions/install_signer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,8 @@ bool HashWithMachineId(const std::string& salt, std::string* result) {
std::string result_bytes(crypto::kSHA256Length, 0);
hash->Finish(string_as_array(&result_bytes), result_bytes.size());

return base::Base64Encode(result_bytes, result);
base::Base64Encode(result_bytes, result);
return true;
}

} // namespace
Expand Down
5 changes: 1 addition & 4 deletions chrome/browser/internal_auth.cc
Original file line number Diff line number Diff line change
Expand Up @@ -166,10 +166,7 @@ void CreatePassport(const std::string& domain,
return;
}
std::string hmac_base64;
if (!base::Base64Encode(hmac, &hmac_base64)) {
NOTREACHED();
return;
}
base::Base64Encode(hmac, &hmac_base64);
if (hmac_base64.size() != BASE64_PER_RAW(kHMACSizeInBytes)) {
NOTREACHED();
return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -150,9 +150,7 @@ ManagedUserRegistrationUtility::Create(Profile* profile) {
// static
std::string ManagedUserRegistrationUtility::GenerateNewManagedUserId() {
std::string new_managed_user_id;
bool success = base::Base64Encode(base::RandBytesAsString(8),
&new_managed_user_id);
DCHECK(success);
base::Base64Encode(base::RandBytesAsString(8), &new_managed_user_id);
return new_managed_user_id;
}

Expand Down
6 changes: 2 additions & 4 deletions chrome/browser/media/desktop_streams_registry.cc
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,8 @@ std::string GenerateRandomStreamId() {
char buffer[kStreamIdLengthBytes];
crypto::RandBytes(buffer, arraysize(buffer));
std::string result;
if (!base::Base64Encode(base::StringPiece(buffer, arraysize(buffer)),
&result)) {
LOG(FATAL) << "Base64Encode failed.";
}
base::Base64Encode(base::StringPiece(buffer, arraysize(buffer)),
&result);
return result;
}

Expand Down
4 changes: 2 additions & 2 deletions chrome/browser/metrics/metrics_log.cc
Original file line number Diff line number Diff line change
Expand Up @@ -803,8 +803,8 @@ void MetricsLog::RecordEnvironment(

std::string serialied_system_profile;
std::string base64_system_profile;
if (system_profile->SerializeToString(&serialied_system_profile) &&
base::Base64Encode(serialied_system_profile, &base64_system_profile)) {
if (system_profile->SerializeToString(&serialied_system_profile)) {
base::Base64Encode(serialied_system_profile, &base64_system_profile);
PrefService* local_state = GetPrefService();
local_state->SetString(prefs::kStabilitySavedSystemProfile,
base64_system_profile);
Expand Down
5 changes: 1 addition & 4 deletions chrome/browser/metrics/metrics_log_serializer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -141,10 +141,7 @@ void MetricsLogSerializer::WriteLogsToPrefList(
it != local_list.end(); ++it) {
// We encode the compressed log as Value::CreateStringValue() expects to
// take a valid UTF8 string.
if (!base::Base64Encode(it->log_text(), &encoded_log)) {
list->Clear();
return;
}
base::Base64Encode(it->log_text(), &encoded_log);
base::MD5Update(&ctx, encoded_log);
list->Append(Value::CreateStringValue(encoded_log));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -163,16 +163,12 @@ void VariationsHttpHeaderProvider::UpdateVariationIDsHeaderValue() {
proto.SerializeToString(&serialized);

std::string hashed;
if (base::Base64Encode(serialized, &hashed)) {
// If successful, swap the header value with the new one.
// Note that the list of IDs and the header could be temporarily out of sync
// if IDs are added as the header is recreated. The receiving servers are OK
// with such discrepancies.
variation_ids_header_ = hashed;
} else {
NOTREACHED() << "Failed to base64 encode Variation IDs value: "
<< serialized;
}
base::Base64Encode(serialized, &hashed);
// If successful, swap the header value with the new one.
// Note that the list of IDs and the header could be temporarily out of sync
// if IDs are added as the header is recreated. The receiving servers are OK
// with such discrepancies.
variation_ids_header_ = hashed;
}

// static
Expand Down
6 changes: 1 addition & 5 deletions chrome/browser/metrics/variations/variations_service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -481,11 +481,7 @@ bool VariationsService::StoreSeedData(const std::string& seed_data,
}

std::string base64_seed_data;
if (!base::Base64Encode(seed_data, &base64_seed_data)) {
VLOG(1) << "Variations Seed data from server fails Base64Encode, rejecting "
<< "the seed.";
return false;
}
base::Base64Encode(seed_data, &base64_seed_data);

local_state_->SetString(prefs::kVariationsSeed, base64_seed_data);
local_state_->SetString(prefs::kVariationsSeedHash, HashSeed(seed_data));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ std::string SerializeSeedBase64(const VariationsSeed& seed, std::string* hash) {
*hash = base::HexEncode(sha1.data(), sha1.size());
}
std::string base64_serialized_seed;
EXPECT_TRUE(base::Base64Encode(serialized_seed, &base64_serialized_seed));
base::Base64Encode(serialized_seed, &base64_serialized_seed);
return base64_serialized_seed;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,9 @@ const char kTestPolicy2JSON[] = "{\"Another\":\"turn_it_off\"}";

// Same encoding as ResourceCache does for its keys.
bool Base64Encode(const std::string& value, std::string* encoded) {
if (value.empty() || !base::Base64Encode(value, encoded))
if (value.empty())
return false;
base::Base64Encode(value, encoded);
base::ReplaceChars(*encoded, "+", "-", encoded);
base::ReplaceChars(*encoded, "/", "_", encoded);
return true;
Expand Down
9 changes: 2 additions & 7 deletions chrome/browser/sync/profile_sync_service_harness.cc
Original file line number Diff line number Diff line change
Expand Up @@ -929,13 +929,8 @@ bool ProfileSyncServiceHarness::MatchesOtherClient(
if (marker != partner_marker) {
if (VLOG_IS_ON(2)) {
std::string marker_base64, partner_marker_base64;
if (!base::Base64Encode(marker, &marker_base64)) {
NOTREACHED();
}
if (!base::Base64Encode(
partner_marker, &partner_marker_base64)) {
NOTREACHED();
}
base::Base64Encode(marker, &marker_base64);
base::Base64Encode(partner_marker, &partner_marker_base64);
DVLOG(2) << syncer::ModelTypeToString(i.Get()) << ": "
<< profile_debug_name_ << " progress marker = "
<< marker_base64 << ", "
Expand Down
5 changes: 1 addition & 4 deletions chrome/browser/ui/ash/screenshot_taker.cc
Original file line number Diff line number Diff line change
Expand Up @@ -71,10 +71,7 @@ void CopyScreenshotToClipboard(scoped_refptr<base::RefCountedString> png_data) {
DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));

std::string encoded;
if (!base::Base64Encode(png_data->data(), &encoded)) {
LOG(ERROR) << "Failed to encode base64";
return;
}
base::Base64Encode(png_data->data(), &encoded);

// Only cares about HTML because ChromeOS doesn't need other formats.
// TODO(dcheng): Why don't we take advantage of the ability to write bitmaps
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2611,8 +2611,7 @@ void AutofillDialogControllerImpl::OnDidLoadRiskFingerprintData(

std::string proto_data;
fingerprint->SerializeToString(&proto_data);
bool success = base::Base64Encode(proto_data, &risk_data_);
DCHECK(success);
base::Base64Encode(proto_data, &risk_data_);

SubmitWithWallet();
}
Expand Down
6 changes: 1 addition & 5 deletions chrome/browser/ui/certificate_dialogs.cc
Original file line number Diff line number Diff line change
Expand Up @@ -49,11 +49,7 @@ std::string WrapAt64(const std::string &str) {

std::string GetBase64String(net::X509Certificate::OSCertHandle cert) {
std::string base64;
if (!base::Base64Encode(
x509_certificate_model::GetDerString(cert), &base64)) {
LOG(ERROR) << "base64 encoding error";
return std::string();
}
base::Base64Encode(x509_certificate_model::GetDerString(cert), &base64);
return "-----BEGIN CERTIFICATE-----\r\n" +
WrapAt64(base64) +
"-----END CERTIFICATE-----\r\n";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1172,9 +1172,7 @@ void PrintPreviewHandler::SendCloudPrintJob(const base::RefCountedBytes* data) {
std::string raw_data(reinterpret_cast<const char*>(data->front()),
data->size());
std::string base64_data;
if (!base::Base64Encode(raw_data, &base64_data)) {
NOTREACHED() << "Base64 encoding PDF data.";
}
base::Base64Encode(raw_data, &base64_data);
StringValue data_value(base64_data);

web_ui()->CallJavascriptFunction("printToCloud", data_value);
Expand Down
5 changes: 1 addition & 4 deletions chrome/common/metrics/caching_permuted_entropy_provider.cc
Original file line number Diff line number Diff line change
Expand Up @@ -66,10 +66,7 @@ void CachingPermutedEntropyProvider::UpdateLocalState() const {
cache_.SerializeToString(&serialized);

std::string base64_encoded;
if (!base::Base64Encode(serialized, &base64_encoded)) {
NOTREACHED();
return;
}
base::Base64Encode(serialized, &base64_encoded);
local_state_->SetString(prefs::kMetricsPermutedEntropyCache, base64_encoded);
}

Expand Down
3 changes: 1 addition & 2 deletions chrome/test/chromedriver/chrome_launcher.cc
Original file line number Diff line number Diff line change
Expand Up @@ -497,8 +497,7 @@ Status ProcessExtension(const std::string& extension,
if (key_len != public_key.size())
return Status(kUnknownError, "invalid public key length");
std::string public_key_base64;
if (!base::Base64Encode(public_key, &public_key_base64))
return Status(kUnknownError, "cannot base64 encode public key");
base::Base64Encode(public_key, &public_key_base64);
std::string id = GenerateExtensionId(public_key);

// Unzip the crx file.
Expand Down
Loading

0 comments on commit 0553072

Please sign in to comment.