Skip to content

Commit

Permalink
[Code Health] Remove uses of Value::GetAsDictionary() in remoting/
Browse files Browse the repository at this point in the history 
Bug: 1187011
Change-Id: Ibecabce4df1cd169bbdb5bbb0ae6ffc2951657e1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2875960
Auto-Submit: Austin Sullivan <asully@chromium.org>
Commit-Queue: Yuwei Huang <yuweih@chromium.org>
Reviewed-by: Yuwei Huang <yuweih@chromium.org>
Cr-Commit-Position: refs/heads/master@{#880632}
  • Loading branch information
@a-sully
a-sully authored and Chromium LUCI CQ committed May 7, 2021
1 parent 6159a88 commit c8edc4c
Show file tree
Hide file tree
Showing 5 changed files with 63 additions and 55 deletions.
6 changes: 3 additions & 3 deletions remoting/host/pairing_registry_delegate_linux.cc
View file
Expand Up @@ -98,13 +98,13 @@ PairingRegistry::Pairing PairingRegistryDelegateLinux::Load(
return PairingRegistry::Pairing();
}

base::DictionaryValue* pairing_dictionary;
if (!pairing->GetAsDictionary(&pairing_dictionary)) {
if (!pairing->is_dict()) {
LOG(WARNING) << "Failed to parse pairing information: not a dictionary.";
return PairingRegistry::Pairing();
}

return PairingRegistry::Pairing::CreateFromValue(*pairing_dictionary);
return PairingRegistry::Pairing::CreateFromValue(
base::Value::AsDictionaryValue(*pairing));
}

bool PairingRegistryDelegateLinux::Save(
Expand Down
49 changes: 28 additions & 21 deletions remoting/host/security_key/security_key_extension_session.cc
View file
Expand Up @@ -4,6 +4,7 @@

#include "remoting/host/security_key/security_key_extension_session.h"

#include <string>
#include <utility>

#include "base/bind.h"
Expand Down Expand Up @@ -42,18 +43,18 @@ unsigned int GetCommandCode(const std::string& data) {

// Creates a string of byte data from a ListValue of numbers. Returns true if
// all of the list elements are numbers.
bool ConvertListValueToString(base::ListValue* bytes, std::string* out) {
bool ConvertListToString(const base::Value& bytes, std::string* out) {
out->clear();

unsigned int byte_count = bytes->GetSize();
unsigned int byte_count = bytes.GetList().size();
if (byte_count != 0) {
out->reserve(byte_count);
for (unsigned int i = 0; i < byte_count; i++) {
int value;
if (!bytes->GetInteger(i, &value)) {
auto value = bytes.GetList()[i].GetIfInt();
if (!value.has_value()) {
return false;
}
out->push_back(static_cast<char>(value));
out->push_back(static_cast<char>(value.value()));
}
}
return true;
Expand Down Expand Up @@ -97,18 +98,19 @@ bool SecurityKeyExtensionSession::OnExtensionMessage(

std::unique_ptr<base::Value> value =
base::JSONReader::ReadDeprecated(message.data());
base::DictionaryValue* client_message;
if (!value || !value->GetAsDictionary(&client_message)) {
if (!value || !value->is_dict()) {
LOG(WARNING) << "Failed to retrieve data from gnubby-auth message.";
return true;
}

std::string type;
if (!client_message->GetString(kMessageType, &type)) {
std::string* maybe_type = value->FindStringKey(kMessageType);
if (!maybe_type) {
LOG(WARNING) << "Invalid gnubby-auth message format.";
return true;
}
std::string type = *maybe_type;

base::Value::DictStorage client_message = value->TakeDict();
if (type == kControlMessage) {
ProcessControlMessage(client_message);
} else if (type == kDataMessage) {
Expand All @@ -123,12 +125,13 @@ bool SecurityKeyExtensionSession::OnExtensionMessage(
}

void SecurityKeyExtensionSession::ProcessControlMessage(
base::DictionaryValue* message_data) const {
std::string option;
if (!message_data->GetString(kControlOption, &option)) {
const base::Value::DictStorage& message_data) const {
auto option_iter = message_data.find(kControlOption);
if (option_iter == message_data.end() || !option_iter->second.is_string()) {
LOG(WARNING) << "Could not extract control option from message.";
return;
}
auto option = option_iter->second.GetString();

if (option == kSecurityKeyAuthV1) {
security_key_auth_handler_->CreateSecurityKeyConnection();
Expand All @@ -138,23 +141,25 @@ void SecurityKeyExtensionSession::ProcessControlMessage(
}

void SecurityKeyExtensionSession::ProcessDataMessage(
base::DictionaryValue* message_data) const {
int connection_id;
if (!message_data->GetInteger(kConnectionId, &connection_id)) {
const base::Value::DictStorage& message_data) const {
auto connection_id_iter = message_data.find(kConnectionId);
if (connection_id_iter == message_data.end() ||
!connection_id_iter->second.is_int()) {
LOG(WARNING) << "Could not extract connection id from message.";
return;
}
auto connection_id = connection_id_iter->second.GetInt();

if (!security_key_auth_handler_->IsValidConnectionId(connection_id)) {
LOG(WARNING) << "Unknown gnubby-auth data connection: '" << connection_id
<< "'";
return;
}

base::ListValue* bytes;
std::string response;
if (message_data->GetList(kDataPayload, &bytes) &&
ConvertListValueToString(bytes, &response)) {
auto bytes_iter = message_data.find(kDataPayload);
if (bytes_iter != message_data.end() &&
ConvertListToString(bytes_iter->second, &response)) {
HOST_LOG << "Sending security key response: " << GetCommandCode(response);
security_key_auth_handler_->SendClientResponse(connection_id, response);
} else {
Expand All @@ -165,12 +170,14 @@ void SecurityKeyExtensionSession::ProcessDataMessage(
}

void SecurityKeyExtensionSession::ProcessErrorMessage(
base::DictionaryValue* message_data) const {
int connection_id;
if (!message_data->GetInteger(kConnectionId, &connection_id)) {
const base::Value::DictStorage& message_data) const {
auto connection_id_iter = message_data.find(kConnectionId);
if (connection_id_iter == message_data.end() ||
!connection_id_iter->second.is_int()) {
LOG(WARNING) << "Could not extract connection id from message.";
return;
}
auto connection_id = connection_id_iter->second.GetInt();

if (security_key_auth_handler_->IsValidConnectionId(connection_id)) {
HOST_LOG << "Sending security key error";
Expand Down
9 changes: 5 additions & 4 deletions remoting/host/security_key/security_key_extension_session.h
View file
Expand Up @@ -12,10 +12,10 @@
#include "base/macros.h"
#include "base/memory/ref_counted.h"
#include "base/threading/thread_checker.h"
#include "base/values.h"
#include "remoting/host/host_extension_session.h"

namespace base {
class DictionaryValue;
class SingleThreadTaskRunner;
} // namespace base

Expand Down Expand Up @@ -48,9 +48,10 @@ class SecurityKeyExtensionSession : public HostExtensionSession {

private:
// These methods process specific security key extension message types.
void ProcessControlMessage(base::DictionaryValue* message_data) const;
void ProcessDataMessage(base::DictionaryValue* message_data) const;
void ProcessErrorMessage(base::DictionaryValue* message_data) const;
void ProcessControlMessage(
const base::Value::DictStorage& message_data) const;
void ProcessDataMessage(const base::Value::DictStorage& message_data) const;
void ProcessErrorMessage(const base::Value::DictStorage& message_data) const;

void SendMessageToClient(int connection_id, const std::string& data) const;

Expand Down
15 changes: 6 additions & 9 deletions remoting/host/token_validator_base.cc
View file
Expand Up @@ -301,24 +301,21 @@ std::string TokenValidatorBase::ProcessResponse(int net_result) {
: data_;

base::Optional<base::Value> value = base::JSONReader::Read(responseData);
base::DictionaryValue* dict;
if (!value || !value->GetAsDictionary(&dict)) {
if (!value || !value->is_dict()) {
LOG(ERROR) << "Invalid token validation response: '" << data_ << "'";
return std::string();
}

std::string token_scope;
dict->GetStringWithoutPathExpansion("scope", &token_scope);
if (!IsValidScope(token_scope)) {
LOG(ERROR) << "Invalid scope: '" << token_scope << "', expected: '"
std::string* token_scope = value->FindStringKey("scope");
if (!token_scope || !IsValidScope(*token_scope)) {
LOG(ERROR) << "Invalid scope: '" << *token_scope << "', expected: '"
<< token_scope_ << "'.";
return std::string();
}

std::string shared_secret;
// Everything is valid, so return the shared secret to the caller.
dict->GetStringWithoutPathExpansion("access_token", &shared_secret);
return shared_secret;
std::string* shared_secret = value->FindStringKey("access_token");
return shared_secret ? *shared_secret : std::string();
}

} // namespace remoting
39 changes: 21 additions & 18 deletions remoting/protocol/ice_config.cc
View file
Expand Up @@ -138,33 +138,37 @@ IceConfig IceConfig::Parse(const base::DictionaryValue& dictionary) {
// Parse iceServers list and store them in |ice_config|.
bool errors_found = false;
ice_config.max_bitrate_kbps = 0;
for (const auto& server : ice_servers_list->GetList()) {
const base::DictionaryValue* server_dict;
if (!server.GetAsDictionary(&server_dict)) {
for (const auto& server : *ice_servers_list) {
if (!server.is_dict()) {
errors_found = true;
continue;
}

const base::ListValue* urls_list = nullptr;
if (!server_dict->GetList("urls", &urls_list)) {
const base::Value* urls_list = server.FindListKey("urls");
if (!urls_list) {
errors_found = true;
continue;
}

std::string username;
server_dict->GetString("username", &username);
const std::string* maybe_username = server.FindStringKey("username");
if (maybe_username)
username = *maybe_username;

std::string password;
server_dict->GetString("credential", &password);
const std::string* maybe_password = server.FindStringKey("credential");
if (maybe_password)
password = *maybe_password;

// Compute the lowest specified bitrate of all the ICE servers.
// Ideally the bitrate would be stored per ICE server, but it is not
// possible (at the application level) to look up which particular
// ICE server was used for the P2P connection.
double new_bitrate_double;
if (server_dict->GetDouble("maxRateKbps", &new_bitrate_double)) {
ice_config.max_bitrate_kbps = MinimumSpecified(
ice_config.max_bitrate_kbps, static_cast<int>(new_bitrate_double));
auto new_bitrate_double = server.FindDoubleKey("maxRateKbps");
if (new_bitrate_double.has_value()) {
ice_config.max_bitrate_kbps =
MinimumSpecified(ice_config.max_bitrate_kbps,
static_cast<int>(new_bitrate_double.value()));
}

for (const auto& url : urls_list->GetList()) {
Expand Down Expand Up @@ -206,20 +210,19 @@ IceConfig IceConfig::Parse(const std::string& config_json) {
return IceConfig();
}

base::DictionaryValue* dictionary = nullptr;
if (!json->GetAsDictionary(&dictionary)) {
if (!json->is_dict()) {
return IceConfig();
}

// Handle the case when the config is wrapped in 'data', i.e. as {'data': {
// 'iceServers': {...} }}.
base::DictionaryValue* data_dictionary = nullptr;
if (!dictionary->HasKey("iceServers") &&
dictionary->GetDictionary("data", &data_dictionary)) {
return Parse(*data_dictionary);
if (!json->FindKey("iceServers")) {
base::Value* data_dictionary = json->FindDictKey("data");
if (data_dictionary)
return Parse(base::Value::AsDictionaryValue(*data_dictionary));
}

return Parse(*dictionary);
return Parse(base::Value::AsDictionaryValue(*json));
}

// static
Expand Down

0 comments on commit c8edc4c

Please sign in to comment.