diff --git a/src/Chainweb/BlockHeaderDB/RestAPI/Server.hs b/src/Chainweb/BlockHeaderDB/RestAPI/Server.hs index d38ffc1031..819c019345 100644 --- a/src/Chainweb/BlockHeaderDB/RestAPI/Server.hs +++ b/src/Chainweb/BlockHeaderDB/RestAPI/Server.hs @@ -1,6 +1,8 @@ {-# LANGUAGE BangPatterns #-} {-# LANGUAGE DataKinds #-} +{-# LANGUAGE DerivingStrategies #-} {-# LANGUAGE FlexibleContexts #-} +{-# LANGUAGE GeneralizedNewtypeDeriving #-} {-# LANGUAGE LambdaCase #-} {-# LANGUAGE OverloadedStrings #-} {-# LANGUAGE ScopedTypeVariables #-} @@ -43,6 +45,7 @@ import Data.Functor.Of import Data.IORef import Data.Proxy import Data.Text.Encoding (decodeUtf8) +import Numeric.Natural(Natural) import Network.Wai.EventSource (ServerEvent(..), eventSourceAppIO) @@ -106,6 +109,14 @@ checkBounds checkBounds db b = b <$ traverse_ (checkKey db . _getUpperBound) (_branchBoundsUpper b) +err400Msg :: ToJSON msg => msg -> ServerError +err400Msg msg = ServerError + { errHTTPCode = 400 + , errReasonPhrase = "Bad request" + , errBody = encode msg + , errHeaders = [] + } + -- -------------------------------------------------------------------------- -- -- Handlers @@ -118,6 +129,21 @@ defaultEntryLimit = 360 p2pEntryLimit :: Num a => a p2pEntryLimit = 20 +newtype BranchBoundsLimit + = BranchBoundsLimit { getBranchBoundsLimit :: Natural } + deriving newtype (Show, Eq, Ord) + +-- | Default limit for the number of bounds in the request of a branch query +-- +defaultBoundsLimit :: BranchBoundsLimit +defaultBoundsLimit = BranchBoundsLimit 32 + +-- | Limit for the number of bounds in the request of a branch query on the P2P +-- API. +-- +p2pBoundsLimit :: BranchBoundsLimit +p2pBoundsLimit = BranchBoundsLimit 4 + -- | Query Branch Hashes of the database. -- -- Cf. "Chainweb.BlockHeaderDB.RestAPI" for more details @@ -132,14 +158,19 @@ branchHashesHandler -> Maybe MaxRank -> BranchBounds db -> Handler (Page (NextItem (DbKey db)) (DbKey db)) -branchHashesHandler db limit next minr maxr bounds = do - nextChecked <- traverse (traverse $ checkKey db) next - checkedBounds <- checkBounds db bounds - liftIO - $ branchKeys db nextChecked (succ <$> effectiveLimit) minr maxr - (_branchBoundsLower checkedBounds) - (_branchBoundsUpper checkedBounds) - $ finiteStreamToPage id effectiveLimit . void +branchHashesHandler db limit next minr maxr bounds + | fromIntegral (length (_branchBoundsUpper bounds)) > getBranchBoundsLimit defaultBoundsLimit = throwError $ err400Msg $ + "upper branch bound limit exceeded. Only " <> show defaultBoundsLimit <> " values are supported." + | fromIntegral (length (_branchBoundsLower bounds)) > getBranchBoundsLimit defaultBoundsLimit = throwError $ err400Msg $ + "lower branch bound limit exceeded. Only " <> show defaultBoundsLimit <> " values are supported." + | otherwise = do + nextChecked <- traverse (traverse $ checkKey db) next + checkedBounds <- checkBounds db bounds + liftIO + $ branchKeys db nextChecked (succ <$> effectiveLimit) minr maxr + (_branchBoundsLower checkedBounds) + (_branchBoundsUpper checkedBounds) + $ finiteStreamToPage id effectiveLimit . void where effectiveLimit = min defaultKeyLimit <$> (limit <|> Just defaultKeyLimit) @@ -151,6 +182,7 @@ branchHeadersHandler :: TreeDb db => ToJSON (DbKey db) => db + -> BranchBoundsLimit -> Limit -- ^ max limit -> Maybe Limit @@ -159,14 +191,19 @@ branchHeadersHandler -> Maybe MaxRank -> BranchBounds db -> Handler (Page (NextItem (DbKey db)) (DbEntry db)) -branchHeadersHandler db maxLimit limit next minr maxr bounds = do - nextChecked <- traverse (traverse $ checkKey db) next - checkedBounds <- checkBounds db bounds - liftIO - $ branchEntries db nextChecked (succ <$> effectiveLimit) minr maxr - (_branchBoundsLower checkedBounds) - (_branchBoundsUpper checkedBounds) - $ finiteStreamToPage key effectiveLimit . void +branchHeadersHandler db (BranchBoundsLimit boundsLimit) maxLimit limit next minr maxr bounds + | fromIntegral (length (_branchBoundsUpper bounds)) > boundsLimit = throwError $ err400Msg $ + "upper branch bound limit exceeded. Only " <> show boundsLimit <> " values are supported." + | fromIntegral (length (_branchBoundsLower bounds)) > boundsLimit = throwError $ err400Msg $ + "lower branch bound limit exceeded. Only " <> show boundsLimit <> " values are supported." + | otherwise = do + nextChecked <- traverse (traverse $ checkKey db) next + checkedBounds <- checkBounds db bounds + liftIO + $ branchEntries db nextChecked (succ <$> effectiveLimit) minr maxr + (_branchBoundsLower checkedBounds) + (_branchBoundsUpper checkedBounds) + $ finiteStreamToPage key effectiveLimit . void where effectiveLimit = min maxLimit <$> (limit <|> Just maxLimit) @@ -243,7 +280,7 @@ blockHeaderDbServer (BlockHeaderDb_ db) :<|> headersHandler db defaultEntryLimit :<|> headerHandler db :<|> branchHashesHandler db - :<|> branchHeadersHandler db defaultEntryLimit + :<|> branchHeadersHandler db defaultBoundsLimit defaultEntryLimit -- Restricted P2P BlockHeader DB API -- @@ -251,7 +288,7 @@ p2pBlockHeaderDbServer :: BlockHeaderDb_ v c -> Server (P2pBlockHeaderDbApi v c) p2pBlockHeaderDbServer (BlockHeaderDb_ db) = headersHandler db p2pEntryLimit :<|> headerHandler db - :<|> branchHeadersHandler db p2pEntryLimit + :<|> branchHeadersHandler db p2pBoundsLimit p2pEntryLimit -- -------------------------------------------------------------------------- -- -- Multichain Server @@ -284,7 +321,7 @@ headerStreamServer . CanReadablePayloadCas tbl => CutDb tbl -> Server (HeaderStreamApi v) -headerStreamServer cdb = headerStreamHandler cdb +headerStreamServer = headerStreamHandler headerStreamHandler :: forall tbl. CanReadablePayloadCas tbl => CutDb tbl -> Tagged Handler Application headerStreamHandler db = Tagged $ \req resp -> do diff --git a/src/Chainweb/Chainweb.hs b/src/Chainweb/Chainweb.hs index 57d38600f9..953f753bc6 100644 --- a/src/Chainweb/Chainweb.hs +++ b/src/Chainweb/Chainweb.hs @@ -668,7 +668,7 @@ runChainweb cw = do . throttle (_chainwebPutPeerThrottler cw) . throttle (_chainwebMempoolThrottler cw) . throttle (_chainwebThrottler cw) - . requestSizeLimit + . p2pRequestSizeLimit . p2pValidationMiddleware -- 2. Start Clients (with a delay of 500ms) @@ -678,7 +678,7 @@ runChainweb cw = do , threadDelay 500000 >> do serveServiceApi $ serviceHttpLog - . requestSizeLimit + . serviceRequestSizeLimit . serviceApiValidationMiddleware ] @@ -786,11 +786,26 @@ runChainweb cw = do mw) (monitorConnectionsClosedByClient clientClosedConnectionsCounter) - requestSizeLimit :: Middleware - requestSizeLimit = requestSizeLimitMiddleware $ + -- Request size limit for the service API + -- + serviceRequestSizeLimit :: Middleware + serviceRequestSizeLimit = requestSizeLimitMiddleware $ setMaxLengthForRequest (\_req -> pure $ Just $ 2 * 1024 * 1024) -- 2MB defaultRequestSizeLimitSettings + -- Request size limit for the P2P API + -- + -- NOTE: this may need to have to be adjusted if the p2p limits for batch + -- sizes or number of branch bound change. It may also need adjustment for + -- other protocol changes, like additional HTTP request headers or changes + -- in the mempool protocol. + -- + -- FIXME: can we make this smaller and still let the mempool work? + -- + p2pRequestSizeLimit :: Middleware + p2pRequestSizeLimit = requestSizeLimitMiddleware $ + setMaxLengthForRequest (\_req -> pure $ Just $ 2 * 1024 * 1024) -- 2MB + defaultRequestSizeLimitSettings httpLog :: Middleware httpLog = requestResponseLogger $ setComponent "http:p2p-api" (_chainwebLogger cw) diff --git a/test/Chainweb/Test/RestAPI.hs b/test/Chainweb/Test/RestAPI.hs index 3e553622bc..5512347870 100644 --- a/test/Chainweb/Test/RestAPI.hs +++ b/test/Chainweb/Test/RestAPI.hs @@ -2,6 +2,9 @@ {-# LANGUAGE FlexibleContexts #-} {-# LANGUAGE OverloadedStrings #-} {-# LANGUAGE PatternSynonyms #-} +{-# LANGUAGE ScopedTypeVariables #-} + +{-# options_ghc -fno-warn-unused-local-binds -fno-warn-unused-imports #-} -- | -- Module: Chainweb.Test.RestAPI @@ -19,6 +22,7 @@ module Chainweb.Test.RestAPI import Control.Monad import Control.Monad.IO.Class +import Data.Bifunctor (first) import qualified Data.ByteString.Char8 as B8 import Data.Either import Data.Foldable @@ -40,6 +44,7 @@ import Text.Read (readEither) -- internal modules +import Chainweb.BlockHash (BlockHash) import Chainweb.BlockHeader import Chainweb.BlockHeaderDB import Chainweb.BlockHeaderDB.Internal (unsafeInsertBlockHeaderDb) @@ -48,6 +53,7 @@ import Chainweb.Graph import Chainweb.Mempool.Mempool (MempoolBackend, MockTx) import Chainweb.RestAPI import Chainweb.Test.RestAPI.Client_ +import Chainweb.Test.RestAPI.Utils (isFailureResponse, clientErrorStatusCode) import Chainweb.Test.Utils import Chainweb.Test.Utils.BlockHeader import Chainweb.Test.TestVersions (barebonesTestVersion) @@ -168,6 +174,7 @@ simpleClientSession envIO cid = assertBool ("test failed: " <> sshow res) (isRight res) where + session :: [(ChainId, BlockHeaderDb)] -> (String -> IO a) -> ClientM () session dbs step = do let gbh0 = genesisBlockHeader version cid @@ -231,6 +238,60 @@ simpleClientSession envIO cid = -- branchHeaders + do + void $ liftIO $ step "branchHeadersClient: BranchBounds limits exceeded" + clientEnv <- liftIO $ do + -- ClientM does not have a MonadFail instance for this failable + -- pattern match; IO does. + BlockHeaderDbsTestClientEnv clientEnv _ _ <- envIO + pure clientEnv + let query bounds = liftIO + $ flip runClientM clientEnv + $ branchHeadersClient + version cid Nothing Nothing Nothing Nothing bounds + let limit = 32 + let blockHeaders = testBlockHeaders (ParentHeader gbh0) + let maxBlockHeaders = take limit blockHeaders + let excessBlockHeaders = take (limit + 1) blockHeaders + + let mkLower :: [BlockHeader] -> HS.HashSet (LowerBound BlockHash) + mkLower hs = HS.fromList $ map (LowerBound . key) hs + let mkUpper :: [BlockHeader] -> HS.HashSet (UpperBound BlockHash) + mkUpper hs = HS.fromList $ map (UpperBound . key) hs + + let emptyLower = mkLower [] + let badLower = mkLower excessBlockHeaders + let goodLower = mkLower maxBlockHeaders + + let emptyUpper = mkUpper [] + let badUpper = mkUpper excessBlockHeaders + let goodUpper = mkUpper maxBlockHeaders + + let badRespCheck :: Int -> ClientError -> Bool + badRespCheck s e = isFailureResponse e && clientErrorStatusCode e == Just s + + badLowerResponse <- query (BranchBounds badLower emptyUpper) + assertExpectation "branchHeadersClient returned a 400 error code on excess lower" + (Expected (Left True)) + (Actual (first (badRespCheck 400) badLowerResponse)) + + badUpperResponse <- query (BranchBounds emptyLower badUpper) + assertExpectation "branchHeadersClient returned a 400 error code on excess upper" + (Expected (Left True)) + (Actual (first (badRespCheck 400) badUpperResponse)) + + -- This will still fail because a bunch of these keys won't be found, + -- but it won't fail the bounds check, which happens first + doesntFailBoundsCheck <- query (BranchBounds goodLower goodUpper) + assertExpectation "branchHeadersClient returned a 404; bounds were within the limits, still fails key exists check" + (Expected (Left True)) + (Actual (first (badRespCheck 404) doesntFailBoundsCheck)) + + doesntFailAtAll <- query (BranchBounds emptyLower emptyUpper) + assertExpectation "branchHeadersClient returned a 200; bounds were within the limits, and no keys to check at all" + (Expected (Right ())) + (Actual (() <$ doesntFailAtAll)) + void $ liftIO $ step "branchHeadersClient: get no block headers" bhs3 <- branchHeadersClient version cid Nothing Nothing Nothing Nothing (BranchBounds mempty mempty) @@ -320,10 +381,13 @@ simpleClientSession envIO cid = void $ liftIO $ step "branchHashesClient: get one block headers with lower and upper bound" hs5 <- branchHashesClient version cid Nothing Nothing Nothing Nothing (BranchBounds (HS.singleton (LowerBound $ key lower)) (HS.singleton (UpperBound $ key h))) + liftIO $ print hs5 assertExpectation "branchHashesClient returned wrong number of entries" (Expected i) (Actual $ _pageLimit hs5) + + -- -------------------------------------------------------------------------- -- -- Paging Tests diff --git a/test/Chainweb/Test/RestAPI/Utils.hs b/test/Chainweb/Test/RestAPI/Utils.hs index ae84c21949..1d3260e14b 100644 --- a/test/Chainweb/Test/RestAPI/Utils.hs +++ b/test/Chainweb/Test/RestAPI/Utils.hs @@ -9,6 +9,9 @@ module Chainweb.Test.RestAPI.Utils -- * Utils , repeatUntil +, clientErrorStatusCode +, isFailureResponse +, getStatusCode -- * Pact client DSL , PactTestFailure(..) @@ -53,6 +56,7 @@ import Data.Text (Text) import Data.Maybe (fromJust) import qualified Data.HashMap.Strict as HM import qualified Data.Text as T +import Network.HTTP.Types.Status (Status(..)) import Rosetta @@ -600,3 +604,19 @@ networkStatus cenv req = h _ = Handler $ \case NetworkStatusFailure _ -> return True _ -> return False + +clientErrorStatusCode :: ClientError -> Maybe Int +clientErrorStatusCode = \case + FailureResponse _ resp -> Just $ getStatusCode resp + DecodeFailure _ resp -> Just $ getStatusCode resp + UnsupportedContentType _ resp -> Just $ getStatusCode resp + InvalidContentTypeHeader resp -> Just $ getStatusCode resp + ConnectionError _ -> Nothing + +isFailureResponse :: ClientError -> Bool +isFailureResponse = \case + FailureResponse {} -> True + _ -> False + +getStatusCode :: ResponseF a -> Int +getStatusCode resp = statusCode (responseStatusCode resp)