From 5c54dfee3adabe4e070dd02020ac3d96972bdec0 Mon Sep 17 00:00:00 2001
From: BlackDex <black.dex@gmail.com>
Date: Wed, 3 Jun 2020 17:07:32 +0200
Subject: [PATCH] Fixed an issue when DNS resolving fails.

In the event of a failed DNS Resolving checking for new versions will
cause a huge delay, and in the end a timeout when loading the page.

- Check if DNS resolving failed, if that is the case, do not check for
  new versions
- Changed `fn get_github_api` to make use of structs
- Added a timeout of 10 seconds for the version check requests
- Moved the "Unknown" lables to the "Latest" lable
---
 src/api/admin.rs                           | 89 ++++++++++++----------
 src/static/templates/admin/diagnostics.hbs | 10 ++-
 2 files changed, 56 insertions(+), 43 deletions(-)

diff --git a/src/api/admin.rs b/src/api/admin.rs
index bd9bf3e9..46338a9c 100644
--- a/src/api/admin.rs
+++ b/src/api/admin.rs
@@ -1,5 +1,6 @@
 use once_cell::sync::Lazy;
 use serde_json::Value;
+use serde::de::DeserializeOwned;
 use std::process::Command;
 
 use rocket::http::{Cookie, Cookies, SameSite};
@@ -315,28 +316,34 @@ fn organizations_overview(_token: AdminToken, conn: DbConn) -> ApiResult<Html<St
     Ok(Html(text))
 }
 
-#[derive(Deserialize, Serialize, Debug)]
-#[allow(non_snake_case)]
-pub struct WebVaultVersion {
+#[derive(Deserialize)]
+struct WebVaultVersion {
     version: String,
 }
 
-fn get_github_api(url: &str) -> Result<Value, Error> {
+#[derive(Deserialize)]
+struct GitRelease {
+    tag_name: String,
+}
+
+#[derive(Deserialize)]
+struct GitCommit {
+    sha: String,
+}
+
+fn get_github_api<T: DeserializeOwned>(url: &str) -> Result<T, Error> {
     use reqwest::{header::USER_AGENT, blocking::Client};
+    use std::time::Duration;
     let github_api = Client::builder().build()?;
 
-    let res = github_api
-        .get(url)
+    Ok(
+        github_api.get(url)
+        .timeout(Duration::from_secs(10))
         .header(USER_AGENT, "Bitwarden_RS")
-        .send()?;
-
-    let res_status = res.status();
-    if res_status != 200 {
-        error!("Could not retrieve '{}', response code: {}", url, res_status);
-    }
-
-    let value: Value = res.error_for_status()?.json()?;
-    Ok(value)
+        .send()?
+        .error_for_status()?
+        .json::<T>()?
+    )
 }
 
 #[get("/diagnostics")]
@@ -350,32 +357,36 @@ fn diagnostics(_token: AdminToken, _conn: DbConn) -> ApiResult<Html<String>> {
     let web_vault_version: WebVaultVersion = serde_json::from_str(&vault_version_str)?;
 
     let github_ips = ("github.com", 0).to_socket_addrs().map(|mut i| i.next());
-    let dns_resolved = match github_ips {
-        Ok(Some(a)) => a.ip().to_string(),
-        _ => "Could not resolve domain name.".to_string(),
+    let (dns_resolved, dns_ok) = match github_ips {
+        Ok(Some(a)) => (a.ip().to_string(), true),
+        _ => ("Could not resolve domain name.".to_string(), false),
     };
 
-    let bitwarden_rs_releases = get_github_api("https://api.github.com/repos/dani-garcia/bitwarden_rs/releases/latest");
-    let latest_release = match &bitwarden_rs_releases {
-        Ok(j) => j["tag_name"].as_str().unwrap(),
-        _ => "-",
+    // If the DNS Check failed, do not even attempt to check for new versions since we were not able to resolve github.com
+    let (latest_release, latest_commit, latest_web_build) = if dns_ok {
+        (
+            match get_github_api::<GitRelease>("https://api.github.com/repos/dani-garcia/bitwarden_rs/releases/latest") {
+                Ok(r) => r.tag_name,
+                _ => "-".to_string()
+            },
+            match get_github_api::<GitCommit>("https://api.github.com/repos/dani-garcia/bitwarden_rs/commits/master") {
+                Ok(mut c) => {
+                    c.sha.truncate(8);
+                    c.sha
+                },
+                _ => "-".to_string()
+            },
+            match get_github_api::<GitRelease>("https://api.github.com/repos/dani-garcia/bw_web_builds/releases/latest") {
+                Ok(r) => r.tag_name.trim_start_matches('v').to_string(),
+                _ => "-".to_string()
+            },
+        )
+    } else {
+        ("-".to_string(), "-".to_string(), "-".to_string())
     };
-
-    let bitwarden_rs_commits = get_github_api("https://api.github.com/repos/dani-garcia/bitwarden_rs/commits/master");
-    let mut latest_commit = match &bitwarden_rs_commits {
-        Ok(j) => j["sha"].as_str().unwrap(),
-        _ => "-",
-    };
-    if latest_commit.len() >= 8 {
-        latest_commit = &latest_commit[..8];
-    }
-
-    let bw_web_builds_releases = get_github_api("https://api.github.com/repos/dani-garcia/bw_web_builds/releases/latest");
-    let latest_web_build = match &bw_web_builds_releases {
-        Ok(j) => j["tag_name"].as_str().unwrap(),
-        _ => "-",
-    };
-
+    
+    // Run the date check as the last item right before filling the json.
+    // This should ensure that the time difference between the browser and the server is as minimal as possible.
     let dt = Utc::now();
     let server_time = dt.format("%Y-%m-%d %H:%M:%S").to_string();
 
@@ -385,7 +396,7 @@ fn diagnostics(_token: AdminToken, _conn: DbConn) -> ApiResult<Html<String>> {
         "web_vault_version": web_vault_version.version,
         "latest_release": latest_release,
         "latest_commit": latest_commit,
-        "latest_web_build": latest_web_build.replace("v", ""),
+        "latest_web_build": latest_web_build,
     });
 
     let text = AdminTemplateData::diagnostics(diagnostics_json).render()?;
diff --git a/src/static/templates/admin/diagnostics.hbs b/src/static/templates/admin/diagnostics.hbs
index b5eca6ce..21aa1f66 100644
--- a/src/static/templates/admin/diagnostics.hbs
+++ b/src/static/templates/admin/diagnostics.hbs
@@ -9,24 +9,26 @@
                     <dt class="col-sm-5">Server Installed
                         <span class="badge badge-success d-none" id="server-success" title="Latest version is installed.">Ok</span>
                         <span class="badge badge-warning d-none" id="server-warning" title="There seems to be an update available.">Update</span>
-                        <span class="badge badge-danger d-none" id="server-failed" title="Unable to determine latest version.">Unknown</span>
                     </dt>
                     <dd class="col-sm-7">
                         <span id="server-installed">{{version}}</span>
                     </dd>
-                    <dt class="col-sm-5">Server Latest</dt>
+                    <dt class="col-sm-5">Server Latest
+                        <span class="badge badge-danger d-none" id="server-failed" title="Unable to determine latest version.">Unknown</span>
+                    </dt>
                     <dd class="col-sm-7">
                         <span id="server-latest">{{diagnostics.latest_release}}<span id="server-latest-commit" class="d-none">-{{diagnostics.latest_commit}}</span></span>
                     </dd>
                     <dt class="col-sm-5">Web Installed
                         <span class="badge badge-success d-none" id="web-success" title="Latest version is installed.">Ok</span>
                         <span class="badge badge-warning d-none" id="web-warning" title="There seems to be an update available.">Update</span>
-                        <span class="badge badge-danger d-none" id="web-failed" title="Unable to determine latest version.">Unknown</span>
                     </dt>
                     <dd class="col-sm-7">
                         <span id="web-installed">{{diagnostics.web_vault_version}}</span>
                     </dd>
-                    <dt class="col-sm-5">Web Latest</dt>
+                    <dt class="col-sm-5">Web Latest
+                        <span class="badge badge-danger d-none" id="web-failed" title="Unable to determine latest version.">Unknown</span>
+                    </dt>
                     <dd class="col-sm-7">
                         <span id="web-latest">{{diagnostics.latest_web_build}}</span>
                     </dd>