From 9375d5b8c212956f3469b86e62e5d7f057194ea2 Mon Sep 17 00:00:00 2001
From: BlackDex <black.dex@gmail.com>
Date: Fri, 24 Sep 2021 18:27:52 +0200
Subject: [PATCH] Updated icon downloading

- Unicode websites could break (www.post.japanpost.jp for example).
  regex would fail because it was missing the unicode-perl feature.
- Be less verbose in logging with icon downloads
- Removed duplicate info/error messages
- Added err_silent! macro to help with the less verbose error/info messages.
---
 Cargo.toml       |  2 +-
 src/api/icons.rs | 17 ++++++++++-------
 src/error.rs     |  9 +++++++++
 3 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/Cargo.toml b/Cargo.toml
index 048201f4..f4b36d0a 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -119,7 +119,7 @@ handlebars = { version = "4.1.3", features = ["dir_source"] }
 # For favicon extraction from main website
 html5ever = "0.25.1"
 markup5ever_rcdom = "0.1.0"
-regex = { version = "1.5.4", features = ["std", "perf"], default-features = false }
+regex = { version = "1.5.4", features = ["std", "perf", "unicode-perl"], default-features = false }
 data-url = "0.1.0"
 
 # Used by U2F, JWT and Postgres
diff --git a/src/api/icons.rs b/src/api/icons.rs
index 36eb286c..675ba43d 100644
--- a/src/api/icons.rs
+++ b/src/api/icons.rs
@@ -250,7 +250,7 @@ fn is_domain_blacklisted(domain: &str) -> bool {
 
             // Use the pre-generate Regex stored in a Lazy HashMap.
             if regex.is_match(domain) {
-                warn!("Blacklisted domain: {:#?} matched {:#?}", domain, blacklist);
+                warn!("Blacklisted domain: {} matched ICON_BLACKLIST_REGEX", domain);
                 is_blacklisted = true;
             }
         }
@@ -555,7 +555,7 @@ fn get_page(url: &str) -> Result<Response, Error> {
 
 fn get_page_with_referer(url: &str, referer: &str) -> Result<Response, Error> {
     if is_domain_blacklisted(url::Url::parse(url).unwrap().host_str().unwrap_or_default()) {
-        err!("Favicon rel linked to a blacklisted domain!");
+        err!("Favicon resolves to a blacklisted domain or IP!", url);
     }
 
     let mut client = CLIENT.get(url);
@@ -563,7 +563,10 @@ fn get_page_with_referer(url: &str, referer: &str) -> Result<Response, Error> {
         client = client.header("Referer", referer)
     }
 
-    client.send()?.error_for_status().map_err(Into::into)
+    match client.send() {
+        Ok(c) => c.error_for_status().map_err(Into::into),
+        Err(e) => err_silent!(format!("{}", e)),
+    }
 }
 
 /// Returns a Integer with the priority of the type of the icon which to prefer.
@@ -647,7 +650,7 @@ fn parse_sizes(sizes: Option<&str>) -> (u16, u16) {
 
 fn download_icon(domain: &str) -> Result<(Vec<u8>, Option<&str>), Error> {
     if is_domain_blacklisted(domain) {
-        err!("Domain is blacklisted", domain)
+        err_silent!("Domain is blacklisted", domain)
     }
 
     let icon_result = get_icon_url(domain)?;
@@ -676,7 +679,7 @@ fn download_icon(domain: &str) -> Result<(Vec<u8>, Option<&str>), Error> {
                         break;
                     }
                 }
-                _ => warn!("Extracted icon from data:image uri is invalid"),
+                _ => debug!("Extracted icon from data:image uri is invalid"),
             };
         } else {
             match get_page_with_referer(&icon.href, &icon_result.referer) {
@@ -692,13 +695,13 @@ fn download_icon(domain: &str) -> Result<(Vec<u8>, Option<&str>), Error> {
                     info!("Downloaded icon from {}", icon.href);
                     break;
                 }
-                _ => warn!("Download failed for {}", icon.href),
+                Err(e) => debug!("{:?}", e),
             };
         }
     }
 
     if buffer.is_empty() {
-        err!("Empty response downloading icon")
+        err_silent!("Empty response or unable find a valid icon", domain);
     }
 
     Ok((buffer, icon_type))
diff --git a/src/error.rs b/src/error.rs
index b944be03..6f11a4ea 100644
--- a/src/error.rs
+++ b/src/error.rs
@@ -220,6 +220,15 @@ macro_rules! err {
     }};
 }
 
+macro_rules! err_silent {
+    ($msg:expr) => {{
+        return Err(crate::error::Error::new($msg, $msg));
+    }};
+    ($usr_msg:expr, $log_value:expr) => {{
+        return Err(crate::error::Error::new($usr_msg, $log_value));
+    }};
+}
+
 #[macro_export]
 macro_rules! err_code {
     ($msg:expr, $err_code: expr) => {{