Skip to content

Make ServeDir return 404 if file does not exists#637

Merged
yoshuawuyts merged 5 commits intohttp-rs:masterfrom
pbzweihander:servedir-404
Jul 10, 2020
Merged

Make ServeDir return 404 if file does not exists#637
yoshuawuyts merged 5 commits intohttp-rs:masterfrom
pbzweihander:servedir-404

Conversation

@pbzweihander
Copy link
Contributor

Description

Make ServeDir return 404 Not Found response when the requested file does not exist.

Motivation and Context

ServeDir, the internal static file serving endpoint, returns 500 when the file does not exist. But I expected 404.

How Has This Been Tested?

ServeDir did not have any test, so I added some unit tests that call the call method directly and some integration tests that request to ServeDir using Route::serve_dir.

Unfortunately, I could not find any test case to produce a 401 Forbidden error. The http_types::Url type seems to normalize the path segments so I couldn't produce any URL path that refers outside of the desired directory.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

Copy link
Member

@yoshuawuyts yoshuawuyts left a comment

Choose a reason for hiding this comment

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

@pbzweihander Thanks so much for this PR; these changes are great! -- a few nits aside I think we can merge this!

@yoshuawuyts yoshuawuyts merged commit a5b911d into http-rs:master Jul 10, 2020
@pbzweihander pbzweihander deleted the servedir-404 branch July 10, 2020 16:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants