Commit ee3066f6 authored by Alex Ilin's avatar Alex Ilin Committed by Commit Bot

[gaia] Refactor base::Value usage in OAuth2MintTokenFlow

This CL updates all deprecated methods of base::Value and
base::JSONReader in OAuth2MintTokenFlow to use recommended APIs.

Bug: 1026237
Change-Id: I4e15970bc6215b74c9d4cea2d96f0923e1c7b878
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1940177Reviewed-by: default avatarDavid Roger <droger@chromium.org>
Commit-Queue: Alex Ilin <alexilin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#719910}
parent 22d34680
...@@ -12,6 +12,7 @@ ...@@ -12,6 +12,7 @@
#include "base/bind.h" #include "base/bind.h"
#include "base/command_line.h" #include "base/command_line.h"
#include "base/json/json_reader.h" #include "base/json/json_reader.h"
#include "base/optional.h"
#include "base/strings/string_number_conversions.h" #include "base/strings/string_number_conversions.h"
#include "base/strings/string_split.h" #include "base/strings/string_split.h"
#include "base/strings/string_util.h" #include "base/strings/string_util.h"
...@@ -58,9 +59,9 @@ static GoogleServiceAuthError CreateAuthError( ...@@ -58,9 +59,9 @@ static GoogleServiceAuthError CreateAuthError(
int net_error, int net_error,
const network::mojom::URLResponseHead* head, const network::mojom::URLResponseHead* head,
std::unique_ptr<std::string> body) { std::unique_ptr<std::string> body) {
if (net_error == net::ERR_ABORTED) { if (net_error == net::ERR_ABORTED)
return GoogleServiceAuthError(GoogleServiceAuthError::REQUEST_CANCELED); return GoogleServiceAuthError(GoogleServiceAuthError::REQUEST_CANCELED);
}
if (net_error != net::OK) { if (net_error != net::OK) {
DLOG(WARNING) << "Server returned error: errno " << net_error; DLOG(WARNING) << "Server returned error: errno " << net_error;
return GoogleServiceAuthError::FromConnectionError(net_error); return GoogleServiceAuthError::FromConnectionError(net_error);
...@@ -70,10 +71,8 @@ static GoogleServiceAuthError CreateAuthError( ...@@ -70,10 +71,8 @@ static GoogleServiceAuthError CreateAuthError(
if (body) if (body)
response_body = std::move(*body); response_body = std::move(*body);
std::unique_ptr<base::Value> value = base::Optional<base::Value> value = base::JSONReader::Read(response_body);
base::JSONReader::ReadDeprecated(response_body); if (!value || !value->is_dict()) {
base::DictionaryValue* response;
if (!value.get() || !value->GetAsDictionary(&response)) {
int http_response_code = -1; int http_response_code = -1;
if (head && head->headers) if (head && head->headers)
http_response_code = head->headers->response_code(); http_response_code = head->headers->response_code();
...@@ -83,25 +82,28 @@ static GoogleServiceAuthError CreateAuthError( ...@@ -83,25 +82,28 @@ static GoogleServiceAuthError CreateAuthError(
"HTTP Status of the response is: %d", "HTTP Status of the response is: %d",
http_response_code)); http_response_code));
} }
base::DictionaryValue* error; const base::Value* error = value->FindDictKey(kError);
if (!response->GetDictionary(kError, &error)) { if (!error) {
return GoogleServiceAuthError::FromUnexpectedServiceResponse( return GoogleServiceAuthError::FromUnexpectedServiceResponse(
"Not able to find a detailed error in a service response."); "Not able to find a detailed error in a service response.");
} }
std::string message; const std::string* message = error->FindStringKey(kMessage);
if (!error->GetString(kMessage, &message)) { if (!message) {
return GoogleServiceAuthError::FromUnexpectedServiceResponse( return GoogleServiceAuthError::FromUnexpectedServiceResponse(
"Not able to find an error message within a service error."); "Not able to find an error message within a service error.");
} }
return GoogleServiceAuthError::FromServiceError(message); return GoogleServiceAuthError::FromServiceError(*message);
} }
} // namespace } // namespace
IssueAdviceInfoEntry::IssueAdviceInfoEntry() {} IssueAdviceInfoEntry::IssueAdviceInfoEntry() = default;
IssueAdviceInfoEntry::~IssueAdviceInfoEntry() = default;
IssueAdviceInfoEntry::IssueAdviceInfoEntry(const IssueAdviceInfoEntry& other) = IssueAdviceInfoEntry::IssueAdviceInfoEntry(const IssueAdviceInfoEntry& other) =
default; default;
IssueAdviceInfoEntry::~IssueAdviceInfoEntry() {} IssueAdviceInfoEntry& IssueAdviceInfoEntry::operator=(
const IssueAdviceInfoEntry& other) = default;
bool IssueAdviceInfoEntry::operator ==(const IssueAdviceInfoEntry& rhs) const { bool IssueAdviceInfoEntry::operator ==(const IssueAdviceInfoEntry& rhs) const {
return description == rhs.description && details == rhs.details; return description == rhs.description && details == rhs.details;
...@@ -191,24 +193,22 @@ void OAuth2MintTokenFlow::ProcessApiCallSuccess( ...@@ -191,24 +193,22 @@ void OAuth2MintTokenFlow::ProcessApiCallSuccess(
std::string response_body; std::string response_body;
if (body) if (body)
response_body = std::move(*body); response_body = std::move(*body);
std::unique_ptr<base::Value> value = base::Optional<base::Value> value = base::JSONReader::Read(response_body);
base::JSONReader::ReadDeprecated(response_body); if (!value || !value->is_dict()) {
base::DictionaryValue* dict = nullptr;
if (!value.get() || !value->GetAsDictionary(&dict)) {
ReportFailure(GoogleServiceAuthError::FromUnexpectedServiceResponse( ReportFailure(GoogleServiceAuthError::FromUnexpectedServiceResponse(
"Not able to parse a JSON object from a service response.")); "Not able to parse a JSON object from a service response."));
return; return;
} }
std::string issue_advice_value; std::string* issue_advice_value = value->FindStringKey(kIssueAdviceKey);
if (!dict->GetString(kIssueAdviceKey, &issue_advice_value)) { if (!issue_advice_value) {
ReportFailure(GoogleServiceAuthError::FromUnexpectedServiceResponse( ReportFailure(GoogleServiceAuthError::FromUnexpectedServiceResponse(
"Not able to find an issueAdvice in a service response.")); "Not able to find an issueAdvice in a service response."));
return; return;
} }
if (issue_advice_value == kIssueAdviceValueConsent) { if (*issue_advice_value == kIssueAdviceValueConsent) {
IssueAdviceInfo issue_advice; IssueAdviceInfo issue_advice;
if (ParseIssueAdviceResponse(dict, &issue_advice)) if (ParseIssueAdviceResponse(&(*value), &issue_advice))
ReportIssueAdviceSuccess(issue_advice); ReportIssueAdviceSuccess(issue_advice);
else else
ReportFailure(GoogleServiceAuthError::FromUnexpectedServiceResponse( ReportFailure(GoogleServiceAuthError::FromUnexpectedServiceResponse(
...@@ -217,7 +217,7 @@ void OAuth2MintTokenFlow::ProcessApiCallSuccess( ...@@ -217,7 +217,7 @@ void OAuth2MintTokenFlow::ProcessApiCallSuccess(
} else { } else {
std::string access_token; std::string access_token;
int time_to_live; int time_to_live;
if (ParseMintTokenResponse(dict, &access_token, &time_to_live)) if (ParseMintTokenResponse(&(*value), &access_token, &time_to_live))
ReportSuccess(access_token, time_to_live); ReportSuccess(access_token, time_to_live);
else else
ReportFailure(GoogleServiceAuthError::FromUnexpectedServiceResponse( ReportFailure(GoogleServiceAuthError::FromUnexpectedServiceResponse(
...@@ -236,49 +236,64 @@ void OAuth2MintTokenFlow::ProcessApiCallFailure( ...@@ -236,49 +236,64 @@ void OAuth2MintTokenFlow::ProcessApiCallFailure(
} }
// static // static
bool OAuth2MintTokenFlow::ParseMintTokenResponse( bool OAuth2MintTokenFlow::ParseMintTokenResponse(const base::Value* dict,
const base::DictionaryValue* dict, std::string* access_token, std::string* access_token,
int* time_to_live) { int* time_to_live) {
CHECK(dict); CHECK(dict);
CHECK(dict->is_dict());
CHECK(access_token); CHECK(access_token);
CHECK(time_to_live); CHECK(time_to_live);
std::string ttl_string;
return dict->GetString(kExpiresInKey, &ttl_string) && const std::string* ttl_string = dict->FindStringKey(kExpiresInKey);
base::StringToInt(ttl_string, time_to_live) && if (!ttl_string || !base::StringToInt(*ttl_string, time_to_live))
dict->GetString(kAccessTokenKey, access_token); return false;
const std::string* access_token_ptr = dict->FindStringKey(kAccessTokenKey);
if (!access_token_ptr)
return false;
*access_token = *access_token_ptr;
return true;
} }
// static // static
bool OAuth2MintTokenFlow::ParseIssueAdviceResponse( bool OAuth2MintTokenFlow::ParseIssueAdviceResponse(
const base::DictionaryValue* dict, IssueAdviceInfo* issue_advice) { const base::Value* dict,
IssueAdviceInfo* issue_advice) {
CHECK(dict); CHECK(dict);
CHECK(dict->is_dict());
CHECK(issue_advice); CHECK(issue_advice);
const base::DictionaryValue* consent_dict = nullptr; const base::Value* consent_dict = dict->FindDictKey(kConsentKey);
if (!dict->GetDictionary(kConsentKey, &consent_dict)) if (!consent_dict)
return false; return false;
const base::ListValue* scopes_list = nullptr; const base::Value* scopes_list = consent_dict->FindListKey(kScopesKey);
if (!consent_dict->GetList(kScopesKey, &scopes_list)) if (!scopes_list)
return false; return false;
bool success = true; bool success = true;
for (size_t index = 0; index < scopes_list->GetSize(); ++index) { for (const auto& scopes_entry : scopes_list->GetList()) {
const base::DictionaryValue* scopes_entry = nullptr; if (!scopes_entry.is_dict()) {
IssueAdviceInfoEntry entry;
base::string16 detail;
if (!scopes_list->GetDictionary(index, &scopes_entry) ||
!scopes_entry->GetString(kDescriptionKey, &entry.description) ||
!scopes_entry->GetString(kDetailKey, &detail)) {
success = false; success = false;
break; break;
} }
const std::string* description =
scopes_entry.FindStringKey(kDescriptionKey);
const std::string* detail = scopes_entry.FindStringKey(kDetailKey);
if (!description || !detail) {
success = false;
break;
}
IssueAdviceInfoEntry entry;
entry.description = base::UTF8ToUTF16(*description);
base::TrimWhitespace(entry.description, base::TRIM_ALL, &entry.description); base::TrimWhitespace(entry.description, base::TRIM_ALL, &entry.description);
entry.details = base::SplitString( entry.details = base::SplitString(
detail, base::ASCIIToUTF16(kDetailSeparators), base::UTF8ToUTF16(*detail), base::ASCIIToUTF16(kDetailSeparators),
base::TRIM_WHITESPACE, base::SPLIT_WANT_NONEMPTY); base::TRIM_WHITESPACE, base::SPLIT_WANT_NONEMPTY);
issue_advice->push_back(entry); issue_advice->push_back(std::move(entry));
} }
if (!success) if (!success)
......
...@@ -20,7 +20,7 @@ class GoogleServiceAuthError; ...@@ -20,7 +20,7 @@ class GoogleServiceAuthError;
class OAuth2MintTokenFlowTest; class OAuth2MintTokenFlowTest;
namespace base { namespace base {
class DictionaryValue; class Value;
} }
// IssueAdvice: messages to show to the user to get a user's approval. // IssueAdvice: messages to show to the user to get a user's approval.
...@@ -37,9 +37,11 @@ class DictionaryValue; ...@@ -37,9 +37,11 @@ class DictionaryValue;
struct IssueAdviceInfoEntry { struct IssueAdviceInfoEntry {
public: public:
IssueAdviceInfoEntry(); IssueAdviceInfoEntry();
IssueAdviceInfoEntry(const IssueAdviceInfoEntry& other);
~IssueAdviceInfoEntry(); ~IssueAdviceInfoEntry();
IssueAdviceInfoEntry(const IssueAdviceInfoEntry& other);
IssueAdviceInfoEntry& operator=(const IssueAdviceInfoEntry& other);
base::string16 description; base::string16 description;
std::vector<base::string16> details; std::vector<base::string16> details;
...@@ -124,11 +126,11 @@ class OAuth2MintTokenFlow : public OAuth2ApiCallFlow { ...@@ -124,11 +126,11 @@ class OAuth2MintTokenFlow : public OAuth2ApiCallFlow {
void ReportIssueAdviceSuccess(const IssueAdviceInfo& issue_advice); void ReportIssueAdviceSuccess(const IssueAdviceInfo& issue_advice);
void ReportFailure(const GoogleServiceAuthError& error); void ReportFailure(const GoogleServiceAuthError& error);
static bool ParseIssueAdviceResponse( static bool ParseIssueAdviceResponse(const base::Value* dict,
const base::DictionaryValue* dict, IssueAdviceInfo* issue_advice); IssueAdviceInfo* issue_advice);
static bool ParseMintTokenResponse( static bool ParseMintTokenResponse(const base::Value* dict,
const base::DictionaryValue* dict, std::string* access_token, std::string* access_token,
int* time_to_live); int* time_to_live);
Delegate* delegate_; Delegate* delegate_;
Parameters parameters_; Parameters parameters_;
......
...@@ -11,6 +11,7 @@ ...@@ -11,6 +11,7 @@
#include <vector> #include <vector>
#include "base/json/json_reader.h" #include "base/json/json_reader.h"
#include "base/optional.h"
#include "base/strings/utf_string_conversions.h" #include "base/strings/utf_string_conversions.h"
#include "base/values.h" #include "base/values.h"
#include "google_apis/gaia/google_service_auth_error.h" #include "google_apis/gaia/google_service_auth_error.h"
...@@ -174,12 +175,12 @@ class OAuth2MintTokenFlowTest : public testing::Test { ...@@ -174,12 +175,12 @@ class OAuth2MintTokenFlowTest : public testing::Test {
device_id, mode)); device_id, mode));
} }
// Helper to parse the given string to DictionaryValue. // Helper to parse the given string to base::Value.
static base::DictionaryValue* ParseJson(const std::string& str) { static std::unique_ptr<base::Value> ParseJson(const std::string& str) {
std::unique_ptr<base::Value> value = base::JSONReader::ReadDeprecated(str); base::Optional<base::Value> value = base::JSONReader::Read(str);
EXPECT_TRUE(value.get()); EXPECT_TRUE(value.has_value());
EXPECT_EQ(base::Value::Type::DICTIONARY, value->type()); EXPECT_TRUE(value->is_dict());
return static_cast<base::DictionaryValue*>(value.release()); return std::make_unique<base::Value>(std::move(*value));
} }
std::unique_ptr<MockMintTokenFlow> flow_; std::unique_ptr<MockMintTokenFlow> flow_;
...@@ -249,8 +250,7 @@ TEST_F(OAuth2MintTokenFlowTest, CreateApiCallBody) { ...@@ -249,8 +250,7 @@ TEST_F(OAuth2MintTokenFlowTest, CreateApiCallBody) {
TEST_F(OAuth2MintTokenFlowTest, ParseMintTokenResponse) { TEST_F(OAuth2MintTokenFlowTest, ParseMintTokenResponse) {
{ // Access token missing. { // Access token missing.
std::unique_ptr<base::DictionaryValue> json( std::unique_ptr<base::Value> json = ParseJson(kTokenResponseNoAccessToken);
ParseJson(kTokenResponseNoAccessToken));
std::string at; std::string at;
int ttl; int ttl;
EXPECT_FALSE(OAuth2MintTokenFlow::ParseMintTokenResponse(json.get(), &at, EXPECT_FALSE(OAuth2MintTokenFlow::ParseMintTokenResponse(json.get(), &at,
...@@ -258,7 +258,7 @@ TEST_F(OAuth2MintTokenFlowTest, ParseMintTokenResponse) { ...@@ -258,7 +258,7 @@ TEST_F(OAuth2MintTokenFlowTest, ParseMintTokenResponse) {
EXPECT_TRUE(at.empty()); EXPECT_TRUE(at.empty());
} }
{ // All good. { // All good.
std::unique_ptr<base::DictionaryValue> json(ParseJson(kValidTokenResponse)); std::unique_ptr<base::Value> json = ParseJson(kValidTokenResponse);
std::string at; std::string at;
int ttl; int ttl;
EXPECT_TRUE(OAuth2MintTokenFlow::ParseMintTokenResponse(json.get(), &at, EXPECT_TRUE(OAuth2MintTokenFlow::ParseMintTokenResponse(json.get(), &at,
...@@ -270,24 +270,22 @@ TEST_F(OAuth2MintTokenFlowTest, ParseMintTokenResponse) { ...@@ -270,24 +270,22 @@ TEST_F(OAuth2MintTokenFlowTest, ParseMintTokenResponse) {
TEST_F(OAuth2MintTokenFlowTest, ParseIssueAdviceResponse) { TEST_F(OAuth2MintTokenFlowTest, ParseIssueAdviceResponse) {
{ // Description missing. { // Description missing.
std::unique_ptr<base::DictionaryValue> json( std::unique_ptr<base::Value> json =
ParseJson(kIssueAdviceResponseNoDescription)); ParseJson(kIssueAdviceResponseNoDescription);
IssueAdviceInfo ia; IssueAdviceInfo ia;
EXPECT_FALSE(OAuth2MintTokenFlow::ParseIssueAdviceResponse( EXPECT_FALSE(OAuth2MintTokenFlow::ParseIssueAdviceResponse(
json.get(), &ia)); json.get(), &ia));
EXPECT_TRUE(ia.empty()); EXPECT_TRUE(ia.empty());
} }
{ // Detail missing. { // Detail missing.
std::unique_ptr<base::DictionaryValue> json( std::unique_ptr<base::Value> json = ParseJson(kIssueAdviceResponseNoDetail);
ParseJson(kIssueAdviceResponseNoDetail));
IssueAdviceInfo ia; IssueAdviceInfo ia;
EXPECT_FALSE(OAuth2MintTokenFlow::ParseIssueAdviceResponse( EXPECT_FALSE(OAuth2MintTokenFlow::ParseIssueAdviceResponse(
json.get(), &ia)); json.get(), &ia));
EXPECT_TRUE(ia.empty()); EXPECT_TRUE(ia.empty());
} }
{ // All good. { // All good.
std::unique_ptr<base::DictionaryValue> json( std::unique_ptr<base::Value> json = ParseJson(kValidIssueAdviceResponse);
ParseJson(kValidIssueAdviceResponse));
IssueAdviceInfo ia; IssueAdviceInfo ia;
EXPECT_TRUE(OAuth2MintTokenFlow::ParseIssueAdviceResponse( EXPECT_TRUE(OAuth2MintTokenFlow::ParseIssueAdviceResponse(
json.get(), &ia)); json.get(), &ia));
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment