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

Expected behaviour when catching TimeoutThread? #602

Closed
agrafix opened this Issue Dec 28, 2016 · 3 comments

Comments

Projects
None yet
3 participants
@agrafix

agrafix commented Dec 28, 2016

In an application I am currently debugging a response handler catches all exceptions via

`catch` \(e :: SomeException) ->

and then logs the expection and continues to do it's work. While I know that this is bad coding and should be fixed right there, I was wondering what the expected behaviour of warp for this kind of situation would be?

@snoyberg

This comment has been minimized.

Member

snoyberg commented Jan 2, 2017

Correct me if you think otherwise @kazu-yamamoto, but I think we do not currently handle this case well in Warp. With this code, I believe today you would end up blocking the slowloris protection and the connection would remain open. I think we need the following improvements in Warp:

  • Whenever we send a TimeoutThread exception, also immediately kill the open connection, so that even if the application catches the exception, the attack is stopped.
  • Modify the Exception instance for TimeoutThread so that it inherits from AsyncException, so packages like safe-exceptions work correctly.
@kazu-yamamoto

This comment has been minimized.

Contributor

kazu-yamamoto commented Jan 2, 2017

+1

@agrafix

This comment has been minimized.

agrafix commented Jan 3, 2017

Sounds good! I hope to find time for a PR soon :-)

snoyberg added a commit that referenced this issue Jan 22, 2017

@snoyberg snoyberg self-assigned this Jan 22, 2017

kazu-yamamoto added a commit that referenced this issue Jan 27, 2017

Merge pull request #605 from yesodweb/602-better-async-timeoutthread
Better async exception behavior for timeouts #602

@snoyberg snoyberg removed the in progress label Jan 27, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment