From 749ed45c5a10af78b581752ee46e0a3a6f7c4858 Mon Sep 17 00:00:00 2001 From: Giorgio Marinelli Date: Mon, 16 Dec 2024 19:05:54 +0100 Subject: [PATCH] Revert "Use Word in MaxHeaderLength and MaxNumberHeaders" This reverts commit c3312b42871d1209b234df90c396aef6ab9e1d31. --- http-client/Network/HTTP/Client.hs | 4 ++-- http-client/Network/HTTP/Client/Body.hs | 2 +- http-client/Network/HTTP/Client/Connection.hs | 12 ++++++------ http-client/Network/HTTP/Client/Headers.hs | 13 ++++++------- http-client/Network/HTTP/Client/Manager.hs | 4 ++-- http-client/Network/HTTP/Client/Response.hs | 4 ++-- http-client/Network/HTTP/Client/Types.hs | 16 ++++------------ .../test-nonet/Network/HTTP/Client/BodySpec.hs | 16 ++++++++-------- .../Network/HTTP/Client/HeadersSpec.hs | 12 ++++++------ .../Network/HTTP/Client/ResponseSpec.hs | 2 +- http-conduit/test/main.hs | 5 +++-- 11 files changed, 41 insertions(+), 49 deletions(-) diff --git a/http-client/Network/HTTP/Client.hs b/http-client/Network/HTTP/Client.hs index d5cd9fe9..f0dbbc17 100644 --- a/http-client/Network/HTTP/Client.hs +++ b/http-client/Network/HTTP/Client.hs @@ -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 diff --git a/http-client/Network/HTTP/Client/Body.hs b/http-client/Network/HTTP/Client/Body.hs index 09c6bc40..a44834cb 100644 --- a/http-client/Network/HTTP/Client/Body.hs +++ b/http-client/Network/HTTP/Client/Body.hs @@ -148,7 +148,7 @@ makeLengthReader cleanup count0 Connection {..} = do return bs makeChunkedReader - :: MaxHeaderLength + :: Maybe MaxHeaderLength -> IO () -- ^ cleanup -> Bool -- ^ raw -> Connection diff --git a/http-client/Network/HTTP/Client/Connection.hs b/http-client/Network/HTTP/Client/Connection.hs index 26db9638..a57da553 100644 --- a/http-client/Network/HTTP/Client/Connection.hs +++ b/http-client/Network/HTTP/Client/Connection.hs @@ -31,19 +31,19 @@ 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 @@ -51,9 +51,9 @@ connectionReadLineWith mhl conn bs0 = 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' diff --git a/http-client/Network/HTTP/Client/Headers.hs b/http-client/Network/HTTP/Client/Headers.hs index d1ba2bfe..f901702d 100644 --- a/http-client/Network/HTTP/Client/Headers.hs +++ b/http-client/Network/HTTP/Client/Headers.hs @@ -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,7 +60,7 @@ 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. @@ -68,7 +68,7 @@ parseStatusHeaders mhl mnh conn timeout' onEarlyHintHeaders cont 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 diff --git a/http-client/Network/HTTP/Client/Manager.hs b/http-client/Network/HTTP/Client/Manager.hs index abe92ff6..6d0de348 100644 --- a/http-client/Network/HTTP/Client/Manager.hs +++ b/http-client/Network/HTTP/Client/Manager.hs @@ -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 diff --git a/http-client/Network/HTTP/Client/Response.hs b/http-client/Network/HTTP/Client/Response.hs index 87de0298..52c3eb58 100644 --- a/http-client/Network/HTTP/Client/Response.hs +++ b/http-client/Network/HTTP/Client/Response.hs @@ -113,8 +113,8 @@ lbsResponse res = do { responseBody = L.fromChunks bss } -getResponse :: MaxHeaderLength - -> MaxNumberHeaders +getResponse :: Maybe MaxHeaderLength + -> Maybe MaxNumberHeaders -> Maybe Int -> Request -> Managed Connection diff --git a/http-client/Network/HTTP/Client/Types.hs b/http-client/Network/HTTP/Client/Types.hs index 3fb1d020..6647ed19 100644 --- a/http-client/Network/HTTP/Client/Types.hs +++ b/http-client/Network/HTTP/Client/Types.hs @@ -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,9 +924,6 @@ 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 @@ -936,6 +931,3 @@ newtype MaxNumberHeaders = MaxNumberHeaders { unMaxNumberHeaders :: Int } deriving (Eq, Show, Ord, Num, Enum, Bounded, T.Typeable) - -noMaxNumberHeaders :: MaxNumberHeaders -noMaxNumberHeaders = maxBound diff --git a/http-client/test-nonet/Network/HTTP/Client/BodySpec.hs b/http-client/test-nonet/Network/HTTP/Client/BodySpec.hs index 5bd28293..a2f2e613 100644 --- a/http-client/test-nonet/Network/HTTP/Client/BodySpec.hs +++ b/http-client/test-nonet/Network/HTTP/Client/BodySpec.hs @@ -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 diff --git a/http-client/test-nonet/Network/HTTP/Client/HeadersSpec.hs b/http-client/test-nonet/Network/HTTP/Client/HeadersSpec.hs index 955505d3..eb0b04c1 100644 --- a/http-client/test-nonet/Network/HTTP/Client/HeadersSpec.hs +++ b/http-client/test-nonet/Network/HTTP/Client/HeadersSpec.hs @@ -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", "") , ("Link", "") @@ -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", "") , ("Link", "") diff --git a/http-client/test-nonet/Network/HTTP/Client/ResponseSpec.hs b/http-client/test-nonet/Network/HTTP/Client/ResponseSpec.hs index 3728fd4f..cdb8d8ec 100644 --- a/http-client/test-nonet/Network/HTTP/Client/ResponseSpec.hs +++ b/http-client/test-nonet/Network/HTTP/Client/ResponseSpec.hs @@ -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 diff --git a/http-conduit/test/main.hs b/http-conduit/test/main.hs index 43144ee2..2a760748 100644 --- a/http-conduit/test/main.hs +++ b/http-conduit/test/main.hs @@ -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,7 +543,7 @@ 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 () @@ -550,7 +551,7 @@ 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 ()