Skip to content

Commit

Permalink
[ONC] Always retain "extra" StaticIPConfig fields
Browse files Browse the repository at this point in the history 
Currently, StaticIPConfig settings will be removed entirely if neither
of {IPAddress,NameServers}ConfigType is 'Static'. This change causes
fields not affected by either of these settings (i.e., not IPAddress,
RoutingPrefix, Gateway, or NameServers) to be retained regardless of
the {IPAddress,NameServers}ConfigType configuration.

Bug: 1048714
Change-Id: Ie3e099bbee9c56788112e6b57b355f998f6ba453
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2042274
Commit-Queue: Alex Khouderchah <akhouderchah@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#739404}
  • Loading branch information
Alex Khouderchah authored and Commit Bot committed Feb 7, 2020
1 parent f02d558 commit c15b3c4
Show file tree
Hide file tree
Showing 3 changed files with 71 additions and 28 deletions.
17 changes: 7 additions & 10 deletions chromeos/network/onc/onc_normalizer.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ Normalizer::~Normalizer() = default;
std::unique_ptr<base::DictionaryValue> Normalizer::NormalizeObject(
const OncValueSignature* object_signature,
const base::DictionaryValue& onc_object) {
CHECK(object_signature != NULL);
CHECK(object_signature != nullptr);
bool error = false;
std::unique_ptr<base::DictionaryValue> result =
MapObject(*object_signature, onc_object, &error);
Expand All @@ -40,11 +40,11 @@ std::unique_ptr<base::DictionaryValue> Normalizer::MapObject(
std::unique_ptr<base::DictionaryValue> normalized =
Mapper::MapObject(signature, onc_object, error);

if (normalized.get() == NULL)
if (normalized.get() == nullptr)
return std::unique_ptr<base::DictionaryValue>();

if (remove_recommended_fields_)
normalized->RemoveWithoutPathExpansion(::onc::kRecommended, NULL);
normalized->RemoveWithoutPathExpansion(::onc::kRecommended, nullptr);

if (&signature == &kCertificateSignature)
NormalizeCertificate(normalized.get());
Expand Down Expand Up @@ -170,11 +170,11 @@ void Normalizer::NormalizeNetworkConfiguration(base::DictionaryValue* network) {
network->GetBooleanWithoutPathExpansion(::onc::kRemove, &remove);
if (remove) {
network->RemoveWithoutPathExpansion(::onc::network_config::kStaticIPConfig,
NULL);
network->RemoveWithoutPathExpansion(::onc::network_config::kName, NULL);
nullptr);
network->RemoveWithoutPathExpansion(::onc::network_config::kName, nullptr);
network->RemoveWithoutPathExpansion(::onc::network_config::kProxySettings,
NULL);
network->RemoveWithoutPathExpansion(::onc::network_config::kType, NULL);
nullptr);
network->RemoveWithoutPathExpansion(::onc::network_config::kType, nullptr);
// Fields dependent on kType are removed afterwards, too.
}

Expand Down Expand Up @@ -264,9 +264,6 @@ void Normalizer::NormalizeStaticIPConfigForNetwork(
const bool name_servers_type_is_static = IsIpConfigTypeStatic(
network, ::onc::network_config::kNameServersConfigType);

RemoveEntryUnless(network, ::onc::network_config::kStaticIPConfig,
ip_config_type_is_static || name_servers_type_is_static);

base::Value* static_ip_config = network->FindKeyOfType(
::onc::network_config::kStaticIPConfig, base::Value::Type::DICTIONARY);
bool all_ip_fields_exist = false;
Expand Down
52 changes: 36 additions & 16 deletions chromeos/network/onc/onc_normalizer_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,16 +14,36 @@
namespace chromeos {
namespace onc {

// Validate that an irrelevant StaticIPConfig dictionary will be removed.
TEST(ONCNormalizerTest, RemoveStaticIPConfig) {
// Validate that StaticIPConfig IPAddress and dependent fields will be removed
// if IPAddressConfigType is not 'Static'.
TEST(ONCNormalizerTest, RemoveUnnecessaryAddressStaticIPConfigFields) {
Normalizer normalizer(true);
std::unique_ptr<const base::DictionaryValue> data(
test_utils::ReadTestDictionary("settings_with_normalization.json"));

const base::DictionaryValue* original = NULL;
const base::DictionaryValue* expected_normalized = NULL;
data->GetDictionary("irrelevant-staticipconfig", &original);
data->GetDictionary("irrelevant-staticipconfig-normalized",
const base::DictionaryValue* original = nullptr;
const base::DictionaryValue* expected_normalized = nullptr;
data->GetDictionary("unnecessary-address-staticipconfig", &original);
data->GetDictionary("unnecessary-address-staticipconfig-normalized",
&expected_normalized);

std::unique_ptr<base::DictionaryValue> actual_normalized =
normalizer.NormalizeObject(&kNetworkConfigurationSignature, *original);
EXPECT_TRUE(test_utils::Equals(expected_normalized, actual_normalized.get()));
}

// Validate that StaticIPConfig fields other than NameServers and IPAddress &
// friends will be retained even without static
// {NameServers,IPAddress}ConfigType.
TEST(ONCNormalizerTest, RetainExtraStaticIPConfigFields) {
Normalizer normalizer(true);
std::unique_ptr<const base::DictionaryValue> data(
test_utils::ReadTestDictionary("settings_with_normalization.json"));

const base::DictionaryValue* original = nullptr;
const base::DictionaryValue* expected_normalized = nullptr;
data->GetDictionary("unnecessary-address-staticipconfig", &original);
data->GetDictionary("unnecessary-address-staticipconfig-normalized",
&expected_normalized);

std::unique_ptr<base::DictionaryValue> actual_normalized =
Expand All @@ -38,8 +58,8 @@ TEST(ONCNormalizerTest, RemoveStaticIPConfigFields) {
std::unique_ptr<const base::DictionaryValue> data(
test_utils::ReadTestDictionary("settings_with_normalization.json"));

const base::DictionaryValue* original = NULL;
const base::DictionaryValue* expected_normalized = NULL;
const base::DictionaryValue* original = nullptr;
const base::DictionaryValue* expected_normalized = nullptr;
data->GetDictionary("irrelevant-staticipconfig-fields", &original);
data->GetDictionary("irrelevant-staticipconfig-fields-normalized",
&expected_normalized);
Expand All @@ -56,8 +76,8 @@ TEST(ONCNormalizerTest, RemoveNameServers) {
std::unique_ptr<const base::DictionaryValue> data(
test_utils::ReadTestDictionary("settings_with_normalization.json"));

const base::DictionaryValue* original = NULL;
const base::DictionaryValue* expected_normalized = NULL;
const base::DictionaryValue* original = nullptr;
const base::DictionaryValue* expected_normalized = nullptr;
data->GetDictionary("irrelevant-nameservers", &original);
data->GetDictionary("irrelevant-nameservers-normalized",
&expected_normalized);
Expand All @@ -74,8 +94,8 @@ TEST(ONCNormalizerTest, RemoveIPFieldsForIncompleteConfig) {
std::unique_ptr<const base::DictionaryValue> data(
test_utils::ReadTestDictionary("settings_with_normalization.json"));

const base::DictionaryValue* original = NULL;
const base::DictionaryValue* expected_normalized = NULL;
const base::DictionaryValue* original = nullptr;
const base::DictionaryValue* expected_normalized = nullptr;
data->GetDictionary("missing-ip-fields", &original);
data->GetDictionary("missing-ip-fields-normalized", &expected_normalized);

Expand All @@ -89,8 +109,8 @@ TEST(ONCNormalizerTest, NormalizeNetworkConfigurationEthernetAndVPN) {
std::unique_ptr<const base::DictionaryValue> data(
test_utils::ReadTestDictionary("settings_with_normalization.json"));

const base::DictionaryValue* original = NULL;
const base::DictionaryValue* expected_normalized = NULL;
const base::DictionaryValue* original = nullptr;
const base::DictionaryValue* expected_normalized = nullptr;
data->GetDictionary("ethernet-and-vpn", &original);
data->GetDictionary("ethernet-and-vpn-normalized", &expected_normalized);

Expand All @@ -105,8 +125,8 @@ TEST(ONCNormalizerTest, NormalizeNetworkConfigurationWifi) {
std::unique_ptr<const base::DictionaryValue> data(
test_utils::ReadTestDictionary("settings_with_normalization.json"));

const base::DictionaryValue* original = NULL;
const base::DictionaryValue* expected_normalized = NULL;
const base::DictionaryValue* original = nullptr;
const base::DictionaryValue* expected_normalized = nullptr;
data->GetDictionary("wifi", &original);
data->GetDictionary("wifi-normalized", &expected_normalized);

Expand Down
30 changes: 28 additions & 2 deletions chromeos/test/data/network/settings_with_normalization.json
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"irrelevant-staticipconfig": {
"unnecessary-address-staticipconfig": {
"GUID": "guid",
"Type": "Ethernet",
"Name": "name",
Expand All @@ -11,14 +11,40 @@
"IPAddress": "127.0.0.1"
}
},
"irrelevant-staticipconfig-normalized": {
"unnecessary-address-staticipconfig-normalized": {
"GUID": "guid",
"Type": "Ethernet",
"Name": "name",
"Ethernet": {
"Authentication": "None"
}
},
"retained-staticipconfig-fields": {
"GUID": "guid",
"Type": "Ethernet",
"Name": "name",
"Ethernet": {
"Authentication": "None"
},
"StaticIPConfig": {
"IncludedRoutes": ["1.1.1.1/24", "2.2.2.2/24"],
"ExcludedRoutes": ["3.3.3.3/24"],
"SearchDomains": ["rick.roll.com"]
}
},
"retained-staticipconfig-fields-normalized": {
"GUID": "guid",
"Type": "Ethernet",
"Name": "name",
"Ethernet": {
"Authentication": "None"
},
"StaticIPConfig": {
"IncludedRoutes": ["1.1.1.1/24", "2.2.2.2/24"],
"ExcludedRoutes": ["3.3.3.3/24"],
"SearchDomains": ["rick.roll.com"]
}
},
"irrelevant-staticipconfig-fields": {
"GUID": "guid",
"Type": "Ethernet",
Expand Down

0 comments on commit c15b3c4

Please sign in to comment.