From b50bd2b9c8c77848a065ad68664e7793ee336a7e Mon Sep 17 00:00:00 2001 From: kstich Date: Fri, 13 Sep 2019 11:22:55 -0700 Subject: [PATCH] Prevent list/set member from targeting container --- docs/source/spec/core.rst | 6 ++++-- .../smithy/model/validation/validators/TargetValidator.java | 6 +++++- .../model/errorfiles/validators/target-validator.errors | 1 + .../model/errorfiles/validators/target-validator.json | 6 ++++++ 4 files changed, 16 insertions(+), 3 deletions(-) diff --git a/docs/source/spec/core.rst b/docs/source/spec/core.rst index abdb7faaecc..08155fbbfc4 100644 --- a/docs/source/spec/core.rst +++ b/docs/source/spec/core.rst @@ -415,7 +415,8 @@ list The :dfn:`list` type represents a homogeneous collection of values. A list is defined using a :token:`list_statement`. A list statement consists of the shape named followed by an object with a single key-value pair of "member" -that defines the :ref:`member ` of the list. +that defines the :ref:`member ` of the list. The member of a list +cannot target the containing list shape. The following example defines a list with a string member from the :ref:`prelude `: @@ -513,7 +514,8 @@ set The :dfn:`set` type represents an unordered collection of unique homogeneous values. A set is defined using a :token:`set_statement` that consists of the shape named followed by an object with a single key-value pair of "member" -that defines the :ref:`member ` of the set. +that defines the :ref:`member ` of the set. The member of a set +cannot target the containing set shape. The following example defines a set of strings: diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/validation/validators/TargetValidator.java b/smithy-model/src/main/java/software/amazon/smithy/model/validation/validators/TargetValidator.java index b8fcba5108d..ffafc7857cf 100644 --- a/smithy-model/src/main/java/software/amazon/smithy/model/validation/validators/TargetValidator.java +++ b/smithy-model/src/main/java/software/amazon/smithy/model/validation/validators/TargetValidator.java @@ -45,9 +45,9 @@ */ public class TargetValidator extends AbstractValidator { - /** Valid member shape targets. */ private static final Set INVALID_MEMBER_TARGETS = SetUtils.of( ShapeType.SERVICE, ShapeType.RESOURCE, ShapeType.OPERATION, ShapeType.MEMBER); + private static final Set INVALID_RECURSIVE_MEMBER_TARGETS = SetUtils.of(ShapeType.LIST, ShapeType.SET); @Override public List validate(Model model) { @@ -85,6 +85,10 @@ private Optional validateTarget(ShapeIndex index, Shape shape, if (INVALID_MEMBER_TARGETS.contains(target.getType())) { return Optional.of(error(shape, format( "Members cannot target %s shapes, but found %s", target.getType(), target))); + } else if (target.getId().equals(shape.getId().withoutMember()) + && INVALID_RECURSIVE_MEMBER_TARGETS.contains(target.getType())) { + return Optional.of(error(shape, format( + "%s shape members cannot target their container", target.getType()))); } case MAP_KEY: return target.asMemberShape().flatMap(m -> validateMapKey(shape, m.getTarget(), index)); diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/target-validator.errors b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/target-validator.errors index 801a679c06f..4c5f4b53980 100644 --- a/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/target-validator.errors +++ b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/target-validator.errors @@ -8,6 +8,7 @@ [ERROR] ns.foo#InvalidListMemberReference$member: member shape targets an unresolved shape `ns.foo#NotFound` | Target [ERROR] ns.foo#InvalidListMemberResource$member: Members cannot target resource shapes, but found (resource: `ns.foo#MyResource`) | Target [ERROR] ns.foo#InvalidListMemberService$member: Members cannot target service shapes, but found (service: `ns.foo#MyService`) | Target +[ERROR] ns.foo#InvalidRecursiveList$member: list shape members cannot target their container | Target [ERROR] ns.foo#InvalidMapType: Map key member targets (structure: `ns.foo#ValidInput`), but is expected to target a string | Target [ERROR] ns.foo#InvalidOperationBadErrorTraits: Operation error targets an invalid structure, `ns.foo#ValidInput`, that is not marked with the `error` trait. | Target [ERROR] ns.foo#InvalidOperationBadErrorTraits: Operation input targets an invalid structure `ns.foo#ValidError` that is marked with the `error` trait. | Target diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/target-validator.json b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/target-validator.json index 44250ce0f81..c3b1dd50d64 100644 --- a/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/target-validator.json +++ b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/target-validator.json @@ -32,6 +32,12 @@ "target": "MyService" } }, + "InvalidRecursiveList": { + "type": "list", + "member": { + "target": "InvalidRecursiveList" + } + }, "MyService": { "type": "service", "version": "2017-01-17",