Skip to content

Commit 774597f

Browse files
committed
WIP: Migrate displayException calls
Drop the backtraces when the resulting string is used in HTTP response bodies.
1 parent 6381307 commit 774597f

File tree

15 files changed

+225
-32
lines changed

15 files changed

+225
-32
lines changed

hls.json

Lines changed: 163 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,163 @@
1+
{
2+
"cabalFormattingProvider": "cabal-gild",
3+
"checkParents": "CheckOnSave",
4+
"checkProject": false,
5+
"formattingProvider": "ormolu",
6+
"maxCompletions": 40,
7+
"plugin": {
8+
"alternateNumberFormat": {
9+
"globalOn": true
10+
},
11+
"cabal": {
12+
"codeActionsOn": true,
13+
"completionOn": true,
14+
"diagnosticsOn": true,
15+
"hoverOn": true,
16+
"symbolsOn": true
17+
},
18+
"cabal-fmt": {
19+
"config": {
20+
"path": "cabal-fmt"
21+
}
22+
},
23+
"cabal-gild": {
24+
"config": {
25+
"path": "cabal-gild"
26+
}
27+
},
28+
"cabalHaskellIntegration": {
29+
"globalOn": true
30+
},
31+
"callHierarchy": {
32+
"globalOn": true
33+
},
34+
"changeTypeSignature": {
35+
"globalOn": true
36+
},
37+
"class": {
38+
"codeActionsOn": true,
39+
"codeLensOn": true
40+
},
41+
"eval": {
42+
"config": {
43+
"diff": true,
44+
"exception": false
45+
},
46+
"globalOn": true
47+
},
48+
"explicit-fields": {
49+
"codeActionsOn": true,
50+
"inlayHintsOn": true
51+
},
52+
"explicit-fixity": {
53+
"globalOn": true
54+
},
55+
"fourmolu": {
56+
"config": {
57+
"external": false,
58+
"path": "fourmolu"
59+
}
60+
},
61+
"gadt": {
62+
"globalOn": true
63+
},
64+
"ghcide-code-actions-bindings": {
65+
"globalOn": true
66+
},
67+
"ghcide-code-actions-fill-holes": {
68+
"globalOn": true
69+
},
70+
"ghcide-code-actions-imports-exports": {
71+
"globalOn": true
72+
},
73+
"ghcide-code-actions-type-signatures": {
74+
"globalOn": true
75+
},
76+
"ghcide-completions": {
77+
"config": {
78+
"autoExtendOn": true,
79+
"snippetsOn": true
80+
},
81+
"globalOn": true
82+
},
83+
"ghcide-hover-and-symbols": {
84+
"hoverOn": true,
85+
"symbolsOn": true
86+
},
87+
"ghcide-type-lenses": {
88+
"config": {
89+
"mode": "always"
90+
},
91+
"globalOn": true
92+
},
93+
"hlint": {
94+
"codeActionsOn": true,
95+
"config": {
96+
"flags": []
97+
},
98+
"diagnosticsOn": true
99+
},
100+
"importLens": {
101+
"codeActionsOn": true,
102+
"codeLensOn": false,
103+
"inlayHintsOn": true
104+
},
105+
"moduleName": {
106+
"globalOn": true
107+
},
108+
"ormolu": {
109+
"config": {
110+
"external": false
111+
}
112+
},
113+
"overloaded-record-dot": {
114+
"globalOn": true
115+
},
116+
"pragmas-completion": {
117+
"globalOn": true
118+
},
119+
"pragmas-disable": {
120+
"globalOn": true
121+
},
122+
"pragmas-suggest": {
123+
"globalOn": true
124+
},
125+
"qualifyImportedNames": {
126+
"globalOn": true
127+
},
128+
"rename": {
129+
"config": {
130+
"crossModule": false
131+
},
132+
"globalOn": true
133+
},
134+
"retrie": {
135+
"globalOn": true
136+
},
137+
"semanticTokens": {
138+
"config": {
139+
"classMethodToken": "method",
140+
"classToken": "class",
141+
"dataConstructorToken": "enumMember",
142+
"functionToken": "function",
143+
"moduleToken": "namespace",
144+
"operatorToken": "operator",
145+
"patternSynonymToken": "macro",
146+
"recordFieldToken": "property",
147+
"typeConstructorToken": "enum",
148+
"typeFamilyToken": "interface",
149+
"typeSynonymToken": "type",
150+
"typeVariableToken": "typeParameter",
151+
"variableToken": "variable"
152+
},
153+
"globalOn": true
154+
},
155+
"splice": {
156+
"globalOn": true
157+
},
158+
"stan": {
159+
"globalOn": false
160+
}
161+
},
162+
"sessionLoading": "singleComponent"
163+
}

libs/saml2-web-sso/saml2-web-sso.cabal

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,7 @@ library
145145
, uuid >=1.3.13
146146
, wai >=3.2.2.1
147147
, wai-extra >=3.0.28
148+
, wai-utilities
148149
, warp >=3.2.28
149150
, word8 >=0.1.3
150151
, xml-conduit >=1.8.0.1

libs/saml2-web-sso/src/Text/XML/DSig.hs

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@ import Data.UUID as UUID
7272
import Data.X509 qualified as X509
7373
import GHC.Stack
7474
import Network.URI (URI (..), parseRelativeReference)
75+
import Network.Wai.Utilities.Exception
7576
import SAML2.XML qualified as HS hiding (Node, URI)
7677
import SAML2.XML.Canonical qualified as HS
7778
import SAML2.XML.Signature qualified as HS
@@ -145,7 +146,7 @@ parseKeyInfo doVerify (cs @LT @LBS -> lbs) = case HS.xmlToSAML @HS.KeyInfo =<< s
145146

146147
-- | Call 'stripWhitespaceDoc' on a rendered bytestring.
147148
stripWhitespaceLBS :: (m ~ Either String) => LBS -> m LBS
148-
stripWhitespaceLBS lbs = renderLBS def . stripWhitespace <$> fmapL show (parseLBS def lbs)
149+
stripWhitespaceLBS lbs = renderLBS def . stripWhitespace <$> fmapL displayExceptionNoBacktrace (parseLBS def lbs)
149150

150151
renderKeyInfo :: (HasCallStack) => X509.SignedCertificate -> LT
151152
renderKeyInfo cert = cs . ourSamlToXML . HS.KeyInfo Nothing $ HS.X509Data (HS.X509Certificate cert :| []) :| []
@@ -224,16 +225,16 @@ mkSignCredsWithCert mValidSince size = do
224225
verify :: forall m. (MonadError String m) => NonEmpty SignCreds -> LBS -> String -> m HXTC.XmlTree
225226
verify creds el sid = case unsafePerformIO (try @SomeException $ verifyIO creds el sid) of
226227
Right (_, Right xml) -> pure xml
227-
Right (_, Left exc) -> throwError $ show exc
228-
Left exc -> throwError $ show exc
228+
Right (_, Left signErr) -> throwError $ show signErr
229+
Left exc -> throwError $ displayExceptionNoBacktrace exc
229230

230231
-- | Convenient wrapper that picks the ID of the root element node and passes it to `verify`.
231232
verifyRoot :: forall m. (MonadError String m) => NonEmpty SignCreds -> LBS -> m HXTC.XmlTree
232233
verifyRoot creds el = do
233234
signedID <- do
234235
XML.Document _ (XML.Element _ attrs _) _ <-
235236
either
236-
(throwError . ("Could not parse signed document: " <>) . cs . show)
237+
(throwError . ("Could not parse signed document: " <>) . cs . displayExceptionNoBacktrace)
237238
pure
238239
(XML.parseLBS XML.def el)
239240
maybe
@@ -272,7 +273,7 @@ verifySignatureUnenvelopedSigs :: HS.PublicKeys -> String -> HXTC.XmlTree -> IO
272273
verifySignatureUnenvelopedSigs pks xid doc = catchAll $ warpResult <$> verifySignature pks xid doc
273274
where
274275
catchAll :: IO (Either HS.SignatureError a) -> IO (Either HS.SignatureError a)
275-
catchAll = handle $ pure . Left . HS.SignatureVerificationLegacyFailure . Left . (show @SomeException)
276+
catchAll = handle $ pure . Left . HS.SignatureVerificationLegacyFailure . Left . (displayExceptionNoBacktrace @SomeException)
276277

277278
warpResult :: Maybe HXTC.XmlTree -> Either HS.SignatureError HXTC.XmlTree
278279
warpResult (Just xml) = Right xml
@@ -413,7 +414,7 @@ signRootAt sigPos (SignPrivCreds hashAlg (SignPrivKeyRSA keypair)) doc =
413414
}
414415
]
415416
docCanonic :: SBS <-
416-
either (throwError . show) (pure . cs) . unsafePerformIO . try @SomeException $
417+
either (throwError . displayExceptionNoBacktrace) (pure . cs) . unsafePerformIO . try @SomeException $
417418
HS.applyTransforms transforms (HXT.mkRoot [] [docInHXT])
418419
let digest :: SBS
419420
digest = case hashAlg of
@@ -437,7 +438,7 @@ signRootAt sigPos (SignPrivCreds hashAlg (SignPrivKeyRSA keypair)) doc =
437438
-- (note that there are two rounds of SHA256 application, hence two mentions of the has alg here)
438439

439440
signedInfoSBS :: SBS <-
440-
either (throwError . show) (pure . cs) . unsafePerformIO . try @SomeException $
441+
either (throwError . displayExceptionNoBacktrace) (pure . cs) . unsafePerformIO . try @SomeException $
441442
HS.applyCanonicalization (HS.signedInfoCanonicalizationMethod signedInfo) Nothing $
442443
HS.samlToDoc signedInfo
443444
sigval :: SBS <-
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
{-# LANGUAGE ImplicitParams #-}
2+
3+
module Network.Wai.Utilities.Exception where
4+
5+
import Control.Exception
6+
import Imports
7+
8+
-- | `displayException` with empty `ExceptionContext`
9+
--
10+
-- Starting with GHC 9.10, exceptions carry a context that contains backtraces.
11+
-- Displaying these is not always desired; e.g. for HTTP response bodies.
12+
displayExceptionNoBacktrace :: (Exception e) => e -> String
13+
displayExceptionNoBacktrace = trim . displayException . toException
14+
where
15+
trim = (dropWhileEnd isSpace) . (dropWhile isSpace)

libs/wai-utilities/wai-utilities.cabal

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ library
6565
exposed-modules:
6666
Network.Wai.Utilities
6767
Network.Wai.Utilities.Error
68+
Network.Wai.Utilities.Exception
6869
Network.Wai.Utilities.Headers
6970
Network.Wai.Utilities.JSONResponse
7071
Network.Wai.Utilities.MockServer

libs/wire-api-federation/src/Wire/API/Federation/Error.hs

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
--
1515
-- You should have received a copy of the GNU Affero General Public License along
1616
-- with this program. If not, see <https://www.gnu.org/licenses/>.
17+
{-# LANGUAGE ImplicitParams #-}
1718

1819
-- | Map federation errors to client-facing errors.
1920
--
@@ -95,6 +96,7 @@ import Network.HTTP.Types.Status
9596
import Network.HTTP.Types.Status qualified as HTTP
9697
import Network.HTTP2.Client qualified as HTTP2
9798
import Network.Wai.Utilities.Error qualified as Wai
99+
import Network.Wai.Utilities.Exception
98100
import OpenSSL.Session (SomeSSLException)
99101
import Servant.Client
100102
import Wire.API.Error
@@ -227,21 +229,21 @@ federationRemoteHTTP2Error target path = \case
227229
( Wai.mkError
228230
unexpectedFederationResponseStatus
229231
"federation-http2-error"
230-
(LT.pack (displayException e))
232+
(LT.pack (displayExceptionNoBacktrace e))
231233
)
232234
& addErrData
233235
(FederatorClientTLSException e) ->
234236
( Wai.mkError
235237
(HTTP.mkStatus 525 "SSL Handshake Failure")
236238
"federation-tls-error"
237-
(LT.pack (displayException e))
239+
(LT.pack (displayExceptionNoBacktrace e))
238240
)
239241
& addErrData
240242
(FederatorClientConnectionError e) ->
241243
( Wai.mkError
242244
federatorConnectionRefusedStatus
243245
"federation-connection-refused"
244-
(LT.pack (displayException e))
246+
(LT.pack (displayExceptionNoBacktrace e))
245247
)
246248
& addErrData
247249
where
@@ -259,12 +261,12 @@ federationClientHTTP2Error (FederatorClientConnectionError e) =
259261
Wai.mkError
260262
HTTP.status500
261263
"federation-not-available"
262-
(LT.pack (displayException e))
264+
(LT.pack (displayExceptionNoBacktrace e))
263265
federationClientHTTP2Error e =
264266
Wai.mkError
265267
HTTP.status500
266268
"federation-local-error"
267-
(LT.pack (displayException e))
269+
(LT.pack (displayExceptionNoBacktrace e))
268270

269271
federationRemoteResponseError :: SrvTarget -> Text -> HTTP.Status -> LByteString -> Wai.Error
270272
federationRemoteResponseError target path status body =
@@ -310,7 +312,7 @@ federationServantErrorToWai (UnsupportedContentType mediaType res) =
310312
<> LT.pack (show mediaType)
311313
)
312314
federationServantErrorToWai (ConnectionError e) =
313-
federationUnavailable . T.pack . displayException $ e
315+
federationUnavailable . T.pack . displayExceptionNoBacktrace $ e
314316

315317
federationErrorContentType :: ResponseF a -> LT.Text
316318
federationErrorContentType =

libs/wire-api/src/Wire/API/MLS/Group/Serialisation.hs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ import Data.Text qualified as T
3939
import Data.Text.Encoding qualified as T
4040
import Data.UUID qualified as UUID
4141
import Imports
42+
import Network.Wai.Utilities.Exception
4243
import Web.HttpApiData (FromHttpApiData (parseHeader))
4344
import Wire.API.Conversation
4445
import Wire.API.MLS.Group
@@ -116,7 +117,7 @@ getParts = do
116117
eDomain <-
117118
T.decodeUtf8' . L.toStrict
118119
<$> getRemainingLazyByteString
119-
domain <- either (fail . displayException) pure eDomain
120+
domain <- either (fail . displayExceptionNoBacktrace) pure eDomain
120121
pure
121122
GroupIdParts
122123
{ convType,
@@ -148,7 +149,7 @@ getDomain = do
148149
len <- fromIntegral <$> getWord16be
149150
domain <- T.decodeUtf8' <$> getByteString len
150151
case domain of
151-
Left e -> fail (displayException e)
152+
Left e -> fail (displayExceptionNoBacktrace e)
152153
Right d -> pure (Domain d)
153154

154155
newGroupId :: ConvType -> Qualified ConvOrSubConvId -> GroupId

libs/wire-api/src/Wire/API/Team/Export.hs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ import Data.Time.Clock
3535
import Data.Time.Format
3636
import Data.Vector (fromList)
3737
import Imports
38+
import Network.Wai.Utilities.Exception
3839
import Test.QuickCheck
3940
import Wire.API.Team.Role (Role)
4041
import Wire.API.User (AccountStatus (..), Name)
@@ -150,7 +151,7 @@ parseByteString bstr =
150151

151152
parseUTCTime :: ByteString -> Parser UTCTime
152153
parseUTCTime b = do
153-
s <- either (fail . displayException) pure $ T.decodeUtf8' b
154+
s <- either (fail . displayExceptionNoBacktrace) pure $ T.decodeUtf8' b
154155
parseTimeM False defaultTimeLocale timestampFormat (T.unpack s)
155156

156157
parseAccountStatus :: ByteString -> Parser AccountStatus

libs/wire-subsystems/src/Wire/Error.hs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import Hasql.Pool
1010
import Imports
1111
import Network.HTTP.Types
1212
import Network.Wai.Utilities.Error qualified as Wai
13+
import Network.Wai.Utilities.Exception
1314
import Network.Wai.Utilities.JSONResponse
1415

1516
-- | Error thrown to the user
@@ -49,6 +50,6 @@ postgresUsageErrorToHttpError err = case err of
4950
-- return "404 not found", not "database crashed"?
5051
-- The problem is that the SessionError is not typed to easily be parsed
5152
-- To prevent foreign key errors we should check the foreign key constraints before inserting
52-
StdError (Wai.mkError status500 "server-error" (LT.pack $ "postgres: " <> show err))
53-
ConnectionUsageError _ -> StdError (Wai.mkError status500 "server-error" (LT.pack $ "postgres: " <> show err))
54-
AcquisitionTimeoutUsageError -> StdError (Wai.mkError status500 "server-error" (LT.pack $ "postgres: " <> show err))
53+
StdError (Wai.mkError status500 "server-error" (LT.pack $ "postgres: " <> displayExceptionNoBacktrace err))
54+
ConnectionUsageError _ -> StdError (Wai.mkError status500 "server-error" (LT.pack $ "postgres: " <> displayExceptionNoBacktrace err))
55+
AcquisitionTimeoutUsageError -> StdError (Wai.mkError status500 "server-error" (LT.pack $ "postgres: " <> displayExceptionNoBacktrace err))

0 commit comments

Comments
 (0)