Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Setting the path_filter and show_files_listing, but list page still show all files. #3207

Open
ahaoboy opened this issue Dec 2, 2023 · 2 comments
Labels
A-files project: actix-files C-improvement Category: an improvement to existing functionality good-first-issue easy to pick up for newcomers

Comments

@ahaoboy
Copy link

ahaoboy commented Dec 2, 2023

Files::new("/", EXAMPLES_DIR)
    .path_filter(|n, _| {
        println!("{:?}", n);
        let s = n.to_str().unwrap();
        if s == "lib.rs" || s == "" {
            true
        } else {
            false
        }
    })
    .show_files_listing(),

Expected Behavior

Only display the files that pass the filter, as in the example above, only the lib.rs file should be displayed.

Current Behavior

image
Display all files, but only those passed by the filter can be accessed.

Possible Solution

Add a filter parameter to the DirectoryRenderer function, or add a filter field to the Directory.

Steps to Reproduce (for bugs)

use actix_files::Files;
use actix_web::{get, guard, middleware, App, HttpServer, Responder};
const EXAMPLES_DIR: &str = concat![env!("CARGO_MANIFEST_DIR"), "/src"];

#[actix_web::main]
async fn main() -> std::io::Result<()> {
    env_logger::init_from_env(env_logger::Env::new().default_filter_or("info"));

    log::info!("starting HTTP server at http://localhost:8080");

    HttpServer::new(|| {
        App::new()
            .service(
                Files::new("/", EXAMPLES_DIR)
                    .path_filter(|n, _| {
                        println!("{:?}", n);
                        let s = n.to_str().unwrap();
                        if s == "lib.rs" || s == "" {
                            true
                        } else {
                            false
                        }
                    })
                    .show_files_listing(),
            )
            .wrap(middleware::Logger::default())
    })
    .bind(("127.0.0.1", 8080))?
    .run()
    .await
}

Context

When I set a filter, it indicates that I don't want users to be aware of the existence of certain files. However, these files are still displayed on the list page and are inaccessible, which can confuse users.

Your Environment

  • Rust Version (I.e, output of rustc -V): rustc 1.76.0-nightly (1e9dda77b 2023-11-22)
  • Actix Web Version: e95c8fe
@robjtede robjtede added C-improvement Category: an improvement to existing functionality A-files project: actix-files good-first-issue easy to pick up for newcomers labels Dec 2, 2023
@Liangdi
Copy link

Liangdi commented Dec 2, 2023

path_filter , is function for filter request , not for dir listing
the first param of filter fn is the request path
as your code:
if you visisted http://localhost:8080/main.rs return 404 because it filtered.

current solution is impl files_listing_renderer yourself ,such as:

Files::new("/", EXAMPLES_DIR)
                    .files_listing_renderer(|dir, req| {
                        Ok(ServiceResponse::new(
                            req.clone(),
                            HttpResponse::Ok().body(dir.path.to_str().unwrap().to_owned()),
                        ))
                    })
                    .show_files_listing()

the DirectoryRenderer is a fn , not a trait or struct , it's hard to extend new function without breaking change.

@ahaoboy
Copy link
Author

ahaoboy commented Dec 3, 2023

path_filter , is function for filter request , not for dir listing the first param of filter fn is the request path as your code: if you visisted http://localhost:8080/main.rs return 404 because it filtered.

current solution is impl files_listing_renderer yourself ,such as:

Files::new("/", EXAMPLES_DIR)
                    .files_listing_renderer(|dir, req| {
                        Ok(ServiceResponse::new(
                            req.clone(),
                            HttpResponse::Ok().body(dir.path.to_str().unwrap().to_owned()),
                        ))
                    })
                    .show_files_listing()

the DirectoryRenderer is a fn , not a trait or struct , it's hard to extend new function without breaking change.

Thx, this is the code that works properly, for future reference for anyone who might need it

.service(
    Files::new("/", EXAMPLES_DIR)
        .files_listing_renderer(|dir, req| {
            let files: Vec<_> = dir
                .path
                .read_dir()
                .unwrap()
                .filter_map(|d| match d {
                    Ok(dir) => {
                        let name = dir.file_name();
                        let s = name.to_string_lossy().to_string();
                        let include_files = ["lib.rs", "range.rs"];
                        if include_files.contains(&s.as_str()) {
                            return Some(s);
                        }
                        None
                    }
                    _ => None,
                })
                .map(|s| format!("<li><a target='_blank' href='/{}'>{}</a></li>", s, s))
                .collect();
            let html = format!(
                r"
<html><head>
<meta charset='utf-8'>
<title>Index of /</title></head><body><h1>Index of /</h1><ul>{}</ul></body>
</html>
",
                files.join("\n")
            );

            Ok(ServiceResponse::new(
                req.clone(),
                HttpResponse::Ok().body(html),
            ))
        })
        .show_files_listing(),```

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-files project: actix-files C-improvement Category: an improvement to existing functionality good-first-issue easy to pick up for newcomers
Projects
None yet
Development

No branches or pull requests

3 participants