Skip to content

Commit

Permalink
Merge PR #998
Browse files Browse the repository at this point in the history
  • Loading branch information
kazu-yamamoto committed Aug 6, 2024
2 parents f132cb8 + 5a9ff6d commit 53da27d
Show file tree
Hide file tree
Showing 5 changed files with 33 additions and 25 deletions.
26 changes: 15 additions & 11 deletions warp/Network/Wai/Handler/Warp/HTTP1.hs
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ http1server
-> Source
-> IO ()
http1server settings ii conn transport app addr th istatus src =
loop True `UnliftIO.catchAny` handler
loop FirstRequest `UnliftIO.catchAny` handler
where
handler e
-- See comment below referencing
Expand Down Expand Up @@ -154,18 +154,22 @@ http1server settings ii conn transport app addr th istatus src =
`UnliftIO.catchAny` \e -> do
settingsOnException settings (Just req) e
-- Don't throw the error again to prevent calling settingsOnException twice.
return False
return CloseConnection

-- When doing a keep-alive connection, the other side may just
-- close the connection. We don't want to treat that as an
-- exceptional situation, so we pass in False to http1 (which
-- in turn passes in False to recvRequest), indicating that
-- exceptional situation, so we pass in SubsequentRequest to http1 (which
-- in turn passes in SubsequentRequest to recvRequest), indicating that
-- this is not the first request. If, when trying to read the
-- request headers, no data is available, recvRequest will
-- throw a NoKeepAliveRequest exception, which we catch here
-- and ignore. See: https://github.com/yesodweb/wai/issues/618

when keepAlive $ loop False
case keepAlive of
ReuseConnection -> loop SubsequentRequest
CloseConnection -> return ()

data ReuseConnection = ReuseConnection | CloseConnection

processRequest
:: Settings
Expand All @@ -179,7 +183,7 @@ processRequest
-> Maybe (IORef Int)
-> IndexedHeader
-> IO ByteString
-> IO Bool
-> IO ReuseConnection
processRequest settings ii conn app th istatus src req mremainingRef idxhdr nextBodyFlush = do
-- Let the application run for as long as it wants
T.pause th
Expand Down Expand Up @@ -226,24 +230,24 @@ processRequest settings ii conn app th istatus src req mremainingRef idxhdr next
Nothing -> do
flushEntireBody nextBodyFlush
T.resume th
return True
return ReuseConnection
Just maxToRead -> do
let tryKeepAlive = do
-- flush the rest of the request body
isComplete <- flushBody nextBodyFlush maxToRead
if isComplete
then do
T.resume th
return True
else return False
return ReuseConnection
else return CloseConnection
case mremainingRef of
Just ref -> do
remaining <- readIORef ref
if remaining <= maxToRead
then tryKeepAlive
else return False
else return CloseConnection
Nothing -> tryKeepAlive
else return False
else return CloseConnection

sendErrorResponse
:: Settings
Expand Down
1 change: 1 addition & 0 deletions warp/Network/Wai/Handler/Warp/Internal.hs
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ module Network.Wai.Handler.Warp.Internal (

-- * Request and response
Source,
FirstRequest (..),
recvRequest,
sendResponse,

Expand Down
15 changes: 9 additions & 6 deletions warp/Network/Wai/Handler/Warp/Request.hs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
{-# OPTIONS_GHC -fno-warn-deprecations #-}

module Network.Wai.Handler.Warp.Request (
FirstRequest(..),
recvRequest,
headerLines,
pauseTimeoutKey,
Expand Down Expand Up @@ -50,11 +51,13 @@ import Network.Wai.Handler.Warp.Settings (

----------------------------------------------------------------

-- | first request on this connection?
data FirstRequest = FirstRequest | SubsequentRequest

-- | Receiving a HTTP request from 'Connection' and parsing its header
-- to create 'Request'.
recvRequest
:: Bool
-- ^ first request on this connection?
:: FirstRequest
-> Settings
-> Connection
-> InternalInfo
Expand Down Expand Up @@ -118,7 +121,7 @@ recvRequest firstRequest settings conn ii th addr src transport = do

----------------------------------------------------------------

headerLines :: Int -> Bool -> Source -> IO [ByteString]
headerLines :: Int -> FirstRequest -> Source -> IO [ByteString]
headerLines maxTotalHeaderLength firstRequest src = do
bs <- readSource src
if S.null bs
Expand All @@ -127,9 +130,9 @@ headerLines maxTotalHeaderLength firstRequest src = do
-- lack of data as a real exception. See the http1 function in
-- the Run module for more details.

if firstRequest
then throwIO ConnectionClosedByPeer
else throwIO NoKeepAliveRequest
case firstRequest of
FirstRequest -> throwIO ConnectionClosedByPeer
SubsequentRequest -> throwIO NoKeepAliveRequest
else push maxTotalHeaderLength src (THStatus 0 0 id id) bs

data NoKeepAliveRequest = NoKeepAliveRequest
Expand Down
4 changes: 2 additions & 2 deletions warp/bench/Parser.hs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import qualified Network.HTTP.Types as H
import UnliftIO.Exception (impureThrow, throwIO)
import Prelude hiding (lines)

import Network.Wai.Handler.Warp.Request (headerLines)
import Network.Wai.Handler.Warp.Request (FirstRequest (..), headerLines)
import Network.Wai.Handler.Warp.Types

#if MIN_VERSION_gauge(0, 2, 0)
Expand Down Expand Up @@ -61,7 +61,7 @@ main = do
]
]
where
testIt req = producer req >>= headerLines 800 False
testIt req = producer req >>= headerLines 800 FirstRequest

----------------------------------------------------------------

Expand Down
12 changes: 6 additions & 6 deletions warp/test/RequestSpec.hs
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ spec = do
describe "headerLines" $ do
let parseHeaderLine chunks = do
src <- mkSourceFunc chunks >>= mkSource
x <- headerLines defaultMaxTotalHeaderLength True src
x <- headerLines defaultMaxTotalHeaderLength FirstRequest src
x `shouldBe` ["Status: 200", "Content-Type: text/plain"]

it "can handle a normal case" $
Expand All @@ -95,9 +95,9 @@ spec = do
it "can (not) handle an illegal case (1)" $ do
let chunks = ["\nStatus:", "\n 200", "\nContent-Type: text/plain", "\r\n\r\n"]
src <- mkSourceFunc chunks >>= mkSource
x <- headerLines defaultMaxTotalHeaderLength True src
x <- headerLines defaultMaxTotalHeaderLength FirstRequest src
x `shouldBe` []
y <- headerLines defaultMaxTotalHeaderLength True src
y <- headerLines defaultMaxTotalHeaderLength FirstRequest src
y `shouldBe` ["Status:", " 200", "Content-Type: text/plain"]

let testLengthHeaders = ["Sta", "tus: 200\r", "\n", "Content-Type: ", "text/plain\r\n\r\n"]
Expand All @@ -106,12 +106,12 @@ spec = do
-- Length is 39, this shouldn't fail
it "doesn't throw on correct length" $ do
src <- mkSourceFunc testLengthHeaders >>= mkSource
x <- headerLines testLength True src
x <- headerLines testLength FirstRequest src
x `shouldBe` ["Status: 200", "Content-Type: text/plain"]
-- Length is still 39, this should fail
it "throws error on correct length too long" $ do
src <- mkSourceFunc testLengthHeaders >>= mkSource
headerLines (testLength - 1) True src `shouldThrow` (== OverLargeHeader)
headerLines (testLength - 1) FirstRequest src `shouldThrow` (== OverLargeHeader)
where
blankSafe = headerLinesList ["f", "oo\n", "bar\nbaz\n\r\n"]
whiteSafe = headerLinesList ["foo\r\nbar\r\nbaz\r\n\r\n hi there"]
Expand All @@ -135,7 +135,7 @@ headerLinesList' orig = do
writeIORef ref z
return y
src' <- mkSource src
res <- headerLines defaultMaxTotalHeaderLength True src'
res <- headerLines defaultMaxTotalHeaderLength FirstRequest src'
return (res, src')

consumeLen :: Int -> Source -> IO S8.ByteString
Expand Down

0 comments on commit 53da27d

Please sign in to comment.