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

restconf: add upstream patches for rousette #780

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,199 @@
From 9622a68eba4aeaa60619b4c33d050ce91b27653d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Pecka?= <[email protected]>
Date: Tue, 8 Oct 2024 12:22:49 +0200
Subject: [PATCH 1/6] restconf: report the list key values when they differ
between URI and data
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Organization: Addiva Elektronik

For creating (leaf-)list instances with PUT and PATCH methods one has
to specify the list key values in the URI and in the data as well.
Those values must be the same. In case they are not, it is an error.
We reported that the values mismatch in case this happens, but the error
message did not report what the values were.
Knowing that might be beneficial when one is about to insert key values
that can be namespace qualified (like identityrefs) and that are
sometimes manipulated by libyang (e.g., when the identity belongs to the
same namespace as the list node, it is not necessary for it to be
namespace qualified by the client, but libyang adds the namespace
internally).

Change-Id: Ie0d42511bde01ab4c39d61370b6601f8808e40c5
Signed-off-by: Mattias Walström <[email protected]>
---
src/restconf/Server.cpp | 29 +++++++++++++++++++++--------
tests/restconf-plain-patch.cpp | 2 +-
tests/restconf-writing.cpp | 14 +++++++-------
tests/restconf-yang-patch.cpp | 2 +-
4 files changed, 30 insertions(+), 17 deletions(-)

diff --git a/src/restconf/Server.cpp b/src/restconf/Server.cpp
index 53d6625..5f560ed 100644
--- a/src/restconf/Server.cpp
+++ b/src/restconf/Server.cpp
@@ -154,9 +154,22 @@ auto rejectYangPatch(const std::string& patchId, const std::string& editId)
};
}

+struct KeyMismatch {
+ libyang::DataNode offendingNode;
+ std::optional<std::string> uriKeyValue;
+
+ std::string message() const {
+ if (uriKeyValue) {
+ return "List key mismatch between URI path ('"s + *uriKeyValue + "') and data ('" + offendingNode.asTerm().valueStr() + "').";
+ } else {
+ return "List key mismatch (key missing in the data).";
+ }
+ }
+};
+
/** @brief In case node is a (leaf-)list check if the key values are the same as the keys specified in the lastPathSegment.
* @return The node where the mismatch occurs */
-std::optional<libyang::DataNode> checkKeysMismatch(const libyang::DataNode& node, const PathSegment& lastPathSegment)
+std::optional<KeyMismatch> checkKeysMismatch(const libyang::DataNode& node, const PathSegment& lastPathSegment)
{
if (node.schema().nodeType() == libyang::NodeType::List) {
const auto& listKeys = node.schema().asList().keys();
@@ -164,18 +177,18 @@ std::optional<libyang::DataNode> checkKeysMismatch(const libyang::DataNode& node
const auto& keyValueURI = lastPathSegment.keys[i];
auto keyNodeData = node.findPath(listKeys[i].module().name() + ':' + listKeys[i].name());
if (!keyNodeData) {
- return node;
+ return KeyMismatch{node, std::nullopt};
}

const auto& keyValueData = keyNodeData->asTerm().valueStr();

if (keyValueURI != keyValueData) {
- return keyNodeData;
+ return KeyMismatch{*keyNodeData, keyValueURI};
}
}
} else if (node.schema().nodeType() == libyang::NodeType::Leaflist) {
if (lastPathSegment.keys[0] != node.asTerm().valueStr()) {
- return node;
+ return KeyMismatch{node, lastPathSegment.keys[0]};
}
}
return std::nullopt;
@@ -350,8 +363,8 @@ libyang::CreatedNodes createEditForPutAndPatch(libyang::Context& ctx, const std:
if (isSameNode(child, lastPathSegment)) {
// 1) a single child that is created by parseSubtree(), its name is the same as `lastPathSegment`.
// It could be a list; then we need to check if the keys in provided data match the keys in URI.
- if (auto offendingNode = checkKeysMismatch(child, lastPathSegment)) {
- throw ErrorResponse(400, "protocol", "invalid-value", "List key mismatch between URI path and data.", offendingNode->path());
+ if (auto keyMismatch = checkKeysMismatch(child, lastPathSegment)) {
+ throw ErrorResponse(400, "protocol", "invalid-value", keyMismatch->message(), keyMismatch->offendingNode.path());
}
replacementNode = child;
} else if (isKeyNode(*node, child)) {
@@ -373,8 +386,8 @@ libyang::CreatedNodes createEditForPutAndPatch(libyang::Context& ctx, const std:
if (!isSameNode(*replacementNode, lastPathSegment)) {
throw ErrorResponse(400, "protocol", "invalid-value", "Data contains invalid node.", replacementNode->path());
}
- if (auto offendingNode = checkKeysMismatch(*parent, lastPathSegment)) {
- throw ErrorResponse(400, "protocol", "invalid-value", "List key mismatch between URI path and data.", offendingNode->path());
+ if (auto keyMismatch = checkKeysMismatch(*parent, lastPathSegment)) {
+ throw ErrorResponse(400, "protocol", "invalid-value", keyMismatch->message(), keyMismatch->offendingNode.path());
}
}
}
diff --git a/tests/restconf-plain-patch.cpp b/tests/restconf-plain-patch.cpp
index 10d653a..d4f3952 100644
--- a/tests/restconf-plain-patch.cpp
+++ b/tests/restconf-plain-patch.cpp
@@ -72,7 +72,7 @@ TEST_CASE("Plain patch")
"error-type": "protocol",
"error-tag": "invalid-value",
"error-path": "/example:tlc/list[name='blabla']/name",
- "error-message": "List key mismatch between URI path and data."
+ "error-message": "List key mismatch between URI path ('libyang') and data ('blabla')."
}
]
}
diff --git a/tests/restconf-writing.cpp b/tests/restconf-writing.cpp
index d46952f..96dbb25 100644
--- a/tests/restconf-writing.cpp
+++ b/tests/restconf-writing.cpp
@@ -432,7 +432,7 @@ TEST_CASE("writing data")
"error-type": "protocol",
"error-tag": "invalid-value",
"error-path": "/example:tlc/list[name='ahoj']/name",
- "error-message": "List key mismatch between URI path and data."
+ "error-message": "List key mismatch between URI path ('netconf') and data ('ahoj')."
}
]
}
@@ -447,7 +447,7 @@ TEST_CASE("writing data")
"error-type": "protocol",
"error-tag": "invalid-value",
"error-path": "/example:top-level-list[name='ahoj']/name",
- "error-message": "List key mismatch between URI path and data."
+ "error-message": "List key mismatch between URI path ('netconf') and data ('ahoj')."
}
]
}
@@ -505,7 +505,7 @@ TEST_CASE("writing data")
"error-type": "protocol",
"error-tag": "invalid-value",
"error-path": "/example:tlc/list[name='netconf']/collection[.='666']",
- "error-message": "List key mismatch between URI path and data."
+ "error-message": "List key mismatch between URI path ('667') and data ('666')."
}
]
}
@@ -520,7 +520,7 @@ TEST_CASE("writing data")
"error-type": "protocol",
"error-tag": "invalid-value",
"error-path": "/example:top-level-leaf-list[.='666']",
- "error-message": "List key mismatch between URI path and data."
+ "error-message": "List key mismatch between URI path ('667') and data ('666')."
}
]
}
@@ -535,7 +535,7 @@ TEST_CASE("writing data")
"error-type": "protocol",
"error-tag": "invalid-value",
"error-path": "/example:tlc/list[name='sysrepo']/name",
- "error-message": "List key mismatch between URI path and data."
+ "error-message": "List key mismatch between URI path ('netconf') and data ('sysrepo')."
}
]
}
@@ -550,7 +550,7 @@ TEST_CASE("writing data")
"error-type": "protocol",
"error-tag": "invalid-value",
"error-path": "/example:tlc/list[name='sysrepo']/name",
- "error-message": "List key mismatch between URI path and data."
+ "error-message": "List key mismatch between URI path ('netconf') and data ('sysrepo')."
}
]
}
@@ -565,7 +565,7 @@ TEST_CASE("writing data")
"error-type": "protocol",
"error-tag": "invalid-value",
"error-path": "/example:tlc/list[name='libyang']/collection[.='42']",
- "error-message": "List key mismatch between URI path and data."
+ "error-message": "List key mismatch between URI path ('5') and data ('42')."
}
]
}
diff --git a/tests/restconf-yang-patch.cpp b/tests/restconf-yang-patch.cpp
index 9d70912..7cc8946 100644
--- a/tests/restconf-yang-patch.cpp
+++ b/tests/restconf-yang-patch.cpp
@@ -436,7 +436,7 @@ TEST_CASE("YANG patch")
"error-type": "protocol",
"error-tag": "invalid-value",
"error-path": "/example:tlc/list[name='asdasdauisbdhaijbsdad']/name",
- "error-message": "List key mismatch between URI path and data."
+ "error-message": "List key mismatch between URI path ('libyang') and data ('asdasdauisbdhaijbsdad')."
}
]
}
--
2.43.0

Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
From 070cffb48fda789910581930265d4624a7213e1b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Pecka?= <[email protected]>
Date: Wed, 16 Oct 2024 11:02:45 +0200
Subject: [PATCH 2/6] uri: rename url-encoding to percent-encoding
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Organization: Addiva Elektronik

RFC 3986 [1] calls it the percent-encoding, let's be consistent.

[1] https://datatracker.ietf.org/doc/html/rfc3986

Change-Id: Iee8b76c980b2694b6643e627b462f8bfc2c21c45
Signed-off-by: Mattias Walström <[email protected]>
---
src/restconf/uri.cpp | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/restconf/uri.cpp b/src/restconf/uri.cpp
index 6e27168..b144d92 100644
--- a/src/restconf/uri.cpp
+++ b/src/restconf/uri.cpp
@@ -29,12 +29,12 @@ auto add = [](auto& ctx) {
char c = std::tolower(_attr(ctx));
_val(ctx) = _val(ctx) * 16 + (c >= 'a' ? c - 'a' + 10 : c - '0');
};
-const auto urlEncodedChar = x3::rule<class urlEncodedChar, unsigned>{"urlEncodedChar"} = x3::lit('%')[set_zero] >> x3::xdigit[add] >> x3::xdigit[add];
+const auto percentEncodedChar = x3::rule<class percentEncodedChar, unsigned>{"percentEncodedChar"} = x3::lit('%')[set_zero] >> x3::xdigit[add] >> x3::xdigit[add];

/* reserved characters according to RFC 3986, sec. 2.2 with '%' added. The '%' character is not specified as reserved but it effectively is because
* "Percent sign serves as the indicator for percent-encoded octets, it must be percent-encoded (...)" [RFC 3986, sec. 2.4]. */
const auto reservedChars = x3::lit(':') | '/' | '?' | '#' | '[' | ']' | '@' | '!' | '$' | '&' | '\'' | '(' | ')' | '*' | '+' | ',' | ',' | ';' | '=' | '%';
-const auto keyValue = x3::rule<class keyValue, std::string>{"keyValue"} = *(urlEncodedChar | (x3::char_ - reservedChars));
+const auto keyValue = x3::rule<class keyValue, std::string>{"keyValue"} = *(percentEncodedChar | (x3::char_ - reservedChars));

const auto keyList = x3::rule<class keyList, std::vector<std::string>>{"keyList"} = keyValue % ',';
const auto identifier = x3::rule<class apiIdentifier, std::string>{"identifier"} = (x3::alpha | x3::char_('_')) >> *(x3::alnum | x3::char_('_') | x3::char_('-') | x3::char_('.'));
@@ -117,7 +117,7 @@ const auto dateAndTime = x3::rule<class dateAndTime, std::string>{"dateAndTime"}
x3::repeat(4)[x3::digit] >> x3::char_('-') >> x3::repeat(2)[x3::digit] >> x3::char_('-') >> x3::repeat(2)[x3::digit] >> x3::char_('T') >>
x3::repeat(2)[x3::digit] >> x3::char_(':') >> x3::repeat(2)[x3::digit] >> x3::char_(':') >> x3::repeat(2)[x3::digit] >> -(x3::char_('.') >> +x3::digit) >>
(x3::char_('Z') | (-(x3::char_('+')|x3::char_('-')) >> x3::repeat(2)[x3::digit] >> x3::char_(':') >> x3::repeat(2)[x3::digit]));
-const auto filter = x3::rule<class filter, std::string>{"filter"} = +(urlEncodedChar | (x3::char_ - '&'));
+const auto filter = x3::rule<class filter, std::string>{"filter"} = +(percentEncodedChar | (x3::char_ - '&'));
const auto depthParam = x3::rule<class depthParam, queryParams::QueryParamValue>{"depthParam"} = x3::uint_[validDepthValues] | (x3::string("unbounded") >> x3::attr(queryParams::UnboundedDepth{}));
const auto queryParamPair = x3::rule<class queryParamPair, std::pair<std::string, queryParams::QueryParamValue>>{"queryParamPair"} =
(x3::string("depth") >> "=" >> depthParam) |
--
2.43.0

34 changes: 34 additions & 0 deletions patches/rousette/v1/0003-uri-correct-x3-rule-class-names.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
From 8a233202647f24538f2bbb8fff1e38d52e3599a4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Pecka?= <[email protected]>
Date: Wed, 16 Oct 2024 11:05:09 +0200
Subject: [PATCH 3/6] uri: correct x3 rule class names
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Organization: Addiva Elektronik

Fixes: e06d5bf ("server: parser for data resources in URI paths")
Change-Id: Ic953e568d841032113ede1c0e896574361c0ebe2
Signed-off-by: Mattias Walström <[email protected]>
---
src/restconf/uri.cpp | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/restconf/uri.cpp b/src/restconf/uri.cpp
index b144d92..8e8dc23 100644
--- a/src/restconf/uri.cpp
+++ b/src/restconf/uri.cpp
@@ -37,8 +37,8 @@ const auto reservedChars = x3::lit(':') | '/' | '?' | '#' | '[' | ']' | '@' | '!
const auto keyValue = x3::rule<class keyValue, std::string>{"keyValue"} = *(percentEncodedChar | (x3::char_ - reservedChars));

const auto keyList = x3::rule<class keyList, std::vector<std::string>>{"keyList"} = keyValue % ',';
-const auto identifier = x3::rule<class apiIdentifier, std::string>{"identifier"} = (x3::alpha | x3::char_('_')) >> *(x3::alnum | x3::char_('_') | x3::char_('-') | x3::char_('.'));
-const auto apiIdentifier = x3::rule<class identifier, ApiIdentifier>{"apiIdentifier"} = -(identifier >> ':') >> identifier;
+const auto identifier = x3::rule<class identifier, std::string>{"identifier"} = (x3::alpha | x3::char_('_')) >> *(x3::alnum | x3::char_('_') | x3::char_('-') | x3::char_('.'));
+const auto apiIdentifier = x3::rule<class apiIdentifier, ApiIdentifier>{"apiIdentifier"} = -(identifier >> ':') >> identifier;
const auto listInstance = x3::rule<class keyList, PathSegment>{"listInstance"} = apiIdentifier >> -('=' >> keyList);
const auto fullyQualifiedApiIdentifier = x3::rule<class identifier, ApiIdentifier>{"apiIdentifier"} = identifier >> ':' >> identifier;
const auto fullyQualifiedListInstance = x3::rule<class keyList, PathSegment>{"listInstance"} = fullyQualifiedApiIdentifier >> -('=' >> keyList);
--
2.43.0

Loading
Loading