Only disable update downloads over expensive connections, not all transfers.
Previous to this CL, all network traffic was disabled from the update engine
if an update wasn't allowed. Instead, in this CL, we switch to only disable
the update application portion (downloading and applying the payload). This
is done by moving the detection logic from the mechanism (the network fetcher)
to the user of said mechanism i.e. the omaha request/response actions.
I have done a little refactoring of the unittests to make this CL easier to
test and found 1 bug in the http_fetcher_unittests where we weren't sending
either the right URL or handling a server not existing correctly.
This CL comes with one caveat:
Technically in the previous impl if a download started over wifi then got
disconnected / retried and bounced to a different connection type that
we couldn't update over, we'd stop the update. This would no longer be true
here if the http_fetcher recovered seamlessly.
BUG=chromium:379832
TEST=Unittests
Change-Id: I6b1a76e4c030d4e384e8ba0b519424a35406aafb
Reviewed-on: https://chromium-review.googlesource.com/202435
Tested-by: Chris Sosa <sosa@chromium.org>
Reviewed-by: David Zeuthen <zeuthen@chromium.org>
Commit-Queue: Chris Sosa <sosa@chromium.org>
diff --git a/http_fetcher_unittest.cc b/http_fetcher_unittest.cc
index d7713c5..3392098 100644
--- a/http_fetcher_unittest.cc
+++ b/http_fetcher_unittest.cc
@@ -23,7 +23,6 @@
#include "update_engine/fake_system_state.h"
#include "update_engine/http_common.h"
#include "update_engine/libcurl_http_fetcher.h"
-#include "update_engine/mock_connection_manager.h"
#include "update_engine/mock_http_fetcher.h"
#include "update_engine/multi_range_http_fetcher.h"
#include "update_engine/proxy_resolver.h"
@@ -194,10 +193,7 @@
class AnyHttpFetcherTest {
public:
- AnyHttpFetcherTest()
- : mock_connection_manager_(&fake_system_state_) {
- fake_system_state_.set_connection_manager(&mock_connection_manager_);
- }
+ AnyHttpFetcherTest() {}
virtual ~AnyHttpFetcherTest() {}
virtual HttpFetcher* NewLargeFetcher(size_t num_proxies) = 0;
@@ -224,7 +220,6 @@
protected:
DirectProxyResolver proxy_resolver_;
FakeSystemState fake_system_state_;
- MockConnectionManager mock_connection_manager_;
};
class MockHttpFetcherTest : public AnyHttpFetcherTest {
@@ -426,17 +421,6 @@
scoped_ptr<HttpFetcher> fetcher(this->test_.NewSmallFetcher());
fetcher->set_delegate(&delegate);
- MockConnectionManager* mock_cm = dynamic_cast<MockConnectionManager*>(
- fetcher->GetSystemState()->connection_manager());
- EXPECT_CALL(*mock_cm, GetConnectionProperties(_,_,_))
- .WillRepeatedly(DoAll(SetArgumentPointee<1>(kNetWifi),
- SetArgumentPointee<2>(NetworkTethering::kUnknown),
- Return(true)));
- EXPECT_CALL(*mock_cm, IsUpdateAllowedOver(kNetWifi, _))
- .WillRepeatedly(Return(true));
- EXPECT_CALL(*mock_cm, StringForConnectionType(kNetWifi))
- .WillRepeatedly(Return(shill::kTypeWifi));
-
scoped_ptr<HttpServer> server(this->test_.CreateServer());
ASSERT_TRUE(server->started_);
@@ -457,17 +441,6 @@
scoped_ptr<HttpFetcher> fetcher(this->test_.NewLargeFetcher());
fetcher->set_delegate(&delegate);
- MockConnectionManager* mock_cm = dynamic_cast<MockConnectionManager*>(
- fetcher->GetSystemState()->connection_manager());
- EXPECT_CALL(*mock_cm, GetConnectionProperties(_,_,_))
- .WillRepeatedly(DoAll(SetArgumentPointee<1>(kNetEthernet),
- SetArgumentPointee<2>(NetworkTethering::kUnknown),
- Return(true)));
- EXPECT_CALL(*mock_cm, IsUpdateAllowedOver(kNetEthernet, _))
- .WillRepeatedly(Return(true));
- EXPECT_CALL(*mock_cm, StringForConnectionType(kNetEthernet))
- .WillRepeatedly(Return(shill::kTypeEthernet));
-
scoped_ptr<HttpServer> server(this->test_.CreateServer());
ASSERT_TRUE(server->started_);
@@ -496,17 +469,6 @@
scoped_ptr<HttpFetcher> fetcher(this->test_.NewSmallFetcher());
fetcher->set_delegate(&delegate);
- MockConnectionManager* mock_cm = dynamic_cast<MockConnectionManager*>(
- fetcher->GetSystemState()->connection_manager());
- EXPECT_CALL(*mock_cm, GetConnectionProperties(_,_,_))
- .WillRepeatedly(DoAll(SetArgumentPointee<1>(kNetWimax),
- SetArgumentPointee<2>(NetworkTethering::kUnknown),
- Return(true)));
- EXPECT_CALL(*mock_cm, IsUpdateAllowedOver(kNetWimax, _))
- .WillRepeatedly(Return(true));
- EXPECT_CALL(*mock_cm, StringForConnectionType(kNetWimax))
- .WillRepeatedly(Return(shill::kTypeWimax));
-
scoped_ptr<HttpServer> server(this->test_.CreateServer());
ASSERT_TRUE(server->started_);
@@ -574,17 +536,6 @@
delegate.fetcher_ = fetcher.get();
fetcher->set_delegate(&delegate);
- MockConnectionManager* mock_cm = dynamic_cast<MockConnectionManager*>(
- fetcher->GetSystemState()->connection_manager());
- EXPECT_CALL(*mock_cm, GetConnectionProperties(_,_,_))
- .WillRepeatedly(DoAll(SetArgumentPointee<1>(kNetCellular),
- SetArgumentPointee<2>(NetworkTethering::kUnknown),
- Return(true)));
- EXPECT_CALL(*mock_cm, IsUpdateAllowedOver(kNetCellular, _))
- .WillRepeatedly(Return(true));
- EXPECT_CALL(*mock_cm, StringForConnectionType(kNetCellular))
- .WillRepeatedly(Return(shill::kTypeCellular));
-
scoped_ptr<HttpServer> server(this->test_.CreateServer());
ASSERT_TRUE(server->started_);
@@ -653,17 +604,6 @@
delegate.loop_ = loop;
delegate.fetcher_->set_delegate(&delegate);
- MockConnectionManager* mock_cm = dynamic_cast<MockConnectionManager*>(
- delegate.fetcher_->GetSystemState()->connection_manager());
- EXPECT_CALL(*mock_cm, GetConnectionProperties(_,_,_))
- .WillRepeatedly(DoAll(SetArgumentPointee<1>(kNetWifi),
- SetArgumentPointee<2>(NetworkTethering::kUnknown),
- Return(true)));
- EXPECT_CALL(*mock_cm, IsUpdateAllowedOver(kNetWifi, _))
- .WillRepeatedly(Return(true));
- EXPECT_CALL(*mock_cm, StringForConnectionType(kNetWifi))
- .WillRepeatedly(Return(shill::kTypeWifi));
-
scoped_ptr<HttpServer> server(this->test_.CreateServer());
this->test_.IgnoreServerAborting(server.get());
ASSERT_TRUE(server->started_);
@@ -713,18 +653,6 @@
scoped_ptr<HttpFetcher> fetcher(this->test_.NewSmallFetcher());
fetcher->set_delegate(&delegate);
- MockConnectionManager* mock_cm = dynamic_cast<MockConnectionManager*>(
- fetcher->GetSystemState()->connection_manager());
- EXPECT_CALL(*mock_cm, GetConnectionProperties(_,_,_))
- .WillRepeatedly(DoAll(SetArgumentPointee<1>(kNetWifi),
- SetArgumentPointee<2>(NetworkTethering::kUnknown),
- Return(true)));
- EXPECT_CALL(*mock_cm, IsUpdateAllowedOver(kNetWifi, _))
- .WillRepeatedly(Return(true));
- EXPECT_CALL(*mock_cm, StringForConnectionType(kNetWifi))
- .WillRepeatedly(Return(shill::kTypeWifi));
-
-
scoped_ptr<HttpServer> server(this->test_.CreateServer());
ASSERT_TRUE(server->started_);
@@ -776,7 +704,7 @@
}
virtual void TransferComplete(HttpFetcher* fetcher, bool successful) {
EXPECT_FALSE(successful);
- EXPECT_EQ(0, fetcher->http_response_code());
+ EXPECT_EQ(404, fetcher->http_response_code());
g_main_loop_quit(loop_);
}
virtual void TransferTerminated(HttpFetcher* fetcher) {
@@ -798,21 +726,9 @@
scoped_ptr<HttpFetcher> fetcher(this->test_.NewSmallFetcher());
fetcher->set_delegate(&delegate);
- MockConnectionManager* mock_cm = dynamic_cast<MockConnectionManager*>(
- fetcher->GetSystemState()->connection_manager());
- EXPECT_CALL(*mock_cm, GetConnectionProperties(_,_,_))
- .WillRepeatedly(DoAll(SetArgumentPointee<1>(kNetEthernet),
- SetArgumentPointee<2>(NetworkTethering::kUnknown),
- Return(true)));
- EXPECT_CALL(*mock_cm, IsUpdateAllowedOver(kNetEthernet, _))
- .WillRepeatedly(Return(true));
- EXPECT_CALL(*mock_cm, StringForConnectionType(kNetEthernet))
- .WillRepeatedly(Return(shill::kTypeEthernet));
-
-
StartTransferArgs start_xfer_args = {
fetcher.get(),
- LocalServerUrlForPath(0, this->test_.SmallUrl(0))
+ this->test_.SmallUrl(0)
};
g_timeout_add(0, StartTransfer, &start_xfer_args);
@@ -833,18 +749,6 @@
scoped_ptr<HttpFetcher> fetcher(this->test_.NewSmallFetcher());
fetcher->set_delegate(&delegate);
- // Don't allow connection to server by denying access over ethernet.
- MockConnectionManager* mock_cm = dynamic_cast<MockConnectionManager*>(
- fetcher->GetSystemState()->connection_manager());
- EXPECT_CALL(*mock_cm, GetConnectionProperties(_,_,_))
- .WillRepeatedly(DoAll(SetArgumentPointee<1>(kNetEthernet),
- SetArgumentPointee<2>(NetworkTethering::kUnknown),
- Return(true)));
- EXPECT_CALL(*mock_cm, IsUpdateAllowedOver(kNetEthernet, _))
- .WillRepeatedly(Return(false));
- EXPECT_CALL(*mock_cm, StringForConnectionType(kNetEthernet))
- .WillRepeatedly(Return(shill::kTypeEthernet));
-
StartTransferArgs start_xfer_args = {
fetcher.get(),
LocalServerUrlForPath(0,
@@ -905,17 +809,6 @@
scoped_ptr<HttpFetcher> fetcher(http_fetcher);
fetcher->set_delegate(&delegate);
- MockConnectionManager* mock_cm = dynamic_cast<MockConnectionManager*>(
- fetcher->GetSystemState()->connection_manager());
- EXPECT_CALL(*mock_cm, GetConnectionProperties(_,_,_))
- .WillRepeatedly(DoAll(SetArgumentPointee<1>(kNetEthernet),
- SetArgumentPointee<2>(NetworkTethering::kUnknown),
- Return(true)));
- EXPECT_CALL(*mock_cm, IsUpdateAllowedOver(kNetEthernet, _))
- .WillRepeatedly(Return(true));
- EXPECT_CALL(*mock_cm, StringForConnectionType(kNetEthernet))
- .WillRepeatedly(Return(shill::kTypeEthernet));
-
StartTransferArgs start_xfer_args =
{ fetcher.get(), LocalServerUrlForPath(server->GetPort(), url) };
@@ -1025,17 +918,6 @@
delegate.loop_ = loop;
delegate.fetcher_.reset(fetcher_in);
- MockConnectionManager* mock_cm = dynamic_cast<MockConnectionManager*>(
- fetcher_in->GetSystemState()->connection_manager());
- EXPECT_CALL(*mock_cm, GetConnectionProperties(_,_,_))
- .WillRepeatedly(DoAll(SetArgumentPointee<1>(kNetWifi),
- SetArgumentPointee<2>(NetworkTethering::kUnknown),
- Return(true)));
- EXPECT_CALL(*mock_cm, IsUpdateAllowedOver(kNetWifi, _))
- .WillRepeatedly(Return(true));
- EXPECT_CALL(*mock_cm, StringForConnectionType(kNetWifi))
- .WillRepeatedly(Return(shill::kTypeWifi));
-
MultiRangeHttpFetcher* multi_fetcher =
dynamic_cast<MultiRangeHttpFetcher*>(fetcher_in);
ASSERT_TRUE(multi_fetcher);
@@ -1226,16 +1108,6 @@
bool is_allowed = (i != 0);
scoped_ptr<HttpFetcher> fetcher(this->test_.NewLargeFetcher());
- MockConnectionManager* mock_cm = dynamic_cast<MockConnectionManager*>(
- fetcher->GetSystemState()->connection_manager());
- EXPECT_CALL(*mock_cm, GetConnectionProperties(_,_,_))
- .WillRepeatedly(DoAll(SetArgumentPointee<1>(kNetWifi),
- SetArgumentPointee<2>(NetworkTethering::kUnknown),
- Return(true)));
- EXPECT_CALL(*mock_cm, IsUpdateAllowedOver(kNetWifi, _))
- .WillRepeatedly(Return(is_allowed));
- EXPECT_CALL(*mock_cm, StringForConnectionType(kNetWifi))
- .WillRepeatedly(Return(shill::kTypeWifi));
bool is_official_build = (i == 1);
LOG(INFO) << "is_update_allowed_over_connection: " << is_allowed;
diff --git a/libcurl_http_fetcher.cc b/libcurl_http_fetcher.cc
index be5df41..1919466 100644
--- a/libcurl_http_fetcher.cc
+++ b/libcurl_http_fetcher.cc
@@ -13,7 +13,6 @@
#include "update_engine/certificate_checker.h"
#include "update_engine/hardware_interface.h"
-#include "update_engine/real_dbus_wrapper.h"
#include "update_engine/utils.h"
using google::protobuf::NewPermanentCallback;
@@ -37,25 +36,6 @@
CleanUp();
}
-// On error, returns false.
-bool LibcurlHttpFetcher::IsUpdateAllowedOverCurrentConnection() const {
- NetworkConnectionType type;
- NetworkTethering tethering;
- RealDBusWrapper dbus_iface;
- ConnectionManager* connection_manager = system_state_->connection_manager();
- if (!connection_manager->GetConnectionProperties(&dbus_iface,
- &type, &tethering)) {
- LOG(INFO) << "We could not determine our connection type. "
- << "Defaulting to allow updates.";
- return true;
- }
- bool is_allowed = connection_manager->IsUpdateAllowedOver(type, tethering);
- LOG(INFO) << "We are connected via "
- << connection_manager->StringForConnectionType(type)
- << ", Updates allowed: " << (is_allowed ? "Yes" : "No");
- return is_allowed;
-}
-
bool LibcurlHttpFetcher::GetProxyType(const std::string& proxy,
curl_proxytype* out_type) {
if (utils::StringHasPrefix(proxy, "socks5://") ||
@@ -161,15 +141,7 @@
CHECK_EQ(curl_easy_setopt(curl_handle_, CURLOPT_WRITEDATA, this), CURLE_OK);
CHECK_EQ(curl_easy_setopt(curl_handle_, CURLOPT_WRITEFUNCTION,
StaticLibcurlWrite), CURLE_OK);
-
- string url_to_use(url_);
- if (!IsUpdateAllowedOverCurrentConnection()) {
- LOG(INFO) << "Not initiating HTTP connection b/c updates are disabled "
- << "over this connection";
- url_to_use = ""; // Sabotage the URL
- }
-
- CHECK_EQ(curl_easy_setopt(curl_handle_, CURLOPT_URL, url_to_use.c_str()),
+ CHECK_EQ(curl_easy_setopt(curl_handle_, CURLOPT_URL, url_.c_str()),
CURLE_OK);
// If the connection drops under |low_speed_limit_bps_| (10
@@ -195,7 +167,7 @@
// Lock down the appropriate curl options for HTTP or HTTPS depending on
// the url.
if (GetSystemState()->hardware()->IsOfficialBuild()) {
- if (StartsWithASCII(url_to_use, "http://", false))
+ if (StartsWithASCII(url_, "http://", false))
SetCurlOptionsForHttp();
else
SetCurlOptionsForHttps();
diff --git a/libcurl_http_fetcher.h b/libcurl_http_fetcher.h
index a3b7919..6008183 100644
--- a/libcurl_http_fetcher.h
+++ b/libcurl_http_fetcher.h
@@ -14,7 +14,6 @@
#include <glib.h>
#include "update_engine/certificate_checker.h"
-#include "update_engine/connection_manager.h"
#include "update_engine/hardware_interface.h"
#include "update_engine/http_fetcher.h"
#include "update_engine/system_state.h"
@@ -195,10 +194,6 @@
// be destroyed.
void ForceTransferTermination();
- // Returns true if updates are allowed over the current type of connection.
- // False otherwise.
- bool IsUpdateAllowedOverCurrentConnection() const;
-
// Sets the curl options for HTTP URL.
void SetCurlOptionsForHttp();
diff --git a/omaha_request_action.cc b/omaha_request_action.cc
index 9c5bbf5..9e43385 100644
--- a/omaha_request_action.cc
+++ b/omaha_request_action.cc
@@ -27,6 +27,7 @@
#include "update_engine/p2p_manager.h"
#include "update_engine/payload_state_interface.h"
#include "update_engine/prefs_interface.h"
+#include "update_engine/real_dbus_wrapper.h"
#include "update_engine/utils.h"
using base::Time;
@@ -879,17 +880,9 @@
output_object.update_exists = true;
SetOutputObject(output_object);
- if (params_->update_disabled()) {
- LOG(INFO) << "Ignoring Omaha updates as updates are disabled by policy.";
+ if (ShouldIgnoreUpdate(output_object)) {
output_object.update_exists = false;
completer.set_code(kErrorCodeOmahaUpdateIgnoredPerPolicy);
- // Note: We could technically delete the UpdateFirstSeenAt state here.
- // If we do, it'll mean a device has to restart the UpdateFirstSeenAt
- // and thus help scattering take effect when the AU is turned on again.
- // On the other hand, it also increases the chance of update starvation if
- // an admin turns AU on/off more frequently. We choose to err on the side
- // of preventing starvation at the cost of not applying scattering in
- // those cases.
return;
}
@@ -1359,4 +1352,55 @@
result, reaction, download_error_code);
}
+bool OmahaRequestAction::ShouldIgnoreUpdate(
+ const OmahaResponse& response) const {
+ if (params_->update_disabled()) {
+ LOG(INFO) << "Ignoring Omaha updates as updates are disabled by policy.";
+ return true;
+ }
+
+ // Note: policy decision to not update to a version we rolled back from.
+ string rollback_version =
+ system_state_->payload_state()->GetRollbackVersion();
+ if(!rollback_version.empty()) {
+ LOG(INFO) << "Detected previous rollback from version " << rollback_version;
+ if(rollback_version == response.version) {
+ LOG(INFO) << "Received version that we rolled back from. Ignoring.";
+ return true;
+ }
+ }
+
+ if(!IsUpdateAllowedOverCurrentConnection()) {
+ LOG(INFO) << "Update is not allowed over current connection.";
+ return true;
+ }
+
+ // Note: We could technically delete the UpdateFirstSeenAt state when we
+ // return true. If we do, it'll mean a device has to restart the
+ // UpdateFirstSeenAt and thus help scattering take effect when the AU is
+ // turned on again. On the other hand, it also increases the chance of update
+ // starvation if an admin turns AU on/off more frequently. We choose to err on
+ // the side of preventing starvation at the cost of not applying scattering in
+ // those cases.
+ return false;
+}
+
+bool OmahaRequestAction::IsUpdateAllowedOverCurrentConnection() const {
+ NetworkConnectionType type;
+ NetworkTethering tethering;
+ RealDBusWrapper dbus_iface;
+ ConnectionManager* connection_manager = system_state_->connection_manager();
+ if (!connection_manager->GetConnectionProperties(&dbus_iface,
+ &type, &tethering)) {
+ LOG(INFO) << "We could not determine our connection type. "
+ << "Defaulting to allow updates.";
+ return true;
+ }
+ bool is_allowed = connection_manager->IsUpdateAllowedOver(type, tethering);
+ LOG(INFO) << "We are connected via "
+ << connection_manager->StringForConnectionType(type)
+ << ", Updates allowed: " << (is_allowed ? "Yes" : "No");
+ return is_allowed;
+}
+
} // namespace chromeos_update_engine
diff --git a/omaha_request_action.h b/omaha_request_action.h
index 1eba719..c8505a9 100644
--- a/omaha_request_action.h
+++ b/omaha_request_action.h
@@ -249,6 +249,13 @@
// Callback used by LookupPayloadViaP2P().
void OnLookupPayloadViaP2PCompleted(const std::string& url);
+ // Returns true if the current update should be ignored.
+ bool ShouldIgnoreUpdate(const OmahaResponse& response) const;
+
+ // Returns true if updates are allowed over the current type of connection.
+ // False otherwise.
+ bool IsUpdateAllowedOverCurrentConnection() const;
+
// Global system context.
SystemState* system_state_;
diff --git a/omaha_request_action_unittest.cc b/omaha_request_action_unittest.cc
index 9de0e9f..e371103 100644
--- a/omaha_request_action_unittest.cc
+++ b/omaha_request_action_unittest.cc
@@ -10,10 +10,12 @@
#include <base/strings/string_util.h>
#include <base/strings/stringprintf.h>
#include <base/time/time.h>
+#include <chromeos/dbus/service_constants.h>
#include "gtest/gtest.h"
#include "update_engine/action_pipe.h"
#include "update_engine/constants.h"
+#include "update_engine/mock_connection_manager.h"
#include "update_engine/mock_http_fetcher.h"
#include "update_engine/omaha_hash_calculator.h"
#include "update_engine/omaha_request_action.h"
@@ -212,6 +214,7 @@
// OmahaRequestAction. |prefs| may be NULL, in which case a local PrefsMock is
// used. |payload_state| may be NULL, in which case a local mock is used.
// |p2p_manager| may be NULL, in which case a local mock is used.
+// |connection_manager| may be NULL, in which case a local mock is used.
// out_response may be NULL. If |fail_http_response_code| is non-negative,
// the transfer will fail with that code. |ping_only| is passed through to the
// OmahaRequestAction constructor. out_post_data may be null; if non-null, the
@@ -225,6 +228,7 @@
bool TestUpdateCheck(PrefsInterface* prefs,
PayloadStateInterface *payload_state,
P2PManager *p2p_manager,
+ ConnectionManager *connection_manager,
OmahaRequestParams& params,
const string& http_response,
int fail_http_response_code,
@@ -249,6 +253,8 @@
fake_system_state.set_payload_state(payload_state);
if (p2p_manager)
fake_system_state.set_p2p_manager(p2p_manager);
+ if (connection_manager)
+ fake_system_state.set_connection_manager(connection_manager);
fake_system_state.set_request_params(¶ms);
OmahaRequestAction action(&fake_system_state,
NULL,
@@ -327,6 +333,7 @@
TestUpdateCheck(NULL, // prefs
NULL, // payload_state
NULL, // p2p_manager
+ NULL, // connection_manager
kDefaultTestParams,
GetNoUpdateResponse(OmahaRequestParams::kAppId),
-1,
@@ -346,6 +353,7 @@
TestUpdateCheck(NULL, // prefs
NULL, // payload_state
NULL, // p2p_manager
+ NULL, // connection_manager
kDefaultTestParams,
GetUpdateResponse(OmahaRequestParams::kAppId,
"1.2.3.4", // version
@@ -384,6 +392,7 @@
TestUpdateCheck(NULL, // prefs
NULL, // payload_state
NULL, // p2p_manager
+ NULL, // connection_manager
params,
GetUpdateResponse(OmahaRequestParams::kAppId,
"1.2.3.4", // version
@@ -406,6 +415,82 @@
EXPECT_FALSE(response.update_exists);
}
+TEST(OmahaRequestActionTest, ValidUpdateBlockedByConnection) {
+ OmahaResponse response;
+ // Set up a connection manager that doesn't allow a valid update over
+ // the current ethernet connection.
+ MockConnectionManager mock_cm(NULL);
+ EXPECT_CALL(mock_cm, GetConnectionProperties(_,_,_))
+ .WillRepeatedly(DoAll(SetArgumentPointee<1>(kNetEthernet),
+ SetArgumentPointee<2>(NetworkTethering::kUnknown),
+ Return(true)));
+ EXPECT_CALL(mock_cm, IsUpdateAllowedOver(kNetEthernet, _))
+ .WillRepeatedly(Return(false));
+ EXPECT_CALL(mock_cm, StringForConnectionType(kNetEthernet))
+ .WillRepeatedly(Return(shill::kTypeEthernet));
+
+ ASSERT_FALSE(
+ TestUpdateCheck(NULL, // prefs
+ NULL, // payload_state
+ NULL, // p2p_manager
+ &mock_cm, // connection_manager
+ kDefaultTestParams,
+ GetUpdateResponse(OmahaRequestParams::kAppId,
+ "1.2.3.4", // version
+ "http://more/info",
+ "true", // prompt
+ "http://code/base/", // dl url
+ "file.signed", // file name
+ "HASH1234=", // checksum
+ "false", // needs admin
+ "123", // size
+ ""), // deadline
+ -1,
+ false, // ping_only
+ kErrorCodeOmahaUpdateIgnoredPerPolicy,
+ metrics::CheckResult::kUpdateAvailable,
+ metrics::CheckReaction::kIgnored,
+ metrics::DownloadErrorCode::kUnset,
+ &response,
+ NULL));
+ EXPECT_FALSE(response.update_exists);
+}
+
+TEST(OmahaRequestActionTest, ValidUpdateBlockedByRollback) {
+ string rollback_version = "1234.0.0";
+ OmahaResponse response;
+
+ MockPayloadState mock_payload_state;
+ EXPECT_CALL(mock_payload_state, GetRollbackVersion())
+ .WillRepeatedly(Return(rollback_version));
+
+ ASSERT_FALSE(
+ TestUpdateCheck(NULL, // prefs
+ &mock_payload_state, // payload_state
+ NULL, // p2p_manager
+ NULL, // connection_manager
+ kDefaultTestParams,
+ GetUpdateResponse(OmahaRequestParams::kAppId,
+ rollback_version, // version
+ "http://more/info",
+ "true", // prompt
+ "http://code/base/", // dl url
+ "file.signed", // file name
+ "HASH1234=", // checksum
+ "false", // needs admin
+ "123", // size
+ ""), // deadline
+ -1,
+ false, // ping_only
+ kErrorCodeOmahaUpdateIgnoredPerPolicy,
+ metrics::CheckResult::kUpdateAvailable,
+ metrics::CheckReaction::kIgnored,
+ metrics::DownloadErrorCode::kUnset,
+ &response,
+ NULL));
+ EXPECT_FALSE(response.update_exists);
+}
+
TEST(OmahaRequestActionTest, NoUpdatesSentWhenBlockedByPolicyTest) {
OmahaResponse response;
OmahaRequestParams params = kDefaultTestParams;
@@ -414,6 +499,7 @@
TestUpdateCheck(NULL, // prefs
NULL, // payload_state
NULL, // p2p_manager
+ NULL, // connection_manager
params,
GetNoUpdateResponse(OmahaRequestParams::kAppId),
-1,
@@ -447,6 +533,7 @@
TestUpdateCheck(&prefs, // prefs
NULL, // payload_state
NULL, // p2p_manager
+ NULL, // connection_manager
params,
GetUpdateResponse2(OmahaRequestParams::kAppId,
"1.2.3.4", // version
@@ -478,6 +565,7 @@
TestUpdateCheck(&prefs, // prefs
NULL, // payload_state
NULL, // p2p_manager
+ NULL, // connection_manager
params,
GetUpdateResponse2(OmahaRequestParams::kAppId,
"1.2.3.4", // version
@@ -527,6 +615,7 @@
TestUpdateCheck(&prefs, // prefs
NULL, // payload_state
NULL, // p2p_manager
+ NULL, // connection_manager
params,
GetUpdateResponse2(OmahaRequestParams::kAppId,
"1.2.3.4", // version
@@ -576,6 +665,7 @@
TestUpdateCheck(&prefs, // prefs
NULL, // payload_state
NULL, // p2p_manager
+ NULL, // connection_manager
params,
GetUpdateResponse2(OmahaRequestParams::kAppId,
"1.2.3.4", // version
@@ -626,6 +716,7 @@
&prefs, // prefs
NULL, // payload_state
NULL, // p2p_manager
+ NULL, // connection_manager
params,
GetUpdateResponse2(OmahaRequestParams::kAppId,
"1.2.3.4", // version
@@ -679,6 +770,7 @@
&prefs, // prefs
NULL, // payload_state
NULL, // p2p_manager
+ NULL, // connection_manager
params,
GetUpdateResponse2(OmahaRequestParams::kAppId,
"1.2.3.4", // version
@@ -714,6 +806,7 @@
TestUpdateCheck(&prefs, // prefs
NULL, // payload_state
NULL, // p2p_manager
+ NULL, // connection_manager
params,
GetUpdateResponse2(OmahaRequestParams::kAppId,
"1.2.3.4", // version
@@ -765,6 +858,7 @@
&prefs, // prefs
NULL, // payload_state
NULL, // p2p_manager
+ NULL, // connection_manager
params,
GetUpdateResponse2(OmahaRequestParams::kAppId,
"1.2.3.4", // version
@@ -802,6 +896,7 @@
TestUpdateCheck(&prefs, // prefs
NULL, // payload_state
NULL, // p2p_manager
+ NULL, // connection_manager
params,
GetUpdateResponse2(OmahaRequestParams::kAppId,
"1.2.3.4", // version
@@ -859,6 +954,7 @@
TestUpdateCheck(NULL, // prefs
NULL, // payload_state
NULL, // p2p_manager
+ NULL, // connection_manager
kDefaultTestParams,
"invalid xml>",
-1,
@@ -878,6 +974,7 @@
TestUpdateCheck(NULL, // prefs
NULL, // payload_state
NULL, // p2p_manager
+ NULL, // connection_manager
kDefaultTestParams,
"",
-1,
@@ -897,6 +994,7 @@
NULL, // prefs
NULL, // payload_state
NULL, // p2p_manager
+ NULL, // connection_manager
kDefaultTestParams,
"<?xml version=\"1.0\" encoding=\"UTF-8\"?><response protocol=\"3.0\">"
"<daystart elapsed_seconds=\"100\"/>"
@@ -920,6 +1018,7 @@
NULL, // prefs
NULL, // payload_state
NULL, // p2p_manager
+ NULL, // connection_manager
kDefaultTestParams,
"<?xml version=\"1.0\" encoding=\"UTF-8\"?><response protocol=\"3.0\">"
"<daystart elapsed_seconds=\"100\"/>"
@@ -943,6 +1042,7 @@
NULL, // prefs
NULL, // payload_state
NULL, // p2p_manager
+ NULL, // connection_manager
kDefaultTestParams,
"<?xml version=\"1.0\" encoding=\"UTF-8\"?><response protocol=\"3.0\">"
"<daystart elapsed_seconds=\"100\"/>"
@@ -984,6 +1084,7 @@
ASSERT_TRUE(TestUpdateCheck(NULL, // prefs
NULL, // payload_state
NULL, // p2p_manager
+ NULL, // connection_manager
kDefaultTestParams,
input_response,
-1,
@@ -1081,6 +1182,7 @@
TestUpdateCheck(NULL, // prefs
NULL, // payload_state
NULL, // p2p_manager
+ NULL, // connection_manager
params,
"invalid xml>",
-1,
@@ -1109,6 +1211,7 @@
TestUpdateCheck(NULL, // prefs
NULL, // payload_state
NULL, // p2p_manager
+ NULL, // connection_manager
kDefaultTestParams,
GetUpdateResponse(OmahaRequestParams::kAppId,
"1.2.3.4", // version
@@ -1140,6 +1243,7 @@
TestUpdateCheck(NULL, // prefs
NULL, // payload_state
NULL, // p2p_manager
+ NULL, // connection_manager
kDefaultTestParams,
GetUpdateResponse(OmahaRequestParams::kAppId,
"1.2.3.4", // version
@@ -1173,6 +1277,7 @@
ASSERT_FALSE(TestUpdateCheck(&prefs,
NULL, // payload_state
NULL, // p2p_manager
+ NULL, // connection_manager
kDefaultTestParams,
"invalid xml>",
-1,
@@ -1209,6 +1314,7 @@
ASSERT_FALSE(TestUpdateCheck(&prefs,
NULL, // payload_state
NULL, // p2p_manager
+ NULL, // connection_manager
params,
"invalid xml>",
-1,
@@ -1324,6 +1430,7 @@
ASSERT_FALSE(TestUpdateCheck(NULL, // prefs
NULL, // payload_state
NULL, // p2p_manager
+ NULL, // connection_manager
params,
"invalid xml>",
-1,
@@ -1371,6 +1478,7 @@
ASSERT_FALSE(TestUpdateCheck(NULL, // prefs
NULL, // payload_state
NULL, // p2p_manager
+ NULL, // connection_manager
params,
"invalid xml>",
-1,
@@ -1431,6 +1539,7 @@
TestUpdateCheck(&prefs,
NULL, // payload_state
NULL, // p2p_manager
+ NULL, // connection_manager
kDefaultTestParams,
GetNoUpdateResponse(OmahaRequestParams::kAppId),
-1,
@@ -1473,6 +1582,7 @@
TestUpdateCheck(&prefs,
NULL, // payload_state
NULL, // p2p_manager
+ NULL, // connection_manager
kDefaultTestParams,
GetNoUpdateResponse(OmahaRequestParams::kAppId),
-1,
@@ -1507,6 +1617,7 @@
TestUpdateCheck(&prefs,
NULL, // payload_state
NULL, // p2p_manager
+ NULL, // connection_manager
kDefaultTestParams,
GetNoUpdateResponse(OmahaRequestParams::kAppId),
-1,
@@ -1542,6 +1653,7 @@
TestUpdateCheck(&prefs,
NULL, // payload_state
NULL, // p2p_manager
+ NULL, // connection_manager
kDefaultTestParams,
GetNoUpdateResponse(OmahaRequestParams::kAppId),
-1,
@@ -1571,6 +1683,7 @@
TestUpdateCheck(&prefs,
NULL, // payload_state
NULL, // p2p_manager
+ NULL, // connection_manager
kDefaultTestParams,
GetNoUpdateResponse(OmahaRequestParams::kAppId),
-1,
@@ -1606,6 +1719,7 @@
TestUpdateCheck(&prefs,
NULL, // payload_state
NULL, // p2p_manager
+ NULL, // connection_manager
kDefaultTestParams,
"<?xml version=\"1.0\" encoding=\"UTF-8\"?><response "
"protocol=\"3.0\"><daystart elapsed_seconds=\"100\"/>"
@@ -1645,6 +1759,7 @@
TestUpdateCheck(&prefs,
NULL, // payload_state
NULL, // p2p_manager
+ NULL, // connection_manager
kDefaultTestParams,
"<?xml version=\"1.0\" encoding=\"UTF-8\"?><response "
"protocol=\"3.0\"><daystart elapsed_seconds=\"200\"/>"
@@ -1670,6 +1785,7 @@
TestUpdateCheck(&prefs,
NULL, // payload_state
NULL, // p2p_manager
+ NULL, // connection_manager
kDefaultTestParams,
"<?xml version=\"1.0\" encoding=\"UTF-8\"?><response "
"protocol=\"3.0\"><daystart blah=\"200\"/>"
@@ -1695,6 +1811,7 @@
TestUpdateCheck(&prefs,
NULL, // payload_state
NULL, // p2p_manager
+ NULL, // connection_manager
kDefaultTestParams,
"<?xml version=\"1.0\" encoding=\"UTF-8\"?><response "
"protocol=\"3.0\"><daystart elapsed_seconds=\"x\"/>"
@@ -1715,6 +1832,7 @@
ASSERT_FALSE(TestUpdateCheck(NULL, // prefs
NULL, // payload_state
NULL, // p2p_manager
+ NULL, // connection_manager
kDefaultTestParams,
"invalid xml>",
-1,
@@ -1737,6 +1855,7 @@
TestUpdateCheck(NULL, // prefs
NULL, // payload_state
NULL, // p2p_manager
+ NULL, // connection_manager
kDefaultTestParams,
"",
501,
@@ -1757,6 +1876,7 @@
TestUpdateCheck(NULL, // prefs
NULL, // payload_state
NULL, // p2p_manager
+ NULL, // connection_manager
kDefaultTestParams,
"",
1500,
@@ -1791,6 +1911,7 @@
&prefs, // prefs
NULL, // payload_state
NULL, // p2p_manager
+ NULL, // connection_manager
params,
GetUpdateResponse2(OmahaRequestParams::kAppId,
"1.2.3.4", // version
@@ -1826,6 +1947,7 @@
TestUpdateCheck(&prefs, // prefs
NULL, // payload_state
NULL, // p2p_manager
+ NULL, // connection_manager
params,
GetUpdateResponse2(OmahaRequestParams::kAppId,
"1.2.3.4", // version
@@ -1877,6 +1999,7 @@
&prefs, // prefs
NULL, // payload_state
NULL, // p2p_manager
+ NULL, // connection_manager
params,
GetUpdateResponse2(OmahaRequestParams::kAppId,
"1.2.3.4", // version
@@ -1940,6 +2063,7 @@
ASSERT_FALSE(TestUpdateCheck(&prefs,
NULL, // payload_state
NULL, // p2p_manager
+ NULL, // connection_manager
params,
"invalid xml>",
-1,
@@ -1990,6 +2114,7 @@
ASSERT_FALSE(TestUpdateCheck(&prefs,
NULL, // payload_state
NULL, // p2p_manager
+ NULL, // connection_manager
params,
"invalid xml>",
-1,
@@ -2040,6 +2165,7 @@
TestUpdateCheck(NULL, // prefs
&mock_payload_state,
&mock_p2p_manager,
+ NULL, // connection_manager
request_params,
GetUpdateResponse2(OmahaRequestParams::kAppId,
"1.2.3.4", // version
@@ -2164,6 +2290,7 @@
TestUpdateCheck(prefs,
NULL, // payload_state
NULL, // p2p_manager
+ NULL, // connection_manager
kDefaultTestParams,
GetUpdateResponse2(OmahaRequestParams::kAppId,
"1.2.3.4", // version
diff --git a/omaha_response_handler_action.cc b/omaha_response_handler_action.cc
index e80dcb0..7a30403 100644
--- a/omaha_response_handler_action.cc
+++ b/omaha_response_handler_action.cc
@@ -10,6 +10,7 @@
#include <base/strings/string_util.h>
#include <policy/device_policy.h>
+#include "update_engine/connection_manager.h"
#include "update_engine/constants.h"
#include "update_engine/delta_performer.h"
#include "update_engine/hardware_interface.h"
@@ -49,17 +50,6 @@
return;
}
- // Note: policy decision to not update to a version we rolled back from.
- string rollback_version =
- system_state_->payload_state()->GetRollbackVersion();
- if(!rollback_version.empty()) {
- LOG(INFO) << "Detected previous rollback from version " << rollback_version;
- if(rollback_version == response.version) {
- LOG(INFO) << "Received version that we rolled back from. Aborting.";
- return;
- }
- }
-
// All decisions as to which URL should be used have already been done. So,
// make the current URL as the download URL.
string current_url = system_state_->payload_state()->GetCurrentUrl();
diff --git a/omaha_response_handler_action_unittest.cc b/omaha_response_handler_action_unittest.cc
index bb35100..82f7369 100644
--- a/omaha_response_handler_action_unittest.cc
+++ b/omaha_response_handler_action_unittest.cc
@@ -85,8 +85,6 @@
string current_url = in.payload_urls.size() ? in.payload_urls[0] : "";
EXPECT_CALL(*(fake_system_state->mock_payload_state()), GetCurrentUrl())
.WillRepeatedly(Return(current_url));
- EXPECT_CALL(*(fake_system_state->mock_payload_state()), GetRollbackVersion())
- .WillRepeatedly(Return(kBadVersion));
OmahaResponseHandlerAction response_handler_action(
fake_system_state,
@@ -198,33 +196,6 @@
EXPECT_EQ("", install_plan.version);
}
-TEST_F(OmahaResponseHandlerActionTest, RollbackVersionTest) {
- string version_ok = "124.0.0";
-
- InstallPlan install_plan;
- OmahaResponse in;
- in.update_exists = true;
- in.version = kBadVersion;
- in.payload_urls.push_back("http://foo/the_update_a.b.c.d.tgz");
- in.more_info_url = "http://more/info";
- in.hash = "HASHj+";
- in.size = 12;
- in.prompt = true;
-
- // Version is blacklisted for first call so no update.
- EXPECT_FALSE(DoTest(in, "/dev/sda5", "", &install_plan));
- EXPECT_EQ("", install_plan.download_url);
- EXPECT_EQ("", install_plan.payload_hash);
- EXPECT_EQ("", install_plan.install_path);
- EXPECT_EQ("", install_plan.version);
-
- // Version isn't blacklisted.
- in.version = version_ok;
- EXPECT_TRUE(DoTest(in, "/dev/sda5", "", &install_plan));
- EXPECT_EQ(in.payload_urls[0], install_plan.download_url);
- EXPECT_EQ(version_ok, install_plan.version);
-}
-
TEST_F(OmahaResponseHandlerActionTest, HashChecksForHttpTest) {
OmahaResponse in;
in.update_exists = true;