MEN-6834: Make all JSON parsing case insensitive #1523
Merged
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
Signed-off-by: Lluis Campos <lluis.campos@northern.tech>
|
@lluiscampos, Let me know if you want to start the integration pipeline by mentioning me and the command "start pipeline". my commands and optionsYou can trigger a pipeline on multiple prs with:
You can start a fast pipeline, disabling full integration tests with:
You can trigger GitHub->GitLab branch sync with:
You can cherry pick to a given branch or branches with:
|
lluiscampos
force-pushed
the
MEN-6834-config-parsing-case-insensitive
branch
from
6f0a11b to
e31bd2c
Compare
lluiscampos
commented
Nov 29, 2023
| template <class Key, class T, class IgnoredLess, class Allocator = allocator<pair<const Key, T>>> | ||
| class CaseInsensitiveMap : public map<const Key, T, CaseInsensitiveLess, Allocator> { | ||
| public: | ||
| using CaseInsensitiveMapType = map<const Key, T, CaseInsensitiveLess, Allocator>; |
|
@mender-test-bot start pipeline |
|
Hello 😸 I created a pipeline for you here: Pipeline-1089545281 Build Configuration Matrix
|
ghost
approved these changes
Nov 30, 2023
| os << R"({ | ||
| "artifactverifykey": "ArtifactVerifyKey_value", | ||
| "deviceTypeFile": "DeviceTypeFile_value", | ||
| "SERVERURL": "ServerURL_value" |
There was a problem hiding this comment.
Maybe we should have a separate test where you have the same key with different casing? I'm a little bit interested to see how it handles it, especially when compared to having two completely identical keys.
There was a problem hiding this comment.
It behaves the same with two identical keys than with same key with different casing: the entry in the map gets overridden and only the last one stands.
I'll add a test, thanks for the suggestion.
Although the JSON standard defines the keys as case sensitive, the golang standard library implements it as case insensitive. Therefore to avoid regressions we need to make all our JSON parsing case insensitive. To implement it, we define a custom `nlohmann::basic_json` with a custom `std::map` that compares the keys as lowercase. See: * https://json.nlohmann.me/api/basic_json/basic_json/ * https://json.nlohmann.me/api/basic_json/object_t/ Ticket: MEN-6834 Changelog: None Signed-off-by: Lluis Campos <lluis.campos@northern.tech>
lluiscampos
force-pushed
the
MEN-6834-config-parsing-case-insensitive
branch
from
e31bd2c to
cb118d4
Compare
|
The last force-push only added a unit-test, not running again the full pipeline. See the last pipeline here. |
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.
Although the JSON standard defines the keys as case sensitive, the
golang standard library implements it as case insensitive. Therefore to
avoid regressions we need to make all our JSON parsing case insensitive.
To implement it, we define a custom
nlohmann::basic_jsonwith a customstd::mapthat compares the keys as lowercase. See: