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

aiohttp.server.ServerHttpProtocol has no mechanism to prevent broken sockets (example broken from client side) #12

Closed
polymorphm opened this issue Feb 16, 2014 · 16 comments
Labels

Comments

@polymorphm
Copy link
Contributor

good day!

aiohttp.server.ServerHttpProtocol has no mechanism to prevent broken sockets.

broken sockets -- will be as resource leak on server-side.

from client side -- attacker may be [specially] creating broken-sockets to make easy DoS-attake to owr server-side..

or broken sockets may be accumulates on server-side [not-specially] due other randomly situations from badly clients.

example of solution of this issue: #11
(variable: self._keep_alive_period it is not solution for all cases)

alternative solution -- create timeout in aiohttp.server.ServerHttpProtocol.
(timeout is good way, but it may has own problems.)

thanks in advance! :-)
and sorry my bad english :(

@fafhrd91
Copy link
Member

could you explain why _keep_alive_period is not a solution?

@polymorphm
Copy link
Contributor Author

could you explain why _keep_alive_period is not a solution?

self._keep_alive_period -- used only in some places of code.

in other places of code -- self._keep_alive_period not preventing from broken socket.

but if rewrite using self._keep_alive_period (more reliable) -- it may be good solution. as I think! but not now.

@fafhrd91
Copy link
Member

it is not clear what is "in other places of code", could you be more specific.
right now i start keep alive timer after response. if keep alive is not configured then socket gets close.

@polymorphm
Copy link
Contributor Author

it is not clear what is "in other places of code", could you be more specific.

for example -- if socket will broken before client was sent all headers to server.

@fafhrd91
Copy link
Member

that's good point. i need to think.

@polymorphm
Copy link
Contributor Author

thank you very much for your time!
I was afraid that my bad english -- would not let me -- pass my meaning :-)

@polymorphm
Copy link
Contributor Author

i think this issue may be closed (merged #11 ). thanks!

@fafhrd91 fafhrd91 reopened this Feb 17, 2014
@fafhrd91
Copy link
Member

i want to add extra timeout. lets keep this open for now.

@polymorphm
Copy link
Contributor Author

i want to add extra timeout

it is will be very good!

@fafhrd91
Copy link
Member

по поводу английского не переживай

@polymorphm
Copy link
Contributor Author

по поводу английского не переживай

а ведь имя "Николай" должно было бы мне подсказать :)


but name "Nikolay" would be suggest to me :-)

@fafhrd91
Copy link
Member

added slow request timeout @1977f04738102c9f3469acc780536b918acd056e

@polymorphm
Copy link
Contributor Author

а было бы большой ошибкой -- если например запускать slow timeout перед самым началом парсинга запроса от клиента (включая prefix)?

то есть -- если например заменить этот код (в aiohttp.server.ServerHttpProtocol):

        try:
            prefix = self.stream.set_parser(self._request_prefix)
            yield from prefix.read()

            # stop keep-alive timer
            if self._keep_alive_handle is not None:
                self._keep_alive_handle.cancel()
                self._keep_alive_handle = None

            # start slow request timer
            if self._timeout:
                self._timeout_handle = self._loop.call_later(
                    self._timeout, self.cancel_slow_request)

на:

        try:
            # start slow request timer (if slow-req-timer and keep-alive timer -- not started yet)
            if self._timeout and \
                    self._timeout_handle is None and \
                    self._keep_alive_handle is None:
                self._timeout_handle = self._loop.call_later(
                    self._timeout, self.cancel_slow_request)

            prefix = self.stream.set_parser(self._request_prefix)
            yield from prefix.read()

            # stop keep-alive timer
            if self._keep_alive_handle is not None:
                self._keep_alive_handle.cancel()
                self._keep_alive_handle = None

            # start slow request timer (if slow-req-timer not started yet)
            if self._timeout and self._timeout_handle is None:
                self._timeout_handle = self._loop.call_later(
                    self._timeout, self.cancel_slow_request)

but will be big error if to start timeout (slow request timeout) before first begin of parsing request (including prefix).

for example -- if replace up code to down code (in aiohttp.server.ServerHttpProtocol)?

@polymorphm
Copy link
Contributor Author

( I fixed last message )

@fafhrd91
Copy link
Member

i think it is better to start timer in connection_made only for first request @3f969b1bb7c4e705c05940625106d2939df53cf0

@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

No branches or pull requests

2 participants