pqi-native 1.0.1.5 → 1.0.1.6
raw patch · 4 files changed
+122/−37 lines, 4 filesdep ~pqi-conformancePVP ok
version bump matches the API change (PVP)
Dependency ranges changed: pqi-conformance
API changes (from Hackage documentation)
Files
- CHANGELOG.md +8/−0
- pqi-native.cabal +2/−2
- src/library/Pqi/Native/Connection.hs +66/−20
- src/library/Pqi/Native/Query.hs +46/−15
CHANGELOG.md view
@@ -1,3 +1,11 @@+# v1.0.1.6++## Fixes++- Fixed a mid-handshake server rejection (e.g. "sorry, too many clients already") escaping as an uncaught `IOException` instead of coming back as a classified `ConnectionBad` (#8). `establish` only wrapped the initial TCP connect in an exception handler; the handshake read that follows it is now caught too and routed through the same `tcpFailureMessage`/`unixSocketFailureMessage` wrapper `failWith` uses, with libpq's own wording for the EOF case. `tcpFailureMessage` also stopped adding a redundant "(ip)" parenthetical when the resolved peer IP is identical to the given host. Caught by the `pqi-conformance` spec added in `nikita-volkov/pqi-conformance@92f5205`.++- Fixed a pipelined command's result getting misattributed to a later, unrelated command after a prior pipeline aborted on a server error (#9). A pipelined command that sends a `Parse` (`sendQueryParams`/`sendPrepare`) records a FIFO entry so the eventual `ParseComplete` can be charged to the right command; that entry was only ever popped by a `ParseComplete` actually arriving. A command whose `Parse` itself fails - a syntax error, or being silently discarded by the server after a pipeline abort - never gets a `ParseComplete`, so its entry was leaked. A `sendPrepare` leaks a `True` entry, and once a later, unrelated command's genuine `ParseComplete` popped that stale `True` instead of its own, that command terminated immediately as `CommandOk` instead of collecting its real result, observed as a `SELECT` returning `CommandOk` instead of `TuplesOk`. Every pipelined command now pops its FIFO entry exactly once, whether via its own `ParseComplete` or, failing that, at its own terminal message (including the synthetic result generated for a command discarded after an abort). Caught by the differential coverage added in `pqi-conformance` 1.0.5.1.+ # v1.0.1.5 ## Fixes
pqi-native.cabal view
@@ -1,6 +1,6 @@ cabal-version: 3.0 name: pqi-native-version: 1.0.1.5+version: 1.0.1.6 category: Database, PostgreSQL synopsis: Native (pure-Haskell) adapter for pqi description:@@ -158,5 +158,5 @@ build-depends: base >=4.11 && <5, hspec >=2.11 && <2.12,- pqi-conformance ^>=1.0.5,+ pqi-conformance ^>=1.0.6, pqi-native,
src/library/Pqi/Native/Connection.hs view
@@ -32,6 +32,7 @@ import Pqi.Native.Types (formatErrorFields) import qualified PtrPoker.Write as Poker import System.Environment (lookupEnv)+import System.IO.Error (isEOFError) #if defined(mingw32_HOST_OS) import System.Win32.Info.Computer (getUserName) #else@@ -280,18 +281,31 @@ pipelineSeparatorPending :: IORef Bool, pendingSyncs :: IORef Int, pendingCommands :: IORef Int,- -- | FIFO, one entry per in-flight pipeline command that sends a @Parse@- -- ('sendQueryParams' or 'sendPrepare'), recording whether that command's- -- @ParseComplete@ is itself the terminal result. 'sendPrepare' pushes- -- @True@ (its @ParseComplete@ ends the command as 'CommandOk');- -- 'sendQueryParams' pushes @False@ (its @ParseComplete@ must fold into the- -- command's accumulating result, like every other extended-protocol- -- message, and never terminate it early). Popping the head at the right- -- @ParseComplete@ keeps the origins in order even when several commands are- -- pipelined together - a plain counter cannot, which is what let a- -- pipelined 'sendPrepare' steal the 'ParseComplete' of an earlier- -- 'sendQueryParams' and shift every later result by one.- pendingParseOrigins :: IORef (Seq.Seq Bool),+ -- | FIFO, one entry per in-flight pipelined command, pushed by every+ -- 'sendAsync' call while in pipeline mode and popped exactly once by+ -- whichever message ends that command's processing. For a command that+ -- sends a @Parse@ ('sendQueryParams' or 'sendPrepare') the entry records+ -- whether that command's @ParseComplete@ is itself the terminal result:+ -- 'sendPrepare' pushes @Just True@ (its @ParseComplete@ ends the command+ -- as 'CommandOk'); 'sendQueryParams' pushes @Just False@ (its+ -- @ParseComplete@ must fold into the command's accumulating result, like+ -- every other extended-protocol message, and never terminate it early).+ -- A command with no @Parse@ step (e.g. 'sendQueryPrepared') pushes+ -- @Nothing@, popped (and discarded) at its own terminal message. Popping+ -- the head at the right message keeps the origins in order even when+ -- several commands are pipelined together - a plain counter cannot,+ -- which is what let a pipelined 'sendPrepare' steal the 'ParseComplete'+ -- of an earlier 'sendQueryParams' and shift every later result by one.+ --+ -- Every command pushes exactly one entry and must pop exactly one, even+ -- when it fails before reaching @ParseComplete@ (e.g. a syntax error, or+ -- being discarded after a pipeline abort): leaving that entry unpopped+ -- would let a later, unrelated command's @ParseComplete@ pop it instead,+ -- misattributing that later command's origin and - when the leaked entry+ -- happens to be @Just True@ - making it terminate early as 'CommandOk'+ -- instead of collecting its actual result (e.g. 'TuplesOk' for a+ -- @SELECT@).+ pendingParseOrigins :: IORef (Seq.Seq (Maybe Bool)), errorVerbosity :: IORef Verbosity, -- | The SQL text of the most recently sent query (set by sendQuery / -- sendQueryParams). Used when formatting error messages for async results@@ -353,6 +367,25 @@ <> "')" | otherwise = "could not connect to server: " <> ByteString.Char8.pack (show err) +-- | Format a handshake-time 'IOException' - e.g. the server closing the+-- socket mid-rejection, as when shedding load with \"sorry, too many clients+-- already\": the rejection is sent but the socket closes before 'handshake'+-- finishes reading it. Routed through the same 'unixSocketFailureMessage'\/+-- 'tcpFailureMessage' wrapper 'failWith' uses for a rejected 'ErrorResponse',+-- so a failure that interrupts the handshake reads exactly like any other+-- classified rejection instead of escaping 'establish' as an uncaught+-- exception. An EOF (the frame never completing) gets libpq's own wording for+-- it; any other handshake-time I\/O error falls back to its 'show'n form.+handshakeFailureMessage :: Connection -> ConnInfo -> IOException -> IO ByteString+handshakeFailureMessage connection connInfo err = do+ let fmtFields+ | isEOFError err =+ "server closed the connection unexpectedly\n\tThis probably means the server terminated abnormally\n\tbefore or while processing the request.\n"+ | otherwise = ByteString.Char8.pack (show err)+ if Transport.isUnixSocketHost (host connInfo)+ then pure (unixSocketFailureMessage connInfo fmtFields)+ else tcpFailureMessage connection connInfo fmtFields+ -- | The handshake-failure message ('failWith', inside 'handshake') for a -- Unix-domain socket connection: names the socket path rather than a -- host\/port pair, matching libpq's phrasing.@@ -364,20 +397,30 @@ <> fmtFields -- | The handshake-failure message for a TCP connection: includes the--- resolved peer IP when available (it may not be, e.g. if the socket has--- already been torn down), matching libpq's phrasing.+-- resolved peer IP when available and distinct from the given host (it may+-- not be resolvable at all, e.g. if the socket has already been torn down;+-- and libpq omits the parenthetical entirely when the host was already the+-- literal numeric address, rather than a name that resolved to it), matching+-- libpq's phrasing. tcpFailureMessage :: Connection -> ConnInfo -> ByteString -> IO ByteString tcpFailureMessage connection connInfo fmtFields = do transport <- readIORef (transport connection) mIp <- catch (Just <$> Transport.peerIp transport) (\(_ :: SomeException) -> pure Nothing) pure $ case mIp of- Nothing -> fmtFields- Just ip ->+ Just ip+ | ip /= host connInfo ->+ "connection to server at \""+ <> host connInfo+ <> "\" ("+ <> ip+ <> "), port "+ <> ByteString.Char8.pack (show (port connInfo))+ <> " failed: "+ <> fmtFields+ _ -> "connection to server at \"" <> host connInfo- <> "\" ("- <> ip- <> "), port "+ <> "\", port " <> ByteString.Char8.pack (show (port connInfo)) <> " failed: " <> fmtFields@@ -413,7 +456,10 @@ Right transport -> do connection <- newConnection False transport info sendMessage connection (startupMessage (startupParams info))- handshake connection+ handshakeResult <- try @IOException (handshake connection)+ case handshakeResult of+ Left err -> setError connection =<< handshakeFailureMessage connection info err+ Right () -> pure () pure connection -- | A \"null\" sentinel connection (the analogue of @PQnewNullConnection@): no
src/library/Pqi/Native/Query.hs view
@@ -144,7 +144,7 @@ -- unnamed @Parse@, whose @ParseComplete@ must fold into this command's own -- result (like 'sendQueryPrepared'). Record its origin so it is not mistaken -- for the terminal @ParseComplete@ of a pipelined 'sendPrepare'.- when (ok && pipeline) $ modifyIORef' (pendingParseOrigins connection) (Seq.|> False)+ when (ok && pipeline) $ modifyIORef' (pendingParseOrigins connection) (Seq.|> Just False) pure ok sendPrepare :: Connection -> ByteString -> ByteString -> Maybe [Word32] -> IO Bool@@ -157,15 +157,24 @@ $ if pipeline then parseMessage name sql (fromMaybe [] parameterTypes) else prepareWrite name sql parameterTypes- when (ok && pipeline) $ modifyIORef' (pendingParseOrigins connection) (Seq.|> True)+ when (ok && pipeline) $ modifyIORef' (pendingParseOrigins connection) (Seq.|> Just True) pure ok +-- | Push a @Nothing@ origin for a pipelined command that sends no @Parse@+-- (so has no @ParseComplete@ to fold or terminate on): it still occupies one+-- slot in the FIFO, popped and discarded at its own terminal message.+sendAsyncNoOrigin :: Connection -> Bool -> ByteString -> Poker.Write -> IO Bool+sendAsyncNoOrigin connection pipeline sql write = do+ ok <- sendAsync connection sql write+ when (ok && pipeline) $ modifyIORef' (pendingParseOrigins connection) (Seq.|> Nothing)+ pure ok+ sendQueryPrepared :: Connection -> ByteString -> [Maybe (ByteString, Format)] -> Format -> IO Bool sendQueryPrepared connection name params resultFormat | tooManyParams params = pure False | otherwise = do pipeline <- inPipeline connection- sendAsync connection ""+ sendAsyncNoOrigin connection pipeline "" $ if pipeline then asyncPreparedWrite name params resultFormat else preparedWrite name params resultFormat@@ -173,7 +182,7 @@ sendDescribePrepared :: Connection -> ByteString -> IO Bool sendDescribePrepared connection name = do pipeline <- inPipeline connection- sendAsync connection ""+ sendAsyncNoOrigin connection pipeline "" $ if pipeline then describeStatementMessage name else describeStatementMessage name <> syncMessage@@ -181,7 +190,7 @@ sendDescribePortal :: Connection -> ByteString -> IO Bool sendDescribePortal connection name = do pipeline <- inPipeline connection- sendAsync connection ""+ sendAsyncNoOrigin connection pipeline "" $ if pipeline then describePortalMessage name else describePortalMessage name <> syncMessage@@ -232,6 +241,14 @@ modifyIORef' (pendingCommands connection) (subtract 1) writeIORef (pipelineSeparatorPending connection) True + -- Pop and discard this command's 'pendingParseOrigins' entry if it hasn't+ -- already been consumed by its own 'ParseComplete' (see the field's+ -- Haddock on 'Connection'). Called at every terminal message besides a+ -- 'ParseComplete' that itself terminates the command.+ popOriginIfPending pipeStatus builder =+ when (pipeStatus /= PipelineOff && not (accOriginPopped builder))+ $ void (popPendingParseOrigin connection)+ go singleRow builder = do pipeStatus <- readIORef (pipelineStatus connection) -- In aborted pipeline mode, if the server has already sent nothing for@@ -241,6 +258,11 @@ pending <- readIORef (pendingCommands connection) if pipeStatus == PipelineAborted && pending > 0 then do+ -- This command is being synthesized without ever reading a+ -- message for it (the server sends nothing for a discarded+ -- command), so its origin - if it pushed one - is necessarily+ -- still unpopped.+ popOriginIfPending pipeStatus builder modifyIORef' (pendingCommands connection) (subtract 1) writeIORef (pipelineSeparatorPending connection) True pure (Just (NativeResult PipelineAbort [] [] Nothing Map.empty [] ""))@@ -273,10 +295,11 @@ then popPendingParseOrigin connection else pure Nothing case origin of- Just True -> do+ Just (Just True) -> do finishCommand pipeStatus pure (Just (NativeResult CommandOk [] [] Nothing Map.empty [] ""))- _ -> go singleRow builder {accHadResponse = True}+ Just _ -> go singleRow builder {accHadResponse = True, accOriginPopped = True}+ Nothing -> go singleRow builder {accHadResponse = True} BindComplete -> go singleRow builder {accHadResponse = True} CloseComplete ->@@ -290,10 +313,12 @@ pure (Just (NativeResult TuplesOk (accFields builder) [] (Just tag) Map.empty [] "")) else do writeIORef (lastError connection) (Just "")+ popOriginIfPending pipeStatus builder finishCommand pipeStatus pure (Just (commandResult builder (Just tag))) EmptyQueryResponse -> do writeIORef (lastError connection) (Just "")+ popOriginIfPending pipeStatus builder finishCommand pipeStatus pure (Just (NativeResult EmptyQuery [] [] Nothing Map.empty [] "")) ErrorResponse fs -> do@@ -302,10 +327,12 @@ PipelineAborted -> do -- Should not normally happen (server discards commands in abort -- mode) but handle defensively.+ popOriginIfPending pipeStatus builder finishCommand pipeStatus pure (Just (NativeResult PipelineAbort [] [] Nothing Map.empty [] "")) PipelineOn -> do writeIORef (pipelineStatus connection) PipelineAborted+ popOriginIfPending pipeStatus builder finishCommand PipelineOn pure (Just (NativeResult FatalError [] [] Nothing errMap [] "")) PipelineOff -> do@@ -335,12 +362,13 @@ pure (Just (NativeResult PipelineSync [] [] Nothing Map.empty [] "")) _ -> go singleRow builder --- | Pop the origin of the next in-flight @ParseComplete@ in pipeline mode:--- @True@ if it is the terminal @ParseComplete@ of a 'sendPrepare' (to be--- materialized as 'CommandOk'), @False@ if it belongs to a 'sendQueryParams'--- and must fold into that command's accumulating result. @Nothing@ when no--- @Parse@ is pending (defensive: the @ParseComplete@ is then folded).-popPendingParseOrigin :: Connection -> IO (Maybe Bool)+-- | Pop the next in-flight pipelined command's origin: @Just (Just True)@ if+-- it is the terminal @ParseComplete@ of a 'sendPrepare' (to be materialized+-- as 'CommandOk'), @Just (Just False)@ if it belongs to a 'sendQueryParams'+-- and must fold into that command's accumulating result, @Just Nothing@ for+-- a command with no @Parse@ step. The outer @Nothing@ means the FIFO is+-- empty (defensive: nothing is popped and the caller folds).+popPendingParseOrigin :: Connection -> IO (Maybe (Maybe Bool)) popPendingParseOrigin connection = atomicModifyIORef' (pendingParseOrigins connection)@@ -402,11 +430,14 @@ accRevRows :: [[Maybe ByteString]], accParamOids :: [Word32], accSawRowDescription :: Bool,- accHadResponse :: Bool+ accHadResponse :: Bool,+ -- | Whether this command's 'pendingParseOrigins' entry has already been+ -- popped (via its own 'ParseComplete'). See 'popOriginIfPending'.+ accOriginPopped :: Bool } emptyBuilder :: Builder-emptyBuilder = Builder [] [] [] False False+emptyBuilder = Builder [] [] [] False False False -- | Collect the (possibly several) results of a simple query, up to -- @ReadyForQuery@. The last is what @PQexec@ returns.