AU: Manual proxy support
Utilize the ChromeProxyResolver to resolve proxies in our network
requests. This means the following changes:
- HttpFetcher classes take a ProxyResolver* in their ctor. Also, a few
useful functions in HttpFetcher to allow subclasses to iterate
through the proxies.
- LibcurlHttpFetcher support for using the ProxyResolver. It will
attempt to use each proxy in the order specified. If any data comes
in from any proxy, it won't continue down the list and will continue
to use that proxy for its lifetime.
- UpdateAttempter can choose, for a given update session, whether or
not to use the ChromeProxyResolver or DirectProxyResolver. For now,
the logic is: for automatic checks, 80% of the time use
ChromeProxyResolver, 20% DirectProxyResolver. For manual checks, the
first 19 manual checks in a row use Chrome, then once it uses
Direct, then starts over again. The idea is that the updater doesn't
necessarily trust Chrome, so some requests should skip it. If a
manual check is performed, the user likely wants her proxy settings
honored, so use them, but don't allow frequent manual checks to
starve out usage of the DirectProxyResolver.
- Updates to tests
BUG=3167
TEST=unittests, tested on device
Review URL: http://codereview.chromium.org/5205002
Change-Id: Iee0f589e5b28d4b804afe1f5b6729ba066d48d62
diff --git a/SConstruct b/SConstruct
index d036745..c9d5068 100644
--- a/SConstruct
+++ b/SConstruct
@@ -253,6 +253,7 @@
full_update_generator.cc
graph_utils.cc
gzip.cc
+ http_fetcher.cc
libcurl_http_fetcher.cc
marshal.glibmarshal.c
omaha_hash_calculator.cc
diff --git a/chrome_proxy_resolver.cc b/chrome_proxy_resolver.cc
index eaa213a..66bc5dc 100644
--- a/chrome_proxy_resolver.cc
+++ b/chrome_proxy_resolver.cc
@@ -24,7 +24,7 @@
bool ChromeProxyResolver::GetProxiesForUrl(
const std::string& url,
- std::vector<std::string>* out_proxies) {
+ std::deque<std::string>* out_proxies) {
// First, query dbus for the currently stored settings
DBusGProxy* proxy = DbusProxy();
TEST_AND_RETURN_FALSE(proxy);
@@ -85,7 +85,7 @@
bool ChromeProxyResolver::GetProxiesForUrlWithSettings(
const string& url,
const string& json_settings,
- std::vector<std::string>* out_proxies) {
+ std::deque<std::string>* out_proxies) {
base::JSONReader parser;
scoped_ptr<Value> root(
diff --git a/chrome_proxy_resolver.h b/chrome_proxy_resolver.h
index 796f8fb..28d96cd 100644
--- a/chrome_proxy_resolver.h
+++ b/chrome_proxy_resolver.h
@@ -5,8 +5,8 @@
#ifndef CHROMEOS_PLATFORM_UPDATE_ENGINE_CHROME_PROXY_RESOLVER_H__
#define CHROMEOS_PLATFORM_UPDATE_ENGINE_CHROME_PROXY_RESOLVER_H__
+#include <deque>
#include <string>
-#include <vector>
#include <curl/curl.h>
#include <gtest/gtest_prod.h> // for FRIEND_TEST
@@ -33,12 +33,11 @@
virtual ~ChromeProxyResolver() {}
virtual bool GetProxiesForUrl(const std::string& url,
- std::vector<std::string>* out_proxies);
+ std::deque<std::string>* out_proxies);
// Get the curl proxy type for a given proxy url. Returns true on success.
// Note: if proxy is kNoProxy, this will return false.
- static bool GetProxyType(const std::string& proxy,
- curl_proxytype* out_type);
+ static bool GetProxyType(const std::string& proxy, curl_proxytype* out_type);
private:
FRIEND_TEST(ChromeProxyResolverTest, GetProxiesForUrlWithSettingsTest);
@@ -54,7 +53,7 @@
// success.
bool GetProxiesForUrlWithSettings(const std::string& url,
const std::string& json_settings,
- std::vector<std::string>* out_proxies);
+ std::deque<std::string>* out_proxies);
DbusGlibInterface* dbus_;
diff --git a/chrome_proxy_resolver_unittest.cc b/chrome_proxy_resolver_unittest.cc
index 66b2401..1cdd5bf 100644
--- a/chrome_proxy_resolver_unittest.cc
+++ b/chrome_proxy_resolver_unittest.cc
@@ -2,16 +2,16 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
+#include <deque>
#include <string>
-#include <vector>
#include <gtest/gtest.h>
#include "update_engine/chrome_proxy_resolver.h"
#include "update_engine/mock_dbus_interface.h"
+using std::deque;
using std::string;
-using std::vector;
using ::testing::_;
using ::testing::Return;
using ::testing::SetArgumentPointee;
@@ -38,7 +38,7 @@
"\"mode\":4}";
ChromeProxyResolver resolver(NULL);
- vector<string> out;
+ deque<string> out;
string urls[] = {"http://foo.com/update", "https://bar.com/foo.gz"};
string multi_settings[] = {kAll, kHttpHttps};
for (size_t i = 0; i < arraysize(urls); i++) {
@@ -119,7 +119,7 @@
SetArgumentPointee<9>(ret_array),
Return(TRUE)));
- vector<string> proxies;
+ deque<string> proxies;
EXPECT_TRUE(resolver.GetProxiesForUrl("http://user:pass@foo.com:22",
&proxies));
EXPECT_EQ(2, proxies.size());
diff --git a/download_action_unittest.cc b/download_action_unittest.cc
index 8135841..16c2507 100644
--- a/download_action_unittest.cc
+++ b/download_action_unittest.cc
@@ -143,7 +143,9 @@
ObjectFeederAction<InstallPlan> feeder_action;
feeder_action.set_obj(install_plan);
PrefsMock prefs;
- MockHttpFetcher* http_fetcher = new MockHttpFetcher(&data[0], data.size());
+ MockHttpFetcher* http_fetcher = new MockHttpFetcher(&data[0],
+ data.size(),
+ NULL);
// takes ownership of passed in HttpFetcher
DownloadAction download_action(&prefs, http_fetcher);
download_action.SetTestFileWriter(&writer);
@@ -289,7 +291,9 @@
feeder_action.set_obj(install_plan);
PrefsMock prefs;
DownloadAction download_action(&prefs,
- new MockHttpFetcher(&data[0], data.size()));
+ new MockHttpFetcher(&data[0],
+ data.size(),
+ NULL));
download_action.SetTestFileWriter(&writer);
DownloadActionDelegateMock download_delegate;
if (use_download_delegate) {
@@ -393,7 +397,7 @@
ObjectFeederAction<InstallPlan> feeder_action;
feeder_action.set_obj(install_plan);
PrefsMock prefs;
- DownloadAction download_action(&prefs, new MockHttpFetcher("x", 1));
+ DownloadAction download_action(&prefs, new MockHttpFetcher("x", 1, NULL));
download_action.SetTestFileWriter(&writer);
DownloadActionTestAction test_action;
@@ -427,7 +431,7 @@
ObjectFeederAction<InstallPlan> feeder_action;
feeder_action.set_obj(install_plan);
PrefsMock prefs;
- DownloadAction download_action(&prefs, new MockHttpFetcher("x", 1));
+ DownloadAction download_action(&prefs, new MockHttpFetcher("x", 1, NULL));
download_action.SetTestFileWriter(&writer);
BondActions(&feeder_action, &download_action);
diff --git a/filesystem_copier_action_unittest.cc b/filesystem_copier_action_unittest.cc
index cd77bef..032a24d 100644
--- a/filesystem_copier_action_unittest.cc
+++ b/filesystem_copier_action_unittest.cc
@@ -192,6 +192,10 @@
EXPECT_TRUE(ExpectVectorsEq(a_loop_data, a_out));
EXPECT_TRUE(collector_action.object() == install_plan);
+ if (terminate_early) {
+ // sleep so OS can clean up
+ sleep(1);
+ }
}
class FilesystemCopierActionTest2Delegate : public ActionProcessorDelegate {
diff --git a/http_fetcher.cc b/http_fetcher.cc
new file mode 100644
index 0000000..5d00660
--- /dev/null
+++ b/http_fetcher.cc
@@ -0,0 +1,31 @@
+// Copyright (c) 2009 The Chromium OS Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "update_engine/http_fetcher.h"
+
+using std::deque;
+using std::string;
+
+namespace chromeos_update_engine {
+
+void HttpFetcher::SetPostData(const void* data, size_t size) {
+ post_data_set_ = true;
+ post_data_.clear();
+ const char *char_data = reinterpret_cast<const char*>(data);
+ post_data_.insert(post_data_.end(), char_data, char_data + size);
+}
+
+// Proxy methods to set the proxies, then to pop them off.
+void HttpFetcher::ResolveProxiesForUrl(const string& url) {
+ if (!proxy_resolver_) {
+ LOG(INFO) << "Not resolving proxies (no proxy resolver).";
+ return;
+ }
+ deque<string> proxies;
+ if (proxy_resolver_->GetProxiesForUrl(url, &proxies)) {
+ SetProxies(proxies);
+ }
+}
+
+} // namespace chromeos_update_engine
diff --git a/http_fetcher.h b/http_fetcher.h
index 6d8608b..a50c760 100644
--- a/http_fetcher.h
+++ b/http_fetcher.h
@@ -5,10 +5,15 @@
#ifndef CHROMEOS_PLATFORM_UPDATE_ENGINE_HTTP_FETCHER_H__
#define CHROMEOS_PLATFORM_UPDATE_ENGINE_HTTP_FETCHER_H__
+#include <deque>
#include <string>
#include <vector>
+
+#include <base/basictypes.h>
+#include <base/logging.h>
#include <glib.h>
-#include "base/basictypes.h"
+
+#include "update_engine/proxy_resolver.h"
// This class is a simple wrapper around an HTTP library (libcurl). We can
// easily mock out this interface for testing.
@@ -23,10 +28,15 @@
class HttpFetcher {
public:
- HttpFetcher()
+ // |proxy_resolver| is the resolver that will be consulted for proxy
+ // settings. It may be null, in which case direct connections will
+ // be used. Does not take ownership of the resolver.
+ explicit HttpFetcher(ProxyResolver* proxy_resolver)
: post_data_set_(false),
http_response_code_(0),
- delegate_(NULL) {}
+ delegate_(NULL),
+ proxies_(1, kNoProxy),
+ proxy_resolver_(proxy_resolver) {}
virtual ~HttpFetcher() {}
void set_delegate(HttpFetcherDelegate* delegate) { delegate_ = delegate; }
@@ -35,12 +45,19 @@
// Optional: Post data to the server. The HttpFetcher should make a copy
// of this data and upload it via HTTP POST during the transfer.
- void SetPostData(const void* data, size_t size) {
- post_data_set_ = true;
- post_data_.clear();
- const char *char_data = reinterpret_cast<const char*>(data);
- post_data_.insert(post_data_.end(), char_data, char_data + size);
+ void SetPostData(const void* data, size_t size);
+
+ // Proxy methods to set the proxies, then to pop them off.
+ void ResolveProxiesForUrl(const std::string& url);
+
+ void SetProxies(const std::deque<std::string>& proxies) {
+ proxies_ = proxies;
}
+ const std::string& GetCurrentProxy() const {
+ return proxies_.front();
+ }
+ bool HasProxy() const { return !proxies_.empty(); }
+ void PopProxy() { proxies_.pop_front(); }
// Downloading should resume from this offset
virtual void SetOffset(off_t offset) = 0;
@@ -84,6 +101,12 @@
// The delegate; may be NULL.
HttpFetcherDelegate* delegate_;
+
+ // Proxy servers
+ std::deque<std::string> proxies_;
+
+ ProxyResolver* const proxy_resolver_;
+
private:
DISALLOW_COPY_AND_ASSIGN(HttpFetcher);
};
diff --git a/http_fetcher_unittest.cc b/http_fetcher_unittest.cc
index 87dda78..a42bb3b 100644
--- a/http_fetcher_unittest.cc
+++ b/http_fetcher_unittest.cc
@@ -8,14 +8,16 @@
#include <utility>
#include <vector>
-#include "base/logging.h"
-#include "base/scoped_ptr.h"
-#include "base/string_util.h"
-#include "glib.h"
-#include "gtest/gtest.h"
+#include <base/logging.h>
+#include <base/scoped_ptr.h>
+#include <base/string_util.h>
+#include <glib.h>
+#include <gtest/gtest.h>
+
#include "update_engine/libcurl_http_fetcher.h"
#include "update_engine/mock_http_fetcher.h"
#include "update_engine/multi_http_fetcher.h"
+#include "update_engine/proxy_resolver.h"
using std::make_pair;
using std::string;
@@ -56,10 +58,16 @@
public:
HttpFetcher* NewLargeFetcher() {
vector<char> big_data(1000000);
- return new MockHttpFetcher(big_data.data(), big_data.size());
+ return new MockHttpFetcher(
+ big_data.data(),
+ big_data.size(),
+ reinterpret_cast<ProxyResolver*>(&proxy_resolver_));
}
HttpFetcher* NewSmallFetcher() {
- return new MockHttpFetcher("x", 1);
+ return new MockHttpFetcher(
+ "x",
+ 1,
+ reinterpret_cast<ProxyResolver*>(&proxy_resolver_));
}
string BigUrl() const {
return "unused://unused";
@@ -71,6 +79,8 @@
bool IsMulti() const { return false; }
typedef NullHttpServer HttpServer;
void IgnoreServerAborting(HttpServer* server) const {}
+
+ DirectProxyResolver proxy_resolver_;
};
class PythonHttpServer {
@@ -131,7 +141,8 @@
class HttpFetcherTest<LibcurlHttpFetcher> : public ::testing::Test {
public:
virtual HttpFetcher* NewLargeFetcher() {
- LibcurlHttpFetcher *ret = new LibcurlHttpFetcher;
+ LibcurlHttpFetcher *ret = new
+ LibcurlHttpFetcher(reinterpret_cast<ProxyResolver*>(&proxy_resolver_));
// Speed up test execution.
ret->set_idle_seconds(1);
ret->set_retry_seconds(1);
@@ -155,6 +166,7 @@
PythonHttpServer *pyserver = reinterpret_cast<PythonHttpServer*>(server);
pyserver->validate_quit_ = false;
}
+ DirectProxyResolver proxy_resolver_;
};
template <>
@@ -163,7 +175,8 @@
public:
HttpFetcher* NewLargeFetcher() {
MultiHttpFetcher<LibcurlHttpFetcher> *ret =
- new MultiHttpFetcher<LibcurlHttpFetcher>;
+ new MultiHttpFetcher<LibcurlHttpFetcher>(
+ reinterpret_cast<ProxyResolver*>(&proxy_resolver_));
MultiHttpFetcher<LibcurlHttpFetcher>::RangesVect
ranges(1, make_pair(0, -1));
ret->set_ranges(ranges);
@@ -175,6 +188,7 @@
return ret;
}
bool IsMulti() const { return true; }
+ DirectProxyResolver proxy_resolver_;
};
typedef ::testing::Types<LibcurlHttpFetcher,
diff --git a/libcurl_http_fetcher.cc b/libcurl_http_fetcher.cc
index 460b980..c6293b1 100644
--- a/libcurl_http_fetcher.cc
+++ b/libcurl_http_fetcher.cc
@@ -9,6 +9,7 @@
#include <base/logging.h>
+#include "update_engine/chrome_proxy_resolver.h"
#include "update_engine/dbus_interface.h"
#include "update_engine/flimflam_proxy.h"
#include "update_engine/utils.h"
@@ -59,6 +60,25 @@
curl_handle_ = curl_easy_init();
CHECK(curl_handle_);
+ CHECK(HasProxy());
+ LOG(INFO) << "Using proxy: " << GetCurrentProxy();
+ if (GetCurrentProxy() == kNoProxy) {
+ CHECK_EQ(curl_easy_setopt(curl_handle_,
+ CURLOPT_PROXY,
+ ""), CURLE_OK);
+ } else {
+ CHECK_EQ(curl_easy_setopt(curl_handle_,
+ CURLOPT_PROXY,
+ GetCurrentProxy().c_str()), CURLE_OK);
+ // Curl seems to require us to set the protocol
+ curl_proxytype type;
+ if (ChromeProxyResolver::GetProxyType(GetCurrentProxy(), &type)) {
+ CHECK_EQ(curl_easy_setopt(curl_handle_,
+ CURLOPT_PROXYTYPE,
+ type), CURLE_OK);
+ }
+ }
+
if (post_data_set_) {
CHECK_EQ(curl_easy_setopt(curl_handle_, CURLOPT_POST, 1), CURLE_OK);
CHECK_EQ(curl_easy_setopt(curl_handle_, CURLOPT_POSTFIELDS,
@@ -133,6 +153,7 @@
resume_offset_ = 0;
retry_count_ = 0;
http_response_code_ = 0;
+ ResolveProxiesForUrl(url);
ResumeTransfer(url);
CurlPerformOnce();
}
@@ -178,6 +199,24 @@
// we're done!
CleanUp();
+ if (!sent_byte_ &&
+ (http_response_code_ < 200 || http_response_code_ >= 300)) {
+ // The transfer completed w/ error and we didn't get any bytes.
+ // If we have another proxy to try, try that.
+
+ PopProxy(); // Delete the proxy we just gave up on.
+
+ if (HasProxy()) {
+ // We have another proxy. Retry immediately.
+ g_idle_add(&LibcurlHttpFetcher::StaticRetryTimeoutCallback, this);
+ } else {
+ // Out of proxies. Give up.
+ if (delegate_)
+ delegate_->TransferComplete(this, false); // success
+ }
+ return;
+ }
+
if ((transfer_size_ >= 0) && (bytes_downloaded_ < transfer_size_)) {
// Need to restart transfer
retry_count_++;
@@ -208,6 +247,9 @@
}
size_t LibcurlHttpFetcher::LibcurlWrite(void *ptr, size_t size, size_t nmemb) {
+ if (size == 0)
+ return 0;
+ sent_byte_ = true;
GetHttpResponseCode();
{
double transfer_size_double;
diff --git a/libcurl_http_fetcher.h b/libcurl_http_fetcher.h
index b3ba518..c41804e 100644
--- a/libcurl_http_fetcher.h
+++ b/libcurl_http_fetcher.h
@@ -22,8 +22,9 @@
public:
static const int kMaxRedirects = 10;
- LibcurlHttpFetcher()
- : curl_multi_handle_(NULL),
+ explicit LibcurlHttpFetcher(ProxyResolver* proxy_resolver)
+ : HttpFetcher(proxy_resolver),
+ curl_multi_handle_(NULL),
curl_handle_(NULL),
timeout_source_(NULL),
transfer_in_progress_(false),
@@ -38,6 +39,7 @@
force_build_type_(false),
forced_official_build_(false),
in_write_callback_(false),
+ sent_byte_(false),
terminate_requested_(false) {}
// Cleans up all internal state. Does not notify delegate
@@ -198,6 +200,10 @@
// If true, we are currently performing a write callback on the delegate.
bool in_write_callback_;
+
+ // If true, we have returned at least one byte in the write callback
+ // to the delegate.
+ bool sent_byte_;
// We can't clean everything up while we're in a write callback, so
// if we get a terminate request, queue it until we can handle it.
diff --git a/main.cc b/main.cc
old mode 100644
new mode 100755
index 0e5d3bc..9dae81a
--- a/main.cc
+++ b/main.cc
@@ -17,6 +17,7 @@
#include <sys/stat.h>
#include "update_engine/dbus_constants.h"
+#include "update_engine/dbus_interface.h"
#include "update_engine/dbus_service.h"
#include "update_engine/prefs.h"
#include "update_engine/subprocess.h"
@@ -180,8 +181,10 @@
metrics_lib.Init();
// Create the update attempter:
+ chromeos_update_engine::ConcreteDbusGlib dbus;
chromeos_update_engine::UpdateAttempter update_attempter(&prefs,
- &metrics_lib);
+ &metrics_lib,
+ &dbus);
// Create the dbus service object:
dbus_g_object_type_install_info(UPDATE_ENGINE_TYPE_SERVICE,
diff --git a/mock_http_fetcher.h b/mock_http_fetcher.h
index dfe61a7..b46e023 100644
--- a/mock_http_fetcher.h
+++ b/mock_http_fetcher.h
@@ -6,8 +6,10 @@
#define CHROMEOS_PLATFORM_UPDATE_ENGINE_MOCK_HTTP_FETCHER_H__
#include <vector>
+
+#include <base/logging.h>
#include <glib.h>
-#include "base/logging.h"
+
#include "update_engine/http_fetcher.h"
// This is a mock implementation of HttpFetcher which is useful for testing.
@@ -26,8 +28,11 @@
public:
// The data passed in here is copied and then passed to the delegate after
// the transfer begins.
- MockHttpFetcher(const char* data, size_t size)
- : sent_size_(0),
+ MockHttpFetcher(const char* data,
+ size_t size,
+ ProxyResolver* proxy_resolver)
+ : HttpFetcher(proxy_resolver),
+ sent_size_(0),
timeout_source_(NULL),
timout_tag_(0),
paused_(false),
diff --git a/multi_http_fetcher.h b/multi_http_fetcher.h
index 533998b..3d5ee11 100644
--- a/multi_http_fetcher.h
+++ b/multi_http_fetcher.h
@@ -5,6 +5,7 @@
#ifndef CHROMEOS_PLATFORM_UPDATE_ENGINE_MULTI_HTTP_FETCHER_H__
#define CHROMEOS_PLATFORM_UPDATE_ENGINE_MULTI_HTTP_FETCHER_H__
+#include <deque>
#include <tr1/memory>
#include <utility>
#include <vector>
@@ -24,9 +25,11 @@
class MultiHttpFetcher : public HttpFetcher, public HttpFetcherDelegate {
public:
typedef std::vector<std::pair<off_t, off_t> > RangesVect;
+ typedef std::vector<std::tr1::shared_ptr<BaseHttpFetcher> > FetchersVect;
- MultiHttpFetcher()
- : sent_transfer_complete_(false),
+ MultiHttpFetcher(ProxyResolver* proxy_resolver)
+ : HttpFetcher(proxy_resolver),
+ sent_transfer_complete_(false),
pending_next_fetcher_(false),
current_index_(0),
bytes_received_this_fetcher_(0) {}
@@ -38,9 +41,11 @@
for (typename std::vector<std::tr1::shared_ptr<BaseHttpFetcher>
>::iterator it = fetchers_.begin(), e = fetchers_.end();
it != e; ++it) {
- (*it) = std::tr1::shared_ptr<BaseHttpFetcher>(new BaseHttpFetcher);
+ (*it) = std::tr1::shared_ptr<BaseHttpFetcher>(
+ new BaseHttpFetcher(proxy_resolver_));
(*it)->set_delegate(this);
}
+ LOG(INFO) << "done w/ list";
}
void SetOffset(off_t offset) {} // for now, doesn't support this
@@ -134,6 +139,13 @@
(*it)->SetBuildType(is_official);
}
}
+
+ virtual void SetProxies(const std::deque<std::string>& proxies) {
+ for (typename FetchersVect::iterator it = fetchers_.begin(),
+ e = fetchers_.end(); it != e; ++it) {
+ (*it)->SetProxies(proxies);
+ }
+ }
private:
void SendTransferComplete(HttpFetcher* fetcher, bool successful) {
@@ -220,7 +232,7 @@
bool pending_next_fetcher_;
RangesVect ranges_;
- std::vector<std::tr1::shared_ptr<BaseHttpFetcher> > fetchers_;
+ FetchersVect fetchers_;
RangesVect::size_type current_index_; // index into ranges_, fetchers_
off_t bytes_received_this_fetcher_;
diff --git a/omaha_request_action_unittest.cc b/omaha_request_action_unittest.cc
index dd71aab..8fe2482 100755
--- a/omaha_request_action_unittest.cc
+++ b/omaha_request_action_unittest.cc
@@ -160,7 +160,8 @@
vector<char>* out_post_data) {
GMainLoop* loop = g_main_loop_new(g_main_context_default(), FALSE);
MockHttpFetcher* fetcher = new MockHttpFetcher(http_response.data(),
- http_response.size());
+ http_response.size(),
+ NULL);
if (fail_http_response_code >= 0) {
fetcher->FailTransfer(fail_http_response_code);
}
@@ -200,7 +201,8 @@
vector<char>* out_post_data) {
GMainLoop* loop = g_main_loop_new(g_main_context_default(), FALSE);
MockHttpFetcher* fetcher = new MockHttpFetcher(http_response.data(),
- http_response.size());
+ http_response.size(),
+ NULL);
NiceMock<PrefsMock> prefs;
OmahaRequestAction action(&prefs, params, event, fetcher);
OmahaRequestActionTestProcessorDelegate delegate;
@@ -267,7 +269,8 @@
NiceMock<PrefsMock> prefs;
OmahaRequestAction action(&prefs, kDefaultTestParams, NULL,
new MockHttpFetcher(http_response.data(),
- http_response.size()));
+ http_response.size(),
+ NULL));
OmahaRequestActionTestProcessorDelegate delegate;
delegate.loop_ = loop;
ActionProcessor processor;
@@ -414,7 +417,8 @@
NiceMock<PrefsMock> prefs;
OmahaRequestAction action(&prefs, kDefaultTestParams, NULL,
new MockHttpFetcher(http_response.data(),
- http_response.size()));
+ http_response.size(),
+ NULL));
TerminateEarlyTestProcessorDelegate delegate;
delegate.loop_ = loop;
ActionProcessor processor;
@@ -599,7 +603,8 @@
kDefaultTestParams,
NULL,
new MockHttpFetcher(http_response.data(),
- http_response.size()));
+ http_response.size(),
+ NULL));
EXPECT_FALSE(update_check_action.IsEvent());
OmahaRequestAction event_action(
@@ -607,7 +612,8 @@
kDefaultTestParams,
new OmahaEvent(OmahaEvent::kTypeUpdateComplete),
new MockHttpFetcher(http_response.data(),
- http_response.size()));
+ http_response.size(),
+ NULL));
EXPECT_TRUE(event_action.IsEvent());
}
diff --git a/proxy_resolver.cc b/proxy_resolver.cc
index 6b3bd93..4ec8d1e 100644
--- a/proxy_resolver.cc
+++ b/proxy_resolver.cc
@@ -4,15 +4,15 @@
#include "update_engine/proxy_resolver.h"
+using std::deque;
using std::string;
-using std::vector;
namespace chromeos_update_engine {
const char kNoProxy[] = "direct://";
bool DirectProxyResolver::GetProxiesForUrl(const string& url,
- vector<string>* out_proxies) {
+ deque<string>* out_proxies) {
out_proxies->clear();
out_proxies->push_back(kNoProxy);
return true;
diff --git a/proxy_resolver.h b/proxy_resolver.h
index 5d9064c..af228d2 100644
--- a/proxy_resolver.h
+++ b/proxy_resolver.h
@@ -7,8 +7,8 @@
#include <base/logging.h>
+#include <deque>
#include <string>
-#include <vector>
namespace chromeos_update_engine {
@@ -26,7 +26,7 @@
// socks{4,5}://<host>[:<port>] - SOCKS4/5 proxy
// kNoProxy - no proxy
virtual bool GetProxiesForUrl(const std::string& url,
- std::vector<std::string>* out_proxies) = 0;
+ std::deque<std::string>* out_proxies) = 0;
private:
DISALLOW_COPY_AND_ASSIGN(ProxyResolver);
@@ -35,8 +35,9 @@
// Always says to not use a proxy
class DirectProxyResolver : public ProxyResolver {
public:
+ DirectProxyResolver() {}
virtual bool GetProxiesForUrl(const std::string& url,
- std::vector<std::string>* out_proxies);
+ std::deque<std::string>* out_proxies);
private:
DISALLOW_COPY_AND_ASSIGN(DirectProxyResolver);
diff --git a/update_attempter.cc b/update_attempter.cc
index ef6a949..4e7d7bf 100644
--- a/update_attempter.cc
+++ b/update_attempter.cc
@@ -14,6 +14,7 @@
#include <tr1/memory>
#include <vector>
+#include <base/rand_util.h>
#include <glib.h>
#include <metrics/metrics_library.h>
@@ -43,6 +44,10 @@
const char* kUpdateCompletedMarker =
"/var/run/update_engine_autoupdate_completed";
+namespace {
+const int kMaxConsecutiveObeyProxyRequests = 20;
+} // namespace {}
+
const char* UpdateStatusToString(UpdateStatus status) {
switch (status) {
case UPDATE_STATUS_IDLE:
@@ -89,7 +94,8 @@
}
UpdateAttempter::UpdateAttempter(PrefsInterface* prefs,
- MetricsLibraryInterface* metrics_lib)
+ MetricsLibraryInterface* metrics_lib,
+ DbusGlibInterface* dbus_iface)
: processor_(new ActionProcessor()),
dbus_service_(NULL),
prefs_(prefs),
@@ -104,7 +110,10 @@
last_checked_time_(0),
new_version_("0.0.0.0"),
new_size_(0),
- is_full_update_(false) {
+ is_full_update_(false),
+ proxy_manual_checks_(0),
+ obeying_proxies_(true),
+ chrome_proxy_resolver_(dbus_iface) {
if (utils::FileExists(kUpdateCompletedMarker))
status_ = UPDATE_STATUS_UPDATED_NEED_REBOOT;
}
@@ -114,7 +123,8 @@
}
void UpdateAttempter::Update(const std::string& app_version,
- const std::string& omaha_url) {
+ const std::string& omaha_url,
+ bool obey_proxies) {
if (status_ == UPDATE_STATUS_UPDATED_NEED_REBOOT) {
LOG(INFO) << "Not updating b/c we already updated and we're waiting for "
<< "reboot";
@@ -129,6 +139,24 @@
LOG(ERROR) << "Unable to initialize Omaha request device params.";
return;
}
+
+ obeying_proxies_ = true;
+ if (obey_proxies || proxy_manual_checks_ == 0) {
+ LOG(INFO) << "forced to obey proxies";
+ // If forced to obey proxies, every 20th request will not use proxies
+ proxy_manual_checks_++;
+ LOG(INFO) << "proxy manual checks: " << proxy_manual_checks_;
+ if (proxy_manual_checks_ >= kMaxConsecutiveObeyProxyRequests) {
+ proxy_manual_checks_ = 0;
+ obeying_proxies_ = false;
+ }
+ } else if (base::RandInt(0, 4) == 0) {
+ obeying_proxies_ = false;
+ }
+ LOG_IF(INFO, !obeying_proxies_) << "To help ensure updates work, this update "
+ "check we are ignoring the proxy settings and using "
+ "direct connections.";
+
DisableDeltaUpdateIfNeeded();
CHECK(!processor_->IsRunning());
processor_->set_delegate(this);
@@ -138,7 +166,7 @@
new OmahaRequestAction(prefs_,
omaha_request_params_,
NULL,
- new LibcurlHttpFetcher));
+ new LibcurlHttpFetcher(GetProxyResolver())));
shared_ptr<OmahaResponseHandlerAction> response_handler_action(
new OmahaResponseHandlerAction(prefs_));
shared_ptr<FilesystemCopierAction> filesystem_copier_action(
@@ -150,22 +178,23 @@
omaha_request_params_,
new OmahaEvent(
OmahaEvent::kTypeUpdateDownloadStarted),
- new LibcurlHttpFetcher));
+ new LibcurlHttpFetcher(GetProxyResolver())));
shared_ptr<DownloadAction> download_action(
- new DownloadAction(prefs_, new MultiHttpFetcher<LibcurlHttpFetcher>));
+ new DownloadAction(prefs_, new MultiHttpFetcher<LibcurlHttpFetcher>(
+ GetProxyResolver())));
shared_ptr<OmahaRequestAction> download_finished_action(
new OmahaRequestAction(prefs_,
omaha_request_params_,
new OmahaEvent(
OmahaEvent::kTypeUpdateDownloadFinished),
- new LibcurlHttpFetcher));
+ new LibcurlHttpFetcher(GetProxyResolver())));
shared_ptr<PostinstallRunnerAction> postinstall_runner_action(
new PostinstallRunnerAction);
shared_ptr<OmahaRequestAction> update_complete_action(
new OmahaRequestAction(prefs_,
omaha_request_params_,
new OmahaEvent(OmahaEvent::kTypeUpdateComplete),
- new LibcurlHttpFetcher));
+ new LibcurlHttpFetcher(GetProxyResolver())));
download_action->set_delegate(this);
response_handler_action_ = response_handler_action;
@@ -212,7 +241,7 @@
<< UpdateStatusToString(status_) << ", so not checking.";
return;
}
- Update(app_version, omaha_url);
+ Update(app_version, omaha_url, true);
}
bool UpdateAttempter::RebootIfNeeded() {
@@ -435,7 +464,7 @@
new OmahaRequestAction(prefs_,
omaha_request_params_,
error_event_.release(), // Pass ownership.
- new LibcurlHttpFetcher));
+ new LibcurlHttpFetcher(GetProxyResolver())));
actions_.push_back(shared_ptr<AbstractAction>(error_event_action));
processor_->EnqueueAction(error_event_action.get());
SetStatusAndNotify(UPDATE_STATUS_REPORTING_ERROR_EVENT);
diff --git a/update_attempter.h b/update_attempter.h
index 3a4ff49..ba0bd94 100644
--- a/update_attempter.h
+++ b/update_attempter.h
@@ -16,9 +16,11 @@
#include <gtest/gtest_prod.h> // for FRIEND_TEST
#include "update_engine/action_processor.h"
+#include "update_engine/chrome_proxy_resolver.h"
#include "update_engine/download_action.h"
#include "update_engine/omaha_request_params.h"
#include "update_engine/omaha_response_handler_action.h"
+#include "update_engine/proxy_resolver.h"
class MetricsLibraryInterface;
struct UpdateEngineService;
@@ -47,14 +49,19 @@
public:
static const int kMaxDeltaUpdateFailures;
- UpdateAttempter(PrefsInterface* prefs, MetricsLibraryInterface* metrics_lib);
+ UpdateAttempter(PrefsInterface* prefs,
+ MetricsLibraryInterface* metrics_lib,
+ DbusGlibInterface* dbus_iface);
virtual ~UpdateAttempter();
// Checks for update and, if a newer version is available, attempts
// to update the system. Non-empty |in_app_version| or
// |in_update_url| prevents automatic detection of the parameter.
+ // If |obey_proxies| is true, the update will likely respect Chrome's
+ // proxy setting. For security reasons, we may still not honor them.
virtual void Update(const std::string& app_version,
- const std::string& omaha_url);
+ const std::string& omaha_url,
+ bool obey_proxies);
// ActionProcessorDelegate methods:
void ProcessingDone(const ActionProcessor* processor, ActionExitCode code);
@@ -153,6 +160,12 @@
// If this was a delta update attempt that failed, count it so that a full
// update can be tried when needed.
void MarkDeltaUpdateFailure();
+
+ ProxyResolver* GetProxyResolver() {
+ return obeying_proxies_ ?
+ reinterpret_cast<ProxyResolver*>(&chrome_proxy_resolver_) :
+ reinterpret_cast<ProxyResolver*>(&direct_proxy_resolver_);
+ }
// Last status notification timestamp used for throttling. Use monotonic
// TimeTicks to ensure that notifications are sent even if the system clock is
@@ -208,6 +221,17 @@
// Device paramaters common to all Omaha requests.
OmahaRequestDeviceParams omaha_request_params_;
+ // Number of consecutive manual update checks we've had where we obeyed
+ // Chrome's proxy settings.
+ int proxy_manual_checks_;
+
+ // If true, this update cycle we are obeying proxies
+ bool obeying_proxies_;
+
+ // Our two proxy resolvers
+ DirectProxyResolver direct_proxy_resolver_;
+ ChromeProxyResolver chrome_proxy_resolver_;
+
DISALLOW_COPY_AND_ASSIGN(UpdateAttempter);
};
diff --git a/update_attempter_mock.h b/update_attempter_mock.h
index 56a0b42..908faeb 100644
--- a/update_attempter_mock.h
+++ b/update_attempter_mock.h
@@ -7,16 +7,19 @@
#include <gmock/gmock.h>
+#include "update_engine/mock_dbus_interface.h"
#include "update_engine/update_attempter.h"
namespace chromeos_update_engine {
class UpdateAttempterMock : public UpdateAttempter {
public:
- UpdateAttempterMock() : UpdateAttempter(NULL, NULL) {}
+ UpdateAttempterMock() : UpdateAttempter(NULL, NULL, &dbus_) {}
- MOCK_METHOD2(Update, void(const std::string& app_version,
- const std::string& omaha_url));
+ MOCK_METHOD3(Update, void(const std::string& app_version,
+ const std::string& omaha_url,
+ bool obey_proxies));
+ MockDbusGlib dbus_;
};
} // namespace chromeos_update_engine
diff --git a/update_attempter_unittest.cc b/update_attempter_unittest.cc
index 89adb03..a65ffea 100644
--- a/update_attempter_unittest.cc
+++ b/update_attempter_unittest.cc
@@ -8,6 +8,7 @@
#include "update_engine/action_mock.h"
#include "update_engine/action_processor_mock.h"
#include "update_engine/filesystem_copier_action.h"
+#include "update_engine/mock_dbus_interface.h"
#include "update_engine/postinstall_runner_action.h"
#include "update_engine/prefs_mock.h"
#include "update_engine/update_attempter.h"
@@ -30,7 +31,8 @@
class UpdateAttempterUnderTest : public UpdateAttempter {
public:
UpdateAttempterUnderTest()
- : UpdateAttempter(NULL, NULL) {}
+ : UpdateAttempter(NULL, NULL, &dbus_) {}
+ MockDbusGlib dbus_;
};
class UpdateAttempterTest : public ::testing::Test {
@@ -181,7 +183,7 @@
}
EXPECT_CALL(*processor_, StartProcessing()).Times(1);
- attempter_.Update("", "");
+ attempter_.Update("", "", false);
EXPECT_EQ(0, attempter_.http_response_code());
EXPECT_EQ(&attempter_, processor_->delegate());
diff --git a/update_check_scheduler.cc b/update_check_scheduler.cc
index 7319ec4..16f1929 100644
--- a/update_check_scheduler.cc
+++ b/update_check_scheduler.cc
@@ -82,7 +82,7 @@
CHECK(me->scheduled_);
me->scheduled_ = false;
if (me->IsOOBEComplete()) {
- me->update_attempter_->Update("", "");
+ me->update_attempter_->Update("", "", false);
} else {
// Skips all automatic update checks if the OOBE process is not complete and
// schedules a new check as if it is the first one.
diff --git a/update_check_scheduler_unittest.cc b/update_check_scheduler_unittest.cc
index 2d2e81d..c267108 100644
--- a/update_check_scheduler_unittest.cc
+++ b/update_check_scheduler_unittest.cc
@@ -270,7 +270,7 @@
TEST_F(UpdateCheckSchedulerTest, StaticCheckOOBECompleteTest) {
scheduler_.scheduled_ = true;
EXPECT_CALL(scheduler_, IsOOBEComplete()).Times(1).WillOnce(Return(true));
- EXPECT_CALL(attempter_, Update("", ""))
+ EXPECT_CALL(attempter_, Update("", "", false))
.Times(1)
.WillOnce(Assign(&scheduler_.scheduled_, true));
scheduler_.enabled_ = true;
@@ -281,7 +281,7 @@
TEST_F(UpdateCheckSchedulerTest, StaticCheckOOBENotCompleteTest) {
scheduler_.scheduled_ = true;
EXPECT_CALL(scheduler_, IsOOBEComplete()).Times(1).WillOnce(Return(false));
- EXPECT_CALL(attempter_, Update("", "")).Times(0);
+ EXPECT_CALL(attempter_, Update("", "", _)).Times(0);
int interval_min, interval_max;
FuzzRange(UpdateCheckScheduler::kTimeoutOnce,
UpdateCheckScheduler::kTimeoutRegularFuzz,