fix parsing of numbers within XML-encoded anyxml #2266
Merged
+17
−3
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jktjkt
added a commit
to Telecominfraproject/oopt-gnpy-libyang
that referenced
this pull request
Jul 6, 2024
...along with CESNET/libyang#2266 which fixes a regression on Windows.
jktjkt
added a commit
to Telecominfraproject/oopt-gnpy-libyang
that referenced
this pull request
Jul 6, 2024
Depends-on: CESNET/libyang#2266
jktjkt
added a commit
to Telecominfraproject/oopt-gnpy-libyang
that referenced
this pull request
Jul 7, 2024
Depends-on: CESNET/libyang#2266
jktjkt
added a commit
to Telecominfraproject/oopt-gnpy-libyang
that referenced
this pull request
Jul 7, 2024
Depends-on: CESNET/libyang#2266 Depends-on: https://gerrit.cesnet.cz/c/CzechLight/libyang-cpp/+/7258
|
Last |
|
...and I do not see a button to do that :(, maybe you as the project owner can either rebase this or somehow make the job unstuck? It's been listed as "queued" for me for 18 hours. |
|
Never mind that, macOS kept failing and now won't even start. I can restart the other checks, but I though I did already yesterday. |
|
macos 11 won't build ever again as the images were removed, actions/runner-images#10097 |
|
Right... not sure how the upgrade will go. As for the WIN checks, they still fail, no idea why. |
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
jktjkt
force-pushed
the
fix-windows-anyxml-naked-number-xml-parsing
branch
from
3839d4a to
47d2611
Compare
michalvasko
approved these changes
Jul 16, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This was caught by the libyang-cpp test suite. On Windows/MSVC, an anyxml snippet that was parsed from this XML:
...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 21eaa39, a change in
lydxml_get_hints_opaqwhich probably tried to silence a compiler warning. The code wrongly assumed that the Clongwill be able to holdUINT32_MAX, which is not the case on Windows wherelonghas the same range asint32_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
strtollin C. The alternative would besscanf(... 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 7ebd607) and a temporary Windows breakage (fixed by a1ecc16). Instead of a simple
git bisect, this required some manual patch backporting.