-
Notifications
You must be signed in to change notification settings - Fork 63
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add decoding option to allow decoding CBOR byte string into Go time.Time #517
Conversation
…yte string into Go time.Time Signed-off-by: Suriyan Subbarayan [email protected]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ssuriyan7 Thanks for opening this PR! I left a couple of suggestions.
b, _ := d.parseByteString() | ||
t, err := time.Parse(time.RFC3339, string(b)) | ||
if err != nil { | ||
return time.Time{}, false, errors.New("cbor: cannot set " + string(b) + " for time.Time: " + err.Error()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should escape control characters and non-printable characters in the error message.
return time.Time{}, false, errors.New("cbor: cannot set " + string(b) + " for time.Time: " + err.Error()) | |
return time.Time{}, false, errors.New("cbor: cannot set " + strconv.Quote(string(b)) + " for time.Time: " + err.Error()) |
name: "Unmarshal an invalid byte string to time.Time when ByteStringToTime is set to ByteStringToTimeAllowed", | ||
opts: DecOptions{ByteStringToTime: ByteStringToTimeAllowed}, | ||
in: hexDecode("4B696E76616C696454657874"), // 'invalidText' | ||
wantErrorMsg: "cbor: cannot set invalidText for time.Time: parsing time \"invalidText\" as \"2006-01-02T15:04:05Z07:00\": cannot parse \"invalidText\" as \"2006\"", | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe add a test with byte string containing some invalid UTF-8 characters.
I opened #524 to nudge this over the finish line. This is necessary for cases that were dependent on the behavior of the bug #501, which has since been fixed. Thank you @ssuriyan7 for your work on this! I took care to retain you as the commit author on my updated branch. |
This PR is completed by @benluddy's PR #524. Thanks @ssuriyan7 for opening this PR and your other contributions too! 👍 |
Currently, when cbor.Unmarshal is called with a byte string data item and time.Time as destination type, it causes UnmarshalTypeError. This change adds a decoding option to specify if this operation can be allowed.
Description
PR Was Proposed and Welcomed in Currently Open Issue
Checklist (for code PR only, ignore for docs PR)
Last line of each commit message should be in this format:
Signed-off-by: Firstname Lastname [email protected]
(see next section).
Certify the Developer's Certificate of Origin 1.1
the Developer Certificate of Origin 1.1.