Skip to content

Commit

Permalink
Revert "Use Word in MaxHeaderLength and MaxNumberHeaders"
Browse files Browse the repository at this point in the history
This reverts commit c3312b4.
marinelli committed Dec 16, 2024

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
1 parent aa8930e commit 749ed45
Showing 11 changed files with 41 additions and 49 deletions.
4 changes: 2 additions & 2 deletions http-client/Network/HTTP/Client.hs
Original file line number Diff line number Diff line change
@@ -325,12 +325,12 @@ managerSetProxy po = managerSetInsecureProxy po . managerSetSecureProxy po
-- @since 0.7.17
managerSetMaxHeaderLength :: Int -> ManagerSettings -> ManagerSettings
managerSetMaxHeaderLength l manager = manager
{ managerMaxHeaderLength = MaxHeaderLength l }
{ managerMaxHeaderLength = Just $ MaxHeaderLength l }

-- @since 0.7.18
managerSetMaxNumberHeaders :: Int -> ManagerSettings -> ManagerSettings
managerSetMaxNumberHeaders n manager = manager
{ managerMaxNumberHeaders = MaxNumberHeaders n }
{ managerMaxNumberHeaders = Just $ MaxNumberHeaders n }

-- $example1
-- = Example Usage
2 changes: 1 addition & 1 deletion http-client/Network/HTTP/Client/Body.hs
Original file line number Diff line number Diff line change
@@ -148,7 +148,7 @@ makeLengthReader cleanup count0 Connection {..} = do
return bs

makeChunkedReader
:: MaxHeaderLength
:: Maybe MaxHeaderLength
-> IO () -- ^ cleanup
-> Bool -- ^ raw
-> Connection
12 changes: 6 additions & 6 deletions http-client/Network/HTTP/Client/Connection.hs
Original file line number Diff line number Diff line change
@@ -31,29 +31,29 @@ import Data.Function (fix)
import Data.Maybe (listToMaybe)
import Data.Word (Word8)

connectionReadLine :: MaxHeaderLength -> Connection -> IO ByteString
connectionReadLine :: Maybe MaxHeaderLength -> Connection -> IO ByteString
connectionReadLine mhl conn = do
bs <- connectionRead conn
when (S.null bs) $ throwHttp IncompleteHeaders
connectionReadLineWith mhl conn bs

-- | Keep dropping input until a blank line is found.
connectionDropTillBlankLine :: MaxHeaderLength -> Connection -> IO ()
connectionDropTillBlankLine :: Maybe MaxHeaderLength -> Connection -> IO ()
connectionDropTillBlankLine mhl conn = fix $ \loop -> do
bs <- connectionReadLine mhl conn
unless (S.null bs) loop

connectionReadLineWith :: MaxHeaderLength -> Connection -> ByteString -> IO ByteString
connectionReadLineWith :: Maybe MaxHeaderLength -> Connection -> ByteString -> IO ByteString
connectionReadLineWith mhl conn bs0 =
go bs0 id 0
where
go bs front total =
case S.break (== charLF) bs of
(_, "") -> do
let total' = total + S.length bs
when (total > unMaxHeaderLength mhl) $ do
-- We reached the maximum length for an header field.
throwHttp OverlongHeaders
case fmap unMaxHeaderLength mhl of
Nothing -> pure ()
Just n -> when (total' > n) $ throwHttp OverlongHeaders
bs' <- connectionRead conn
when (S.null bs') $ throwHttp IncompleteHeaders
go bs' (front . (bs:)) total'
13 changes: 6 additions & 7 deletions http-client/Network/HTTP/Client/Headers.hs
Original file line number Diff line number Diff line change
@@ -28,7 +28,7 @@ charColon = 58
charPeriod = 46


parseStatusHeaders :: MaxHeaderLength -> MaxNumberHeaders -> Connection -> Maybe Int -> ([Header] -> IO ()) -> Maybe (IO ()) -> IO StatusHeaders
parseStatusHeaders :: Maybe MaxHeaderLength -> Maybe MaxNumberHeaders -> Connection -> Maybe Int -> ([Header] -> IO ()) -> Maybe (IO ()) -> IO StatusHeaders
parseStatusHeaders mhl mnh conn timeout' onEarlyHintHeaders cont
| Just k <- cont = getStatusExpectContinue k
| otherwise = getStatus
@@ -60,15 +60,15 @@ parseStatusHeaders mhl mnh conn timeout' onEarlyHintHeaders cont
return $ Just $ StatusHeaders s' v' (earlyHeaders <> earlyHeaders') reqHeaders
| otherwise -> (Just <$>) $ StatusHeaders s v mempty A.<$> parseHeaders 0 id

nextStatusLine :: MaxHeaderLength -> IO (Status, HttpVersion)
nextStatusLine :: Maybe MaxHeaderLength -> IO (Status, HttpVersion)
nextStatusLine mhl = do
-- Ensure that there is some data coming in. If not, we want to signal
-- this as a connection problem and not a protocol problem.
bs <- connectionRead conn
when (S.null bs) $ throwHttp NoResponseDataReceived
connectionReadLineWith mhl conn bs >>= parseStatus mhl 3

parseStatus :: MaxHeaderLength -> Int -> S.ByteString -> IO (Status, HttpVersion)
parseStatus :: Maybe MaxHeaderLength -> Int -> S.ByteString -> IO (Status, HttpVersion)
parseStatus mhl i bs | S.null bs && i > 0 = connectionReadLine mhl conn >>= parseStatus mhl (i - 1)
parseStatus _ _ bs = do
let (ver, bs2) = S.break (== charSpace) bs
@@ -92,10 +92,9 @@ parseStatusHeaders mhl mnh conn timeout' onEarlyHintHeaders cont
_ -> Nothing

guardMaxNumberHeaders :: Int -> IO ()
guardMaxNumberHeaders count =
when (count >= unMaxNumberHeaders mnh) $ do
-- We reached the maximum number of header fields.
throwHttp TooManyHeaders
guardMaxNumberHeaders count = case fmap unMaxNumberHeaders mnh of
Nothing -> pure ()
Just n -> when (count >= n) $ throwHttp TooManyHeaders

parseHeaders :: Int -> ([Header] -> [Header]) -> IO [Header]
parseHeaders count front = do
4 changes: 2 additions & 2 deletions http-client/Network/HTTP/Client/Manager.hs
Original file line number Diff line number Diff line change
@@ -92,8 +92,8 @@ defaultManagerSettings = ManagerSettings
, managerModifyResponse = return
, managerProxyInsecure = defaultProxy
, managerProxySecure = defaultProxy
, managerMaxHeaderLength = 4096
, managerMaxNumberHeaders = 100
, managerMaxHeaderLength = Just $ MaxHeaderLength 4096
, managerMaxNumberHeaders = Just $ MaxNumberHeaders 100
}

-- | Create a 'Manager'. The @Manager@ will be shut down automatically via
4 changes: 2 additions & 2 deletions http-client/Network/HTTP/Client/Response.hs
Original file line number Diff line number Diff line change
@@ -113,8 +113,8 @@ lbsResponse res = do
{ responseBody = L.fromChunks bss
}

getResponse :: MaxHeaderLength
-> MaxNumberHeaders
getResponse :: Maybe MaxHeaderLength
-> Maybe MaxNumberHeaders
-> Maybe Int
-> Request
-> Managed Connection
16 changes: 4 additions & 12 deletions http-client/Network/HTTP/Client/Types.hs
Original file line number Diff line number Diff line change
@@ -40,9 +40,7 @@ module Network.HTTP.Client.Types
, ResponseTimeout (..)
, ProxySecureMode (..)
, MaxHeaderLength (..)
, noMaxHeaderLength
, MaxNumberHeaders (..)
, noMaxNumberHeaders
) where

import qualified Data.Typeable as T (Typeable)
@@ -826,13 +824,13 @@ data ManagerSettings = ManagerSettings
-- Default: respect the @proxy@ value on the @Request@ itself.
--
-- Since 0.4.7
, managerMaxHeaderLength :: MaxHeaderLength
, managerMaxHeaderLength :: Maybe MaxHeaderLength
-- ^ Configure the maximum size, in bytes, of an HTTP header field.
--
-- Default: 4096
--
-- @since 0.7.17
, managerMaxNumberHeaders :: MaxNumberHeaders
, managerMaxNumberHeaders :: Maybe MaxNumberHeaders
-- ^ Configure the maximum number of HTTP header fields.
--
-- Default: 100
@@ -864,8 +862,8 @@ data Manager = Manager
, mSetProxy :: Request -> Request
, mModifyResponse :: Response BodyReader -> IO (Response BodyReader)
-- ^ See 'managerProxy'
, mMaxHeaderLength :: MaxHeaderLength
, mMaxNumberHeaders :: MaxNumberHeaders
, mMaxHeaderLength :: Maybe MaxHeaderLength
, mMaxNumberHeaders :: Maybe MaxNumberHeaders
}
deriving T.Typeable

@@ -926,16 +924,10 @@ newtype MaxHeaderLength = MaxHeaderLength
}
deriving (Eq, Show, Ord, Num, Enum, Bounded, T.Typeable)

noMaxHeaderLength :: MaxHeaderLength
noMaxHeaderLength = maxBound

-- | The maximum number of header fields.
--
-- @since 0.7.18
newtype MaxNumberHeaders = MaxNumberHeaders
{ unMaxNumberHeaders :: Int
}
deriving (Eq, Show, Ord, Num, Enum, Bounded, T.Typeable)

noMaxNumberHeaders :: MaxNumberHeaders
noMaxNumberHeaders = maxBound
16 changes: 8 additions & 8 deletions http-client/test-nonet/Network/HTTP/Client/BodySpec.hs
Original file line number Diff line number Diff line change
@@ -22,7 +22,7 @@ spec = describe "BodySpec" $ do
(conn, _, input) <- dummyConnection
[ "5\r\nhello\r\n6\r\n world\r\n0\r\n\r\nnot consumed"
]
reader <- makeChunkedReader noMaxHeaderLength (return ()) False conn
reader <- makeChunkedReader Nothing (return ()) False conn
body <- brConsume reader
S.concat body `shouldBe` "hello world"
input' <- input
@@ -33,7 +33,7 @@ spec = describe "BodySpec" $ do
(conn, _, input) <- dummyConnection
[ "5\r\nhello\r\n6\r\n world\r\n0\r\ntrailers-are: ignored\r\nbut: consumed\r\n\r\nnot consumed"
]
reader <- makeChunkedReader noMaxHeaderLength (return ()) False conn
reader <- makeChunkedReader Nothing (return ()) False conn
body <- brConsume reader
S.concat body `shouldBe` "hello world"
input' <- input
@@ -43,7 +43,7 @@ spec = describe "BodySpec" $ do
it "chunked, pieces" $ do
(conn, _, input) <- dummyConnection $ map S.singleton $ S.unpack
"5\r\nhello\r\n6\r\n world\r\n0\r\n\r\nnot consumed"
reader <- makeChunkedReader noMaxHeaderLength (return ()) False conn
reader <- makeChunkedReader Nothing (return ()) False conn
body <- brConsume reader
S.concat body `shouldBe` "hello world"
input' <- input
@@ -53,7 +53,7 @@ spec = describe "BodySpec" $ do
it "chunked, pieces, with trailers" $ do
(conn, _, input) <- dummyConnection $ map S.singleton $ S.unpack
"5\r\nhello\r\n6\r\n world\r\n0\r\ntrailers-are: ignored\r\nbut: consumed\r\n\r\nnot consumed"
reader <- makeChunkedReader noMaxHeaderLength (return ()) False conn
reader <- makeChunkedReader Nothing (return ()) False conn
body <- brConsume reader
S.concat body `shouldBe` "hello world"
input' <- input
@@ -64,7 +64,7 @@ spec = describe "BodySpec" $ do
(conn, _, input) <- dummyConnection
[ "5\r\nhello\r\n6\r\n world\r\n0\r\n\r\nnot consumed"
]
reader <- makeChunkedReader noMaxHeaderLength (return ()) True conn
reader <- makeChunkedReader Nothing (return ()) True conn
body <- brConsume reader
S.concat body `shouldBe` "5\r\nhello\r\n6\r\n world\r\n0\r\n\r\n"
input' <- input
@@ -75,7 +75,7 @@ spec = describe "BodySpec" $ do
(conn, _, input) <- dummyConnection
[ "5\r\nhello\r\n6\r\n world\r\n0\r\ntrailers-are: returned\r\nin-raw: body\r\n\r\nnot consumed"
]
reader <- makeChunkedReader noMaxHeaderLength (return ()) True conn
reader <- makeChunkedReader Nothing (return ()) True conn
body <- brConsume reader
S.concat body `shouldBe` "5\r\nhello\r\n6\r\n world\r\n0\r\ntrailers-are: returned\r\nin-raw: body\r\n\r\n"
input' <- input
@@ -85,7 +85,7 @@ spec = describe "BodySpec" $ do
it "chunked, pieces, raw" $ do
(conn, _, input) <- dummyConnection $ map S.singleton $ S.unpack
"5\r\nhello\r\n6\r\n world\r\n0\r\n\r\nnot consumed"
reader <- makeChunkedReader noMaxHeaderLength (return ()) True conn
reader <- makeChunkedReader Nothing (return ()) True conn
body <- brConsume reader
S.concat body `shouldBe` "5\r\nhello\r\n6\r\n world\r\n0\r\n\r\n"
input' <- input
@@ -95,7 +95,7 @@ spec = describe "BodySpec" $ do
it "chunked, pieces, raw, with trailers" $ do
(conn, _, input) <- dummyConnection $ map S.singleton $ S.unpack
"5\r\nhello\r\n6\r\n world\r\n0\r\ntrailers-are: returned\r\nin-raw: body\r\n\r\nnot consumed"
reader <- makeChunkedReader noMaxHeaderLength (return ()) True conn
reader <- makeChunkedReader Nothing (return ()) True conn
body <- brConsume reader
S.concat body `shouldBe` "5\r\nhello\r\n6\r\n world\r\n0\r\ntrailers-are: returned\r\nin-raw: body\r\n\r\n"
input' <- input
12 changes: 6 additions & 6 deletions http-client/test-nonet/Network/HTTP/Client/HeadersSpec.hs
Original file line number Diff line number Diff line change
@@ -23,7 +23,7 @@ spec = describe "HeadersSpec" $ do
, "\nignored"
]
(connection, _, _) <- dummyConnection input
statusHeaders <- parseStatusHeaders noMaxHeaderLength noMaxNumberHeaders connection Nothing (\_ -> return ()) Nothing
statusHeaders <- parseStatusHeaders Nothing Nothing connection Nothing (\_ -> return ()) Nothing
statusHeaders `shouldBe` StatusHeaders status200 (HttpVersion 1 1) mempty
[ ("foo", "bar")
, ("baz", "bin")
@@ -37,7 +37,7 @@ spec = describe "HeadersSpec" $ do
]
(conn, out, _) <- dummyConnection input
let sendBody = connectionWrite conn "data"
statusHeaders <- parseStatusHeaders noMaxHeaderLength noMaxNumberHeaders conn Nothing (\_ -> return ()) (Just sendBody)
statusHeaders <- parseStatusHeaders Nothing Nothing conn Nothing (\_ -> return ()) (Just sendBody)
statusHeaders `shouldBe` StatusHeaders status200 (HttpVersion 1 1) [] [ ("foo", "bar") ]
out >>= (`shouldBe` ["data"])

@@ -47,7 +47,7 @@ spec = describe "HeadersSpec" $ do
]
(conn, out, _) <- dummyConnection input
let sendBody = connectionWrite conn "data"
statusHeaders <- parseStatusHeaders noMaxHeaderLength noMaxNumberHeaders conn Nothing (\_ -> return ()) (Just sendBody)
statusHeaders <- parseStatusHeaders Nothing Nothing conn Nothing (\_ -> return ()) (Just sendBody)
statusHeaders `shouldBe` StatusHeaders status417 (HttpVersion 1 1) [] []
out >>= (`shouldBe` [])

@@ -59,7 +59,7 @@ spec = describe "HeadersSpec" $ do
, "result"
]
(conn, out, inp) <- dummyConnection input
statusHeaders <- parseStatusHeaders noMaxHeaderLength noMaxNumberHeaders conn Nothing (\_ -> return ()) Nothing
statusHeaders <- parseStatusHeaders Nothing Nothing conn Nothing (\_ -> return ()) Nothing
statusHeaders `shouldBe` StatusHeaders status200 (HttpVersion 1 1) [] [ ("foo", "bar") ]
out >>= (`shouldBe` [])
inp >>= (`shouldBe` ["result"])
@@ -78,7 +78,7 @@ spec = describe "HeadersSpec" $ do
callbackResults :: MVar (Seq.Seq [Header]) <- newMVar mempty
let onEarlyHintHeader h = modifyMVar_ callbackResults (return . (Seq.|> h))

statusHeaders <- parseStatusHeaders noMaxHeaderLength noMaxNumberHeaders conn Nothing onEarlyHintHeader Nothing
statusHeaders <- parseStatusHeaders Nothing Nothing conn Nothing onEarlyHintHeader Nothing
statusHeaders `shouldBe` StatusHeaders status200 (HttpVersion 1 1)
[("Link", "</foo.js>")
, ("Link", "</bar.js>")
@@ -110,7 +110,7 @@ spec = describe "HeadersSpec" $ do
callbackResults :: MVar (Seq.Seq [Header]) <- newMVar mempty
let onEarlyHintHeader h = modifyMVar_ callbackResults (return . (Seq.|> h))

statusHeaders <- parseStatusHeaders noMaxHeaderLength noMaxNumberHeaders conn Nothing onEarlyHintHeader Nothing
statusHeaders <- parseStatusHeaders Nothing Nothing conn Nothing onEarlyHintHeader Nothing
statusHeaders `shouldBe` StatusHeaders status200 (HttpVersion 1 1)
[("Link", "</foo.js>")
, ("Link", "</bar.js>")
2 changes: 1 addition & 1 deletion http-client/test-nonet/Network/HTTP/Client/ResponseSpec.hs
Original file line number Diff line number Diff line change
@@ -16,7 +16,7 @@ main = hspec spec

spec :: Spec
spec = describe "ResponseSpec" $ do
let getResponse' conn = getResponse noMaxHeaderLength noMaxNumberHeaders Nothing req (dummyManaged conn) Nothing
let getResponse' conn = getResponse Nothing Nothing Nothing req (dummyManaged conn) Nothing
req = parseRequest_ "http://localhost"
it "basic" $ do
(conn, _, _) <- dummyConnection
5 changes: 3 additions & 2 deletions http-conduit/test/main.hs
Original file line number Diff line number Diff line change
@@ -56,6 +56,7 @@ import qualified Data.Aeson as A
import qualified Network.HTTP.Simple as Simple
import Data.Monoid (mempty)
import Control.Monad.Trans.Resource (runResourceT)
import Data.Maybe (fromJust)

past :: UTCTime
past = UTCTime (ModifiedJulianDay 56200) (secondsToDiffTime 0)
@@ -542,15 +543,15 @@ tooManyHeaderFields :: (Int -> IO ()) -> IO ()
tooManyHeaderFields =
withCApp $ \app' -> runConduit $ src .| appSink app'
where
limit = fromEnum (managerMaxNumberHeaders defaultManagerSettings)
limit = fromEnum (fromJust $ managerMaxNumberHeaders defaultManagerSettings)
src = sourceList $ "HTTP/1.0 200 OK\r\n" : replicate limit "foo: bar\r\n"

notTooManyHeaderFields :: (Int -> IO ()) -> IO ()
notTooManyHeaderFields = withCApp $ \app' -> do
runConduit $ appSource app' .| CL.drop 1
runConduit $ src .| appSink app'
where
limit = fromEnum (managerMaxNumberHeaders defaultManagerSettings) - 1
limit = fromEnum (fromJust $ managerMaxNumberHeaders defaultManagerSettings) - 1
src = sourceList $ ["HTTP/1.0 200 OK\r\n"] <> replicate limit "foo: bar\r\n" <> ["\r\n"]

redir :: (Int -> IO ()) -> IO ()

0 comments on commit 749ed45

Please sign in to comment.