From eb5ea74052360c816b88f33680116e545f01c8a7 Mon Sep 17 00:00:00 2001 From: Robert Varga Date: Fri, 27 Nov 2020 17:02:43 +0100 Subject: [PATCH] Enforce augment statement argument well-formedness 'augment' statement has dual use, depending on where it is declared. Each use requires a different schema node identifier: - plain augment requires SchemaNodeIdentifier.Absolute - uses/augment requires SchemaNodeIdentifier.Descendant Make sure we enforce proper parsing, so that our argument will end up being correctly captured (and can be used for further reasoning). JIRA: YANGTOOLS-1189 Change-Id: I4e628fd1aee29e103a045cfe90bae58f91ce32ed Signed-off-by: Robert Varga --- .../AbstractAugmentStatementSupport.java | 21 +++++++++- .../yangtools/yang/stmt/YT1189Test.java | 39 +++++++++++++++++++ .../src/test/resources/bugs/YT1189/bar.yang | 17 ++++++++ .../src/test/resources/bugs/YT1189/foo.yang | 11 ++++++ .../test/resources/bugs/bug5481/module2.yang | 2 +- 5 files changed, 88 insertions(+), 2 deletions(-) create mode 100644 yang/yang-parser-rfc7950/src/test/java/org/opendaylight/yangtools/yang/stmt/YT1189Test.java create mode 100644 yang/yang-parser-rfc7950/src/test/resources/bugs/YT1189/bar.yang create mode 100644 yang/yang-parser-rfc7950/src/test/resources/bugs/YT1189/foo.yang diff --git a/yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/augment/AbstractAugmentStatementSupport.java b/yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/augment/AbstractAugmentStatementSupport.java index fc62ae4cc1..9966b17432 100644 --- a/yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/augment/AbstractAugmentStatementSupport.java +++ b/yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/augment/AbstractAugmentStatementSupport.java @@ -25,10 +25,13 @@ import org.opendaylight.yangtools.yang.model.api.Status; import org.opendaylight.yangtools.yang.model.api.YangStmtMapping; import org.opendaylight.yangtools.yang.model.api.meta.DeclaredStatement; import org.opendaylight.yangtools.yang.model.api.meta.EffectiveStatement; +import org.opendaylight.yangtools.yang.model.api.meta.StatementDefinition; import org.opendaylight.yangtools.yang.model.api.stmt.AugmentEffectiveStatement; import org.opendaylight.yangtools.yang.model.api.stmt.AugmentStatement; import org.opendaylight.yangtools.yang.model.api.stmt.DataDefinitionStatement; import org.opendaylight.yangtools.yang.model.api.stmt.SchemaNodeIdentifier; +import org.opendaylight.yangtools.yang.model.api.stmt.SchemaNodeIdentifier.Absolute; +import org.opendaylight.yangtools.yang.model.api.stmt.SchemaNodeIdentifier.Descendant; import org.opendaylight.yangtools.yang.model.api.stmt.StatusEffectiveStatement; import org.opendaylight.yangtools.yang.model.api.stmt.UsesStatement; import org.opendaylight.yangtools.yang.model.api.stmt.WhenEffectiveStatement; @@ -72,7 +75,23 @@ abstract class AbstractAugmentStatementSupport "Augment argument \'%s\' is not valid, it can be only absolute path; or descendant if used in uses", value); - return ArgumentUtils.nodeIdentifierFromPath(ctx, value); + // As per: + // https://tools.ietf.org/html/rfc6020#section-7.15 + // https://tools.ietf.org/html/rfc7950#section-7.17 + // + // The argument is either Absolute or Descendant based on whether the statement is declared within a 'uses' + // statement. The mechanics differs wildly between the two cases, so let's start by ensuring our argument + // is in the correct domain. + final SchemaNodeIdentifier result = ArgumentUtils.nodeIdentifierFromPath(ctx, value); + final StatementDefinition parent = ctx.coerceParentContext().publicDefinition(); + if (parent == YangStmtMapping.USES) { + SourceException.throwIf(result instanceof Absolute, ctx.sourceReference(), + "Absolute schema node identifier is not allowed when used within a uses statement"); + } else { + SourceException.throwIf(result instanceof Descendant, ctx.sourceReference(), + "Descendant schema node identifier is not allowed when used outside of a uses statement"); + } + return result; } @Override diff --git a/yang/yang-parser-rfc7950/src/test/java/org/opendaylight/yangtools/yang/stmt/YT1189Test.java b/yang/yang-parser-rfc7950/src/test/java/org/opendaylight/yangtools/yang/stmt/YT1189Test.java new file mode 100644 index 0000000000..c87205066e --- /dev/null +++ b/yang/yang-parser-rfc7950/src/test/java/org/opendaylight/yangtools/yang/stmt/YT1189Test.java @@ -0,0 +1,39 @@ +/* + * Copyright (c) 2020 PANTHEON.tech, s.r.o. and others. All rights reserved. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License v1.0 which accompanies this distribution, + * and is available at http://www.eclipse.org/legal/epl-v10.html + */ +package org.opendaylight.yangtools.yang.stmt; + +import static org.hamcrest.CoreMatchers.instanceOf; +import static org.hamcrest.CoreMatchers.startsWith; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.junit.Assert.assertThrows; + +import org.junit.Test; +import org.opendaylight.yangtools.yang.parser.spi.meta.ReactorException; +import org.opendaylight.yangtools.yang.parser.spi.source.SourceException; + +public class YT1189Test { + @Test + public void testDescendantAugment() { + final ReactorException ex = assertThrows(ReactorException.class, + () -> StmtTestUtils.parseYangSource("/bugs/YT1189/foo.yang")); + final Throwable cause = ex.getCause(); + assertThat(cause, instanceOf(SourceException.class)); + assertThat(cause.getMessage(), + startsWith("Descendant schema node identifier is not allowed when used outside of a uses statement [at ")); + } + + @Test + public void testAbsoluteUsesAugment() { + final ReactorException ex = assertThrows(ReactorException.class, + () -> StmtTestUtils.parseYangSource("/bugs/YT1189/bar.yang")); + final Throwable cause = ex.getCause(); + assertThat(cause, instanceOf(SourceException.class)); + assertThat(cause.getMessage(), + startsWith("Absolute schema node identifier is not allowed when used within a uses statement [at ")); + } +} diff --git a/yang/yang-parser-rfc7950/src/test/resources/bugs/YT1189/bar.yang b/yang/yang-parser-rfc7950/src/test/resources/bugs/YT1189/bar.yang new file mode 100644 index 0000000000..d10d552eb1 --- /dev/null +++ b/yang/yang-parser-rfc7950/src/test/resources/bugs/YT1189/bar.yang @@ -0,0 +1,17 @@ +module bar { + namespace "bar"; + prefix "bar"; + + grouping grp { + container grp-cont; + } + + container cont { + uses grp { + // Invalid: the path should be descendant + augment "/grp-cont" { + + } + } + } +} diff --git a/yang/yang-parser-rfc7950/src/test/resources/bugs/YT1189/foo.yang b/yang/yang-parser-rfc7950/src/test/resources/bugs/YT1189/foo.yang new file mode 100644 index 0000000000..c2896b821b --- /dev/null +++ b/yang/yang-parser-rfc7950/src/test/resources/bugs/YT1189/foo.yang @@ -0,0 +1,11 @@ +module foo { + namespace "foo"; + prefix "foo"; + + container cont; + + // Invalid: the path should be absolute + augment "cont" { + + } +} diff --git a/yang/yang-parser-rfc7950/src/test/resources/bugs/bug5481/module2.yang b/yang/yang-parser-rfc7950/src/test/resources/bugs/bug5481/module2.yang index f6b1fa3450..22431b0778 100644 --- a/yang/yang-parser-rfc7950/src/test/resources/bugs/bug5481/module2.yang +++ b/yang/yang-parser-rfc7950/src/test/resources/bugs/bug5481/module2.yang @@ -12,7 +12,7 @@ module module2 { description "Initial version."; } - augment "module1:top" { + augment "/module1:top" { when "module1:top = 'extended'"; description "text"; status deprecated; -- 2.36.6