Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent list/set member from targeting container #162

Merged
merged 1 commit into from
Sep 13, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions docs/source/spec/core.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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 <member>` of the list.
that defines the :ref:`member <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 <prelude>`:
Expand Down Expand Up @@ -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 <member>` of the set.
that defines the :ref:`member <member>` of the set. The member of a set
cannot target the containing set shape.

The following example defines a set of strings:

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,9 @@
*/
public class TargetValidator extends AbstractValidator {

/** Valid member shape targets. */
private static final Set<ShapeType> INVALID_MEMBER_TARGETS = SetUtils.of(
ShapeType.SERVICE, ShapeType.RESOURCE, ShapeType.OPERATION, ShapeType.MEMBER);
private static final Set<ShapeType> INVALID_RECURSIVE_MEMBER_TARGETS = SetUtils.of(ShapeType.LIST, ShapeType.SET);

@Override
public List<ValidationEvent> validate(Model model) {
Expand Down Expand Up @@ -85,6 +85,10 @@ private Optional<ValidationEvent> 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));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,12 @@
"target": "MyService"
}
},
"InvalidRecursiveList": {
"type": "list",
"member": {
"target": "InvalidRecursiveList"
}
},
"MyService": {
"type": "service",
"version": "2017-01-17",
Expand Down