pqi-conformance 1.0.8.0 → 1.0.9.0

raw patch · 9 files changed

+301/−5 lines, 9 filesPVP ok

version bump matches the API change (PVP)

API changes (from Hackage documentation)

Files

• CHANGELOG.md +11/−1
• pqi-conformance.cabal +4/−1
• src/library/Pqi/Conformance/Operation/Connectdb.hs +4/−0
• src/library/Pqi/Conformance/Operation/Connectdb/HandshakeReset.hs +111/−0
• src/library/Pqi/Conformance/Operation/Connectdb/MissingUnixSocketDirectory.hs +1/−1
• src/library/Pqi/Conformance/Operation/Connectdb/UnixSocketUri.hs +1/−1
• src/library/Pqi/Conformance/Operation/Connectdb/UnresolvableHost.hs +44/−0
• src/library/Pqi/Conformance/Operation/Exec.hs +4/−1
• src/library/Pqi/Conformance/Operation/Exec/ConnectionLostMidQuery.hs +121/−0

@@ -1,8 +1,18 @@+# v1.0.9.0++## Non-breaking++- Added a `connectdb` spec covering a host name that DNS cannot resolve, asserting `errorMessage` exactly against the reference. Found `pqi-native`'s failure message is the raw `Show`n `getAddrInfo` exception, worded unrelatedly to libpq's own (locale-translated) sentence for the same failure, so a downstream classifier that pattern-matches libpq's wording reacts inconsistently between the two adapters. Found via `hasql` issue #329.++- Added a `connectdb` spec covering a handshake-time connection reset (`ECONNRESET`, forced via `SO_LINGER` 0) as distinct from a clean EOF. Found `pqi-native`'s `handshakeFailureMessage` only special-cases EOF with libpq-matching wording ("server closed the connection unexpectedly") and falls back to the raw `Show`n `IOException` (e.g. `Network.Socket.recvBuf: resource vanished (Connection reset by peer)`) for any other handshake-time I/O error, even though libpq itself reports a hard reset with the very same EOF wording. Found via `hasql` issue #329.++- Added an `exec` spec covering a connection reset while a query response is in flight, after a valid handshake. Found none of `pqi-native`'s query functions (`exec`, `execParams`, `prepare`, `execPrepared`, `describePrepared`, `describePortal`) wrap their read loop in any exception handler at all, so a connection lost mid-query escapes as a raw, uncaught `IOException` instead of any classified failure - unlike libpq, which returns a `FatalError` result with its own "server closed the connection unexpectedly" message and never throws. Found via `hasql` issue #329.+ # v1.0.8.0  ## Non-breaking -- Added a `connectdb` spec covering a Unix-socket directory that doesn't exist, asserting `errorMessage` byte-for-byte against the reference. Found `pqi-native`'s failure message is the raw `Show`n `IOException`, which happens to contain `"could not connect to server"` - one of `Hasql.Connection`'s networking (transient) patterns - while libpq's message for the identical `ENOENT` doesn't, so the two adapters drive `Hasql.Connection.acquire` to opposite classifications of the same failure. Found via `hasql` issue #329.+- Added a `connectdb` spec covering a Unix-socket directory that doesn't exist, asserting `errorMessage` exactly against the reference. Found `pqi-native`'s failure message is the raw `Show`n `IOException`, which happens to contain `"could not connect to server"` - one of `Hasql.Connection`'s networking (transient) patterns - while libpq's message for the identical `ENOENT` doesn't, so the two adapters drive `Hasql.Connection.acquire` to opposite classifications of the same failure. Found via `hasql` issue #329.  # v1.0.7.0 

@@ -1,6 +1,6 @@ cabal-version: 3.0 name: pqi-conformance-version: 1.0.8.0+version: 1.0.9.0 category: Database, PostgreSQL, Testing synopsis: Differential conformance tests for pqi adapters description:@@ -94,9 +94,11 @@     Pqi.Conformance.Operation.CmdStatus     Pqi.Conformance.Operation.CmdTuples     Pqi.Conformance.Operation.Connectdb+    Pqi.Conformance.Operation.Connectdb.HandshakeReset     Pqi.Conformance.Operation.Connectdb.MissingUnixSocketDirectory     Pqi.Conformance.Operation.Connectdb.Rejection     Pqi.Conformance.Operation.Connectdb.UnixSocketUri+    Pqi.Conformance.Operation.Connectdb.UnresolvableHost     Pqi.Conformance.Operation.ConnectionNeedsPassword     Pqi.Conformance.Operation.ConnectionUsedPassword     Pqi.Conformance.Operation.ConnectPoll@@ -113,6 +115,7 @@     Pqi.Conformance.Operation.EscapeIdentifier     Pqi.Conformance.Operation.EscapeStringConn     Pqi.Conformance.Operation.Exec+    Pqi.Conformance.Operation.Exec.ConnectionLostMidQuery     Pqi.Conformance.Operation.ExecParams     Pqi.Conformance.Operation.ExecPrepared     Pqi.Conformance.Operation.ExitPipelineMode

@@ -14,9 +14,11 @@ import qualified Pqi import Pqi.Conformance.Harness import Pqi.Conformance.Observation+import qualified Pqi.Conformance.Operation.Connectdb.HandshakeReset as HandshakeReset import qualified Pqi.Conformance.Operation.Connectdb.MissingUnixSocketDirectory as MissingUnixSocketDirectory import qualified Pqi.Conformance.Operation.Connectdb.Rejection as Rejection import qualified Pqi.Conformance.Operation.Connectdb.UnixSocketUri as UnixSocketUri+import qualified Pqi.Conformance.Operation.Connectdb.UnresolvableHost as UnresolvableHost import Pqi.Conformance.Prelude import qualified Pqi.Conformance.Reference as Reference import Test.Hspec@@ -27,6 +29,8 @@   Rejection.spec adapter   UnixSocketUri.spec adapter   MissingUnixSocketDirectory.spec adapter+  UnresolvableHost.spec adapter+  HandshakeReset.spec adapter    describe "connectdb" do     it "opens a usable connection" \conninfo ->

@@ -0,0 +1,111 @@+-- | Coverage for a handshake-time failure that is a hard reset+-- (@ECONNRESET@) rather than a clean EOF: the server accepts the TCP+-- connection, reads the startup packet, then tears the socket down with+-- @SO_LINGER@ set to a zero timeout so the close sends a TCP @RST@ instead+-- of the usual @FIN@.+--+-- Unlike a graceful close (see+-- 'Pqi.Conformance.Operation.Connectdb.Rejection', which truncates an+-- @ErrorResponse@ frame and closes normally), a reset is a genuinely+-- different I\/O failure shape, and was not routed through the same code+-- path in @pqi-native@:+-- 'Pqi.Native.Connection.handshakeFailureMessage' special-cased an EOF+-- (\"server closed the connection unexpectedly\", matching libpq's own+-- wording) but fell back to the raw 'Show'n 'System.IO.Error.IOException'+-- for anything else - and @ECONNRESET@ landed in that \"anything else\"+-- branch.+--+-- Was found in @pqi-native@+-- (<https://github.com/nikita-volkov/hasql/issues/329>): the candidate's+-- 'Pqi.errorMessage' for this failure was e.g. @Network.Socket.recvBuf:+-- resource vanished (Connection reset by peer)@, entirely unlike libpq's+-- own \"server closed the connection unexpectedly\" sentence for the same+-- underlying reset. Fixed by widening+-- 'Pqi.Native.Connection.handshakeFailureMessage's classification to+-- also recognize a hard reset (@ioe_type == ResourceVanished@), not just+-- a clean EOF.+module Pqi.Conformance.Operation.Connectdb.HandshakeReset+  ( 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 handshake-time connection reset (not a clean EOF)" do+      it "the candidate reports a failure message shaped like the reference's" \_ ->+        -- 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.+        withResettingServer \port -> do+          candidate <- attempt adapter port+          reference <- attempt Reference.adapter port+          candidate `shouldBe` reference++-- | Run 'Pqi.connectdb' against the given port (see 'withResettingServer')+-- 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 reset would+-- land during a preamble @pqi-native@ (which never attempts SSL) does not+-- even send. 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 (the startup+-- packet), then reset the connection - via @SO_LINGER@ with a zero timeout,+-- which makes the kernel send a @RST@ instead of the usual @FIN@ on close -+-- rather than closing it gracefully.+withResettingServer :: (Socket.PortNumber -> IO a) -> IO a+withResettingServer action =+  bracket open Socket.close \listener -> do+    port <- Socket.socketPort listener+    _ <- forkIO (try @SomeException (forever (serveOneReset 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++serveOneReset :: Socket.Socket -> IO ()+serveOneReset listener = do+  (conn, _) <- Socket.accept listener+  _ <- Socket.ByteString.recv conn 4096+  Socket.setSockOptValue conn Socket.Linger (Socket.SockOptValue (Socket.StructLinger 1 0))+  Socket.close conn

@@ -6,7 +6,7 @@ -- @ENOENT@ - so unlike -- 'Pqi.Conformance.Operation.Connectdb.UnixSocketUri' (whose two adapters -- fail via genuinely unrelated machinery: DNS resolution vs. a filesystem--- check), this spec does compare 'Pqi.errorMessage' byte-for-byte. It's+-- check), this spec does compare 'Pqi.errorMessage' exactly. It's -- expected to fail on @pqi-native@ right now, and deliberately left that -- way: the mismatch it demonstrates is exactly what confuses downstream -- string-matching classifiers.

@@ -14,7 +14,7 @@ -- 'Pqi.errorMessage': both adapters fail to connect either way (there is no -- such socket directory), but their failure text is produced by unrelated, -- adapter-specific machinery (DNS resolution vs. a filesystem check) and--- isn't expected to match byte-for-byte. What must match is the host each+-- isn't expected to match exactly. What must match is the host each -- adapter parsed out of the conninfo, which is deterministic and -- OS\/locale-independent. module Pqi.Conformance.Operation.Connectdb.UnixSocketUri

@@ -0,0 +1,44 @@+-- | Coverage for @connectdb@ against a host name that DNS cannot resolve.+--+-- Was found in @pqi-native@+-- (<https://github.com/nikita-volkov/hasql/issues/329>): the candidate's+-- 'Pqi.errorMessage' was the raw 'Show'n form of the underlying+-- 'Network.Socket.getAddrInfo' exception (@could not connect to server:+-- Network.Socket.getAddrInfo (called with preferred socket+-- type\/protocol: ...): does not exist (Name or service not known)@),+-- whereas libpq's own message for the identical failure is its own,+-- differently-worded (and locale-translated) sentence (@could not+-- translate host name "..." to address: Name or service not known@).+-- Both mention the resolver failure, but in unrelated phrasing, so a+-- downstream classifier that pattern-matches on libpq's wording (e.g.+-- @Hasql.Connection@'s networking-error patterns) reacted differently to+-- the two adapters for what is otherwise the same underlying failure.+-- Fixed in @pqi-native@'s @Pqi.Native.Connection.connectFailureMessage@,+-- which now recognizes a resolver failure by its 'ioe_location' and+-- reproduces libpq's own wording for it.+module Pqi.Conformance.Operation.Connectdb.UnresolvableHost+  ( spec,+  )+where++import qualified Pqi+import Pqi.Conformance.Harness+import Pqi.Conformance.Prelude+import Test.Hspec++spec :: Pqi.Adapter -> SpecWith ByteString+spec adapter =+  describe "connectdb" do+    describe "a host name that DNS cannot resolve" do+      it "reports a failure message shaped like the reference's" \_ ->+        differentialConnect adapter conninfo \adapter' conninfo' -> do+          connection <- Pqi.connectdb adapter' conninfo'+          observedStatus <- Pqi.status connection+          observedError <- Pqi.errorMessage connection+          Pqi.finish connection+          pure (observedStatus, observedError)+  where+    -- A hostname reserved by RFC 2606 conventions for never resolving:+    -- this test is only about the shape of the failure, not about a real+    -- connection succeeding.+    conninfo = "host=nonexistent.invalid.host"

@@ -7,12 +7,15 @@  import qualified Pqi import Pqi.Conformance.Harness+import qualified Pqi.Conformance.Operation.Exec.ConnectionLostMidQuery as ConnectionLostMidQuery import Pqi.Conformance.Prelude import Pqi.Conformance.Scenario import Test.Hspec  spec :: Pqi.Adapter -> SpecWith ByteString-spec adapter =+spec adapter = do+  ConnectionLostMidQuery.spec adapter+   describe "exec" do     let forCase title sql =           it title \conninfo -> differential adapter conninfo (execScenario sql)

@@ -0,0 +1,121 @@+-- | Coverage for a connection reset while a query response is in flight -+-- after a valid handshake, not during one (contrast+-- 'Pqi.Conformance.Operation.Connectdb.HandshakeReset|). A hand-rolled+-- server completes the startup handshake normally (@AuthenticationOk@ then+-- @ReadyForQuery@), reads the client's @Query@ message, then resets the+-- connection - via @SO_LINGER@ with a zero timeout, so the close sends a+-- TCP @RST@ - instead of ever sending a result.+--+-- Was found in @pqi-native@: none of 'Pqi.Native.Query.exec',+-- @execParams@, @prepare@, @execPrepared@, @describePrepared@, or+-- @describePortal@ wrapped their read loop in any exception handler at all -+-- unlike 'Pqi.Native.Connection.establish', which at least classifies a+-- broken read into a 'Pqi.errorMessage'. A connection lost mid-query used+-- to escape 'Pqi.exec' as a raw, uncaught 'System.IO.Error.IOException',+-- crashing the caller's thread outright rather than surfacing as any kind+-- of classified failure. libpq itself never throws here: @PQexec@ returns a+-- result with 'Pqi.FatalError' status and its own \"server closed the+-- connection unexpectedly\" message, the same wording it uses for a reset+-- during the handshake.+--+-- Fixed by catching the read loop's 'System.IO.Error.IOException' in each of+-- those six flows and converting it into a classified 'Pqi.FatalError'+-- result via+-- 'Pqi.Native.Connection.connectionLostMessage'\/'Pqi.Native.Query.connectionLostResult',+-- the same classification 'HandshakeReset' exercises for the connect-time+-- case.+module Pqi.Conformance.Operation.Exec.ConnectionLostMidQuery+  ( 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 "exec" do+    describe "a connection reset while a query response is in flight" do+      it "the candidate reports a classified failed result 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.+        withHandshakingThenResettingServer \port -> do+          candidate <- attempt adapter port+          reference <- attempt Reference.adapter port+          candidate `shouldBe` reference++-- | Connect, run one query against the given port (see+-- 'withHandshakingThenResettingServer'), and report the resulting result's+-- status and error message, or the exception's 'Show'n form if one escaped+-- 'Pqi.exec' - 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 exchange+-- would never reach the fake server's plain-protocol handshake, which+-- @pqi-native@ (which never attempts SSL) does not even send. The host is+-- given as a literal IP rather than a name so a 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 (Maybe (Pqi.ExecStatus, 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"+  bracket (Pqi.connectdb adapter conninfo) Pqi.finish \connection -> do+    outcome <- try @SomeException (Pqi.exec connection "select 1")+    case outcome of+      Left err -> pure (Left (show err))+      Right mResult -> do+        observed <- traverse (\r -> (,) <$> Pqi.resultStatus r <*> Pqi.resultErrorMessage r) mResult+        pure (Right observed)++-- | 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: complete a minimal but valid startup handshake+-- (@AuthenticationOk@ then @ReadyForQuery@), read whatever the client sends+-- next (its @Query@ message), then reset the connection - via @SO_LINGER@+-- with a zero timeout, which makes the kernel send a @RST@ instead of the+-- usual @FIN@ on close - rather than ever answering it.+withHandshakingThenResettingServer :: (Socket.PortNumber -> IO a) -> IO a+withHandshakingThenResettingServer action =+  bracket open Socket.close \listener -> do+    port <- Socket.socketPort listener+    _ <- forkIO (try @SomeException (forever (serveOneQueryThenReset 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++serveOneQueryThenReset :: Socket.Socket -> IO ()+serveOneQueryThenReset listener = do+  (conn, _) <- Socket.accept listener+  _ <- Socket.ByteString.recv conn 4096 -- the startup packet+  Socket.ByteString.sendAll conn authenticationOk+  Socket.ByteString.sendAll conn readyForQuery+  _ <- Socket.ByteString.recv conn 4096 -- the Query message+  Socket.setSockOptValue conn Socket.Linger (Socket.SockOptValue (Socket.StructLinger 1 0))+  Socket.close conn+  where+    -- 'R', length 8 (self-inclusive), auth type 0 (Ok).+    authenticationOk = ByteString.pack [0x52, 0x00, 0x00, 0x00, 0x08, 0x00, 0x00, 0x00, 0x00]+    -- 'Z', length 5 (self-inclusive), status 'I' (idle).+    readyForQuery = ByteString.pack [0x5A, 0x00, 0x00, 0x00, 0x05, 0x49]