cd82d85 · typeable/wai

git.iko.soy/typeable/wai

Handle a timeout bug during request body

Here's the problem situation. As soon as we are done with processing the request headers, the timeout handler is paused. The purpose is to prevent timeouts during application code. The problematic case this commit solves is a when the request body has at least one chunk >= 2048. In this case, this chunk will tickle the timeout handler, which will start the clock again. If the application then delays longer than the timeout period, it will be terminated. Instead, we resume the timeout handler as soon as we start reading the request body, and then pause it as soon as we exit the request body processing code. Note that if the Sink provided by the application takes a long time, we could theoretically still have a problem. This should be looked into in more detail at some point.

commitcd82d85b060b720e2cd89b035b2a101b943c8273 authorMichael Snoyman <michael@snoyman.com> dateThu 20 Jun 2013, 22:04 (13 years ago) parentf7322f3

4 files changed+53−12

compare with master

unifiedsplit

warp/Network/Wai/Handler/Warp/Request.hs+19−6 warp/Network/Wai/Handler/Warp/Run.hs+5−4 warp/test/RunSpec.hs+28−1 warp/warp.cabal+1−1

warp/Network/Wai/Handler/Warp/Request.hs+19−6

@@ -25,17 +25,20 @@ import Network.Wai.Handler.Warp.Conduit

2525import Network.Wai.Handler.Warp.ReadInt

2626import Network.Wai.Handler.Warp.Types

2727import Prelude hiding (lines)

28+import qualified Network.Wai.Handler.Warp.Timeout as Timeout

2829

2930-- FIXME come up with good values here

3031maxTotalHeaderLength :: Int

3132maxTotalHeaderLength = 50 * 1024

3233

33−parseRequest :: Connection -> Port -> SockAddr

34+parseRequest :: Connection

35+ -> Timeout.Handle

36+ -> Port -> SockAddr

3437 -> Source (ResourceT IO) ByteString

3538 -> ResourceT IO (Request, IO (ResumableSource (ResourceT IO) ByteString))

36−parseRequest conn port remoteHost' src1 = do

39+parseRequest conn timeoutHandle port remoteHost' src1 = do

3740 (src2, headers') <- src1 $$+ takeHeaders

38− parseRequest' conn port headers' remoteHost' src2

41+ parseRequest' conn timeoutHandle port headers' remoteHost' src2

3942

4043handleExpect :: Connection

4144 -> H.HttpVersion

@@ -53,13 +56,14 @@ handleExpect conn hv front (x:xs) = handleExpect conn hv (front . (x:)) xs

5356

5457-- | Parse a set of header lines and body into a 'Request'.

5558parseRequest' :: Connection

59+ -> Timeout.Handle

5660 -> Port

5761 -> [ByteString]

5862 -> SockAddr

5963 -> ResumableSource (ResourceT IO) ByteString -- FIXME was buffered

6064 -> ResourceT IO (Request, IO (ResumableSource (ResourceT IO) ByteString))

61−parseRequest' _ _ [] _ _ = throwIO $ NotEnoughLines []

62−parseRequest' conn port (firstLine:otherLines) remoteHost' src = do

65+parseRequest' _ _ _ [] _ _ = throwIO $ NotEnoughLines []

66+parseRequest' conn timeoutHandle port (firstLine:otherLines) remoteHost' src = do

6367 (method, rpath', gets, httpversion) <- parseFirst firstLine

6468 let (host',rpath)

6569 | S.null rpath' = ("", "/")

@@ -97,7 +101,16 @@ parseRequest' conn port (firstLine:otherLines) remoteHost' src = do

97101 , requestHeaders = heads

98102 , isSecure = False

99103 , remoteHost = remoteHost'

100− , requestBody = rbody

104+ , requestBody = do

105+ -- Timeout handling was paused after receiving the full request

106+ -- headers. Now we need to resume it to avoid a slowloris

107+ -- attack during request body sending.

108+ liftIO $ Timeout.resume timeoutHandle

109+ -- As soon as we finish receiving the request body, whether

110+ -- because the application is not interested in more bytes, or

111+ -- because there is no more data available, pause the timeout

112+ -- handler again.

113+ addCleanup (const $ liftIO $ Timeout.pause timeoutHandle) rbody

101114 , vault = mempty

102115#if MIN_VERSION_wai(1, 4, 0)

103116 , requestBodyLength =

warp/Network/Wai/Handler/Warp/Run.hs+5−4

@@ -206,7 +206,7 @@ runSettingsConnectionMaker set getConn app = do

206206 bracket_ onOpen onClose $

207207

208208 -- Actually serve this connection.

209− serveConnection set cleaner port app conn addr

209+ serveConnection th set cleaner port app conn addr

210210 where

211211 -- FIXME: only IOEception is caught. What about other exceptions?

212212 getConnLoop = getConn `catch` \(e :: IOException) -> do

@@ -228,10 +228,11 @@ runSettingsConnectionMaker set getConn app = do

228228 f

229229 Just tm -> f tm

230230

231−serveConnection :: Settings

231+serveConnection :: T.Handle

232+ -> Settings

232233 -> Cleaner

233234 -> Port -> Application -> Connection -> SockAddr-> IO ()

234−serveConnection settings cleaner port app conn remoteHost' =

235+serveConnection timeoutHandle settings cleaner port app conn remoteHost' =

235236 runResourceT serveConnection'

236237 where

237238 innerRunResourceT

@@ -243,7 +244,7 @@ serveConnection settings cleaner port app conn remoteHost' =

243244 serveConnection' = serveConnection'' $ connSource conn th

244245

245246 serveConnection'' fromClient = do

246− (env, getSource) <- parseRequest conn port remoteHost' fromClient

247+ (env, getSource) <- parseRequest conn timeoutHandle port remoteHost' fromClient

247248 case settingsIntercept settings env of

248249 Nothing -> do

249250 -- Let the application run for as long as it wants

warp/test/RunSpec.hs+28−1

@@ -8,6 +8,7 @@ import Control.Monad (forM_)

88import Control.Monad.IO.Class (MonadIO, liftIO)

99import Data.ByteString (ByteString, hPutStr, hGetSome)

1010import qualified Data.ByteString as S

11+import qualified Data.ByteString.Char8 as S8

1112import qualified Data.ByteString.Lazy as L

1213import Data.Conduit (($$))

1314import qualified Data.Conduit.List

@@ -20,7 +21,7 @@ import System.IO (hFlush, hClose)

2021import System.IO.Unsafe (unsafePerformIO)

2122import Test.Hspec

2223import Control.Concurrent.MVar (newEmptyMVar, takeMVar, putMVar)

23−import Control.Exception (bracket, try, IOException, onException)

24+import Control.Exception.Lifted (bracket, try, IOException, onException)

2425import Data.Conduit.Network (bindPort)

2526import Network.Socket (sClose)

2627

@@ -300,3 +301,29 @@ spec = do

300301 [ "Hello World\nBye"

301302 , "Hello World"

302303 ]

304+ it "timeout in request body" $ do

305+ ifront <- I.newIORef id

306+ let app req = do

307+ bss <- (requestBody req $$ Data.Conduit.List.consume) `onException`

308+ liftIO (I.atomicModifyIORef ifront (\front -> (front . ("consume interrupted":), ())))

309+ liftIO $ threadDelay 4000000 `onException`

310+ I.atomicModifyIORef ifront (\front -> (front . ("threadDelay interrupted":), ()))

311+ liftIO $ I.atomicModifyIORef ifront $ \front -> (front . (S.concat bss:), ())

312+ return $ responseLBS status200 [] ""

313+ withApp defaultSettings { settingsTimeout = 1 } app $ \port -> do

314+ let bs1 = S.replicate 2048 88

315+ bs2 = "This is short"

316+ bs = S.append bs1 bs2

317+ handle <- connectTo "127.0.0.1" $ PortNumber $ fromIntegral port

318+ hPutStr handle "POST / HTTP/1.1\r\n"

319+ hPutStr handle "content-length: "

320+ hPutStr handle $ S8.pack $ show $ S.length bs

321+ hPutStr handle "\r\n\r\n"

322+ threadDelay 100000

323+ hPutStr handle bs1

324+ threadDelay 100000

325+ hPutStr handle bs2

326+ hClose handle

327+ threadDelay 5000000

328+ front <- I.readIORef ifront

329+ S.concat (front []) `shouldBe` bs

warp/warp.cabal+1−1

@@ -1,5 +1,5 @@

11Name: warp

2−Version: 1.3.8.2

2+Version: 1.3.8.3

33Synopsis: A fast, light-weight web server for WAI applications.

44License: MIT

55License-file: LICENSE