pqi-conformance 1.0.5.0 → 1.0.6.0

raw patch · 6 files changed

+202/−1 lines, 6 filesdep +networkPVP ok

version bump matches the API change (PVP)

Dependencies added: network

API changes (from Hackage documentation)

Files

• CHANGELOG.md +14/−0
• pqi-conformance.cabal +4/−1
• src/library/Pqi/Conformance/Operation/Connectdb.hs +3/−0
• src/library/Pqi/Conformance/Operation/Connectdb/Rejection.hs +110/−0
• src/library/Pqi/Conformance/Operation/PipelineSync.hs +2/−0
• src/library/Pqi/Conformance/Operation/PipelineSync/Desync.hs +69/−0

@@ -1,3 +1,17 @@+# v1.0.6.0++## Non-breaking++- Added a `pipelineSync` spec covering a pipelined command whose result gets misattributed to a later, unrelated command after a prior pipeline aborted on a server error: a named-statement prepare discarded by the server after an earlier command in the same pipeline fails, followed by a plain `SELECT` in the next pipeline on the same connection, which must come back `TuplesOk` rather than short-circuiting as `CommandOk`. `pqi-native` 1.0.1.5 fails it, 1.0.1.6 fixes it. Found via `pqi-native` issue #9.++- Added a `connectdb` spec covering a mid-handshake server rejection: a listener that accepts the connection, writes the first three bytes of an `E`rrorResponse frame, then closes before the frame completes - mirroring how a server sheds load with e.g. "sorry, too many clients already". A sound adapter reports `ConnectionBad` with a classified error message, the same way it reports any other rejected handshake, instead of letting the underlying I/O exception escape `connectdb`.++  Unlike most operation specs, this one drives a raw listener rather than the shared PostgreSQL container: the failure is about how an adapter reacts to a truncated read, which a real server only triggers racily (e.g. under `max_connections` pressure). A hand-rolled listener reproduces the exact byte pattern deterministically. Adds a new `network` dependency.++  The spec asserts full equality on both `status` and `errorMessage`, not just `status`: an adapter that swaps in a made-up message instead of reproducing libpq's actual wording would otherwise slip through unnoticed. Getting that message comparison to hold required two adjustments once real libpq's behavior was checked against: the conninfo passes `sslmode=disable`, since libpq negotiates SSL before the startup packet by default and the candidate adapters under test never do, so without it the two sides would be reacting to the truncated bytes at different points in the protocol; and the listener's host is given as a literal IP (`127.0.0.1`) rather than a name, since libpq only parenthesizes a resolved IP in its failure message when it differs from the given host string, which a literal IP never does. Both attempts now also share one listener/port, since the failure message embeds the port number and two independently-bound ephemeral listeners would otherwise never produce an equal message even when every other detail matches.++  `pqi-native` throws an uncaught `IOException` out of `connectdb` here, since only the initial TCP connect was wrapped in an exception handler, not the handshake read that follows it. Found via [`pqi-native` issue #8](https://github.com/nikita-volkov/pqi-native/issues/8), itself found while investigating `hasql` issue #329.+ # v1.0.5.0  ## Non-breaking

@@ -1,6 +1,6 @@ cabal-version: 3.0 name: pqi-conformance-version: 1.0.5.0+version: 1.0.6.0 category: Database, PostgreSQL, Testing synopsis: Differential conformance tests for pqi adapters description:@@ -94,6 +94,7 @@     Pqi.Conformance.Operation.CmdStatus     Pqi.Conformance.Operation.CmdTuples     Pqi.Conformance.Operation.Connectdb+    Pqi.Conformance.Operation.Connectdb.Rejection     Pqi.Conformance.Operation.ConnectionNeedsPassword     Pqi.Conformance.Operation.ConnectionUsedPassword     Pqi.Conformance.Operation.ConnectPoll@@ -160,6 +161,7 @@     Pqi.Conformance.Operation.Pass     Pqi.Conformance.Operation.PipelineStatus     Pqi.Conformance.Operation.PipelineSync+    Pqi.Conformance.Operation.PipelineSync.Desync     Pqi.Conformance.Operation.PipelineSync.InterruptedWait     Pqi.Conformance.Operation.PipelineSync.Interruption     Pqi.Conformance.Operation.PipelineSync.Parity@@ -203,6 +205,7 @@     containers >=0.6 && <0.9,     directory >=1.3 && <1.4,     hspec >=2.11 && <2.12,+    network >=3.1 && <3.3,     postgresql-libpq >=0.11 && <0.12,     pqi ^>=1.1,     testcontainers-postgresql ^>=0.2,

@@ -14,6 +14,7 @@ import qualified Pqi import Pqi.Conformance.Harness import Pqi.Conformance.Observation+import qualified Pqi.Conformance.Operation.Connectdb.Rejection as Rejection import Pqi.Conformance.Prelude import qualified Pqi.Conformance.Reference as Reference import Test.Hspec@@ -21,6 +22,8 @@  spec :: Pqi.Adapter -> SpecWith ByteString spec adapter = do+  Rejection.spec adapter+   describe "connectdb" do     it "opens a usable connection" \conninfo ->       differential adapter conninfo observeConnection

@@ -0,0 +1,110 @@+-- | Coverage for a mid-handshake server rejection: the server accepts the TCP+-- connection, starts writing an error frame back (mirroring how it sheds load+-- with e.g. \"sorry, too many clients already\"), then closes the socket+-- before the frame is complete. A sound adapter reports this as a classified,+-- \'ConnectionBad\' failure - the same way it reports any other rejected+-- handshake - instead of letting the underlying I\/O exception escape+-- 'Pqi.connectdb'.+--+-- Found in @pqi-native@ (<https://github.com/nikita-volkov/pqi-native/issues/8>):+-- 'establish' only wrapped the initial TCP connect in an exception handler,+-- leaving the handshake read that follows it able to throw an uncaught+-- 'System.IO.Error.IOException' whenever the server's rejection didn't arrive+-- as a complete frame.+--+-- Unlike most operation specs, this one drives a raw listener instead of the+-- shared PostgreSQL container: the failure is about how an adapter reacts to+-- a truncated read, which a real server only triggers racily (e.g. under+-- @max_connections@ pressure). A hand-rolled listener reproduces the exact+-- byte pattern deterministically.+module Pqi.Conformance.Operation.Connectdb.Rejection+  ( spec,+  )+where++import Control.Concurrent (forkIO)+import Control.Exception (SomeException, bracket, try)+import qualified Data.ByteString as ByteString+import qualified Data.ByteString.Char8 as ByteString.Char8+import qualified Network.Socket as Socket+import qualified Network.Socket.ByteString as Socket.ByteString+import qualified Pqi+import Pqi.Conformance.Prelude+import qualified Pqi.Conformance.Reference as Reference+import Test.Hspec++spec :: Pqi.Adapter -> SpecWith ByteString+spec adapter =+  describe "connectdb" do+    describe "a mid-handshake server rejection" do+      it "the candidate reports a classified error like the reference, instead of throwing" \_ ->+        -- Both attempts share one listener (and so one port): the failure+        -- message embeds the port number, and the candidate and reference+        -- would otherwise always disagree on that one detail despite+        -- matching in every way that matters.+        withRejectingServer \port -> do+          candidate <- attempt adapter port+          reference <- attempt Reference.adapter port+          candidate `shouldBe` reference++-- | Run 'Pqi.connectdb' against the given port (see 'withRejectingServer')+-- and report the resulting status and error message, or the exception's+-- 'Show'n form if one escaped - which is exactly what should never happen.+--+-- @sslmode=disable@ keeps this comparable across adapters: libpq negotiates+-- SSL before the startup packet by default, so without it the rejection+-- would land during a preamble @pqi-native@ (which never attempts SSL) does+-- not even send, and the two adapters would be reacting to the truncated+-- bytes at different points in the protocol. The host is given as a literal+-- IP rather than a name so libpq's failure message doesn't gain a resolved-IP+-- parenthetical the candidate would then also have to reproduce.+attempt :: Pqi.Adapter -> Socket.PortNumber -> IO (Either String (Pqi.ConnStatus, Maybe ByteString))+attempt adapter port = do+  let conninfo =+        "host=127.0.0.1 port="+          <> ByteString.Char8.pack (show port)+          <> " dbname=x user=x sslmode=disable"+  result <- try @SomeException (Pqi.connectdb adapter conninfo)+  case result of+    Left err -> pure (Left (show err))+    Right connection -> do+      observedStatus <- Pqi.status connection+      observedError <- Pqi.errorMessage connection+      Pqi.finish connection+      pure (Right (observedStatus, observedError))++-- | Bind a loopback listener on an ephemeral port and hand its port number to+-- the action, while a background thread serves every connection made to it+-- in turn: read whatever the client has sent so far, write the first three+-- bytes of an \'E\'rrorResponse frame (a type byte and two of its four+-- length bytes), and close - never completing the frame.+--+-- The truncated write is safe to race against each client: the kernel+-- queues a connection at 'Socket.listen' time, so a client's own 'connect'+-- and initial 'send' succeed regardless of whether the server thread has+-- reached 'Socket.accept' yet, and the client only blocks once it starts+-- reading the (never-completed) response.+withRejectingServer :: (Socket.PortNumber -> IO a) -> IO a+withRejectingServer action =+  bracket open Socket.close \listener -> do+    port <- Socket.socketPort listener+    _ <- forkIO (try @SomeException (forever (serveOneRejection listener)) >> pure ())+    action port+  where+    open = do+      address : _ <-+        Socket.getAddrInfo+          (Just Socket.defaultHints {Socket.addrSocketType = Socket.Stream})+          (Just "127.0.0.1")+          (Just "0")+      sock <- Socket.socket (Socket.addrFamily address) (Socket.addrSocketType address) (Socket.addrProtocol address)+      Socket.bind sock (Socket.addrAddress address)+      Socket.listen sock 8+      pure sock++serveOneRejection :: Socket.Socket -> IO ()+serveOneRejection listener = do+  (conn, _) <- Socket.accept listener+  _ <- Socket.ByteString.recv conn 4096+  Socket.ByteString.sendAll conn (ByteString.pack [0x45, 0x00, 0x00])+  Socket.close conn

@@ -8,6 +8,7 @@ import qualified Pqi import qualified Pqi as Lq import Pqi.Conformance.Harness+import qualified Pqi.Conformance.Operation.PipelineSync.Desync as Desync import qualified Pqi.Conformance.Operation.PipelineSync.InterruptedWait as InterruptedWait import qualified Pqi.Conformance.Operation.PipelineSync.Interruption as Interruption import qualified Pqi.Conformance.Operation.PipelineSync.Parity as Parity@@ -21,6 +22,7 @@     Parity.spec adapter     Interruption.spec adapter     InterruptedWait.spec adapter+    Desync.spec adapter     it "collects pipelined queries per sync" \conninfo ->       differential adapter conninfo \connection -> do         entered <- Lq.enterPipelineMode connection

@@ -0,0 +1,69 @@+-- | Reproduces+-- <https://github.com/nikita-volkov/pqi-native/issues/9>: after a pipelined+-- statement fails with a server error, a command still queued behind it in+-- that same pipeline is discarded by the server without any response at+-- all (per the pipelining protocol, everything after the failure is+-- skipped until the next @Sync@). A later pipeline sent on the same+-- connection can then misattribute its own @ParseComplete@ to that silently+-- discarded command instead of to itself.+--+-- The discarded command needs to be sent via 'Lq.sendPrepare' specifically:+-- a discarded 'Lq.sendQueryParams' command leaks the same way internally,+-- but every leaked entry it produces is indistinguishable from a correctly+-- popped one, so the symptom never surfaces. A discarded 'Lq.sendPrepare'+-- leaks an entry that, if later popped by an unrelated command's+-- @ParseComplete@, makes that command terminate immediately as+-- 'Lq.CommandOk' instead of collecting its real result - observable here as+-- the second pipeline's plain @SELECT@ coming back 'Lq.CommandOk' instead of+-- 'Lq.TuplesOk' with an empty row list.+module Pqi.Conformance.Operation.PipelineSync.Desync+  ( spec,+  )+where++import qualified Pqi+import qualified Pqi as Lq+import Pqi.Conformance.Harness+import Pqi.Conformance.Prelude+import Pqi.Conformance.Scenario (takeCommandResults, takeResult)+import Test.Hspec++spec :: Pqi.Adapter -> SpecWith ByteString+spec adapter =+  describe "desync after a mid-pipeline server error" do+    it "a later pipeline's results are not misattributed" \conninfo ->+      differential adapter conninfo \connection -> do+        entered <- Lq.enterPipelineMode connection++        -- First pipeline: a statement that fails with a server error,+        -- followed by a named-statement prepare that gets silently+        -- discarded as a consequence (the server sends nothing for it).+        failingSent <- Lq.sendQueryParams connection "select 1 / 0" [] Lq.Text+        discardedSent <- Lq.sendPrepare connection "" "select 99" Nothing+        firstSynced <- Lq.pipelineSync connection+        failed <- takeCommandResults connection+        discarded <- takeCommandResults connection+        firstSyncResult <- takeResult connection++        -- Second pipeline, on the same connection: a plain SELECT that must+        -- come back as TuplesOk with an empty row list, not CommandOk.+        selectSent <- Lq.sendQueryParams connection "select 1 where false" [] Lq.Text+        secondSynced <- Lq.pipelineSync connection+        selected <- takeCommandResults connection+        secondSyncResult <- takeResult connection++        exited <- Lq.exitPipelineMode connection+        pure+          ( entered,+            failingSent,+            discardedSent,+            firstSynced,+            failed,+            discarded,+            firstSyncResult,+            selectSent,+            secondSynced,+            selected,+            secondSyncResult,+            exited+          )