Skip to content

Add ServeFile and ServeDir #45

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

Merged
merged 17 commits into from
Apr 27, 2021
Merged

Add ServeFile and ServeDir #45

merged 17 commits into from
Apr 27, 2021

Conversation

davidpdrsn
Copy link
Member

Service that serves a file and guesses the mime type with https://crates.io/crates/mime_guess.

In the future we'll also have `ServeDir` so makes sense for these to
live in the same module.

Also added a test for `ServeFile`.
So it can be used by the upcoming `ServeFile`
@davidpdrsn davidpdrsn changed the title Add ServeFile Add ServeFile and ServeDir Feb 3, 2021
@davidpdrsn davidpdrsn added the A-new-middleware Area: new middleware proposals label Feb 11, 2021
@davidpdrsn davidpdrsn added this to the 0.1.0 milestone Feb 12, 2021
@davidpdrsn
Copy link
Member Author

@tesaguri Is this something you would be up for reviewing?

Comment on lines 84 to 93
if append_index_html_on_directories {
let is_dir = tokio::fs::metadata(full_path.clone())
.await
.map(|m| m.is_dir())
.unwrap_or(false);

if is_dir {
full_path.push("index.html");
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the path points to a directory and has no trailing slash, the service should redirect to a URI with a trailing slash.

Suppose that there is ${base}/foo/index.html with a relative URI like <script src="script.js"></script>. When a user agent requests /foo/, the URI script.js resolves to /foo/script.js as expected. But when it requests /foo, script.js resolves to /script.js, which is not the desired behavior.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Uh thats a good point. Didn't consider that. Fixed in 467dd06

Copy link
Member

@LucioFranco LucioFranco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM just one question

@davidpdrsn davidpdrsn merged commit b1892d9 into master Apr 27, 2021
@davidpdrsn davidpdrsn deleted the serve-file branch April 27, 2021 20:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-new-middleware Area: new middleware proposals
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants