more perf

Cargo.lock

@@ -1263,7 +1263,7 @@ dependencies = [
 
 [[package]]
 name = "mangadex-home"
-version = "0.2.0"
+version = "0.2.1"
 dependencies = [
  "actix-web",
  "base64 0.13.0",

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "mangadex-home"
-version = "0.2.0"
+version = "0.2.1"
 license = "MIT OR Apache-2.0"
 authors = ["Edward Shen <code@eddie.sh>"]
 edition = "2018"

README.md

@@ -1,5 +1,11 @@
 A Rust implementation of a Mangadex @ Home client.
 
+This client contains the following features:
+
+ - Multi-threaded
+ - HTTP/2 support
+ - No support for TLS 1.1 or 1.0
+
 ## Building
 
 ```sh
@@ -21,24 +27,42 @@ Note that these quotas are closer to a rough estimate, and is not guaranteed to
 be strictly below these values, so it's recommended to under set your config
 values to make sure you don't exceed the actual quota.
 
+## Installation
+
+Either build it from source or run `cargo install mangadex-home`.
+
 ## Running
 
-This version relies on loading configurations from `env`, or from a file called
-`.env`. The config options are below:
+Run `mangadex-home`, and make sure the advertised port is open on your firewall.
+Do note that some configuration fields are required. See the next section for
+details.
 
-```
-# Your MD@H client secret
-CLIENT_SECRET=
-# The port to use
-PORT=
-# The maximum disk cache size, in bytes
-DISK_CACHE_QUOTA_BYTES=
-# The path where the on-disk cache should be stored
-DISK_CACHE_PATH="./cache" # Optional, default is "./cache"
-# The maximum memory cache size, in bytes
-MEM_CACHE_QUOTA_BYTES=
-# The maximum memory speed, in bytes per second.
-MAX_NETWORK_SPEED=
-```
+## Configuration
 
-After these values have been set, simply run the client.
+Most configuration options can be either provided on the command line, sourced
+from a `.env` file, or sourced directly from the environment. Do not that the
+client secret is an exception. You must provide the client secret from the
+environment or from the `.env` file, as providing client secrets in a shell is a
+operation security risk.
+
+The following options are required:
+
+ - Client Secret
+ - Memory cache quota
+ - Disk cache quota
+ - Advertised network speed
+
+The following are optional as a default value will be set for you:
+
+ - Port
+ - Disk cache path
+
+ ### Advanced configuration
+
+ This implementation prefers to act more secure by default. As a result, some
+ features that the official specification requires are not enabled by default.
+ If you don't know why these features are disabled by default, then don't enable
+ these, as they may generally weaken the security stance of the client for more
+ compatibility.
+
+ - Sending Server version string

src/config.rs

@@ -1,8 +1,15 @@
 use std::num::{NonZeroU16, NonZeroUsize};
 use std::path::PathBuf;
+use std::sync::atomic::AtomicBool;
 
 use clap::Clap;
 
+// Validate tokens is an atomic because it's faster than locking on rwlock.
+pub static VALIDATE_TOKENS: AtomicBool = AtomicBool::new(false);
+// We use an atomic here because it's better for us to not pass the config
+// everywhere.
+pub static SEND_SERVER_VERSION: AtomicBool = AtomicBool::new(false);
+
 #[derive(Clap)]
 pub struct CliArgs {
     /// The port to listen on.
@@ -25,6 +32,6 @@ pub struct CliArgs {
     /// Whether or not to provide the Server HTTP header to clients. This is
     /// useful for debugging, but is generally not recommended for security
     /// reasons.
-    #[clap(long, env = "ENABLE_SERVER_STRING")]
+    #[clap(long, env = "ENABLE_SERVER_STRING", takes_value = false)]
     pub enable_server_string: bool,
 }

src/main.rs

@@ -2,13 +2,11 @@
 // We're end users, so these is ok
 #![allow(clippy::future_not_send, clippy::module_name_repetitions)]
 
+use std::env::{self, VarError};
+use std::process;
 use std::sync::atomic::AtomicBool;
 use std::sync::Arc;
 use std::time::Duration;
-use std::{
-    env::{self, VarError},
-    process,
-};
 use std::{num::ParseIntError, sync::atomic::Ordering};
 
 use actix_web::rt::{spawn, time, System};
@@ -37,6 +35,7 @@ macro_rules! client_api_version {
         "30"
     };
 }
+
 #[derive(Error, Debug)]
 enum ServerError {
     #[error("There was a failure parsing config")]
@@ -50,22 +49,33 @@ async fn main() -> Result<(), std::io::Error> {
     // It's ok to fail early here, it would imply we have a invalid config.
     dotenv::dotenv().ok();
     let cli_args = CliArgs::parse();
+    let port = cli_args.port;
 
     SimpleLogger::new()
         .with_level(LevelFilter::Info)
         .init()
         .unwrap();
-    let port = cli_args.port;
 
     let client_secret = if let Ok(v) = env::var("CLIENT_SECRET") {
         v
     } else {
-        eprintln!("Client secret not found in ENV. Please set CLIENT_SECRET.");
+        error!("Client secret not found in ENV. Please set CLIENT_SECRET.");
         process::exit(1);
     };
     let client_secret_1 = client_secret.clone();
 
     let server = ServerState::init(&client_secret, &cli_args).await.unwrap();
+    let data_0 = Arc::new(RwLockServerState(RwLock::new(server)));
+    let data_1 = Arc::clone(&data_0);
+
+    // What's nice is that Rustls only supports TLS 1.2 and 1.3.
+    let mut tls_config = ServerConfig::new(NoClientAuth::new());
+    tls_config.cert_resolver = data_0.clone();
+
+    //
+    // At this point, the server is ready to start, and starts the necessary
+    // threads.
+    //
 
     // Set ctrl+c to send a stop message
     let running = Arc::new(AtomicBool::new(true));
@@ -79,10 +89,7 @@ async fn main() -> Result<(), std::io::Error> {
     })
     .expect("Error setting Ctrl-C handler");
 
-    let data_0 = Arc::new(RwLockServerState(RwLock::new(server)));
-    let data_1 = Arc::clone(&data_0);
-    let data_2 = Arc::clone(&data_0);
-
+    // Spawn ping task
     spawn(async move {
         let mut interval = time::interval(Duration::from_secs(90));
         let mut data = Arc::clone(&data_0);
@@ -93,9 +100,7 @@ async fn main() -> Result<(), std::io::Error> {
         }
     });
 
-    let mut tls_config = ServerConfig::new(NoClientAuth::new());
-    tls_config.cert_resolver = data_2;
-
+    // Start HTTPS server
     HttpServer::new(move || {
         App::new()
             .service(routes::token_data)

src/ping.rs

@@ -1,6 +1,7 @@
+use std::sync::Arc;
 use std::{
     num::{NonZeroU16, NonZeroUsize},
-    sync::Arc,
+    sync::atomic::Ordering,
 };
 
 use log::{error, info, warn};
@@ -8,6 +9,7 @@ use serde::{Deserialize, Serialize};
 use sodiumoxide::crypto::box_::PrecomputedKey;
 use url::Url;
 
+use crate::config::VALIDATE_TOKENS;
 use crate::state::RwLockServerState;
 use crate::{client_api_version, config::CliArgs};
 
@@ -96,13 +98,13 @@ pub async fn update_server_state(secret: &str, req: &CliArgs, data: &mut Arc<RwL
                     }
                 }
 
-                if write_guard.force_tokens != resp.force_tokens {
-                    write_guard.force_tokens = resp.force_tokens;
+                if VALIDATE_TOKENS.load(Ordering::Acquire) != resp.force_tokens {
                     if resp.force_tokens {
                         info!("Client received command to enforce token validity.");
                     } else {
                         info!("Client received command to no longer enforce token validity");
                     }
+                    VALIDATE_TOKENS.store(resp.force_tokens, Ordering::Release);
                 }
 
                 if let Some(tls) = resp.tls {

src/routes.rs

@@ -1,7 +1,5 @@
-use std::{
-    convert::Infallible,
-    sync::atomic::{AtomicBool, Ordering},
-};
+use std::convert::Infallible;
+use std::sync::atomic::Ordering;
 
 use actix_web::dev::HttpResponseBuilder;
 use actix_web::http::header::{
@@ -21,12 +19,11 @@ use thiserror::Error;
 
 use crate::cache::{CacheKey, CachedImage};
 use crate::client_api_version;
+use crate::config::{SEND_SERVER_VERSION, VALIDATE_TOKENS};
 use crate::state::RwLockServerState;
 
 pub const BASE64_CONFIG: base64::Config = base64::Config::new(base64::CharacterSet::UrlSafe, false);
 
-pub static SEND_SERVER_VERSION: AtomicBool = AtomicBool::new(false);
-
 const SERVER_ID_STRING: &str = concat!(
     env!("CARGO_CRATE_NAME"),
     " ",
@@ -57,7 +54,7 @@ async fn token_data(
     path: Path<(String, String, String)>,
 ) -> impl Responder {
     let (token, chapter_hash, file_name) = path.into_inner();
-    if state.0.read().force_tokens {
+    if VALIDATE_TOKENS.load(Ordering::Acquire) {
         if let Err(e) = validate_token(&state.0.read().precomputed_key, token, &chapter_hash) {
             return ServerResponse::TokenValidationError(e);
         }
@@ -72,7 +69,7 @@ async fn token_data_saver(
     path: Path<(String, String, String)>,
 ) -> impl Responder {
     let (token, chapter_hash, file_name) = path.into_inner();
-    if state.0.read().force_tokens {
+    if VALIDATE_TOKENS.load(Ordering::Acquire) {
         if let Err(e) = validate_token(&state.0.read().precomputed_key, token, &chapter_hash) {
             return ServerResponse::TokenValidationError(e);
         }
@@ -185,15 +182,23 @@ async fn fetch_image(
         return construct_response(cached);
     }
 
-    let mut state = state.0.write();
+    // It's important to not get a write lock before this request, else we're
+    // holding the read lock until the await resolves.
 
     let resp = if is_data_saver {
         reqwest::get(format!(
             "{}/data-saver/{}/{}",
-            state.image_server, &key.1, &key.2
+            state.0.read().image_server,
+            &key.1,
+            &key.2
         ))
     } else {
-        reqwest::get(format!("{}/data/{}/{}", state.image_server, &key.1, &key.2))
+        reqwest::get(format!(
+            "{}/data/{}/{}",
+            state.0.read().image_server,
+            &key.1,
+            &key.2
+        ))
     }
     .await;
 
@@ -238,7 +243,7 @@ async fn fetch_image(
                         last_modified,
                     };
                     let resp = construct_response(&cached);
-                    state.cache.put(key, cached).await;
+                    state.0.write().cache.put(key, cached).await;
                     return resp;
                 }
                 Err(e) => {

src/state.rs

@@ -1,10 +1,8 @@
-use std::{
-    io::BufReader,
-    sync::{atomic::Ordering, Arc},
-};
+use std::io::BufReader;
+use std::sync::{atomic::Ordering, Arc};
 
+use crate::config::{SEND_SERVER_VERSION, VALIDATE_TOKENS};
 use crate::ping::{Request, Response, Tls, CONTROL_CENTER_PING_URL};
-use crate::routes::SEND_SERVER_VERSION;
 use crate::{cache::Cache, config::CliArgs};
 use log::{error, info, warn};
 use parking_lot::RwLock;
@@ -18,7 +16,6 @@ pub struct ServerState {
     pub precomputed_key: PrecomputedKey,
     pub image_server: Url,
     pub tls_config: Tls,
-    pub force_tokens: bool,
     pub url: String,
     pub cache: Cache,
     pub log_state: LogState,
@@ -36,7 +33,10 @@ impl ServerState {
             .send()
             .await;
 
-        SEND_SERVER_VERSION.store(config.enable_server_string, Ordering::Release);
+        if config.enable_server_string {
+            warn!("Client will send Server header in responses. This is not recommended!");
+            SEND_SERVER_VERSION.store(true, Ordering::Release);
+        }
 
         match resp {
             Ok(resp) => match resp.json::<Response>().await {
@@ -72,11 +72,12 @@ impl ServerState {
                         info!("This client will not validate tokens.");
                     }
 
+                    VALIDATE_TOKENS.store(resp.force_tokens, Ordering::Release);
+
                     Ok(Self {
                         precomputed_key: key,
                         image_server: resp.image_server,
                         tls_config: resp.tls.unwrap(),
-                        force_tokens: resp.force_tokens,
                         url: resp.url,
                         cache: Cache::new(
                             config.memory_quota.get(),