kilork / actix-web-static-files

actix-web static files as resources support
The Unlicense
75 stars 18 forks source link

`ResourceFiles::new("", resources::generate())` as a root resolver doesn't correctly resolve `http://localhost:8080/` to `index.html` #49

Closed 05storm26 closed 2 years ago

05storm26 commented 2 years ago

I am using this crate with this pull request: https://github.com/kilork/actix-web-static-files/pull/45 with actix 4.0 beta so it is somewhat unoffical:

However I have found one bug: If I try to add ResourceFiles as a root resolver like this: ResourceFiles::new("/", resources::generate()) this doesn't actually work for any paths other than the actual path of "/" so "/" gets resolved to index.html but no other file can be correctly resolved like "/index.js".

If I add it like this: ResourceFiles::new("", resources::generate()) this works all the paths that are not handled by previous services are handled by this services and paths like "/index.html" and "/index.js" are correctly resolved and served by this crate however now the most simple request to http://localhost:8080/ doesn't get resolved to index.html. (resolve_defaults is true)

Stepping through the code the issue is this:

in resource_files.rs:163 Service<ServiceRequest> for ResourceFilesService::call:

 let req_path = req.match_info().path();

        let mut item = self.files.get(req_path);

        if item.is_none()
            && self.resolve_defaults
            && (req_path.is_empty() || req_path.ends_with('/'))
        {
            let index_req_path = req_path.to_string() + INDEX_HTML;    // <------------
            item = self.files.get(index_req_path.as_str());                           // <--------------
        }

index_req_path becomes "/index.html" and then it is tried to be read from the self.files hashmap.

A way to fix this would be to change this function so that in all places where the self.files hashmap is being read (line numbers: 165, 172, 187, 191) it does it though a helper method that will check the path and if that starts with a slash it will remove that slash before attempting to read the file from the hashmap (so when the path is for example "/index.html" we should try to read from the map using the key "index.html" (the leading slash removed)).

Workarounds:

  1. register both ResourceFiles::new("", resources::generate()) and ResourceFiles::new("/", resources::generate()) at the same time.
  2. if acceptable use resolve_not_found_to_root with ResourceFiles::new("", resources::generate())
kilork commented 2 years ago

Can you check this example, this is pretty simple, and it works, at least worked with old version:

https://github.com/kilork/actix-web-static-files-examples/blob/main/webpack/src/main.rs

More complex example:

https://github.com/kilork/actix-web-static-files-example-angular-router

05storm26 commented 2 years ago

1

I have tried the example: https://github.com/kilork/actix-web-static-files-examples/tree/main/resource-dir

applying:

diff --git a/resource-dir/Cargo.toml b/resource-dir/Cargo.toml
index a5ef637..00da229 100644
--- a/resource-dir/Cargo.toml
+++ b/resource-dir/Cargo.toml
@@ -5,8 +5,8 @@ authors = ["Alexander Korolev <alexander.korolev.germany@gmail.com>"]
 edition = "2018"

 [dependencies]
-actix-web = "3"
-actix-web-static-files = "3.0"
+actix-web = "4.0.0-beta.19"
+actix-web-static-files = {version = "3.0.5", git ="https://github.com/enaut/actix-web-static-files.git"}
 static-files = "0.2.1"

 [build-dependencies]

and then cargo run makes the http://localhost:8080/ load the index.html content, http://localhost:8080/index.html is returning 404.

2

Trying out https://github.com/kilork/actix-web-static-files-examples/tree/main/webpack

with the same kind of patch is similar. An empty page loads for http://localhost:8080/ because the inital GET "/" loads successfully with 200 and the proper content. But the referenced script inside "main.js" cannot be loaded, it is 404.

05storm26 commented 2 years ago

(Also looking at the downloads/month of actix-web versions: https://lib.rs/crates/actix-web/versions it seems that a lot of people like 50% already is using 4.0 so it would be great to have this crate support 4.0 beta.)

kilork commented 2 years ago

Fixed in master and v4.0.0.