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

Fixed bug in serving static directory #803

Merged
merged 6 commits into from
Feb 27, 2016
Merged

Conversation

realcr
Copy link
Contributor

@realcr realcr commented Feb 25, 2016

When using app.add_static(...) to serve a directory, if the root of the directory is requested, the connection between the server and client is crashed.
This happens because the server tries to read a directory using open with 'rb' and fails.

The fix first makes sure that the requested resource is a file. If it is not a file, it returns a 404 error. I think this is the correct behaviour, feel free to change it if you think it should be something else.

I also include a relevant test case, to avoid regressions with this problem.

import asyncio

SERVER_HOST = '127.0.0.1'
SERVER_PORT = 8080
Copy link
Member

Choose a reason for hiding this comment

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

@asvetlov
Copy link
Member

Thanks! I'll merge after you'll fix two mentioned tiny comments.

@realcr
Copy link
Contributor Author

realcr commented Feb 26, 2016

I added the required modifications. Thank you for teaching me about flake8 :)

pass


def run_timeout(cor, loop, timeout=ASYNC_TEST_TIMEOUT):
Copy link
Member

Choose a reason for hiding this comment

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

Do you really need the function? What the purpose of timeout parameter -- looks like you don't use it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is used for making sure the test doesn't run too long. Do you have other mechanism to do this in the code base? If not, maybe we can add it in a more global location, to be used with other tests. The ASYNC_TEST_TIMEOUT is the default timeout.

Copy link
Member

Choose a reason for hiding this comment

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

Got it.
Right now aiohttp test suite has no such feature. I didn't feel the need for it.
Let's add the feature later to hooks that provide @pytest.mark.run_loop functionality.

@realcr
Copy link
Contributor Author

realcr commented Feb 27, 2016

I fixed the test case. It now uses mark.run_loop and create_app_and_client, so it is much shorter.

asvetlov added a commit that referenced this pull request Feb 27, 2016
Fixed bug in serving static directory
@asvetlov asvetlov merged commit 3b4eef8 into aio-libs:master Feb 27, 2016
@asvetlov
Copy link
Member

Thanks!

@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