Remove ignored UTF8 setting in diagMode() #417
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
Unreleased functions Diagnose() and DiagnoseFirst() added in v2.5.0-beta3 were ignoring the setting of DecOptions.UTF8 = DecodeInvalidUTF8 in the underlying decoder. Given this, removing the ignored code is enough to make the new diagnostic functions match the default behavior of CBOR decoder which is to reject invalid UTF-8 in CBOR text strings. For now, it doesn't make sense to print invalid UTF-8 text because the Diagnostic Notation described by RFC 8949 and the Extended Diagnostic Notation in Appendix G of RFC 8610 don't specify how to represent invalid UTF-8.
|
@x448 can you take a look? This is probably the last coding change before tagging v2.5.0 and it doesn't require re-fuzzing the unaffected non-diagnostic codec functions. Also need to update README before tagging v2.5.0. |
x448
reviewed
Jul 30, 2023
x448
approved these changes
Jul 30, 2023
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.
Closes #416
Unreleased functions
Diagnose()andDiagnoseFirst()added in v2.5.0-beta3 were ignoring the setting of DecOptions.UTF8 = DecodeInvalidUTF8 (to allow decoding invalid UTF-8) in the underlying decoder.This PR is a NOP which simply removes the ignored code (instead of making it work) because:
Diagnostic functions rejecting invalid UTF-8 matches the default behavior of CBOR decoder.
It currently doesn't make sense to display invalid UTF-8 text. Diagnostic Notation described by RFC 8949 and the Extended Diagnostic Notation in Appendix G of RFC 8610 don't specify how to represent invalid UTF-8.