Repository: svenstaro/miniserve
Pull request #848 (2022-07-20 09:47:26)

Switch to `fast_qr` lib
As mentioned in #846 , there is a clear mistake but I'm unsure how to fix.

cyqsimon: Seems like VSCode auto-sorted `Cargo.toml`. Looks okay though.
svenstaro: Please rebase, sorry, I updated some deps. Won't update deps anymore until this is merged.
cyqsimon: Also I've noticed an issue in the QR code dropdown menu when run with `-q`. Give me some time to fix it. Meanwhile I'll mark this PR as draft.
svenstaro: Hey @cyqsimon, I'd like to cut a new release of miniserve with this in it after I merge the other PRs soon. Do you think you can finish this one up in the coming days? No problem if it's not in the release, just curious.
cyqsimon: @svenstaro please go ahead with the release. Life got in the way for me during this past week and I am unable to squeeze out extra time. Sorry about that. I'll try to work on this PR this weekend.
svenstaro: No worries!
cyqsimon: Rebased.
cyqsimon: Still working on fixing the QR code in the drop-down menu. The previous implementation seems rather jerry-rigged. I'd like to spend some time to do it properly, if I can say so.

Patch:
diff --git a/Cargo.lock b/Cargo.lock
index a7e1793..cfcf744 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -800,6 +800,12 @@ version = "1.0.1"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "b5320ae4c3782150d900b79807611a59a99fc9a1d61d686faafc24b93fc8d7ca"
 
+[[package]]
+name = "fake-tty"
+version = "0.2.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "1caac348c61d1f9a7d43c3629abc75540754a4971944953f84b98cb8280e3971"
+
 [[package]]
 name = "fancy-regex"
 version = "0.7.1"
@@ -810,6 +816,15 @@ dependencies = [
  "regex",
 ]
 
+[[package]]
+name = "fast_qr"
+version = "0.3.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "b798dfd6e29b85c0bcf434272db4cde0100ab4d82c5db0a4f422e77b30d0b4e4"
+dependencies = [
+ "wasm-bindgen",
+]
+
 [[package]]
 name = "fastrand"
 version = "1.8.0"
@@ -1525,6 +1540,8 @@ dependencies = [
  "clap_complete",
  "clap_mangen",
  "comrak",
+ "fake-tty",
+ "fast_qr",
  "futures",
  "get_if_addrs",
  "grass",
@@ -1540,7 +1557,6 @@ dependencies = [
  "port_check",
  "predicates",
  "pretty_assertions",
- "qrcodegen",
  "regex",
  "reqwest",
  "rstest",
@@ -2017,12 +2033,6 @@ dependencies = [
  "unicode-ident",
 ]
 
-[[package]]
-name = "qrcodegen"
-version = "1.8.0"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "4339fc7a1021c9c1621d87f5e3505f2805c8c105420ba2f2a4df86814590c142"
-
 [[package]]
 name = "quote"
 version = "1.0.21"
diff --git a/Cargo.toml b/Cargo.toml
index 70773ff..05baaab 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -32,6 +32,7 @@ clap = { version = "3.2", features = ["derive", "cargo", "wrap_help"] }
 clap_complete = "3.2.3"
 clap_mangen = "0.1"
 comrak = "0.14.0"
+fast_qr = "0.3.1"
 futures = "0.3"
 get_if_addrs = "0.5"
 hex = "0.4"
@@ -44,7 +45,6 @@ mime = "0.3"
 nanoid = "0.4"
 percent-encoding = "2"
 port_check = "0.1"
-qrcodegen = "1"
 rustls = { version = "0.20", optional = true }
 rustls-pemfile = { version = "1.0", optional = true }
 serde = { version = "1", features = ["derive"] }
@@ -77,5 +77,9 @@ rstest = "0.15"
 select = "0.5"
 url = "2"
 
+[target.'cfg(not(windows))'.dev-dependencies]
+# fake_tty does not support Windows for now
+fake-tty = "0.2.0"
+
 [build-dependencies]
 grass = "0.11"
diff --git a/data/style.scss b/data/style.scss
index 0fcc90d..fb76a9a 100644
--- a/data/style.scss
+++ b/data/style.scss
@@ -17,8 +17,6 @@ $themes: squirrel, archlinux, monokai, zenburn;
     @return $s;
 }
 
-
-
 html {
     font-smoothing: antialiased;
     text-rendering: optimizeLegibility;
@@ -180,7 +178,7 @@ nav .qrcode {
     background: var(--switch_theme_background);
 }
 
-nav .qrcode img {
+nav .qrcode svg {
     display: block;
 }
 
diff --git a/src/consts.rs b/src/consts.rs
new file mode 100644
index 0000000..d864683
--- /dev/null
+++ b/src/consts.rs
@@ -0,0 +1,7 @@
+use fast_qr::ECL;
+
+/// The error correction level to use for all QR code generation.
+pub const QR_EC_LEVEL: ECL = ECL::L;
+
+/// The margin size for the SVG QR code on the webpage.
+pub const SVG_QR_MARGIN: usize = 1;
diff --git a/src/listing.rs b/src/listing.rs
index 851f4ac..7477599 100644
--- a/src/listing.rs
+++ b/src/listing.rs
@@ -9,7 +9,6 @@ use actix_web::{HttpMessage, HttpRequest, HttpResponse};
 use bytesize::ByteSize;
 use comrak::{markdown_to_html, ComrakOptions};
 use percent_encoding::{percent_decode_str, utf8_percent_encode};
-use qrcodegen::{QrCode, QrCodeEcc};
 use serde::Deserialize;
 use strum_macros::{Display, EnumString};
 
@@ -38,7 +37,6 @@ pub struct QueryParameters {
     pub order: Option<SortingOrder>,
     pub raw: Option<bool>,
     pub mkdir_name: Option<String>,
-    qrcode: Option<String>,
     download: Option<ArchiveMethod>,
 }
 
@@ -166,6 +164,12 @@ pub fn directory_listing(
 
     let base = Path::new(serve_path);
     let random_route_abs = format!("/{}", conf.route_prefix);
+    let abs_url = format!(
+        "{}://{}{}",
+        req.connection_info().scheme(),
+        req.connection_info().host(),
+        req.uri()
+    );
     let is_root = base.parent().is_none() || Path::new(&req.path()) == Path::new(&random_route_abs);
 
     let encoded_dir = match base.strip_prefix(random_route_abs) {
@@ -218,20 +222,6 @@ pub fn directory_listing(
 
     let query_params = extract_query_parameters(req);
 
-    // If the `qrcode` parameter is included in the url, then should respond to the QR code
-    if let Some(url) = query_params.qrcode {
-        let res = match QrCode::encode_text(&url, QrCodeEcc::Medium) {
-            Ok(qr) => HttpResponse::Ok()
-                .append_header(("Content-Type", "image/svg+xml"))
-                .body(qr_to_svg_string(&qr, 2)),
-            Err(err) => {
-                log::error!("URL is invalid (too long?): {:?}", err);
-                HttpResponse::UriTooLong().finish()
-            }
-        };
-        return Ok(ServiceResponse::new(req.clone(), res));
-    }
-
     let mut entries: Vec<Entry> = Vec::new();
     let mut readme: Option<(String, String)> = None;
 
@@ -384,9 +374,10 @@ pub fn directory_listing(
                 renderer::page(
                     entries,
                     readme,
+                    abs_url,
                     is_root,
                     query_params,
-                    breadcrumbs,
+                    &breadcrumbs,
                     &encoded_dir,
                     conf,
                     current_user,
@@ -407,34 +398,3 @@ pub fn extract_query_parameters(req: &HttpRequest) -> QueryParameters {
         }
     }
 }
-
-// Returns a string of SVG code for an image depicting
-// the given QR Code, with the given number of border modules.
-// The string always uses Unix newlines (\n), regardless of the platform.
-fn qr_to_svg_string(qr: &QrCode, border: i32) -> String {
-    assert!(border >= 0, "Border must be non-negative");
-    let mut result = String::new();
-    result += "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n";
-    result += "<!DOCTYPE svg PUBLIC \"-//W3C//DTD SVG 1.1//EN\" \"http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd\">\n";
-    let dimension = qr
-        .size()
-        .checked_add(border.checked_mul(2).unwrap())
-        .unwrap();
-    result += &format!(
-		"<svg xmlns=\"http://www.w3.org/2000/svg\" version=\"1.1\" viewBox=\"0 0 {0} {0}\" stroke=\"none\">\n", dimension);
-    result += "\t<rect width=\"100%\" height=\"100%\" fill=\"#FFFFFF\"/>\n";
-    result += "\t<path d=\"";
-    for y in 0..qr.size() {
-        for x in 0..qr.size() {
-            if qr.get_module(x, y) {
-                if x != 0 || y != 0 {
-                    result += " ";
-                }
-                result += &format!("M{},{}h1v1h-1z", x + border, y + border);
-            }
-        }
-    }
-    result += "\" fill=\"#000000\"/>\n";
-    result += "</svg>\n";
-    result
-}
diff --git a/src/main.rs b/src/main.rs
index 08c6680..b49089b 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -11,14 +11,15 @@ use actix_web::{middleware, App, HttpRequest, HttpResponse};
 use actix_web_httpauth::middleware::HttpAuthentication;
 use anyhow::Result;
 use clap::{crate_version, IntoApp, Parser};
+use fast_qr::QRBuilder;
 use log::{error, warn};
-use qrcodegen::{QrCode, QrCodeEcc};
 use yansi::{Color, Paint};
 
 mod archive;
 mod args;
 mod auth;
 mod config;
+mod consts;
 mod errors;
 mod file_upload;
 mod listing;
@@ -238,13 +239,13 @@ async fn run(miniserve_config: MiniserveConfig) -> Result<(), ContextualError> {
             .iter()
             .filter(|url| !url.contains("//127.0.0.1:") && !url.contains("//[::1]:"))
         {
-            match QrCode::encode_text(url, QrCodeEcc::Low) {
+            match QRBuilder::new(url.clone()).ecl(consts::QR_EC_LEVEL).build() {
                 Ok(qr) => {
                     println!("QR code for {}:", Color::Green.paint(url).bold());
-                    print_qr(&qr);
+                    qr.print();
                 }
                 Err(e) => {
-                    error!("Failed to render QR to terminal: {}", e);
+                    error!("Failed to render QR to terminal: {:?}", e);
                 }
             };
         }
@@ -350,31 +351,3 @@ async fn css() -> impl Responder {
         .insert_header(ContentType(mime::TEXT_CSS))
         .body(css)
 }
-
-// Prints to the console two inverted QrCodes side by side.
-fn print_qr(qr: &QrCode) {
-    let border = 4;
-    let size = qr.size() + 2 * border;
-
-    for y in (0..size).step_by(2) {
-        for x in 0..2 * size {
-            let inverted = x >= size;
-            let (x, y) = (x % size - border, y - border);
-
-            //each char represents two vertical modules
-            let (mod1, mod2) = match inverted {
-                false => (qr.get_module(x, y), qr.get_module(x, y + 1)),
-                true => (!qr.get_module(x, y), !qr.get_module(x, y + 1)),
-            };
-            let c = match (mod1, mod2) {
-                (false, false) => ' ',
-                (true, false) => '▀',
-                (false, true) => '▄',
-                (true, true) => '█',
-            };
-            print!("{0}", c);
-        }
-        println!();
-    }
-    println!();
-}
diff --git a/src/renderer.rs b/src/renderer.rs
index 40aa7cd..c8958fe 100644
--- a/src/renderer.rs
+++ b/src/renderer.rs
@@ -2,11 +2,15 @@ use actix_web::http::StatusCode;
 use chrono::{DateTime, Utc};
 use chrono_humanize::Humanize;
 use clap::{crate_name, crate_version};
+use fast_qr::convert::svg::SvgBuilder;
+use fast_qr::qr::QRCodeError;
+use fast_qr::QRBuilder;
 use maud::{html, Markup, PreEscaped, DOCTYPE};
 use std::time::SystemTime;
 use strum::IntoEnumIterator;
 
 use crate::auth::CurrentUser;
+use crate::consts;
 use crate::listing::{Breadcrumb, Entry, QueryParameters, SortingMethod, SortingOrder};
 use crate::{archive::ArchiveMethod, MiniserveConfig};
 
@@ -15,9 +19,10 @@ use crate::{archive::ArchiveMethod, MiniserveConfig};
 pub fn page(
     entries: Vec<Entry>,
     readme: Option<(String, String)>,
+    abs_url: impl AsRef<str>,
     is_root: bool,
     query_params: QueryParameters,
-    breadcrumbs: Vec<Breadcrumb>,
+    breadcrumbs: &[Breadcrumb],
     encoded_dir: &str,
     conf: &MiniserveConfig,
     current_user: Option<&CurrentUser>,
@@ -33,11 +38,7 @@ pub fn page(
     let upload_action = build_upload_action(&upload_route, encoded_dir, sort_method, sort_order);
     let mkdir_action = build_mkdir_action(&upload_route, encoded_dir);
 
-    let title_path = breadcrumbs
-        .iter()
-        .map(|el| el.name.clone())
-        .collect::<Vec<_>>()
-        .join("/");
+    let title_path = breadcrumbs_to_path_string(breadcrumbs);
 
     html! {
         (DOCTYPE)
@@ -89,7 +90,10 @@ pub fn page(
                         }
                     }
                 }
-                (color_scheme_selector(conf.show_qrcode, conf.hide_theme_selector))
+                nav {
+                    (qr_spoiler(conf.show_qrcode, abs_url))
+                    (color_scheme_selector(conf.hide_theme_selector))
+                }
                 div.container {
                     span #top { }
                     h1.title dir="ltr" {
@@ -226,6 +230,25 @@ pub fn raw(entries: Vec<Entry>, is_root: bool) -> Markup {
     }
 }
 
+/// Renders the QR code SVG
+fn qr_code_svg(url: impl AsRef<str>, margin: usize) -> Result<String, QRCodeError> {
+    let qr = QRBuilder::new(url.as_ref().into())
+        .ecl(consts::QR_EC_LEVEL)
+        .build()?;
+    let svg = SvgBuilder::new().margin(margin).build_qr(qr);
+
+    Ok(svg)
+}
+
+/// Build a path string from a list of breadcrumbs.
+fn breadcrumbs_to_path_string(breadcrumbs: &[Breadcrumb]) -> String {
+    breadcrumbs
+        .iter()
+        .map(|el| el.name.clone())
+        .collect::<Vec<_>>()
+        .join("/")
+}
+
 // Partial: version footer
 fn version_footer() -> Markup {
     html! {
@@ -292,30 +315,37 @@ const THEME_PICKER_CHOICES: &[(&str, &str)] = &[
 
 pub const THEME_SLUGS: &[&str] = &["squirrel", "archlinux", "zenburn", "monokai"];
 
-/// Partial: color scheme selector
-fn color_scheme_selector(show_qrcode: bool, hide_theme_selector: bool) -> Markup {
+/// Partial: qr code spoiler
+fn qr_spoiler(show_qrcode: bool, content: impl AsRef<str>) -> Markup {
     html! {
-        nav {
-            @if show_qrcode {
-                div {
-                    p onmouseover="document.querySelector('#qrcode').src = `?qrcode=${encodeURIComponent(window.location.href)}`" {
-                        "QR code"
-                    }
-                    div.qrcode {
-                        img #qrcode alt="QR code" title="QR code of this page";
+        @if show_qrcode {
+            div {
+                p {
+                    "QR code"
+                }
+                div.qrcode #qrcode title=(PreEscaped(content.as_ref())) {
+                    @match qr_code_svg(content, consts::SVG_QR_MARGIN) {
+                        Ok(svg) => (PreEscaped(svg)),
+                        Err(err) => (format!("QR generation error: {:?}", err)),
                     }
                 }
             }
-            @if !hide_theme_selector {
-                div {
-                    p {
-                        "Change theme..."
-                    }
-                    ul.theme {
-                        @for color_scheme in THEME_PICKER_CHOICES {
-                            li.(format!("theme_{}", color_scheme.1)) {
-                                (color_scheme_link(color_scheme))
-                            }
+        }
+    }
+}
+
+/// Partial: color scheme selector
+fn color_scheme_selector(hide_theme_selector: bool) -> Markup {
+    html! {
+        @if !hide_theme_selector {
+            div {
+                p {
+                    "Change theme..."
+                }
+                ul.theme {
+                    @for color_scheme in THEME_PICKER_CHOICES {
+                        li.(format!("theme_{}", color_scheme.1)) {
+                            (color_scheme_link(color_scheme))
                         }
                     }
                 }


Test patch:
diff --git a/tests/qrcode.rs b/tests/qrcode.rs
index a9c27fe..98a3c67 100644
--- a/tests/qrcode.rs
+++ b/tests/qrcode.rs
@@ -1,14 +1,17 @@
 mod fixtures;
 
-use fixtures::{server, server_no_stderr, Error, TestServer};
-use reqwest::StatusCode;
+use assert_cmd::prelude::CommandCargoExt;
+use assert_fs::TempDir;
+use fixtures::{port, server, tmpdir, Error, TestServer};
 use rstest::rstest;
 use select::document::Document;
 use select::predicate::Attr;
-use std::iter::repeat_with;
+use std::process::{Command, Stdio};
+use std::thread::sleep;
+use std::time::Duration;
 
 #[rstest]
-fn hide_qrcode_element(server: TestServer) -> Result<(), Error> {
+fn webpage_hides_qrcode_when_disabled(server: TestServer) -> Result<(), Error> {
     let body = reqwest::blocking::get(server.url())?.error_for_status()?;
     let parsed = Document::from_read(body)?;
     assert!(parsed.find(Attr("id", "qrcode")).next().is_none());
@@ -17,30 +20,92 @@ fn hide_qrcode_element(server: TestServer) -> Result<(), Error> {
 }
 
 #[rstest]
-fn show_qrcode_element(#[with(&["-q"])] server: TestServer) -> Result<(), Error> {
+fn webpage_shows_qrcode_when_enabled(#[with(&["-q"])] server: TestServer) -> Result<(), Error> {
     let body = reqwest::blocking::get(server.url())?.error_for_status()?;
     let parsed = Document::from_read(body)?;
-    assert!(parsed.find(Attr("id", "qrcode")).next().is_some());
+    let qr_container = parsed
+        .find(Attr("id", "qrcode"))
+        .next()
+        .ok_or("QR container not found")?;
+    let tooltip = qr_container
+        .attr("title")
+        .ok_or("QR container has no title")?;
+    assert_eq!(tooltip, server.url().as_str());
 
     Ok(())
 }
 
+#[cfg(not(windows))]
+fn run_in_faketty_kill_and_get_stdout(template: &Command) -> Result<String, Error> {
+    use fake_tty::{bash_command, get_stdout};
+
+    let cmd = {
+        let bin = template.get_program().to_str().expect("not UTF8");
+        let args = template
+            .get_args()
+            .map(|s| s.to_str().expect("not UTF8"))
+            .collect::<Vec<_>>()
+            .join(" ");
+        format!("{} {}", bin, args)
+    };
+    let mut child = bash_command(&cmd).stdin(Stdio::null()).spawn()?;
+
+    sleep(Duration::from_secs(1));
+
+    child.kill()?;
+    let output = child.wait_with_output().expect("Failed to read stdout");
+    let all_text = get_stdout(output.stdout)?;
+
+    Ok(all_text)
+}
+
 #[rstest]
-fn get_svg_qrcode(#[from(server_no_stderr)] server: TestServer) -> Result<(), Error> {
-    // Ok
-    let resp = reqwest::blocking::get(server.url().join("/?qrcode=test")?)?;
+// Disabled for Windows because `fake_tty` does not currently support it.
+#[cfg(not(windows))]
+fn qrcode_hidden_in_tty_when_disabled(tmpdir: TempDir, port: u16) -> Result<(), Error> {
+    let mut template = Command::cargo_bin("miniserve")?;
+    template.arg("-p").arg(port.to_string()).arg(tmpdir.path());
+
+    let output = run_in_faketty_kill_and_get_stdout(&template)?;
 
-    assert_eq!(resp.status(), StatusCode::OK);
-    assert_eq!(resp.headers()["Content-Type"], "image/svg+xml");
-    let body = resp.text()?;
-    assert!(body.starts_with("<?xml"));
-    assert_eq!(body.len(), 3530);
+    assert!(!output.contains("QR code for "));
+    Ok(())
+}
+
+#[rstest]
+// Disabled for Windows because `fake_tty` does not currently support it.
+#[cfg(not(windows))]
+fn qrcode_shown_in_tty_when_enabled(tmpdir: TempDir, port: u16) -> Result<(), Error> {
+    let mut template = Command::cargo_bin("miniserve")?;
+    template
+        .arg("-p")
+        .arg(port.to_string())
+        .arg("-q")
+        .arg(tmpdir.path());
+
+    let output = run_in_faketty_kill_and_get_stdout(&template)?;
+
+    assert!(output.contains("QR code for "));
+    Ok(())
+}
+
+#[rstest]
+fn qrcode_hidden_in_non_tty_when_enabled(tmpdir: TempDir, port: u16) -> Result<(), Error> {
+    let mut child = Command::cargo_bin("miniserve")?
+        .arg("-p")
+        .arg(port.to_string())
+        .arg("-q")
+        .arg(tmpdir.path())
+        .stdout(Stdio::piped())
+        .stderr(Stdio::piped())
+        .spawn()?;
 
-    // Err
-    let content: String = repeat_with(|| '0').take(8 * 1024).collect();
-    let resp = reqwest::blocking::get(server.url().join(&format!("?qrcode={}", content))?)?;
+    sleep(Duration::from_secs(1));
 
-    assert_eq!(resp.status(), StatusCode::URI_TOO_LONG);
+    child.kill()?;
+    let output = child.wait_with_output().expect("Failed to read stdout");
+    let stdout = String::from_utf8(output.stdout)?;
 
+    assert!(!stdout.contains("QR code for "));
     Ok(())
 }