Checking mirror status only if necessary

When doing an update, we check to see if a mirror is stale, this only
needs to be done if there is a mirror set and if the upstream server is
up and reachable.

This commit checks to see if a mirror is set and there is an upstream
server to compare against, and only then it checks to see if it is stale.

Closes #922

Signed-off-by: Castulo Martinez <castulo.martinez@intel.com>

src/curl.c

@@ -123,13 +123,18 @@ exit:
 	return curl_ret;
 }
 
-static int check_connection(const char *test_capath)
+int check_connection(const char *test_capath, char *url)
 {
 	CURLcode curl_ret;
 	long response = 0;
 
-	debug("Curl - check_connection url: %s\n", version_url);
-	curl_ret = swupd_curl_set_basic_options(curl, version_url, false);
+	if (!url) {
+		debug("Please provide the url to use to test the connection\n");
+		return -1;
+	}
+
+	debug("Curl - check_connection url: %s\n", url);
+	curl_ret = swupd_curl_set_basic_options(curl, url, false);
 	if (curl_ret != CURLE_OK) {
 		return -1;
 	}
@@ -150,6 +155,9 @@ static int check_connection(const char *test_capath)
 
 	switch (curl_ret) {
 	case CURLE_OK:
+		/* Note: when using http and the user is behind a proxy, we may get
+		 * a valid response here from the proxy even if the http server is
+		 * down, allow it to continue for now, it will fail further in the process */
 		return 0;
 	case CURLE_SSL_CACERT:
 		debug("Curl - Unable to verify server SSL certificate\n");
@@ -173,7 +181,7 @@ static int check_connection(const char *test_capath)
 	}
 }
 
-int swupd_curl_init(void)
+int swupd_curl_init(char *url)
 {
 	CURLcode curl_ret;
 	char *str;
@@ -203,15 +211,17 @@ int swupd_curl_init(void)
 	/* enforce the use of https or file */
 	if (!is_url_allowed(version_url) ||
 	    (strcmp(content_url, version_url) != 0 && !is_url_allowed(content_url))) {
+		swupd_curl_deinit();
 		return -1;
 	}
 
-	ret = check_connection(NULL);
+	ret = check_connection(NULL, url);
 	if (ret == 0) {
 		return 0;
 	} else if (ret == -CURLE_OPERATION_TIMEDOUT) {
 		error("Curl - Communicating with server timed out\n");
-		return -1;
+		ret = -1;
+		goto exit;
 	}
 
 	if (FALLBACK_CAPATHS[0]) {
@@ -225,7 +235,7 @@ int swupd_curl_init(void)
 			}
 
 			debug("Curl - Trying fallback CA path %s\n", tok);
-			ret = check_connection(tok);
+			ret = check_connection(tok, url);
 			if (ret == 0) {
 				capath = strdup_or_die(tok);
 				break;
@@ -234,13 +244,16 @@ int swupd_curl_init(void)
 		free_string(&str);
 	}
 
+exit:
 	if (ret != 0) {
+		/* curl failed to initialize */
 		error("Failed to connect to update server: %s\n", version_url);
 		info("Possible solutions for this problem are:\n"
 		     "\tCheck if your network connection is working\n"
 		     "\tFix the system clock\n"
 		     "\tRun 'swupd info' to check if the urls are correct\n"
 		     "\tCheck if the server SSL certificate is trusted by your system ('clrtrust generate' may help)\n");
+		swupd_curl_deinit();
 	}
 
 	return ret;

src/globals.c

@@ -87,16 +87,6 @@ void global_set_opt_from_config(bool state)
 	from_config = state;
 }
 
-static void remove_trailing_slash(char *url)
-{
-	int len = strlen(url);
-
-	while (len > 0 && url[len - 1] == '/') {
-		len--;
-		url[len] = 0;
-	}
-}
-
 /* If the MIX_BUNDLES_DIR has the valid-mix flag file we can run through
  * adding the mix data to the OS */
 bool check_mix_exists(void)

src/helpers.c

@@ -518,6 +518,16 @@ void swupd_deinit(void)
 	dump_file_descriptor_leaks();
 }
 
+void remove_trailing_slash(char *url)
+{
+	int len = strlen(url);
+
+	while (len > 0 && url[len - 1] == '/') {
+		len--;
+		url[len] = 0;
+	}
+}
+
 /* this function is intended to encapsulate the basic swupd
 * initializations for the majority of commands, that is:
 * 	- Make sure root is the user running the code
@@ -579,7 +589,7 @@ enum swupd_code swupd_init(enum swupd_init_config config)
 	}
 
 	if ((config & SWUPD_NO_NETWORK) == 0) {
-		if (swupd_curl_init() != 0) {
+		if (swupd_curl_init(version_url) != 0) {
 			ret = SWUPD_CURL_INIT_FAILED;
 			goto out_close_lock;
 		}

src/mirror.c

@@ -114,6 +114,13 @@ static int unset_mirror_url()
 		goto out;
 	}
 
+	/* we need to also unset the mirror urls from the cache and set it to
+	 * the central version */
+	free_string(&version_url);
+	free_string(&content_url);
+	set_default_version_url();
+	set_default_content_url();
+	get_latest_version("");
 out:
 	free_string(&content_path);
 	free_string(&version_path);
@@ -215,46 +222,85 @@ out:
 	return ret;
 }
 
+static bool mirror_is_set(void)
+{
+	bool mirror_set;
+	char *version_path;
+	char *content_path;
+
+	version_path = mk_full_filename(path_prefix, MIRROR_VERSION_URL_PATH);
+	content_path = mk_full_filename(path_prefix, MIRROR_CONTENT_URL_PATH);
+	mirror_set = file_exists(version_path) && file_exists(content_path);
+
+	free_string(&version_path);
+	free_string(&content_path);
+
+	return mirror_set;
+}
+
 void handle_mirror_if_stale(void)
 {
 	char *ret_str = NULL;
 	char *fullpath = NULL;
 
+	/* make sure there is a mirror set up first */
+	if (!mirror_is_set()) {
+		return;
+	}
+
+	info("Checking mirror status\n");
+
 	fullpath = mk_full_filename(path_prefix, DEFAULT_VERSION_URL_PATH);
 	int ret = get_value_from_path(&ret_str, fullpath, true);
 	if (ret != 0 || ret_str == NULL) {
 		/* no versionurl file here, might not exist under --path argument */
 		goto out;
 	}
+
+	/* before trying to get the latest version let's make sure the central version is up */
+	if (!content_url_is_local && !check_connection(NULL, ret_str) != 0) {
+		warn("Upstream server %s not responding, cannot determine upstream version\n", ret_str);
+		warn("Unable to determine if the mirror is up to date\n");
+		goto out;
+	}
+
 	int central_version = get_latest_version(ret_str);
 	int mirror_version = get_latest_version("");
 
-	/* note that negative mirror_version and negative central_version are handled
-	 * correctly, as the difference will be great for a mirror_version of -1
-	 * (it will resolve to central_version + 1 and the diff will be very large).
-	 * When the upstream central_version is an error -1 version the mirror version
-	 * will be used due to the diff being negative. */
-	int diff = central_version - mirror_version;
-	if (diff > MIRROR_STALE_UNSET || mirror_version == -1) {
-		warn("removing stale mirror configuration. "
-		     "Mirror version (%d) is too far behind upstream version (%d)\n",
-		     mirror_version,
-		     central_version);
-		unset_mirror_url();
-		/* we need to re-set the cached_version to latest using the central version,
-		 * since at the moment the cached_version is the outdated mirror version */
-		free_string(&version_url);
-		free_string(&content_url);
-		set_default_version_url();
-		set_default_content_url();
-		get_latest_version(ret_str);
+	/* if the latest version could not be retrieved from the central server we cannot
+	 * check if mirror is stale, this would be very odd since we already check connection
+	 * with the server earlier */
+	if (central_version < 0) {
+		warn("Upstream server %s not responding, cannot determine upstream version\n", ret_str);
+		warn("Unable to determine if the mirror is up to date\n");
 		goto out;
 	}
-	if (diff > MIRROR_STALE_WARN) {
-		warn("mirror version (%d) is behind upstream version (%d)\n",
+
+	/* if the mirror_version has an error or it's outdated and central_version works,
+	 * we should unset the mirror*/
+	int diff = central_version - mirror_version;
+	if (mirror_version > 0 && diff <= MIRROR_STALE_UNSET) {
+		if (diff > MIRROR_STALE_WARN) {
+			warn("mirror version (%d) is behind upstream version (%d)\n",
+			     mirror_version,
+			     central_version);
+		}
+		/* no need to unset */
+		goto out;
+	}
+
+	/* if we've made it this far we need to unset */
+	if (mirror_version < 0) {
+		warn("the mirror version could not be determined\n");
+	} else {
+		warn("the mirror version (%d) is too far behind upstream version (%d)\n",
 		     mirror_version,
 		     central_version);
 	}
+
+	info("Removing mirror configuration\n");
+	unset_mirror_url();
+
 out:
 	free_string(&fullpath);
 	free_string(&ret_str);

src/swupd.h

@@ -356,6 +356,7 @@ extern bool on_new_format(void);
 extern char *get_printable_bundle_name(const char *bundle_name, bool is_experimental);
 extern void print_regexp_error(int errcode, regex_t *regexp);
 extern bool is_url_allowed(char *url);
+extern void remove_trailing_slash(char *url);
 
 /* subscription.c */
 struct list *free_list_file(struct list *item);

src/swupd_curl.h

@@ -64,9 +64,21 @@ typedef int (*swupd_curl_progress_cb)(void *clientp, int64_t dltotal, int64_t dl
 /**
  * @brief Init the swupd curl.
  *
+ * @param url The url to use to test server connectivity
+ *
  * @note Must be called before any curl operation.
  */
-int swupd_curl_init(void);
+int swupd_curl_init(char *url);
+
+/**
+ * @brief Test a connection with a server.
+ *
+ * @param url The url to use to test server connectivity
+ * @param test_capath path for the certificate
+ *
+ * @returns 0 if the server is responding
+ */
+int check_connection(const char *test_capath, char *url);
 
 /**
  * @brief Close curl and release all memory allocated by swupd_curl_init().

test/functional/update/update-non-responsive.bats

@@ -53,10 +53,16 @@ test_teardown() {
 		Warning: This is an insecure connection
 		The --allow-insecure-http flag was used, be aware that this poses a threat to the system
 		Error: Curl - Communicating with server timed out
+		Error: Failed to connect to update server: http://localhost:$port
+		Possible solutions for this problem are:
+		.Check if your network connection is working
+		.Fix the system clock
+		.Run 'swupd info' to check if the urls are correct
+		.Check if the server SSL certificate is trusted by your system \('clrtrust generate' may help\)
 		Error: Updater failed to initialize, exiting now.
 	EOM
 	)
-	assert_is_output "$expected_output"
+	assert_regex_is_output "$expected_output"
 	assert_file_not_exists "$TARGETDIR"/test_file1
 
 }
@@ -77,10 +83,16 @@ test_teardown() {
 		Warning: This is an insecure connection
 		The --allow-insecure-http flag was used, be aware that this poses a threat to the system
 		Error: Curl - Communicating with server timed out
+		Error: Failed to connect to update server: http://localhost:$port
+		Possible solutions for this problem are:
+		.Check if your network connection is working
+		.Fix the system clock
+		.Run 'swupd info' to check if the urls are correct
+		.Check if the server SSL certificate is trusted by your system \('clrtrust generate' may help\)
 		Error: Updater failed to initialize, exiting now.
 	EOM
 	)
-	assert_is_output "$expected_output"
+	assert_regex_is_output "$expected_output"
 	assert_file_not_exists "$TARGETDIR"/test_file1
 
 }
@@ -99,7 +111,10 @@ test_teardown() {
 	assert_status_is 0
 	expected_output=$(cat <<-EOM
 		Update started.
-		Error: Curl - Communicating with server timed out - '.*/version/formatstaging/latest'
+		Checking mirror status
+		Error: Curl - Communicating with server timed out - 'http://localhost:$port//version/formatstaging/latest'
+		Warning: Upstream server http://localhost:$port/ not responding, cannot determine upstream version
+		Warning: Unable to determine if the mirror is up to date
 		Preparing to update from 10 to 20
 		Downloading packs for:
 		 - os-core

test/functional/update/update-with-mirror.bats

@@ -33,6 +33,7 @@ test_setup() {
 	assert_status_is 0
 	expected_output=$(cat <<-EOM
 		Update started.
+		Checking mirror status
 		Preparing to update from 10 to 20
 		Downloading packs for:
 		 - test-bundle1

test/functional/update/update-with-old-mirror.bats

@@ -34,7 +34,9 @@ test_setup() {
 	assert_status_is 0
 	expected_output=$(cat <<-EOM
 		Update started.
-		Warning: removing stale mirror configuration. Mirror version (20) is too far behind upstream version (1000)
+		Checking mirror status
+		Warning: the mirror version (20) is too far behind upstream version (1000)
+		Removing mirror configuration
 		Preparing to update from 10 to 1000
 		Downloading packs for:
 		 - test-bundle1

test/functional/update/update-with-slightly-old-mirror.bats

@@ -35,6 +35,7 @@ test_setup() {
 	assert_status_is 0
 	expected_output=$(cat <<-EOM
 		Update started.
+		Checking mirror status
 		Warning: mirror version (20) is behind upstream version (100)
 		Preparing to update from 10 to 20
 		Downloading packs for: