From 20f7b538e52ce47eefbe0dbb8e44fb5298df7820 Mon Sep 17 00:00:00 2001 From: tcely Date: Mon, 29 May 2023 07:12:23 -0400 Subject: [PATCH 1/2] Docker entrypoint fixes (#753) * docker entrypoint: DRY store log backup Don't Repeat Yourself Set a variable for the full path to the source file. Create the backup path by appending an extension from `date` output. Also fixed quoting and switched to an `if` block. Clean up block level variables. * docker entrypoint: use ISO 8601 format The previous format discards information about the local time zone. * docker entrypoint: always use UTC Now the format always ends in +00:00, and we can ignore that part again. * docker entrypoint: provide the date format again * docker entrypoint: remove time zone from the date format * docker entrypoint: use an unambiguous date format Present a leading zero before the month: YYYY-0MM-DD Both YYYY-MM-DD and YYYY-DD-MM are used by people and can be confusing in the beginning of the month. * docker entrypoint: use appropriate quoting Without avoiding field splitting a password containing a space changes the number of arguments being set. * docker entrypoint: use explicit braces for custom variables Make the intentions clear and don't assume the user knows the special cases for when variables won't be extended. Example: word=animal words=mistake echo "$words vs $word vs ${word}s" * docker entrypoint-xftp-server: braces and quoting * docker entrypoint-xftp-server: backup block * docker entrypoints: explain date format in a comment * switched from long to short option to date for POSIX * docker entrypoint-smp-server: explain date format * docker entrypoint-xftp-server: explain date format * docker entrypoints: further explain format I fixed the case to match the date format letters. Also, use words to explain since I don't want everyone to need to read about date formats to understand. * docker entrypoints: only quote letters I was either going to quote the dashes too or stop quoting the colons. Having less quotes was more readable. * Revert "docker entrypoint: use an unambiguous date format" This reverts commit ba2a93bad98d5dbd9a27b737bcfab36bae7bd0f7. * docker entrypoints: remove %F * docker entrypoint-smp-server: remove %F Used part of the explicit ISO 8601 format, provided by the coreutils date invocation guide. * docker entrypoint-xftp-server: remove %F Used part of the explicit ISO 8601 format, provided by the coreutils date invocation guide. --- scripts/docker/entrypoint-smp-server | 37 ++++++++++++++++--------- scripts/docker/entrypoint-xftp-server | 39 ++++++++++++++++++--------- 2 files changed, 51 insertions(+), 25 deletions(-) diff --git a/scripts/docker/entrypoint-smp-server b/scripts/docker/entrypoint-smp-server index 8958874f8..5817b7b56 100755 --- a/scripts/docker/entrypoint-smp-server +++ b/scripts/docker/entrypoint-smp-server @@ -1,25 +1,25 @@ #!/usr/bin/env sh -confd="/etc/opt/simplex" -logd="/var/opt/simplex/" +confd='/etc/opt/simplex' +logd='/var/opt/simplex/' # Check if server has been initialized -if [ ! -f "$confd/smp-server.ini" ]; then +if [ ! -f "${confd}/smp-server.ini" ]; then # If not, determine ip or domain - case "$ADDR" in - '') printf "Please specify \$ADDR environment variable.\n"; exit 1 ;; + case "${ADDR}" in + '') printf 'Please specify $ADDR environment variable.\n'; exit 1 ;; *[a-zA-Z]*) - case "$ADDR" in - *:*) set -- --ip "$ADDR" ;; - *) set -- -n "$ADDR" ;; + case "${ADDR}" in + *:*) set -- --ip "${ADDR}" ;; + *) set -- -n "${ADDR}" ;; esac ;; - *) set -- --ip "$ADDR" ;; + *) set -- --ip "${ADDR}" ;; esac # Optionally, set password - case "$PASS" in + case "${PASS}" in '') set -- "$@" --no-password ;; - *) set -- "$@" --password "$PASS" ;; + *) set -- "$@" --password "${PASS}" ;; esac # And init certificates and configs @@ -27,7 +27,20 @@ if [ ! -f "$confd/smp-server.ini" ]; then fi # Backup store log just in case -[ -f "$logd/smp-server-store.log" ] && cp "$logd"/smp-server-store.log "$logd"/smp-server-store.log."$(date +'%FT%T')" +# +# Uses the UTC (universal) time zone and this +# format: YYYY-mm-dd'T'HH:MM:SS +# year, month, day, letter T, hour, minute, second +# +# This is the ISO 8601 format without the time zone at the end. +# +_file="${logd}/smp-server-store.log" +if [ -f "${_file}" ]; then + _backup_extension="$(date -u '+%Y-%m-%dT%H:%M:%S')" + cp -v -p "${_file}" "${_file}.${_backup_extension:-date-failed}" + unset -v _backup_extension +fi +unset -v _file # Finally, run smp-sever. Notice that "exec" here is important: # smp-server replaces our helper script, so that it can catch INT signal diff --git a/scripts/docker/entrypoint-xftp-server b/scripts/docker/entrypoint-xftp-server index c23262670..55757401e 100755 --- a/scripts/docker/entrypoint-xftp-server +++ b/scripts/docker/entrypoint-xftp-server @@ -1,25 +1,25 @@ #!/usr/bin/env sh -confd="/etc/opt/simplex-xftp" -logd="/var/opt/simplex-xftp" +confd='/etc/opt/simplex-xftp' +logd='/var/opt/simplex-xftp' # Check if server has been initialized -if [ ! -f "$confd/file-server.ini" ]; then +if [ ! -f "${confd}/file-server.ini" ]; then # If not, determine ip or domain - case "$ADDR" in - '') printf "Please specify \$ADDR environment variable.\n"; exit 1 ;; + case "${ADDR}" in + '') printf 'Please specify $ADDR environment variable.\n'; exit 1 ;; *[a-zA-Z]*) - case "$ADDR" in - *:*) set -- --ip "$ADDR" ;; - *) set -- -n "$ADDR" ;; + case "${ADDR}" in + *:*) set -- --ip "${ADDR}" ;; + *) set -- -n "${ADDR}" ;; esac ;; - *) set -- --ip "$ADDR" ;; + *) set -- --ip "${ADDR}" ;; esac # Set quota - case "$QUOTA" in - '') printf "Please specify \$QUOTA environment variable.\n"; exit 1 ;; - *) set -- "$@" --quota "$QUOTA" ;; + case "${QUOTA}" in + '') printf 'Please specify $QUOTA environment variable.\n'; exit 1 ;; + *) set -- "$@" --quota "${QUOTA}" ;; esac # Init the certificates and configs @@ -27,7 +27,20 @@ if [ ! -f "$confd/file-server.ini" ]; then fi # Backup store log just in case -[ -f "$logd/file-server-store.log" ] && cp "$logd"/file-server-store.log "$logd"/file-server-store.log."$(date +'%FT%T')" +# +# Uses the UTC (universal) time zone and this +# format: YYYY-mm-dd'T'HH:MM:SS +# year, month, day, letter T, hour, minute, second +# +# This is the ISO 8601 format without the time zone at the end. +# +_file="${logd}/file-server-store.log" +if [ -f "${_file}" ]; then + _backup_extension="$(date -u '+%Y-%m-%dT%H:%M:%S')" + cp -v -p "${_file}" "${_file}.${_backup_extension:-date-failed}" + unset -v _backup_extension +fi +unset -v _file # Finally, run xftp-sever. Notice that "exec" here is important: # smp-server replaces our helper script, so that it can catch INT signal From b747080db3f4ec507849d463ec4f52f0bc8ecabd Mon Sep 17 00:00:00 2001 From: spaced4ndy <8711996+spaced4ndy@users.noreply.github.com> Date: Fri, 2 Jun 2023 18:00:24 +0400 Subject: [PATCH 2/2] add more message delivery tests (#763) --- .gitignore | 4 + tests/AgentTests/FunctionalAPITests.hs | 123 +++++++++++++++++++++++-- 2 files changed, 118 insertions(+), 9 deletions(-) diff --git a/.gitignore b/.gitignore index 3e7898442..fe58f6e91 100644 --- a/.gitignore +++ b/.gitignore @@ -7,3 +7,7 @@ dist-newstyle/ cabal.project.local cabal.project.local~ + +.hpc/ +*.tix +.coverage diff --git a/tests/AgentTests/FunctionalAPITests.hs b/tests/AgentTests/FunctionalAPITests.hs index ea3bc918a..79bb4b6bb 100644 --- a/tests/AgentTests/FunctionalAPITests.hs +++ b/tests/AgentTests/FunctionalAPITests.hs @@ -61,6 +61,7 @@ import Simplex.Messaging.Server.Expiration import Simplex.Messaging.Transport (ATransport (..)) import Simplex.Messaging.Util (tryError) import Simplex.Messaging.Version +import System.Directory (copyFile, renameFile) import Test.Hspec import UnliftIO import XFTPClient (testXFTPServer) @@ -147,9 +148,13 @@ functionalAPITests t = do testAsyncServerOffline t it "should notify after HELLO timeout" $ withSmpServer t testAsyncHelloTimeout - describe "Duplicate message delivery" $ + describe "Message delivery" $ do it "should deliver messages to the user once, even if repeat delivery is made by the server (no ACK)" $ testDuplicateMessage t + it "should report error via msg integrity on skipped messages" $ + testSkippedMessages t + it "should report decryption error on ratchet becoming out of sync" $ + testDecryptionError t describe "Inactive client disconnection" $ do it "should disconnect clients if it was inactive longer than TTL" $ testInactiveClientDisconnected t @@ -163,8 +168,10 @@ functionalAPITests t = do it "should suspend agent on timeout, even if pending messages not sent" $ testSuspendingAgentTimeout t describe "Batching SMP commands" $ do - it "should subscribe to multiple subscriptions with batching" $ - testBatchedSubscriptions t + it "should subscribe to multiple (200) subscriptions with batching" $ + testBatchedSubscriptions 200 10 t + it "should subscribe to multiple (6) subscriptions with batching" $ + testBatchedSubscriptions 6 3 t describe "Async agent commands" $ do it "should connect using async agent commands" $ withSmpServer t testAsyncCommands @@ -491,6 +498,101 @@ testDuplicateMessage t = do get alice2 ##> ("", bobId, SENT 6) get bob2 =##> \case ("", c, Msg "hello 3") -> c == aliceId; _ -> False +testSkippedMessages :: HasCallStack => ATransport -> IO () +testSkippedMessages t = do + alice <- getSMPAgentClient' agentCfg initAgentServers testDB + bob <- getSMPAgentClient' agentCfg initAgentServers testDB2 + (aliceId, bobId) <- withSmpServerStoreLogOn t testPort $ \_ -> do + (aliceId, bobId) <- runRight $ makeConnection alice bob + runRight_ $ do + 4 <- sendMessage alice bobId SMP.noMsgFlags "hello" + get alice ##> ("", bobId, SENT 4) + get bob =##> \case ("", c, Msg "hello") -> c == aliceId; _ -> False + ackMessage bob aliceId 4 + + disconnectAgentClient bob + + runRight_ $ do + 5 <- sendMessage alice bobId SMP.noMsgFlags "hello 2" + get alice ##> ("", bobId, SENT 5) + 6 <- sendMessage alice bobId SMP.noMsgFlags "hello 3" + get alice ##> ("", bobId, SENT 6) + 7 <- sendMessage alice bobId SMP.noMsgFlags "hello 4" + get alice ##> ("", bobId, SENT 7) + + pure (aliceId, bobId) + + nGet alice =##> \case ("", "", DOWN _ [c]) -> c == bobId; _ -> False + threadDelay 200000 + + disconnectAgentClient alice + + alice2 <- getSMPAgentClient' agentCfg initAgentServers testDB + bob2 <- getSMPAgentClient' agentCfg initAgentServers testDB2 + + withSmpServerStoreLogOn t testPort $ \_ -> do + runRight_ $ do + subscribeConnection bob2 aliceId + subscribeConnection alice2 bobId + + 8 <- sendMessage alice2 bobId SMP.noMsgFlags "hello 5" + get alice2 ##> ("", bobId, SENT 8) + get bob2 =##> \case ("", c, MSG MsgMeta {integrity = MsgError {errorInfo = MsgSkipped {fromMsgId = 4, toMsgId = 6}}} _ "hello 5") -> c == aliceId; _ -> False + ackMessage bob2 aliceId 5 + + 9 <- sendMessage alice2 bobId SMP.noMsgFlags "hello 6" + get alice2 ##> ("", bobId, SENT 9) + get bob2 =##> \case ("", c, Msg "hello 6") -> c == aliceId; _ -> False + ackMessage bob2 aliceId 6 + +testDecryptionError :: HasCallStack => ATransport -> IO () +testDecryptionError t = do + alice <- getSMPAgentClient' agentCfg initAgentServers testDB + bob <- getSMPAgentClient' agentCfg initAgentServers testDB2 + withSmpServerStoreMsgLogOn t testPort $ \_ -> do + (aliceId, bobId) <- runRight $ makeConnection alice bob + runRight_ $ do + 4 <- sendMessage alice bobId SMP.noMsgFlags "hello" + get alice ##> ("", bobId, SENT 4) + get bob =##> \case ("", c, Msg "hello") -> c == aliceId; _ -> False + ackMessage bob aliceId 4 + + 5 <- sendMessage bob aliceId SMP.noMsgFlags "hello 2" + get bob ##> ("", aliceId, SENT 5) + get alice =##> \case ("", c, Msg "hello 2") -> c == bobId; _ -> False + ackMessage alice bobId 5 + + liftIO $ copyFile testDB2 (testDB2 <> ".bak") + + 6 <- sendMessage alice bobId SMP.noMsgFlags "hello 3" + get alice ##> ("", bobId, SENT 6) + get bob =##> \case ("", c, Msg "hello 3") -> c == aliceId; _ -> False + ackMessage bob aliceId 6 + + 7 <- sendMessage bob aliceId SMP.noMsgFlags "hello 4" + get bob ##> ("", aliceId, SENT 7) + get alice =##> \case ("", c, Msg "hello 4") -> c == bobId; _ -> False + ackMessage alice bobId 7 + + disconnectAgentClient bob + + -- importing database backup after progressing ratchet de-synchronizes ratchet, + -- this will be fixed by ratchet re-negotiation + liftIO $ renameFile (testDB2 <> ".bak") testDB2 + + bob2 <- getSMPAgentClient' agentCfg initAgentServers testDB2 + + runRight_ $ do + subscribeConnection bob2 aliceId + + 8 <- sendMessage alice bobId SMP.noMsgFlags "hello 5" + get alice ##> ("", bobId, SENT 8) + get bob2 =##> \case ("", c, ERR AGENT {agentErr = A_CRYPTO {cryptoErr = RATCHET_HEADER}}) -> c == aliceId; _ -> False + + 6 <- sendMessage bob2 aliceId SMP.noMsgFlags "hello 6" + get bob2 ##> ("", aliceId, SENT 6) + get alice =##> \case ("", c, ERR AGENT {agentErr = A_CRYPTO {cryptoErr = RATCHET_HEADER}}) -> c == bobId; _ -> False + makeConnection :: AgentClient -> AgentClient -> ExceptT AgentErrorType IO (ConnId, ConnId) makeConnection alice bob = makeConnectionForUsers alice 1 bob 1 @@ -612,14 +714,14 @@ testSuspendingAgentTimeout t = do ("", "", SUSPENDED) <- nGet b pure () -testBatchedSubscriptions :: ATransport -> IO () -testBatchedSubscriptions t = do +testBatchedSubscriptions :: Int -> Int -> ATransport -> IO () +testBatchedSubscriptions nCreate nDel t = do a <- getSMPAgentClient' agentCfg initAgentServers2 testDB b <- getSMPAgentClient' agentCfg initAgentServers2 testDB2 conns <- runServers $ do - conns <- forM [1 .. 200 :: Int] . const $ makeConnection a b + conns <- forM [1 .. nCreate :: Int] . const $ makeConnection a b forM_ conns $ \(aId, bId) -> exchangeGreetings a bId b aId - let (aIds', bIds') = unzip $ take 10 conns + let (aIds', bIds') = unzip $ take nDel conns delete a bIds' delete b aIds' liftIO $ threadDelay 1000000 @@ -635,11 +737,14 @@ testBatchedSubscriptions t = do ("", "", UP {}) <- nGet b liftIO $ threadDelay 1000000 let (aIds, bIds) = unzip conns - conns' = drop 10 conns + conns' = drop nDel conns (aIds', bIds') = unzip conns' subscribe a bIds subscribe b aIds forM_ conns' $ \(aId, bId) -> exchangeGreetingsMsgId 6 a bId b aId + void $ resubscribeConnections a bIds + void $ resubscribeConnections b aIds + forM_ conns' $ \(aId, bId) -> exchangeGreetingsMsgId 8 a bId b aId delete a bIds' delete b aIds' deleteFail a bIds' @@ -649,7 +754,7 @@ testBatchedSubscriptions t = do subscribe c cs = do r <- subscribeConnections c cs liftIO $ do - let dc = S.fromList $ take 10 cs + let dc = S.fromList $ take nDel cs all isRight (M.withoutKeys r dc) `shouldBe` True all (== Left (CONN NOT_FOUND)) (M.restrictKeys r dc) `shouldBe` True M.keys r `shouldMatchList` cs