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

#921 show directory index [WIP] #1033

Closed
wants to merge 8 commits into from

Conversation

trimailov
Copy link

What do these changes do?

  • By default throws HTTP 403 - Forbidden if we access static directory
  • If static directory indexing is enabled - lists the static directory contents when we access it

Are there changes in behavior for the user?

UrlDispatcher.add_static got new flag - show_index, which will show index of a directory if explicitly set to True. By default, accessing directory index is forbidden (HTTP/403, show_index=False)

Related issue number

#921

Checklist

  • StaticRoute._directory_as_html():
    • Maybe it should be outside the class? Unit-testing would be easier that way..
    • I would want a unit-test for this method to explicitly check that HTML is as we want
    • Method looks cumbersome and has lots of newlines, so the output would be human readable (is this a priority?)
    • Example output (from a test test_access_root_of_static_handler):
<html>
<head>
<title>Index of /</title>
</head>
<body>
<h1>Index of /</h1>
<ul>
<li><a href="/my_file">my_file</a></li>
</ul>
</body>
</html>
  • There is currently missing coverage for L506 and L531
  • Documentation should reflect the changes, though I have build problems ATM and cannot check docs as HTML

If we want to access a static file dir, it should return `Forbidden
(403)` by default.

Related: aio-libs#921
XXX: need for a response to return proper html

Related: aio-libs#921
Also I now return in place, instead of creating variable and returning
later, I am not a fan of returning somewehere inside a method, though if
we tried to return `ret` at the end as before, but I guess it's the most
clean pattern to do this.

This is because we have to conditional blocks, either of which can
return from the method. If first condtitonal creates `ret` variable,
later conditional may just raise `HTTPNotFound` which we do not want.
Though, I do not want to check that `ret` is populated either. Thus
return in place.

Related: aio-libs#921
- Better method name
- More human readable output (newlines)
- Title is shown as relative path to static directory and not as posix
  path
- Show directories with slash at the end of the name
And fix a minor typo
asvetlov pushed a commit that referenced this pull request Aug 22, 2016
* Add `show_index` flag to StaticRoute

Related: #921

* Accessing static dir, should return 403 by default

If we want to access a static file dir, it should return `Forbidden
(403)` by default.

Related: #921

* WIP: if allowed - return directory's index, otherwise - 403

XXX: need for a response to return proper html

Related: #921

* Return directory index list if we allow to show it

Also I now return in place, instead of creating variable and returning
later, I am not a fan of returning somewehere inside a method, though if
we tried to return `ret` at the end as before, but I guess it's the most
clean pattern to do this.

This is because we have to conditional blocks, either of which can
return from the method. If first condtitonal creates `ret` variable,
later conditional may just raise `HTTPNotFound` which we do not want.
Though, I do not want to check that `ret` is populated either. Thus
return in place.

Related: #921

* Test if correct statuses are returned if we allow acces index or not

Related: #921

* Prettier directory output

- Better method name
- More human readable output (newlines)
- Title is shown as relative path to static directory and not as posix
  path
- Show directories with slash at the end of the name

* Remove unnecessary comment

* Update docs

And fix a minor typo

* Check of html content is added for the response page with a directory index

Related: #921

* Test of accessing non-existing static resource added

Related: #921

* 403 Forbidden returned if the permission error occurred on listing requested folder

Related: #921

* show_index parameter usage of app.router.add_static method documented

Related: #921

* Import order fixed

Related: #921

* Make directory index sorted so stable for tests

Related: #921

* Improve tests coverage

Related: #921

* Improve tests coverage

Related: #921
@asvetlov
Copy link
Member

Fixed by #1094

@asvetlov asvetlov closed this Aug 22, 2016
@lock
Copy link

lock bot commented Oct 29, 2019

This thread has been automatically locked since there has not been
any recent activity after it was closed. Please open a new issue for
related bugs.

If you feel like there's important points made in this discussion,
please include those exceprts into that new issue.

@lock lock bot added the outdated label Oct 29, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Oct 29, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants