From 23bc41c8c289eb49eff747bdb844ad2e58dc29e0 Mon Sep 17 00:00:00 2001 From: steiler Date: Mon, 1 Jun 2026 16:31:04 +0200 Subject: [PATCH] fix: normalize identityref prefix aliases in must-statement xpath literals getMustStatement now calls normalizeXPathLiterals on each must expression before returning it. This rewrites quoted literals of the form 'alias:IdentityName' where alias is a local import alias (not the module's own declared prefix) to 'declaredPrefix:IdentityName', so that xpath evaluation at runtime sees a consistent, canonical prefix regardless of how the consuming module chose to import the identity module. Adds normalizeXPathLiterals and normalizeIdentityrefLiteral helpers. Plain literals, unknown prefixes, and already-canonical prefixes are passed through unchanged (idempotent). Regression tests: TestGetMustStatement_NormalizesAlias, TestGetMustStatement_AlreadyDeclaredPrefix, TestGetMustStatement_PlainLiteralUnchanged, TestGetMustStatement_UnknownPrefixUnchanged. YANG fixtures in pkg/schema/testdata/must-alias/. Companion fix in data-server: sdcio/data-server fix/identityref-leafref-normalization Co-authored-by: Cursor --- pkg/schema/leaf.go | 53 ++++++++ pkg/schema/leaf_test.go | 127 ++++++++++++++++++ .../must-alias/must-alias-consumer.yang | 35 +++++ .../must-correct-prefix-consumer.yang | 19 +++ .../testdata/must-alias/must-identity.yang | 15 +++ 5 files changed, 249 insertions(+) create mode 100644 pkg/schema/leaf_test.go create mode 100644 pkg/schema/testdata/must-alias/must-alias-consumer.yang create mode 100644 pkg/schema/testdata/must-alias/must-correct-prefix-consumer.yang create mode 100644 pkg/schema/testdata/must-alias/must-identity.yang diff --git a/pkg/schema/leaf.go b/pkg/schema/leaf.go index 2a5a7d5..f3755a3 100644 --- a/pkg/schema/leaf.go +++ b/pkg/schema/leaf.go @@ -170,6 +170,7 @@ func getMustStatement(e *yang.Entry) []*sdcpb.MustStatement { if m, ok := m.(*yang.Must); ok { // newlines might appear in the yang file, replace them with space stmt := strings.ReplaceAll(m.Name, "\n", " ") + stmt = normalizeXPathLiterals(stmt, e) ms := &sdcpb.MustStatement{ Statement: stmt, } @@ -182,6 +183,58 @@ func getMustStatement(e *yang.Entry) []*sdcpb.MustStatement { return rs } +// normalizeXPathLiterals rewrites quoted string literals in an xpath expression +// of the form 'prefix:name' (or "prefix:name") where prefix is a local import +// alias to use the imported module's own declared prefix instead. +// Literals whose prefix is not in the import table are left unchanged. +func normalizeXPathLiterals(xpath string, e *yang.Entry) string { + var b strings.Builder + i := 0 + for i < len(xpath) { + ch := xpath[i] + if ch == '\'' || ch == '"' { + end := strings.IndexByte(xpath[i+1:], ch) + if end < 0 { + b.WriteString(xpath[i:]) + return b.String() + } + end = i + 1 + end + literal := xpath[i+1 : end] + b.WriteByte(ch) + b.WriteString(normalizeIdentityrefLiteral(literal, e)) + b.WriteByte(ch) + i = end + 1 + } else { + b.WriteByte(ch) + i++ + } + } + return b.String() +} + +// normalizeIdentityrefLiteral rewrites a single literal value 'prefix:name' +// to use the declared prefix of the imported module. +func normalizeIdentityrefLiteral(literal string, e *yang.Entry) string { + colonIdx := strings.IndexByte(literal, ':') + if colonIdx < 0 { + return literal + } + prefix := literal[:colonIdx] + name := literal[colonIdx+1:] + if strings.ContainsAny(prefix, " \t\n/[]@=") { + return literal + } + mod := yang.FindModuleByPrefix(e.Node, prefix) + if mod == nil { + return literal + } + declaredPrefix := mod.GetPrefix() + if declaredPrefix == prefix { + return literal + } + return declaredPrefix + ":" + name +} + func getIfFeature(e *yang.Entry) []string { ifFeatures, ok := e.Extra["if-feature"] if !ok { diff --git a/pkg/schema/leaf_test.go b/pkg/schema/leaf_test.go new file mode 100644 index 0000000..b012f15 --- /dev/null +++ b/pkg/schema/leaf_test.go @@ -0,0 +1,127 @@ +// Copyright 2024 Nokia +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package schema + +import ( + "testing" + + "github.com/sdcio/schema-server/pkg/config" +) + +func mustAliasSchema(t *testing.T) *Schema { + t.Helper() + sc, err := NewSchema(&config.SchemaConfig{ + Name: "must-alias-test", + Vendor: "test", + Version: "0.0.0", + Files: []string{"testdata/must-alias"}, + Directories: []string{}, + Excludes: []string{}, + }) + if err != nil { + t.Fatalf("NewSchema: %v", err) + } + return sc +} + +// TestGetMustStatement_NormalizesAlias verifies that getMustStatement rewrites +// quoted xpath literals that use a non-standard import alias to the declared +// prefix of the imported module. +func TestGetMustStatement_NormalizesAlias(t *testing.T) { + sc := mustAliasSchema(t) + + e, err := sc.GetEntry([]string{"protocol-with-alias"}) + if err != nil { + t.Fatalf("GetEntry: %v", err) + } + + stmts := getMustStatement(e) + if len(stmts) != 1 { + t.Fatalf("expected 1 must statement, got %d", len(stmts)) + } + + got := stmts[0].Statement + want := ". = 'must-id:BGP' or . = 'must-id:OSPF'" + if got != want { + t.Errorf("must statement not normalized\n got: %q\n want: %q", got, want) + } +} + +// TestGetMustStatement_AlreadyDeclaredPrefix verifies that getMustStatement +// leaves literals unchanged when they already use the module's declared prefix +// (idempotent behaviour). +func TestGetMustStatement_AlreadyDeclaredPrefix(t *testing.T) { + sc := mustAliasSchema(t) + + e, err := sc.GetEntry([]string{"protocol-correct-prefix"}) + if err != nil { + t.Fatalf("GetEntry: %v", err) + } + + stmts := getMustStatement(e) + if len(stmts) != 1 { + t.Fatalf("expected 1 must statement, got %d", len(stmts)) + } + + got := stmts[0].Statement + want := ". = 'must-id:BGP'" + if got != want { + t.Errorf("must statement changed unexpectedly\n got: %q\n want: %q", got, want) + } +} + +// TestGetMustStatement_PlainLiteralUnchanged verifies that string literals +// without a colon prefix are left unchanged. +func TestGetMustStatement_PlainLiteralUnchanged(t *testing.T) { + sc := mustAliasSchema(t) + + e, err := sc.GetEntry([]string{"plain-string-must"}) + if err != nil { + t.Fatalf("GetEntry: %v", err) + } + + stmts := getMustStatement(e) + if len(stmts) != 1 { + t.Fatalf("expected 1 must statement, got %d", len(stmts)) + } + + got := stmts[0].Statement + want := ". != 'just-a-string'" + if got != want { + t.Errorf("plain literal changed unexpectedly\n got: %q\n want: %q", got, want) + } +} + +// TestGetMustStatement_UnknownPrefixUnchanged verifies that literals whose +// prefix is not in the module's import table are left unchanged. +func TestGetMustStatement_UnknownPrefixUnchanged(t *testing.T) { + sc := mustAliasSchema(t) + + e, err := sc.GetEntry([]string{"unknown-prefix-must"}) + if err != nil { + t.Fatalf("GetEntry: %v", err) + } + + stmts := getMustStatement(e) + if len(stmts) != 1 { + t.Fatalf("expected 1 must statement, got %d", len(stmts)) + } + + got := stmts[0].Statement + want := ". != 'unknown-mod:FOO'" + if got != want { + t.Errorf("unknown-prefix literal changed unexpectedly\n got: %q\n want: %q", got, want) + } +} diff --git a/pkg/schema/testdata/must-alias/must-alias-consumer.yang b/pkg/schema/testdata/must-alias/must-alias-consumer.yang new file mode 100644 index 0000000..d165134 --- /dev/null +++ b/pkg/schema/testdata/must-alias/must-alias-consumer.yang @@ -0,0 +1,35 @@ +module must-alias-consumer { + yang-version 1.1; + namespace "urn:test/must-alias-consumer"; + prefix "mac"; + + import must-identity { + prefix alias-id; + } + + // must uses non-standard import alias — should be rewritten to declared prefix + leaf protocol-with-alias { + type identityref { + base alias-id:protocol-type; + } + must ". = 'alias-id:BGP' or . = 'alias-id:OSPF'" { + error-message "Must be BGP or OSPF"; + } + } + + // must uses a literal that has no colon — must remain unchanged + leaf plain-string-must { + type string; + must ". != 'just-a-string'" { + error-message "Must not be just-a-string"; + } + } + + // must uses a prefix not in the import table — must remain unchanged + leaf unknown-prefix-must { + type string; + must ". != 'unknown-mod:FOO'" { + error-message "Must not be unknown-mod:FOO"; + } + } +} diff --git a/pkg/schema/testdata/must-alias/must-correct-prefix-consumer.yang b/pkg/schema/testdata/must-alias/must-correct-prefix-consumer.yang new file mode 100644 index 0000000..9a86bd6 --- /dev/null +++ b/pkg/schema/testdata/must-alias/must-correct-prefix-consumer.yang @@ -0,0 +1,19 @@ +module must-correct-prefix-consumer { + yang-version 1.1; + namespace "urn:test/must-correct-prefix-consumer"; + prefix "mcpc"; + + import must-identity { + prefix must-id; + } + + // must already uses the declared prefix — must remain unchanged (idempotent) + leaf protocol-correct-prefix { + type identityref { + base must-id:protocol-type; + } + must ". = 'must-id:BGP'" { + error-message "Must be BGP"; + } + } +} diff --git a/pkg/schema/testdata/must-alias/must-identity.yang b/pkg/schema/testdata/must-alias/must-identity.yang new file mode 100644 index 0000000..225a101 --- /dev/null +++ b/pkg/schema/testdata/must-alias/must-identity.yang @@ -0,0 +1,15 @@ +module must-identity { + yang-version 1.1; + namespace "urn:test/must-identity"; + prefix "must-id"; + + identity protocol-type; + + identity BGP { + base protocol-type; + } + + identity OSPF { + base protocol-type; + } +}