Skip to content

Commit

Permalink
fix parsing of numbers within XML-encoded anyxml
Browse files Browse the repository at this point in the history
This was caught by the libyang-cpp test suite. On Windows/MSVC, an
anyxml snippet that was parsed from this XML:

 <ax xmlns="http://example.com/coze"><x>1</x><x>2</x><x>3</x></ax>

...would get printed like this:

 {"example-schema:ax":{"x":["1","2","3"]}}

...while the correct form is:

 {"example-schema:ax":{"x":[1,2,3]}}

This was caused by a change in `lydxml_get_hints_opaq` which probably
tried to silence a compiler warning. The code wrongly assumed that the C
`long` will be able to hold `UINT32_MAX`, which is not the case on
Windows where `long` has the same range as `int32_t`. As a result, when
the XML is parsed, these numeric values are compared against -1 on
Windows, and against 4294967295 on many other platforms, and therefore
any positive number was tagged with the hint of "this needs a 64bit
serialization" on Windows, which in turn triggers printing these values
as quoted string in JSON.

The fix simply makes sure that we never perform a narrowing conversion
from a constant. I was looking for a way of using explicitly sized types
(i.e., int64_t in this case), but there's only `strtoll` in C. The
alternative would be `sscanf(... SCNd64 ...)`, but the semantics is
subtly different when it comes to whitespace.

The bisecting process was non-trivial for me because the workflow which
bypasses CI introduced a test regression (fixed by commit 7ebd607,
"test UPDATE message changed") and a temporary Windows breakage (fixed
by commit a1ecc16, "compat BUGFIX regression in win timegm support").
Instead of a simple `git bisect`, this required some manual patch
backporting.

Fixes: 21eaa39 libyang BUGFIX type size problems
  • Loading branch information
jktjkt committed Jul 6, 2024
1 parent 3dfd2b6 commit 3839d4a
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 3 deletions.
8 changes: 5 additions & 3 deletions src/parser_xml.c
Original file line number Diff line number Diff line change
Expand Up @@ -441,7 +441,9 @@ lydxml_get_hints_opaq(const char *name, size_t name_len, const char *value, size
{
struct lyd_node_opaq *opaq;
char *ptr;
long num;
/* this needs to be at least 64bit, and it "should not" be an explicit int64_t
* because the code calls strtoll later on, which "might" return a bigger type */
long long num;

*hints = 0;
*anchor = NULL;
Expand All @@ -453,11 +455,11 @@ lydxml_get_hints_opaq(const char *name, size_t name_len, const char *value, size
/* boolean value */
*hints |= LYD_VALHINT_BOOLEAN;
} else {
num = strtol(value, &ptr, 10);
num = strtoll(value, &ptr, 10);
if ((unsigned)(ptr - value) == value_len) {
/* number value */
*hints |= LYD_VALHINT_DECNUM;
if ((num < INT32_MIN) || (num > (long)UINT32_MAX)) {
if ((num < INT32_MIN) || (num > UINT32_MAX)) {
/* large number */
*hints |= LYD_VALHINT_NUM64;
}
Expand Down
12 changes: 12 additions & 0 deletions tests/utests/data/test_parser_xml.c
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,18 @@ test_anyxml(void **state)
free(str);
CHECK_LYD_STRING(tree, LYD_PRINT_WITHSIBLINGS, data_expected);
lyd_free_all(tree);

data = "<anyx xmlns=\"urn:tests:a\"><x>1</x><x>0</x><x>-1</x><x>4294967295</x><x>4294967296</x><x>-2147483648</x><x>-2147483649</x></anyx>";
CHECK_PARSE_LYD(data, 0, LYD_VALIDATE_PRESENT, tree);
assert_non_null(tree);
tree = tree->next;
assert_int_equal(LY_SUCCESS, lyd_print_mem(&str, tree, LYD_XML, LYD_PRINT_SHRINK));
CHECK_STRING(str, data);
free(str);
assert_int_equal(LY_SUCCESS, lyd_print_mem(&str, tree, LYD_JSON, LYD_PRINT_SHRINK));
CHECK_STRING(str, "{\"a:anyx\":{\"x\":[1,0,-1,4294967295,\"4294967296\",-2147483648,\"-2147483649\"]}}");
free(str);
lyd_free_all(tree);
}

static void
Expand Down

0 comments on commit 3839d4a

Please sign in to comment.