Skip to content
This repository has been archived by the owner on Aug 18, 2020. It is now read-only.

[CO-370] Make Content-Type parsing more lenient #3596

Merged
merged 5 commits into from
Sep 15, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,9 @@

- Small refactor of wallet Errors implementation to be more maintainable (CBR-26)

- Content-Type parser is now more lenient and accepts `application/json`, `application/json;charset=utf-8` and
no Content-Type at all (defaulting to `application/json`).

### Specifications

### Documentation
Expand Down
2 changes: 0 additions & 2 deletions pkgs/default.nix
Original file line number Diff line number Diff line change
Expand Up @@ -17759,7 +17759,6 @@ license = stdenv.lib.licenses.mit;
, unordered-containers
, vector
, wai
, wai-cors
, wai-middleware-throttle
, warp
, x509
Expand Down Expand Up @@ -17861,7 +17860,6 @@ unliftio-core
unordered-containers
vector
wai
wai-cors
wai-middleware-throttle
warp
x509
Expand Down
2 changes: 1 addition & 1 deletion wallet-new/cardano-sl-wallet-new.cabal
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@ library
Cardano.Wallet.Orphans.Bi
Cardano.Wallet.Server
Cardano.Wallet.Server.CLI
Cardano.Wallet.Server.Middlewares
Cardano.Wallet.Server.Plugins
Cardano.Wallet.Server.Plugins.AcidState
Cardano.Wallet.Server.LegacyPlugins
Expand Down Expand Up @@ -245,7 +246,6 @@ library
, unordered-containers
, vector
, wai
, wai-cors
, wai-middleware-throttle
, warp
, x509
Expand Down
20 changes: 16 additions & 4 deletions wallet-new/server/Main.hs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import Pos.DB.DB (initNodeDBs)
import Pos.DB.Txp (txpGlobalSettings)
import Pos.Infra.Diffusion.Types (Diffusion)
import Pos.Launcher (NodeParams (..), NodeResources (..),
WalletConfiguration, bpLoggingParams,
WalletConfiguration (..), bpLoggingParams,
bracketNodeResources, loggerBracket, lpDefaultName,
runNode, withConfigurations)
import Pos.Launcher.Configuration (AssetLockPath (..),
Expand All @@ -39,17 +39,20 @@ import Pos.Wallet.Web.Tracking.Sync (syncWallet)

import qualified Cardano.Wallet.Kernel.Mode as Kernel.Mode

import qualified Cardano.Wallet.API.V1.Headers as Headers
import Cardano.Wallet.Kernel (PassiveWallet)
import qualified Cardano.Wallet.Kernel as Kernel
import qualified Cardano.Wallet.Kernel.Internal as Kernel.Internal
import qualified Cardano.Wallet.Kernel.Keystore as Keystore
import qualified Cardano.Wallet.Kernel.NodeStateAdaptor as NodeStateAdaptor
import Cardano.Wallet.Server.CLI (ChooseWalletBackend (..),
NewWalletBackendParams (..), WalletBackendParams (..),
NewWalletBackendParams, WalletBackendParams (..),
WalletStartupOptions (..), getWalletDbOptions,
getWalletNodeOptions, walletDbPath, walletFlushDb,
walletRebuildDb)
import qualified Cardano.Wallet.Server.LegacyPlugins as LegacyPlugins
import Cardano.Wallet.Server.Middlewares (throttleMiddleware,
withDefaultHeader)
import qualified Cardano.Wallet.Server.Plugins as Plugins
import Cardano.Wallet.WalletLayer (PassiveWalletLayer)
import qualified Cardano.Wallet.WalletLayer.Kernel as WalletLayer.Kernel
Expand Down Expand Up @@ -106,7 +109,10 @@ actionWithLegacyWallet genesisConfig walletConfig txpConfig sscParams nodeParams
plugins :: TVar NtpStatus -> LegacyPlugins.Plugin WalletWebMode
plugins ntpStatus =
mconcat [ LegacyPlugins.conversation wArgs
, LegacyPlugins.legacyWalletBackend genesisConfig walletConfig txpConfig wArgs ntpStatus
, LegacyPlugins.legacyWalletBackend genesisConfig txpConfig wArgs ntpStatus
[ throttleMiddleware (ccThrottle walletConfig)
, withDefaultHeader Headers.applicationJson
]
, LegacyPlugins.walletDocumentation wArgs
, LegacyPlugins.acidCleanupWorker wArgs
, LegacyPlugins.syncWalletWorker genesisConfig
Expand All @@ -117,13 +123,14 @@ actionWithLegacyWallet genesisConfig walletConfig txpConfig sscParams nodeParams
-- | The "workhorse" responsible for starting a Cardano edge node plus a number of extra plugins.
actionWithWallet :: (HasConfigurations, HasCompileInfo)
=> Genesis.Config
-> WalletConfiguration
-> TxpConfiguration
-> SscParams
-> NodeParams
-> NtpConfiguration
-> NewWalletBackendParams
-> IO ()
actionWithWallet genesisConfig txpConfig sscParams nodeParams ntpConfig params =
actionWithWallet genesisConfig walletConfig txpConfig sscParams nodeParams ntpConfig params =
bracketNodeResources
genesisConfig
nodeParams
Expand Down Expand Up @@ -171,6 +178,10 @@ actionWithWallet genesisConfig txpConfig sscParams nodeParams ntpConfig params =
plugins w dbMode = mconcat
-- The actual wallet backend server.
[ Plugins.apiServer pm params w
-- Throttle requests.
[ throttleMiddleware (ccThrottle walletConfig)
, withDefaultHeader Headers.applicationJson
]

-- The corresponding wallet documention, served as a different
-- server which doesn't require client x509 certificates to
Expand Down Expand Up @@ -217,6 +228,7 @@ startEdgeNode wso =
legacyParams
WalletNew newParams -> actionWithWallet
genesisConfig
walletConfig
txpConfig
sscParams
nodeParams
Expand Down
2 changes: 1 addition & 1 deletion wallet-new/src/Cardano/Wallet/API/Response.hs
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ instance FromJSON a => MimeUnrender ValidJSON a where
Right v -> return v

instance Accept ValidJSON where
contentType _ = contentType (Proxy @ JSON)
contentTypes _ = contentTypes (Proxy @ JSON)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This simple change already makes the parser accept both application/json and application/json;charset=utf-8.

Actually, contentType is defined using the head of what's returned by contentTypes, which in the case of JSON is application/json;charset=utf-8 !

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@KtorZ Interesting, good catch! I am wondering if this change, alone, suffice to make the content type lenient or if we do need the extra middleware machinery at all? 🤔

Copy link
Contributor Author

@KtorZ KtorZ Sep 14, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This alone covers the requirements 1, 2 and 4 from CO-369

The middleware machinery covers the requirement 3.


instance ToJSON a => MimeRender ValidJSON a where
mimeRender _ = mimeRender (Proxy @ JSON)
Expand Down
60 changes: 8 additions & 52 deletions wallet-new/src/Cardano/Wallet/Server/LegacyPlugins.hs
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,9 @@ import Cardano.Wallet.API as API
import Cardano.Wallet.API.V1.Headers (applicationJson)
import qualified Cardano.Wallet.API.V1.Types as V1
import qualified Cardano.Wallet.LegacyServer as LegacyServer
import Cardano.Wallet.Server.CLI (RunMode, WalletBackendParams (..),
import Cardano.Wallet.Server.CLI (WalletBackendParams (..),
isDebugMode, walletAcidInterval, walletDbOptions)
import Cardano.Wallet.Server.Middlewares (withMiddlewares)
import qualified Pos.Wallet.Web.Error.Types as V0

import Control.Exception (fromException)
Expand All @@ -32,10 +33,6 @@ import Network.HTTP.Types.Status (badRequest400)
import Network.Wai (Application, Middleware, Response, responseLBS)
import Network.Wai.Handler.Warp (defaultSettings,
setOnExceptionResponse)
import Network.Wai.Middleware.Cors (cors, corsMethods,
corsRequestHeaders, simpleCorsResourcePolicy,
simpleMethods)
import qualified Network.Wai.Middleware.Throttle as Throttle
import Ntp.Client (NtpStatus)
import Pos.Chain.Txp (TxpConfiguration)
import Pos.Infra.Diffusion.Types (Diffusion (..))
Expand All @@ -56,8 +53,7 @@ import Cardano.NodeIPC (startNodeJsIPC)
import Pos.Configuration (walletProductionApi,
walletTxCreationDisabled)
import Pos.Infra.Shutdown.Class (HasShutdownContext (shutdownContext))
import Pos.Launcher.Configuration (HasConfigurations,
ThrottleSettings (..), WalletConfiguration (..))
import Pos.Launcher.Configuration (HasConfigurations)
import Pos.Util.CompileInfo (HasCompileInfo)
import Pos.Wallet.Web.Mode (WalletWebMode)
import Pos.Wallet.Web.Server.Launcher (walletDocumentationImpl,
Expand Down Expand Up @@ -105,19 +101,19 @@ walletDocumentation WalletBackendParams {..} = pure $ \_ ->
application :: WalletWebMode Application
application = do
let app = Servant.serve API.walletDocAPI LegacyServer.walletDocServer
withMiddleware (WalletConfiguration Nothing) walletRunMode app
return $ withMiddlewares [] app
tls =
if isDebugMode walletRunMode then Nothing else walletTLSParams

-- | A @Plugin@ to start the wallet backend API.
legacyWalletBackend :: (HasConfigurations, HasCompileInfo)
=> Genesis.Config
-> WalletConfiguration
-> TxpConfiguration
-> WalletBackendParams
-> TVar NtpStatus
-> [Middleware]
-> Plugin WalletWebMode
legacyWalletBackend genesisConfig walletConfig txpConfig WalletBackendParams {..} ntpStatus = pure $ \diffusion -> do
legacyWalletBackend genesisConfig txpConfig WalletBackendParams {..} ntpStatus middlewares = pure $ \diffusion -> do
modifyLoggerName (const "legacyServantBackend") $ do
logWarning $ sformat "RUNNING THE OLD LEGACY DATA LAYER IS NOT RECOMMENDED!"
logInfo $ sformat ("Production mode for API: "%build)
Expand All @@ -143,7 +139,8 @@ legacyWalletBackend genesisConfig walletConfig txpConfig WalletBackendParams {..
logInfo "Wallet Web API has STARTED!"
wsConn <- getWalletWebSockets
ctx <- V0.walletWebModeContext
withMiddleware walletConfig walletRunMode
return
$ withMiddlewares middlewares
$ upgradeApplicationWS wsConn
$ Servant.serve API.walletAPI
$ LegacyServer.walletServer
Expand Down Expand Up @@ -212,44 +209,3 @@ syncWalletWorker :: Genesis.Config -> Plugin WalletWebMode
syncWalletWorker genesisConfig = pure $ const $
modifyLoggerName (const "syncWalletWorker") $
(view (lensOf @SyncQueue) >>= processSyncRequest genesisConfig)

corsMiddleware :: Middleware
corsMiddleware = cors (const $ Just policy)
where
policy = simpleCorsResourcePolicy
{ corsRequestHeaders = ["Content-Type"]
, corsMethods = "PUT" : simpleMethods
}


-- | "Attaches" the middleware to this 'Application', if any. When running in
-- debug mode, chances are we want to at least allow CORS to test the API with
-- a Swagger editor, locally.
withMiddleware
:: MonadIO m
=> WalletConfiguration
-> RunMode
-> Application
-> m Application
withMiddleware walletConfiguration wrm app = do
throttling <- case ccThrottle walletConfiguration of
Nothing ->
pure identity
Just ts -> do
throttler <- liftIO $ Throttle.initThrottler
pure (Throttle.throttle (throttleSettings ts) throttler)
pure
. (if isDebugMode wrm then corsMiddleware else identity)
. throttling
$ app
where
throttleSettings ts = Throttle.defaultThrottleSettings
{ Throttle.onThrottled = \microsTilRetry ->
let
err = V1.RequestThrottled microsTilRetry
in
responseLBS (V1.toHttpErrorStatus err) [applicationJson] (encode err)
, Throttle.throttleRate = fromIntegral $ tsRate ts
, Throttle.throttlePeriod = fromIntegral $ tsPeriod ts
, Throttle.throttleBurst = fromIntegral $ tsBurst ts
}
69 changes: 69 additions & 0 deletions wallet-new/src/Cardano/Wallet/Server/Middlewares.hs
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
{- | A collection of middlewares used by this edge node.
@Middleware@ is a component that sits between the server and application.
It can do such tasks as GZIP encoding or response caching.
-}

module Cardano.Wallet.Server.Middlewares
( withMiddlewares
, throttleMiddleware
, withDefaultHeader
) where

import Universum

import Data.Aeson (encode)
import qualified Data.List as List
import Network.HTTP.Types.Header (Header)
import Network.HTTP.Types.Method (methodPatch, methodPost, methodPut)
import Network.Wai (Application, Middleware, ifRequest,
requestHeaders, requestMethod, responseLBS)
import qualified Network.Wai.Middleware.Throttle as Throttle

import Cardano.Wallet.API.V1.Headers (applicationJson)
import qualified Cardano.Wallet.API.V1.Types as V1

import Pos.Launcher.Configuration (ThrottleSettings (..))


-- | "Attaches" the middlewares to this 'Application'.
withMiddlewares :: [Middleware] -> Application -> Application
withMiddlewares = flip $ foldr ($)

-- | Only apply a @Middleware@ to request with bodies (we don't consider
-- "DELETE" as one of them).
ifRequestWithBody :: Middleware -> Middleware
ifRequestWithBody =
ifRequest ((`List.elem` [methodPost, methodPut, methodPatch]) . requestMethod)

-- | A @Middleware@ to throttle requests.
throttleMiddleware :: Maybe ThrottleSettings -> Middleware
throttleMiddleware Nothing app = app
throttleMiddleware (Just ts) app = \req respond -> do
throttler <- Throttle.initThrottler
Throttle.throttle throttleSettings throttler app req respond
where
throttleSettings = Throttle.defaultThrottleSettings
{ Throttle.onThrottled = \microsTilRetry ->
let
err = V1.RequestThrottled microsTilRetry
in
responseLBS (V1.toHttpErrorStatus err) [applicationJson] (encode err)
, Throttle.throttleRate = fromIntegral $ tsRate ts
, Throttle.throttlePeriod = fromIntegral $ tsPeriod ts
, Throttle.throttleBurst = fromIntegral $ tsBurst ts
}

-- | A @Middleware@ to default a specific Header when not provided
withDefaultHeader :: Header -> Middleware
withDefaultHeader header = ifRequestWithBody $ \app req send ->
let
headers =
requestHeaders req

req' =
if any (on (==) fst header) headers then
req
else
req { requestHeaders = header : headers }
in
app req' send
40 changes: 11 additions & 29 deletions wallet-new/src/Cardano/Wallet/Server/Plugins.hs
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,12 @@ import Data.Aeson (encode)
import qualified Data.ByteString.Char8 as BS8
import qualified Data.Text as T
import Data.Typeable (typeOf)
import qualified Servant

import Network.HTTP.Types.Status (badRequest400)
import Network.Wai (Application, Middleware, Response, responseLBS)
import Network.Wai.Handler.Warp (defaultSettings,
setOnExceptionResponse)
import Network.Wai.Middleware.Cors (cors, corsMethods,
corsRequestHeaders, simpleCorsResourcePolicy,
simpleMethods)

import Cardano.NodeIPC (startNodeJsIPC)
import Cardano.Wallet.API as API
Expand All @@ -39,8 +37,9 @@ import qualified Cardano.Wallet.Kernel.Diffusion as Kernel
import qualified Cardano.Wallet.Kernel.Mode as Kernel
import qualified Cardano.Wallet.Server as Server
import Cardano.Wallet.Server.CLI (NewWalletBackendParams (..),
RunMode, WalletBackendParams (..), getWalletDbOptions,
isDebugMode, walletAcidInterval)
WalletBackendParams (..), getWalletDbOptions, isDebugMode,
walletAcidInterval)
import Cardano.Wallet.Server.Middlewares (withMiddlewares)
import Cardano.Wallet.Server.Plugins.AcidState
(createAndArchiveCheckpoints)
import Cardano.Wallet.WalletLayer (ActiveWalletLayer,
Expand All @@ -59,9 +58,6 @@ import Pos.Util.Wlog (logInfo, modifyLoggerName, usingLoggerName)
import Pos.Web (serveDocImpl, serveImpl)
import qualified Pos.Web.Server

import qualified Servant


-- Needed for Orphan Instance 'Buildable Servant.NoContent' :|
import Pos.Wallet.Web ()

Expand All @@ -75,8 +71,9 @@ apiServer
:: ProtocolMagic
-> NewWalletBackendParams
-> (PassiveWalletLayer IO, PassiveWallet)
-> [Middleware]
-> Plugin Kernel.WalletMode
apiServer protocolMagic (NewWalletBackendParams WalletBackendParams{..}) (passiveLayer, passiveWallet) =
apiServer protocolMagic (NewWalletBackendParams WalletBackendParams{..}) (passiveLayer, passiveWallet) middlewares =
pure $ \diffusion -> do
env <- ask
let diffusion' = Kernel.fromDiffusion (lower env) diffusion
Expand Down Expand Up @@ -113,9 +110,11 @@ apiServer protocolMagic (NewWalletBackendParams WalletBackendParams{..}) (passiv

getApplication :: ActiveWalletLayer IO -> Kernel.WalletMode Application
getApplication active = do
logInfo "New wallet API has STARTED!"
return $ withMiddleware walletRunMode $
Servant.serve API.newWalletAPI $ Server.walletServer active walletRunMode
logInfo "New wallet API has STARTED!"
return
$ withMiddlewares middlewares
$ Servant.serve API.newWalletAPI
$ Server.walletServer active walletRunMode

lower :: env -> ReaderT env IO a -> IO a
lower env m = runReaderT m env
Expand All @@ -124,23 +123,6 @@ apiServer protocolMagic (NewWalletBackendParams WalletBackendParams{..}) (passiv
portCallback ctx =
usingLoggerName "NodeIPC" . flip runReaderT ctx . startNodeJsIPC

-- | "Attaches" the middleware to this 'Application', if any.
-- When running in debug mode, chances are we want to at least allow CORS to test the API
-- with a Swagger editor, locally.
withMiddleware :: RunMode -> Application -> Application
withMiddleware wrm app
| isDebugMode wrm = corsMiddleware app
| otherwise = app

corsMiddleware :: Middleware
corsMiddleware = cors (const $ Just policy)
where
policy = simpleCorsResourcePolicy
{ corsRequestHeaders = ["Content-Type"]
, corsMethods = "PUT" : simpleMethods
}


-- | A @Plugin@ to serve the wallet documentation
docServer
:: (HasConfigurations, HasCompileInfo)
Expand Down