Fix Union Encoding #1861
Merged
Fix Union Encoding #1861
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
When encoding a union value back into zjson, we were calling value.toString() instead of value.serialize(). The resulted in bad zjson being sent to the store. When it was read and decoded, it choked and caused the error. A test has been added that replicated the error and confirmed that it was fixed.
jameskerr
commented
Oct 1, 2021
| @@ -13,15 +13,17 @@ export class TypeMap implements ContainerTypeInterface { | |||
| return `|{` + typeId(keyType) + ":" + typeId(valType) + "}|" | |||
| } | |||
|
|
|||
| create(value: [Value, Value][], typedefs) { | |||
| create(value: [Value, Value][] | null, typedefs) { | |||
There was a problem hiding this comment.
Values can always be null. This was another bug the test caught. The Map and Union types are more rare and have not been battle tested as much so it makes sense these bugs are here.
jameskerr
commented
Oct 1, 2021
| @@ -18,7 +18,7 @@ export class Union implements ZedValueInterface { | |||
|
|
|||
| serialize() { | |||
| if (isNull(this.index) || isNull(this.value)) return null | |||
| return [this.index.toString(), this.value.toString()] | |||
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.
fixes #1855
When encoding a union value back into zjson, we were calling value.toString() instead of value.serialize(). The resulted in the string representation of the value rather than the json value.
In the original repro, the tags field was a union of "string", "string array", and "null". When we received the data the value was:
["CobaltStrike", "HKKFGL-AS-AP HK Kwaifong Group Limited"]. When we encoded it back it became"[CobaltStrike,HKKFGL-AS-AP HK Kwaifong Group Limited]". This resulted in bad zjson being saved. When it was read and decoded, it choked on that string value and caused the error.A test has been added that replicated the error and confirmed that it was fixed.