git.iko.soy/typeable/compaREST
on master help sign in
Detect unguarded recursion when inspecting object properties and similar things (#128)

* Do check for unguarded recursion in the "silent" branches of schema processing * Add test * Make test more throrough * Fixed tests Signed-off-by: iko <ilyakooo0@gmail.com> Co-authored-by: iko <ilyakooo0@gmail.com>

commitdb716ee5b2d6e5649042fa32f4cff4d211d92064 authormniip <mniip@users.noreply.github.com> dateMon 24 Jan 2022, 17:50 (5 years ago) parent5727f6b
11 files changed+88−19
compaREST.cabal+1−1 src/Data/OpenApi/Compare/Validate/Schema/Process.hs+10−14 test/Spec/Golden/TraceTree.hs+2−4 test/golden/common/unguarded-recursive/a.yaml → test/golden/common/unguarded-recursive/basic/a.yaml test/golden/common/unguarded-recursive/b.yaml → test/golden/common/unguarded-recursive/basic/b.yaml test/golden/common/unguarded-recursive/report.md → test/golden/common/unguarded-recursive/basic/report.md test/golden/common/unguarded-recursive/trace-tree.yaml → test/golden/common/unguarded-recursive/basic/trace-tree.yaml test/golden/common/unguarded-recursive/silent/a.yaml+25 test/golden/common/unguarded-recursive/silent/b.yaml+19 test/golden/common/unguarded-recursive/silent/report.md+15 test/golden/common/unguarded-recursive/silent/trace-tree.yaml+16
compaREST.cabal+1−1
@@ -14,7 +14,7 @@ extra-doc-files: README.md
1414 CHANGELOG.md
1515tested-with: GHC == 8.10.4
1616extra-source-files: awsm-css/dist/awsm.min.css
17−data-files:
17+extra-source-files:
1818 test/golden/**/*.yaml
1919 test/golden/**/*.md
2020
src/Data/OpenApi/Compare/Validate/Schema/Process.hs+10−14
@@ -39,15 +39,11 @@ instance Monoid w => MonadWriter w (Silent w) where
3939 listen (Silent x) = Silent (x, mempty)
4040 pass (Silent (x, _)) = Silent x
4141
42−instance MonadState (MemoState ()) (Silent w) where
43− get = Silent $ runIdentity $ runMemo () get
44− put _ = pure ()
45−
4642type ProcessM = StateT (MemoState ()) (ReaderT (Traced (Definitions Schema)) (Writer (P.PathsPrefixTree Behave AnIssue 'SchemaLevel)))
4743
48−type SilentM = ReaderT (Traced (Definitions Schema)) (Silent (P.PathsPrefixTree Behave AnIssue 'SchemaLevel))
44+type SilentM = StateT (MemoState ()) (ReaderT (Traced (Definitions Schema)) (Silent (P.PathsPrefixTree Behave AnIssue 'SchemaLevel)))
4945
50−-- Either SilentM or ProcessM
46+-- Either ProcessM or SilentM
5147type MonadProcess m =
5248 ( MonadReader (Traced (Definitions Schema)) m
5349 , MonadWriter (P.PathsPrefixTree Behave AnIssue 'SchemaLevel) m
@@ -57,11 +53,11 @@ type MonadProcess m =
5753warn :: MonadProcess m => Issue 'SchemaLevel -> m ()
5854warn issue = tell $ P.singleton $ AnItem Root $ anIssue issue
5955
60−-- | Ignore warnings but allow a recursive loop that lazily computes a recursive 'Condition'.
61−silently :: MonadProcess m => SilentM a -> m a
62−silently m = do
56+-- Perform a computation lazily, ignoring the warnings and discarding memoization/loop detection information.
57+lazily :: MonadProcess m => SilentM a -> m a
58+lazily m = do
6359 defs <- R.ask
64− pure . runSilent $ runReaderT m defs
60+ pure $ runSilent $ runReaderT (runMemo () m) defs
6561
6662warnKnot :: MonadProcess m => KnotTier (ForeachType JsonFormula) () m
6763warnKnot =
@@ -243,11 +239,11 @@ processSchema sch@(extract -> Schema {..}) = do
243239 itemsClause <- case tracedItems sch of
244240 Nothing -> pure top
245241 Just (Left rs) -> do
246− f <- silently $ processRefSchema rs
242+ f <- lazily $ processRefSchema rs
247243 pure top {forArray = singletonFormula $ Items f rs}
248244 Just (Right rss) -> do
249245 fsrs <- forM rss $ \rs -> do
250− f <- silently $ processRefSchema rs
246+ f <- lazily $ processRefSchema rs
251247 pure (f, rs)
252248 pure top {forArray = singletonFormula $ TupleItems fsrs}
253249
@@ -273,12 +269,12 @@ processSchema sch@(extract -> Schema {..}) = do
273269 _ -> top
274270
275271 (addProps, addPropSchema) <- case tracedAdditionalProperties sch of
276− Just (Right rs) -> (,Just rs) <$> silently (processRefSchema rs)
272+ Just (Right rs) -> (,Just rs) <$> lazily (processRefSchema rs)
277273 Just (Left False) -> pure (bottom, Nothing)
278274 _ -> pure (top, Just $ traced (ask sch `Snoc` AdditionalPropertiesStep) $ Inline mempty)
279275 propList <- forM (S.toList . S.fromList $ IOHM.keys _schemaProperties <> _schemaRequired) $ \k -> do
280276 (f, psch) <- case IOHM.lookup k $ tracedProperties sch of
281− Just rs -> (,rs) <$> silently (processRefSchema rs)
277+ Just rs -> (,rs) <$> lazily (processRefSchema rs)
282278 Nothing ->
283279 let fakeSchema = traced (ask sch `Snoc` AdditionalPropertiesStep) $ Inline mempty
284280 in -- The mempty here is incorrect, but if addPropSchema was Nothing, then
test/Spec/Golden/TraceTree.hs+2−4
@@ -12,7 +12,6 @@ import Data.OpenApi.Compare.Validate.OpenApi ()
1212import Data.Text (Text)
1313import qualified Data.Text.Encoding as T
1414import qualified Data.Yaml as Yaml
15−import Paths_compaREST
1615import Spec.Golden.Extra
1716import Test.Tasty (TestTree, testGroup)
1817import Text.Pandoc.Builder
@@ -23,11 +22,10 @@ import Prelude hiding (id, (.))
2322
2423tests :: IO TestTree
2524tests = do
26− dir <- getDataDir
2725 traceTreeTests <-
2826 goldenInputsTreeUniform
2927 "TraceTree"
30− dir
28+ "test/golden"
3129 "trace-tree.yaml"
3230 ("a.yaml", "b.yaml")
3331 Yaml.decodeFileThrow
@@ -36,7 +34,7 @@ tests = do
3634 reportTests <-
3735 goldenInputsTreeUniform
3836 "Report"
39− dir
37+ "test/golden"
4038 "report.md"
4139 ("a.yaml", "b.yaml")
4240 Yaml.decodeFileThrow
test/golden/common/unguarded-recursive/a.yaml → test/golden/common/unguarded-recursive/basic/a.yamlrenamed

The content is the same.

test/golden/common/unguarded-recursive/b.yaml → test/golden/common/unguarded-recursive/basic/b.yamlrenamed

The content is the same.

test/golden/common/unguarded-recursive/report.md → test/golden/common/unguarded-recursive/basic/report.mdrenamed

The content is the same.

test/golden/common/unguarded-recursive/trace-tree.yaml → test/golden/common/unguarded-recursive/basic/trace-tree.yamlrenamed

The content is the same.

test/golden/common/unguarded-recursive/silent/a.yamlnew file+25
@@ -0,0 +1,25 @@
1+components:
2+ schemas:
3+ A:
4+ type: object
5+ properties:
6+ bar:
7+ $ref: "#/components/schemas/B"
8+ B:
9+ anyOf:
10+ - $ref: "#/components/schemas/B"
11+ type: object
12+openapi: 3.0.0
13+info:
14+ version: ""
15+ title: ""
16+paths:
17+ /foo:
18+ get:
19+ responses:
20+ "200":
21+ content:
22+ application/json;charset=utf-8:
23+ schema:
24+ $ref: "#/components/schemas/A"
25+ description: ""
test/golden/common/unguarded-recursive/silent/b.yamlnew file+19
@@ -0,0 +1,19 @@
1+components:
2+ schemas: {}
3+openapi: 3.0.0
4+info:
5+ version: ""
6+ title: ""
7+paths:
8+ /foo:
9+ get:
10+ responses:
11+ "200":
12+ content:
13+ application/json;charset=utf-8:
14+ schema:
15+ type: object
16+ properties:
17+ bar:
18+ type: object
19+ description: ""
test/golden/common/unguarded-recursive/silent/report.mdnew file+15
@@ -0,0 +1,15 @@
1+# Summary
2+
3+| ❌ Breaking changes | ⚠️ Non-breaking changes | [❓ Unsupported feature changes](#unsupported-changes) |
4+|---------------------|-------------------------|--------------------------------------------------------|
5+| 0 | 0 | 1 |
6+
7+# <span id="unsupported-changes"></span>❓ Unsupported feature changes
8+
9+## **GET** /foo
10+
11+### ⬅️☁️ JSON Response – 200
12+
13+#### `$.bar`
14+
15+Encountered recursion that is too complex for CompaREST to untangle.
test/golden/common/unguarded-recursive/silent/trace-tree.yamlnew file+16
@@ -0,0 +1,16 @@
1+forwardChanges:
2+ AtPath "/foo":
3+ InOperation GetMethod:
4+ WithStatusCode 200:
5+ ResponsePayload:
6+ PayloadSchema:
7+ OfType Object:
8+ InProperty "bar": UnguardedRecursion
9+backwardChanges:
10+ AtPath "/foo":
11+ InOperation GetMethod:
12+ WithStatusCode 200:
13+ ResponsePayload:
14+ PayloadSchema:
15+ OfType Object:
16+ InProperty "bar": UnguardedRecursion