fix(services)!: reject trailing slash for file paths by Isvane · Pull Request #678 · tower-rs/tower-http

Isvane · 2026-05-12T10:19:44Z

BREAKING CHANGE: Requests to file paths with a trailing slash (e.g., /index.html/)
will now correctly return a 404 FileNotFound instead of serving the file.
This alters previous behavior where the trailing slash was ignored.

Motivation

Hi. This is my first time contributing to global library and I wanted to help out with #654.

The goal here was to make sure that if someone tries to access a file with a trailing slash (like /index.html/), the server doesn't get confused and try to serve it as a file. It should only allow the trailing slash if it’s actually a directory.

Solution

In serve_dir/mod.rs, I added a check to see if the requested URI ends with a /.
I passed that info into the open_file function.
Inside open_file.rs, I added a check: if there’s a trailing slash but the metadata says "this is a file," we return FileNotFound.
I made sure this doesn't break the way the it looks for index.html inside real folders.
I added a new test case in tests.rs to make sure requesting /index.html/ now correctly returns a 404.

I'm very open to feedback if there’s a cleaner way to handle this or if I missed any project conventions, please let me know.

jlizen · 2026-05-12T23:23:07Z

@jplatte this feels like probably the right default, but something that needs to ship across a breaking semver release due to the behavioral change, would you agree? One could call it a bugfix, but that seems a bit sketchy to me.

(specific PR code aside, I will review this more closely)

jplatte · 2026-05-13T00:15:37Z

Yeah, seems likely enough that some users are currently (accidentally) relying on file/ paths working.

jlizen

Thanks for the contribution! Implementation could use a bit of tweaking, see comments.

minor: It might be worth applying similar validation to ServeVariant::SingleFile as well, no? Probably more confusing to have handling be inconsistent?

jlizen · 2026-05-19T14:31:36Z

-            // Might already at this point know a redirect or not found result should be
-            // returned which corresponds to a Some(output). Otherwise the path might be
-            // modified and proceed to the open file/metadata future.
+            let was_directory = is_dir(&path_to_file).await;


This will result in a redundant syscall since we anyway call is_dir inside maybe_redirect_or_append_path.

We could instead just check for the trailing slash AFTER that call, since if it doesn't have an early return, we know that we are dealing with file rather than a directory (perhaps because index.html was appended)

jlizen · 2026-05-19T14:46:49Z

        ServeVariant::Directory {
            append_index_html_on_directories,
        } => {
-            // Might already at this point know a redirect or not found result should be


I would probably keep this comment, it is largely accurate still? Instead of we moved the slash check until after per this comment we would want a new comment explaining, "at this point we know we are are dealing with a file, so make sure it doesn't have a trailing slash"

jlizen · 2026-05-19T14:51:55Z

    negotiated_encodings: Vec<(Encoding, QValue)>,
    range_header: Option<String>,
    buf_chunk_size: usize,
+    trailing_slash: bool,


I would probably just check the path directly inline where we need it rather than threading throughout.

jlizen · 2026-05-19T14:54:22Z

 }

+#[tokio::test]
+async fn not_found_when_file_requested_with_trailing_slash() {


Can we also have a check where there is a trailing slash + file where there is a fallback configured?

Isvane · 2026-05-19T15:59:02Z

Thanks for the contribution! Implementation could use a bit of tweaking, see comments.

minor: It might be worth applying similar validation to ServeVariant::SingleFile as well, no? Probably more confusing to have handling be inconsistent?

Hi @jlizen,

Thanks so much for the review. I've updated the PR with your feedback.

I took out the redundant is_dir check and moved the trailing slash logic to happen inline right after maybe_redirect_or_append_path. I also made sure to leave the original comment alone and added a new one like you suggested.

I also did the suggestion for SingleFile. Since this is my first PR ever, I mostly just copied and pasted the existing test structures to write the two new tests (one for the fallback behavior and one for the single file trailing slash check). Hopefully everything looks okay.

I really appreciate you taking the time for the review and the helpful guidance. I learned a lot messing around with this. Let me know if there's anything else I should tweak.

jplatte · 2026-05-19T18:38:47Z

@jlizen are you sure about SingleFile? Isn't that (only) used for ServeFile, where we (should) just ignore the request path entirely?

jlizen · 2026-05-21T04:20:43Z

@jlizen are you sure about SingleFile? Isn't that (only) used for ServeFile, where we (should) just ignore the request path entirely?

Yeah, after thinking more with it, I agree that this check doesn't belong with ServeFile.

My knee-jerk previously was, "well, it is intrinsically confusing to serve a file from a directory-like URI". But, it is literally the contract of ServeFile to serve a given file regardless of path, which is already off the rail for "URI intuition", so I think the concern is misplaced.

jlizen · 2026-05-21T04:22:05Z

                Some(path_to_file)
            }
-            ServeVariant::SingleFile { mime: _ } => Some(base_path.to_path_buf()),
+            ServeVariant::SingleFile { mime: _ } => {


Sorry for the churn on this, but let's rip out the SingleFile check as discussed here.

jlizen · 2026-05-21T04:33:42Z

+            // At this point we know we are dealing with a file mapping.
+            // A trailing slash is only valid if it's the root path "/" fetching an index file, otherwise it's invalid.
+            let uri_path = req.uri().path();
+            if uri_path.ends_with('/') && uri_path != "/" {


This introduces a bug where /foo/ with append_index_html_on_directories: true will now 404 out.

The check should instead be something like:

if uri_path.ends_with('/') && !path_to_file.ends_with("index.html") { ...

Another option would be to return an enum from maybe_redirect_or_append_path:

enum PathResolution { EarlyOutput(OpenFileOutput), AppendedIndexHtml, // was a directory, index.html appended NotADirectory, // not a directory, path unchanged }

I have no strong preference for one versus the other, dealer's choice. The enum is cleaner in principal but probably a bit overengineered compared to the one-liner.

Also, let's definitely have a test case with /foo/ that appends index.html and makes sure it is still served properly.

Let's throw in a test against / that appends index.html while we're at it.

fix(services): handle trailing slash

da88c5e

Isvane marked this pull request as draft May 12, 2026 15:15

fix(services): handle trailing slashes on file requests

4a19359

Isvane marked this pull request as ready for review May 12, 2026 16:12

Isvane changed the title ~~fix(services): handle trailing slash~~ fix(services)!: reject trailing slash for file paths May 14, 2026

jlizen requested changes May 19, 2026

View reviewed changes

Isvane added 3 commits May 19, 2026 22:25

fix(services): return 404 for file requests with trailing slash

7a5f33d

test: add test for file request with trailing slash and fallback

1b7ce8d

fix(services): return 404 for single file requests with trailing slash

7724b8a

Isvane requested a review from jlizen May 19, 2026 16:02

jlizen requested changes May 21, 2026

View reviewed changes

Conversation

Isvane commented May 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Solution

Uh oh!

jlizen commented May 12, 2026

Uh oh!

jplatte commented May 13, 2026

Uh oh!

jlizen left a comment

Choose a reason for hiding this comment

Uh oh!

jlizen May 19, 2026

Choose a reason for hiding this comment

Uh oh!

jlizen May 19, 2026

Choose a reason for hiding this comment

Uh oh!

jlizen May 19, 2026

Choose a reason for hiding this comment

Uh oh!

jlizen May 19, 2026

Choose a reason for hiding this comment

Uh oh!

Isvane commented May 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jplatte commented May 19, 2026

Uh oh!

jlizen commented May 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jlizen May 21, 2026

Choose a reason for hiding this comment

Uh oh!

jlizen May 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jlizen May 21, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Isvane commented May 12, 2026 •

edited

Loading

Isvane commented May 19, 2026 •

edited

Loading

jlizen commented May 21, 2026 •

edited

Loading

jlizen May 21, 2026 •

edited

Loading