From 0f276b6ec8383cea3f3da726682594e5e683033a Mon Sep 17 00:00:00 2001 From: Ali MJ Al-Nasrawy Date: Sat, 27 Mar 2021 07:13:39 +0300 Subject: [PATCH 1/9] Resolve symlinks when listing This has the benefit of showing the size and modification date of the pointed-to file. Symlink to directories now respects '--dirs-first' option and broken symlinks don't show in directory listing. --- src/listing.rs | 22 +++------------------- src/renderer.rs | 4 ---- 2 files changed, 3 insertions(+), 23 deletions(-) diff --git a/src/listing.rs b/src/listing.rs index 66aea6b69..2fe583cf6 100644 --- a/src/listing.rs +++ b/src/listing.rs @@ -65,9 +65,6 @@ pub enum EntryType { /// Entry is a file File, - - /// Entry is a symlink - Symlink, } /// Entry @@ -114,11 +111,6 @@ impl Entry { pub fn is_file(&self) -> bool { self.entry_type == EntryType::File } - - /// Returns wether the entry is a symlink - pub fn is_symlink(&self) -> bool { - self.entry_type == EntryType::Symlink - } } /// One entry in the path to the listed directory @@ -260,7 +252,7 @@ pub fn directory_listing( let file_name = entry.file_name().to_string_lossy().to_string(); // if file is a directory, add '/' to the end of the name - if let Ok(metadata) = entry.metadata() { + if let Ok(metadata) = std::fs::metadata(entry.path()) { if skip_symlinks && metadata.file_type().is_symlink() { continue; } @@ -269,15 +261,7 @@ pub fn directory_listing( Err(_) => None, }; - if metadata.file_type().is_symlink() { - entries.push(Entry::new( - file_name, - EntryType::Symlink, - file_url, - None, - last_modification_date, - )); - } else if metadata.is_dir() { + if metadata.is_dir() { entries.push(Entry::new( file_name, EntryType::Directory, @@ -285,7 +269,7 @@ pub fn directory_listing( None, last_modification_date, )); - } else { + } else if metadata.is_file() { entries.push(Entry::new( file_name, EntryType::File, diff --git a/src/renderer.rs b/src/renderer.rs index c9ec9cdbd..248769695 100644 --- a/src/renderer.rs +++ b/src/renderer.rs @@ -341,10 +341,6 @@ fn entry_row( } } } - } @else if entry.is_symlink() { - a.symlink href=(parametrized_link(&entry.link, sort_method, sort_order)) { - (entry.name) span.symlink-symbol { "⇢" } - } } } } From 81e65554b4da37399ef9b0ac6c647198a6e81ebf Mon Sep 17 00:00:00 2001 From: Ali MJ Al-Nasrawy Date: Sat, 27 Mar 2021 07:19:59 +0300 Subject: [PATCH 2/9] Bring back the symlink symbol --- src/listing.rs | 11 +++++++++++ src/renderer.rs | 6 ++++++ 2 files changed, 17 insertions(+) diff --git a/src/listing.rs b/src/listing.rs index 2fe583cf6..c5eefe6c1 100644 --- a/src/listing.rs +++ b/src/listing.rs @@ -75,6 +75,9 @@ pub struct Entry { /// Type of the entry pub entry_type: EntryType, + /// Entry is symlink. Not mutually exclusive with entry_type + pub is_symlink: bool, + /// URL of the entry pub link: String, @@ -89,6 +92,7 @@ impl Entry { fn new( name: String, entry_type: EntryType, + is_symlink: bool, link: String, size: Option, last_modification_date: Option, @@ -96,6 +100,7 @@ impl Entry { Entry { name, entry_type, + is_symlink, link, size, last_modification_date, @@ -250,6 +255,10 @@ pub fn directory_listing( // show file url as relative to static path let file_url = utf8_percent_encode(&p.to_string_lossy(), FRAGMENT).to_string(); let file_name = entry.file_name().to_string_lossy().to_string(); + let is_symlink = match entry.metadata() { + Ok(metadata) => metadata.file_type().is_symlink(), + Err(_) => continue, + }; // if file is a directory, add '/' to the end of the name if let Ok(metadata) = std::fs::metadata(entry.path()) { @@ -265,6 +274,7 @@ pub fn directory_listing( entries.push(Entry::new( file_name, EntryType::Directory, + is_symlink, file_url, None, last_modification_date, @@ -273,6 +283,7 @@ pub fn directory_listing( entries.push(Entry::new( file_name, EntryType::File, + is_symlink, file_url, Some(ByteSize::b(metadata.len())), last_modification_date, diff --git a/src/renderer.rs b/src/renderer.rs index 248769695..266f0350a 100644 --- a/src/renderer.rs +++ b/src/renderer.rs @@ -329,11 +329,17 @@ fn entry_row( @if entry.is_dir() { a.directory href=(parametrized_link(&entry.link, sort_method, sort_order)) { (entry.name) "/" + @if entry.is_symlink { + span.symlink-symbol { "⇢" } + } } } @else if entry.is_file() { div.file-entry { a.file href=(&entry.link) { (entry.name) + @if entry.is_symlink { + span.symlink-symbol { "⇢" } + } } @if let Some(size) = entry.size { span.mobile-info.size { From 57ec279491db97d2933f07127235d44e918472ce Mon Sep 17 00:00:00 2001 From: Ali MJ Al-Nasrawy Date: Mon, 29 Mar 2021 12:11:58 +0300 Subject: [PATCH 3/9] Move symlink symbol from html to css This should facilitate testing because this symbol will no longer a part of the entry text shown in html. --- data/style.scss | 3 ++- src/renderer.rs | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/data/style.scss b/data/style.scss index 9bfbeaf30..be193427e 100644 --- a/data/style.scss +++ b/data/style.scss @@ -94,7 +94,8 @@ a.symlink:hover { color: var(--symlink_link_color); } -.symlink-symbol { +.symlink-symbol::after { + content: "⇢"; display: inline-block; border: 1px solid var(--symlink_link_color); margin-left: 0.5rem; diff --git a/src/renderer.rs b/src/renderer.rs index 266f0350a..c51f364f7 100644 --- a/src/renderer.rs +++ b/src/renderer.rs @@ -330,7 +330,7 @@ fn entry_row( a.directory href=(parametrized_link(&entry.link, sort_method, sort_order)) { (entry.name) "/" @if entry.is_symlink { - span.symlink-symbol { "⇢" } + span.symlink-symbol { } } } } @else if entry.is_file() { @@ -338,7 +338,7 @@ fn entry_row( a.file href=(&entry.link) { (entry.name) @if entry.is_symlink { - span.symlink-symbol { "⇢" } + span.symlink-symbol { } } } @if let Some(size) = entry.size { From 18a3465da2dd5630a760a6f106d1ceff6f78cd9f Mon Sep 17 00:00:00 2001 From: Ali MJ Al-Nasrawy Date: Mon, 29 Mar 2021 12:31:22 +0300 Subject: [PATCH 4/9] Test for symlink directories and files Replace some of the testing files and directories with symbolic links. They should behave exactly the same. --- tests/serve_request.rs | 64 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 64 insertions(+) diff --git a/tests/serve_request.rs b/tests/serve_request.rs index 95449f5d2..385feeb80 100644 --- a/tests/serve_request.rs +++ b/tests/serve_request.rs @@ -8,10 +8,16 @@ use regex::Regex; use rstest::rstest; use select::document::Document; use select::node::Node; +use std::path::Path; use std::process::{Command, Stdio}; use std::thread::sleep; use std::time::Duration; +#[cfg(unix)] +use std::os::unix::fs::{symlink as symlink_dir, symlink as symlink_file}; +#[cfg(windows)] +use std::os::windows::fs::{symlink_dir, symlink_file}; + #[rstest] fn serves_requests_with_no_options(tmpdir: TempDir) -> Result<(), Error> { let mut child = Command::cargo_bin("miniserve")? @@ -153,6 +159,64 @@ fn serves_requests_no_hidden_files_without_flag(tmpdir: TempDir, port: u16) -> R Ok(()) } +#[rstest] +fn serves_requests_symlinks(tmpdir: TempDir, port: u16) -> Result<(), Error> { + let mut child = Command::cargo_bin("miniserve")? + .arg(tmpdir.path()) + .arg("-p") + .arg(port.to_string()) + .stdout(Stdio::null()) + .spawn()?; + + sleep(Duration::from_secs(1)); + + let files = &["symlink-file.html"]; + let dirs = &["symlink-dir/"]; + let broken = &["symlink broken"]; + + for &directory in dirs { + let orig = Path::new(DIRECTORIES[0].strip_suffix("/").unwrap()); + let link = tmpdir + .path() + .join(Path::new(directory.strip_suffix("/").unwrap())); + symlink_dir(orig, link).expect("Couldn't create symlink"); + } + for &file in files { + let orig = Path::new(FILES[0]); + let link = tmpdir.path().join(Path::new(file)); + symlink_file(orig, link).expect("Couldn't create symlink"); + } + for &file in broken { + let orig = Path::new("should-not-exist.xxx"); + let link = tmpdir.path().join(Path::new(file)); + symlink_file(orig, link).expect("Couldn't create symlink"); + } + + let body = reqwest::blocking::get(format!("http://localhost:{}", port).as_str())? + .error_for_status()?; + let parsed = Document::from_read(body)?; + + for &entry in files.into_iter().chain(dirs) { + let node = parsed + .find(|x: &Node| x.name().unwrap_or_default() == "a" && x.text() == entry) + .next() + .unwrap(); + assert_eq!(node.attr("href").unwrap().strip_prefix("/").unwrap(), entry); + if entry.ends_with("/") { + assert_eq!(node.attr("class").unwrap(), "directory"); + } else { + assert_eq!(node.attr("class").unwrap(), "file"); + } + } + for &entry in broken { + assert!(parsed.find(|x: &Node| x.text() == entry).next().is_none()); + } + + child.kill()?; + + Ok(()) +} + #[rstest] fn serves_requests_with_randomly_assigned_port(tmpdir: TempDir) -> Result<(), Error> { let mut child = Command::cargo_bin("miniserve")? From 97f5772ca5b1d23ef623a0aea7a5a7f84b9345f1 Mon Sep 17 00:00:00 2001 From: Ali MJ Al-Nasrawy Date: Mon, 5 Apr 2021 23:21:26 +0300 Subject: [PATCH 5/9] Honor --no-symlinks option when listing --- src/listing.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/listing.rs b/src/listing.rs index c5eefe6c1..53d7233bf 100644 --- a/src/listing.rs +++ b/src/listing.rs @@ -262,7 +262,7 @@ pub fn directory_listing( // if file is a directory, add '/' to the end of the name if let Ok(metadata) = std::fs::metadata(entry.path()) { - if skip_symlinks && metadata.file_type().is_symlink() { + if skip_symlinks && is_symlink { continue; } let last_modification_date = match metadata.modified() { From 60695fac0957a4032ee84057de676ab8e3ddbb5d Mon Sep 17 00:00:00 2001 From: Ali MJ Al-Nasrawy Date: Mon, 5 Apr 2021 23:21:34 +0300 Subject: [PATCH 6/9] Avoid unneccessary syscalls for entry metadata For non-symlink files and directories, there is no need to call `std::fs::metadata()` as the metadata are already obtained via `entry.metadata()` --- src/listing.rs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/listing.rs b/src/listing.rs index 53d7233bf..9d749063a 100644 --- a/src/listing.rs +++ b/src/listing.rs @@ -255,13 +255,16 @@ pub fn directory_listing( // show file url as relative to static path let file_url = utf8_percent_encode(&p.to_string_lossy(), FRAGMENT).to_string(); let file_name = entry.file_name().to_string_lossy().to_string(); - let is_symlink = match entry.metadata() { - Ok(metadata) => metadata.file_type().is_symlink(), - Err(_) => continue, + let (is_symlink, metadata) = match entry.metadata() { + Ok(metadata) if metadata.file_type().is_symlink() => { + // for symlinks, get the metadata of the original file + (true, std::fs::metadata(entry.path())) + } + res => (false, res), }; // if file is a directory, add '/' to the end of the name - if let Ok(metadata) = std::fs::metadata(entry.path()) { + if let Ok(metadata) = metadata { if skip_symlinks && is_symlink { continue; } From 50d43daaf605bb25f22f374f1098839ef0445810 Mon Sep 17 00:00:00 2001 From: Ali MJ Al-Nasrawy Date: Sun, 11 Apr 2021 20:30:08 +0300 Subject: [PATCH 7/9] Test for --no-symlink option --- tests/serve_request.rs | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/tests/serve_request.rs b/tests/serve_request.rs index 385feeb80..1e92339e2 100644 --- a/tests/serve_request.rs +++ b/tests/serve_request.rs @@ -159,15 +159,18 @@ fn serves_requests_no_hidden_files_without_flag(tmpdir: TempDir, port: u16) -> R Ok(()) } -#[rstest] -fn serves_requests_symlinks(tmpdir: TempDir, port: u16) -> Result<(), Error> { - let mut child = Command::cargo_bin("miniserve")? - .arg(tmpdir.path()) +#[rstest(no_symlinks, case(true), case(false))] +fn serves_requests_symlinks(tmpdir: TempDir, port: u16, no_symlinks: bool) -> Result<(), Error> { + let mut comm = Command::cargo_bin("miniserve")?; + comm.arg(tmpdir.path()) .arg("-p") .arg(port.to_string()) - .stdout(Stdio::null()) - .spawn()?; + .stdout(Stdio::null()); + if no_symlinks { + comm.arg("--no-symlinks"); + } + let mut child = comm.spawn()?; sleep(Duration::from_secs(1)); let files = &["symlink-file.html"]; @@ -199,9 +202,16 @@ fn serves_requests_symlinks(tmpdir: TempDir, port: u16) -> Result<(), Error> { for &entry in files.into_iter().chain(dirs) { let node = parsed .find(|x: &Node| x.name().unwrap_or_default() == "a" && x.text() == entry) - .next() - .unwrap(); + .next(); + assert_eq!(node.is_none(), no_symlinks); + if no_symlinks { + continue; + } + + let node = node.unwrap(); assert_eq!(node.attr("href").unwrap().strip_prefix("/").unwrap(), entry); + reqwest::blocking::get(format!("http://localhost:{}/{}", port, entry))? + .error_for_status()?; if entry.ends_with("/") { assert_eq!(node.attr("class").unwrap(), "directory"); } else { From cbbc438d5d5a1c9ac326a255270e56f3624eca0f Mon Sep 17 00:00:00 2001 From: Ali MJ Al-Nasrawy Date: Sun, 11 Apr 2021 20:54:19 +0300 Subject: [PATCH 8/9] CSS: remove unused symlink class --- data/style.scss | 16 +--------------- 1 file changed, 1 insertion(+), 15 deletions(-) diff --git a/data/style.scss b/data/style.scss index be193427e..82ab66378 100644 --- a/data/style.scss +++ b/data/style.scss @@ -77,11 +77,6 @@ a.file:visited, color: var(--file_link_color); } -a.symlink, -a.symlink:visited { - color: var(--symlink_link_color); -} - a.directory:hover { color: var(--directory_link_color); } @@ -90,10 +85,6 @@ a.file:hover { color: var(--file_link_color); } -a.symlink:hover { - color: var(--symlink_link_color); -} - .symlink-symbol::after { content: "⇢"; display: inline-block; @@ -484,17 +475,12 @@ th span.active span { } a.root, - a.file, - a.symlink { + a.file { display: inline-block; flex: 1; padding: 0.5625rem 0; } - a.symlink { - width: 100%; - } - .back { display: flex; } From b9807ce23dff8a8ac0ae258b4d62d4042a6ff43c Mon Sep 17 00:00:00 2001 From: Ali MJ Al-Nasrawy Date: Sun, 11 Apr 2021 21:21:26 +0300 Subject: [PATCH 9/9] Don't use different syle for symlink symbol --- data/style.scss | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/data/style.scss b/data/style.scss index 82ab66378..e94436a02 100644 --- a/data/style.scss +++ b/data/style.scss @@ -88,7 +88,7 @@ a.file:hover { .symlink-symbol::after { content: "⇢"; display: inline-block; - border: 1px solid var(--symlink_link_color); + border: 1px solid; margin-left: 0.5rem; border-radius: 0.2rem; padding: 0 0.1rem; @@ -521,7 +521,6 @@ th span.active span { --text_color: #323232; --directory_link_color: #d02474; --file_link_color: #0086b3; - --symlink_link_color: #ed6a43; --table_background: #ffffff; --table_text_color: #323232; --table_header_background: #323232; @@ -566,7 +565,6 @@ th span.active span { --text_color: #fefefe; --directory_link_color: #03a9f4; --file_link_color: #ea95ff; - --symlink_link_color: #ff9800; --table_background: #353946; --table_text_color: #eeeeee; --table_header_background: #5294e2; @@ -611,7 +609,6 @@ th span.active span { --text_color: #efefef; --directory_link_color: #f0dfaf; --file_link_color: #87d6d5; - --symlink_link_color: #ffccee; --table_background: #4a4949; --table_text_color: #efefef; --table_header_background: #7f9f7f; @@ -656,7 +653,6 @@ th span.active span { --text_color: #f8f8f2; --directory_link_color: #f92672; --file_link_color: #a6e22e; - --symlink_link_color: #fd971f; --table_background: #3b3a32; --table_text_color: #f8f8f0; --table_header_background: #75715e;